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

Wed Apr 21 13:31:24 PDT 2010

On Wed, 2010-04-21 at 16:01 -0400, Gaetan Nadon wrote:

> 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. 

In the makefile, $@ works. It's to get from the macro to there that is
tricky.

\$(@)    --> $(@)      works
\$\@     --> $\@        works
\$@      --> blank      fails
$@       --> blank      fails

> > 
> > > 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
> 
> _______________________________________________
> 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/20100421/f551b94a/attachment-0001.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/f551b94a/attachment-0001.pgp>