On Fri, Aug 31, 2018 at 4:07 PM Jeff King [off-list ref] wrote:
On Fri, Aug 31, 2018 at 02:33:18AM -0400, Eric Sunshine wrote:
quoted
diff --git a/Documentation/Makefile b/Documentation/Makefile
@@ -332,6 +332,7 @@ clean:
$(RM) manpage-base-url.xsl
+ '$(SHELL_PATH_SQ)' ./doc-diff --clean
This spelling took me by surprise. The doc-diff script itself specifies
/bin/sh, and we do not build it, so the #! line is never replaced. [...]
I don't think the script does anything complicated that would choke a
lesser /bin/sh. But it doesn't hurt to be defensive, since this bit of
the Makefile will be run for everyone, whether they care about doc-diff
or not.
So that all makes sense. I initially wrote this to suggest that we call
out this subtlety in the commit message. But I see this is based on
existing instances from ee7ec2f9de (documentation: Makefile accounts for
SHELL_PATH setting, 2009-03-22). So maybe I am just showing my
ignorance. ;)
Correct. I was concerned that invoking it simply as "./doc-diff
--clean" could be problematic, so, knowing that the Makefile invoked
other scripts in Documentation/, I mirrored their invocation. If it
didn't follow existing practice of invoking the command with
$(SHELL_PATH_SQ), then that would merit mention in the commit message,
but as it is, the commit message is probably fine.
Thanks for the review.