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

Wed Apr 21 13:01:32 PDT 2010

On Wed, 2010-04-21 at 06:03 -0700, Dan Nicholson wrote:

> On Tue, Apr 20, 2010 at 11:10 AM, Gaetan Nadon <memsize at videotron.ca> wrote:
> > 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.
> 
> Yeah, I think you'll need to, anyway, to pass distcheck.
> 
> > 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;
> 
> I would personally replace all occurrences of $$OUTFILE with $@. I
> don't think the () are necessary, either.

I got there by trial and error. It looks like if I want to access a
variable defined outside the script,
I need $(VAR) whereby if I define a variable inside the script I need
$$VAR toa cess it.
 It does work, but I hope it's not by accident.

> 
> > 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.
> 
> I'd rather not have it since it doesn't tell me anything useful, but
> if other people do want it, that's OK.
> 

Peter is not too much in favor of it either. I'll remove it and it'll be
the "short" solution. If we want more functions and robustness then we
should copy the radeonhd one.

> --
> Dan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20100421/806a9054/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/20100421/806a9054/attachment.pgp>