[PATCH util-macros] XORG_GIT_MODULE_VERSION: writes module version in xorg-git-version.h

Tue Apr 20 11:10:18 PDT 2010

On Mon, 2010-04-19 at 21:34 -0700, Dan Nicholson wrote:

> On Mon, Apr 19, 2010 at 5:54 PM, Peter Hutterer
> <peter.hutterer at who-t.net> wrote:
> > On Mon, Apr 19, 2010 at 11:00:08PM +0200, Julien Cristau wrote:
> >> On Mon, Apr 19, 2010 at 14:52:55 -0400, Gaetan Nadon wrote:
> >>
> >> > Generates the git module version according to the "git describe HEAD"
> >> > If the git module has pending changes, it appends "-dirty" to the version tag
> >> >
> >> > Signed-off-by: Gaetan Nadon <memsize at videotron.ca>
> >> > ---
> >> >  xorg-macros.m4.in |   18 ++++++++++++++++++
> >> >  1 files changed, 18 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/xorg-macros.m4.in b/xorg-macros.m4.in
> >> > index 916b472..efb2e53 100644
> >> > --- a/xorg-macros.m4.in
> >> > +++ b/xorg-macros.m4.in
> >> > @@ -1009,3 +1009,21 @@ mv \$(top_srcdir)/.INSTALL.tmp \$(top_srcdir)/INSTALL) \
> >> >  echo 'util-macros \"pkgdatadir\" from xorg-macros.pc not found: installing possibly empty INSTALL.' >&2)"
> >> >  AC_SUBST([INSTALL_CMD])
> >> >  ]) # XORG_INSTALL
> >> > +
> >> > +# XORG_GIT_MODULE_VERSION()
> >> > +# -------------------------
> >> > +# Minimum version: 1.8.0
> >> > +#
> >> > +# Generates the git module version according to the "git describe HEAD"
> >> > +# If the git module has pending changes, it appends "-dirty" to the version tag
> >> > +#
> >> > +AC_DEFUN([XORG_GIT_MODULE_VERSION], [
> >> > +GIT_MODULE_VERSION_CMD="VER=\`GIT_DIR=\$(top_srcdir)/.git git describe HEAD 2>/dev/null\`; \
> >> > +DVER=\`GIT_DIR=\$(top_srcdir)/.git git diff-index HEAD 2>/dev/null\`; \
> >> > +OUTSTR=\"\#undef XORG_GIT_VERSION\" ; \
> >> > +OUTFILE=\"xorg-git-version.h\"; \
> >> > +test -n \"\$\$VER\" && OUTSTR=\"\#define XORG_GIT_VERSION \$\$VER\" && test -n \"\$\$DVER\" && OUTSTR=\"\#define XORG_GIT_VERSION \$\$VER-dirty\"; \
> >> > +test -e \"\$\$OUTFILE\" || echo \"\$\$OUTSTR\" > \"\$\$OUTFILE\"; \
> >> > +CONTENT=\`cat \$\$OUTFILE\` && test \"\$\$CONTENT\" = \"\$\$OUTSTR\" || echo \$\$OUTSTR > \$\$OUTFILE;"
> >> > +AC_SUBST([GIT_MODULE_VERSION_CMD])
> >> > +]) # XORG_GIT_MODULE_VERSION
> >
> > my eyes!
> >
> >> As I said in reply to the initial patch by Peter I'd like a way to
> >> disable this, because packages might be built from a git tree, which
> >> might be unrelated to the xorg one, or outside of any git tree.  What's
> >> the output like if not building from git?  Does 'make
> >> GIT_MODULE_VERSION_CMD=:' work to disable it (I guess not, because
> >> anything trying to include xorg-git-version.h will be unhappy)?
> >
> > it simply sets the #undef and nothing will be printed to the log file. Is
> > there any specific argument against _running_ the macro as long if it
> > doesn't add anything to the logfile?
> >
> > Gaetan:
> > I'm not a big fan of the -dirty either but I guess if others want it we can
> > leave it in.  Two questions though:
> > - If modules start using this macro, do they stillhave to add to
> >   DISTCLEANFILES and friends?
> 
> I think it would be run all the time depending on .git/HEAD (or
> .git/`git symbolic-ref HEAD` like krh suggested), but it entirely
> depends what you do in the Makefile.am.
> 
> > - This macro only gets invoked on automake runs, right? So if I pull and
> >  just rebuild, would it update the git version?
> 
> It depends how fancy you want to be with the Makefile.am. To me, the
> following is "the right way to do it", but it requires touching more
> files.
> 
> configure.ac:
> AM_CONDITIONAL([USING_GIT], [test -f "$srcdir/.git/HEAD"])
> 
> Makefile.am:
> if USING_GIT
> GIT_HEAD = $(top_srcdir)/.git/HEAD
> endif
> noinst_HEADERS = git-xorg-version.h
> git-xorg-version.h: $(GIT_HEAD)
>     $(AM_V_GEN)$(GIT_MODULE_VERSION_CMD) $@
> 
> If you have a checkout, the header depends on .git/HEAD, so it will
> get rebuilt any time HEAD gets updated. Actually, that doesn't seem
> like it would work that well since HEAD seems to take the modification
> time of the ref it's following, so the header wouldn't get updated if
> you checked out an older branch. Probably best would be:
> 
> noinst_HEADERS = git-xorg-version.h
> git-xorg-version.h:
>     $(AM_V_GEN)$(GIT_MODULE_VERSION_CMD) $@
> .PHONY: git-xorg-version.h
> 

Sounds the best to me. I would put the generated file in
DISTCLEANFILES. 
It will cause less rebuilds than if it is in CLEANFILES, in the case the
generated
.h file is included by millions of other .h files and you just clean one
subtree.

> And then you'd just have to depend on GIT_MODULE_VERSION_CMD not
> updating the file unnecessarily to prevent spurious rebuilds. Gaeton's
> version seems to do that.
> 

It does.

I posted the patch v2 yesterday. Is the usage of $@ is correct? 

        VER=`GIT_DIR=$(top_srcdir)/.git git describe HEAD 2>/dev/null`; 
        DVER=`GIT_DIR=$(top_srcdir)/.git git diff-index HEAD 2>/dev/null`; 
        OUTSTR="\#undef XORG_GIT_VERSION" ; 
        OUTFILE=$(@); 
        test -n "$$VER" && OUTSTR="\#define XORG_GIT_VERSION $$VER"; 
        test -n "$$DVER" && OUTSTR="$$OUTSTR-with-uncommitted-changes"; 
        test -e "$$OUTFILE" || echo "$$OUTSTR" > "$$OUTFILE"; 
        CONTENT=`cat $$OUTFILE` && test "$$CONTENT" = "$$OUTSTR" || echo $$OUTSTR > $$OUTFILE;

It's no so bad when you look at it in the Makefile, with a different
hair do. Just a couple of backslashes.

Last call if reviewers want to remove with-uncommitted-changes.

> --
> Dan
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20100420/1d1ac273/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20100420/1d1ac273/attachment.pgp>