[PATCH weston v2] Makefile: Fix compilation error when git commit message has quotes

[Chokshi, Mitul]

Do we really need the git commit message (subject) in version string? What if we use commit hash instead? That will be a simple solution of replacing %s with %H 
i.e.
git --git-dir=$(top_srcdir)/.git log -1 --format='%H (%ci)'

Regards,
Mitul

-----Original Message-----
From: Pekka Paalanen [mailto:ppaalanen at gmail.com] 
Sent: Wednesday, April 13, 2016 3:46 PM
To: Chokshi, Mitul <mitul.chokshi at intel.com>
Cc: Bryce Harrington <bryce at osg.samsung.com>; ybakos at humanoriented.com; wayland-devel at lists.freedesktop.org
Subject: Re: [PATCH weston v2] Makefile: Fix compilation error when git commit message has quotes

On Thu, 7 Apr 2016 10:56:23 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:

> On Thu, Apr 07, 2016 at 04:53:31PM +0000, Chokshi, Mitul wrote:
> > 
> > A double-quote in log message prematurely ends the enquoted string
> > in src/git-version.h, causing an error during compilation.
> > Used stream editor to replace " with \"
> > 
> > Signed-off-by: Mitul Chokshi <mitul.chokshi at intel.com>
> > ---
> >  Makefile.am | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index d1644ac..a09ea0b 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -162,7 +162,7 @@ endif
> >  
> >  if HAVE_GIT_REPO
> >  src/git-version.h : $(top_srcdir)/.git/logs/HEAD
> > -	$(AM_V_GEN)echo "#define BUILD_ID \"$(shell git --git-dir=$(top_srcdir)/.git describe --always --dirty) $(shell git --git-dir=$(top_srcdir)/.git log -1 --format='%s (%ci)')\"" > $@
> > +	$(AM_V_GEN)echo "#define BUILD_ID \"$(shell git --git-dir=$(top_srcdir)/.git describe --always --dirty) $(shell git --git-dir=$(top_srcdir)/.git log -1 --format='%s (%ci)' | $(SED) 's|\"|\\\"|g' )\"" > $@
> >  else
> >  src/git-version.h :
> >  	$(AM_V_GEN)echo "#define BUILD_ID \"unknown (not built from git or tarball)\"" > $@  
> 
> Confirmed:
> 
> $ echo "foo" >> README
> $ git commit README -m "testing \""
> 
> $ make
> make  all-am
> make[1]: Entering directory `/home/bryce/src/Wayland/weston'
> /bin/bash: -c: line 0: syntax error near unexpected token `('
> /bin/bash: -c: line 0: `echo "  GEN     " src/git-version.h;echo "#define BUILD_ID \"1.9.0-168-gb1f52e1 testing " (2016-04-07 10:53:15 -0700)\"" > src/git-version.h'
> make[1]: *** [src/git-version.h] Error 1
> make[1]: Leaving directory `/home/bryce/src/Wayland/weston'
> make: *** [all] Error 2
> 
> With the patch applied, the make passes properly.
> 
> Tested-by: Bryce Harrington <bryce at osg.samsung.com>

Hi,

hmm, it doesn't work for me. I did the same test, having the subject
line 'testing "', and I get:

$ make
make  all-am
make[1]: Entering directory '/home/pq/build/weston'
  GEN      src/git-version.h
  CC       src/weston-compositor.o
In file included from /home/pq/git/weston/src/compositor.c:62:0:
./src/git-version.h:1:81: warning: missing terminating " character [enabled by default]
 #define BUILD_ID "1.10.0-90-g610e0a7-dirty testing " (2016-04-13 17:32:41 +0300)"
                                                                                 ^
  CC       src/weston-main.o
In file included from /home/pq/git/weston/src/main.c:47:0:
./src/git-version.h:1:81: warning: missing terminating " character [enabled by default]
 #define BUILD_ID "1.10.0-90-g610e0a7-dirty testing " (2016-04-13 17:32:41 +0300)"
                                                                                 ^
/home/pq/git/weston/src/main.c: In function ‘main’:
./src/git-version.h:1:66: error: expected ‘)’ before numeric constant
 #define BUILD_ID "1.10.0-90-g610e0a7-dirty testing " (2016-04-13 17:32:41 +0300)"
                                                                  ^
/home/pq/git/weston/src/main.c:737:6: note: in expansion of macro ‘BUILD_ID’
      BUILD_ID);
      ^
./src/git-version.h:1:54: error: called object is not a function or function pointer
 #define BUILD_ID "1.10.0-90-g610e0a7-dirty testing " (2016-04-13 17:32:41 +0300)"
                                                      ^
/home/pq/git/weston/src/main.c:737:6: note: in expansion of macro ‘BUILD_ID’
      BUILD_ID);
      ^
./src/git-version.h:1:66: error: missing terminating " character
 #define BUILD_ID "1.10.0-90-g610e0a7-dirty testing " (2016-04-13 17:32:41 +0300)"
                                                                  ^
/home/pq/git/weston/src/main.c:737:6: note: in expansion of macro ‘BUILD_ID’
      BUILD_ID);
      ^
Makefile:5398: recipe for target 'src/weston-main.o' failed
make[1]: *** [src/weston-main.o] Error 1
make[1]: Leaving directory '/home/pq/build/weston'
Makefile:2982: recipe for target 'all' failed
make: *** [all] Error 2


So even if the Makefile command invocation was fixed, the C
preprocessor token is still broken. Touching just README probably did
not cause any .c files to be rebuilt.

Maybe the git strings should not go as arguments to 'echo', so we avoid
one level of quoting. So something like this in bash:

( echo -n '#define BUILD_ID "' &&
  ( git --git-dir=$(top_srcdir)/.git describe --always --dirty &&
    git --git-dir=$(top_srcdir)/.git log -1 --format='%s (%ci)' ) | sed ... &&
  echo '"' ) > $@

An excercise for the reader is to figure out if there are any bashisms
in that, how to write it in Makefile.am, and what the sed script needs
to be. And perhaps make sure to remove stray newlines from the string.


Thanks,
pq
--------------------------------------------------------------
Intel Research and Development Ireland Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263


This e-mail and any attachments may contain confidential material for the sole
use of the intended recipient(s). Any review or distribution by others is
strictly prohibited. If you are not the intended recipient, please contact the
sender and delete all copies.