Hi Peff, On Thu, 28 Jun 2018, Jeff King wrote: > On Wed, Jun 27, 2018 at 09:35:23PM +0200, Johannes Schindelin via GitGitGadget wrote: > > > To prevent erroneous commits from being reported (e.g. when unpacking > > Git's source code from a .tar.gz file into a subdirectory of a different > > Git project, as e.g. git_osx_installer does), we painstakingly set > > GIT_CEILING_DIRECTORIES when trying to determine the current commit. > > > > Except that we got the quoting wrong, and that variable therefore does > > not have the desired effect. > > > > Let's fix that quoting, and while at it, also suppress the unhelpful > > message > > I had to stare at the code for a bit to figure out what was wrong: Do you want me to update the commit message? > > - '-DGIT_BUILT_FROM_COMMIT="$(shell GIT_CEILING_DIRECTORIES=\"$(CURDIR)/..\" \ > > - git rev-parse -q --verify HEAD || :)"' > > + '-DGIT_BUILT_FROM_COMMIT="$(shell \ > > + GIT_CEILING_DIRECTORIES="$(CURDIR)/.." \ > > + git rev-parse -q --verify HEAD 2>/dev/null)"' > > The issue is that the $(shell) is resolved before the output is stuffed > into the command-line with -DGIT_BUILT_FROM_COMMIT, and therefore is > _not_ inside quotes. And thus backslashing the quotes is wrong, as the > quote gets literally inserted into the CEILING_DIRECTORIES variable. > > I thought at first we could not need the quotes in the post-image > either, because shell variable assignments do not do word-splitting. > I.e.: > > FOO='with spaces' > BAR=$FOO sh -c 'echo $BAR' > > works just fine. $ x="two spaces" $ echo $x two spaces Maybe we should quote a little bit more religiously. > But $(CURDIR) here is not a shell variable, but rather a Makefile > variable, so it's expanded before we hit the shell. So we need the > quotes. And unfortunately it also breaks if $(CURDIR) contains exotic > metacharacters. If we cared we could use single quotes and $(CURDIR_SQ), > but I suspect it would be far from the first thing to break in such a > case. > > Which is a long-winded way of saying the patch looks correct to me. Thanks ;-) Ciao, Dscho
