Обсуждение: patch: version_stamp.pl: Add Git commit info to version if 'git' is specified

Hi,
this is a patch for src/tools/version_stamp.pl that adds the current Git
commit information from "git describe --tags --long" to the current
version string if "git" is specified on the command line. I've been
testing for regressions and such lately (none found, yay), and the
current output from for example "psql --version" only shows "psql
(PostgreSQL) 9.6devel" without any reference to which commit it was
compiled from.

The following is an example from current master when I compile it
locally:

  $ src/tools/version_stamp.pl git
  Stamped these files with version number 9.6devel+REL9_5_ALPHA1-331-g119cf76:
          configure.in
          doc/bug.template
          src/include/pg_config.h.win32
          src/interfaces/libpq/libpq.rc.in
          src/port/win32ver.rc
  Don't forget to run autoconf 2.69 before committing.
  $ autoconf
  $ ./configure
  [snip]
  $ make
  [snip]
  $ src/bin/psql/psql --version
  psql (PostgreSQL) 9.6devel+REL9_5_ALPHA1-331-g119cf76

The format I suggest for this is "[version]+[git_info]", as recommended
by the Semantic Version Specification (http://semver.org).

This format is not meant for public releases, of course, but I've found
it very useful when compiling local versions and I quickly want to see
which commit I have compiled from. The version string can also be used
together with git(1). For example, right now I have an ancient version
of REL9_5_STABLE from yesterday installed. To see if there are any new
commits I should check out, I can just use copy+paste to see what's
happened since then:

  $ git log --format=oneline 9.5alpha2+REL9_5_ALPHA2-82-gce56a64..
  440fc48cac7f450bb71d1f06f0d1326c63e3e42f dblink docs:  fix typo to use "connname" (3 n's), not "conname"

The patch applies cleanly with -p1 to all REL9_*_STABLE branches and
master except REL9_0_STABLE and REL9_1_STABLE (due to perltidy in
2bc09ff three years ago) and has been tested with all minor version
variatons that version_stamp.pl accepts.

In case something fishy happens to the mail, the commit is also
available from <https://github.com/sunny256/postgres/> in these
branches:

  git-versioning - squashed commit with suggested log message
  git-versioning.wip - the branch I worked on

Online patch:

  <https://github.com/sunny256/postgres/commit/0a6b8728f8105d05fd5b13c4b7ae80a44ff57576.patch>

Regards,
Øyvind

+-| Øyvind A. Holm <sunny@sunbase.org> - N 60.37604° E 5.33339° |-+
| OpenPGP: 0xFB0CBEE894A506E5 - http://www.sunbase.org/pubkey.asc |
| Fingerprint: A006 05D6 E676 B319 55E2  E77E FB0C BEE8 94A5 06E5 |
+------------| 681035c8-4d32-11e5-ab81-fefdb24f8e10 |-------------+

On Fri, Aug 28, 2015 at 12:14 PM, Øyvind A. <sunny@sunbase.org> wrote:
> this is a patch for src/tools/version_stamp.pl that adds the current Git
> commit information from "git describe --tags --long" to the current
> version string if "git" is specified on the command line. I've been
> testing for regressions and such lately (none found, yay), and the
> current output from for example "psql --version" only shows "psql
> (PostgreSQL) 9.6devel" without any reference to which commit it was
> compiled from.

What's the point? version_stamp.pl is used only by maintainers to bump
easily the version number in the code tree. And you can already append
a custom string after the version string with --with-extra-version in
configure. Here is for example one I use for development:
GIT_CURRENT=`cd $PGSOURCE && git rev-parse --short HEAD`
./configure --with-extra-version=-$GIT_CURRENT
This custom string is actually added in PG_VERSION and PG_VERSION_STR,
and generates things like that:
$ psql --version
psql (PostgreSQL) 9.6devel-2edb949

To be short, the method currently in place is far more flexible than
what you propose, and the patch you are proposing has the disadvantage
to create diffs in the code tree, which is not that helpful for
development purposes.

Regards,
--
Michael

Øyvind A. Holm <sunny@sunbase.org> writes:
> this is a patch for src/tools/version_stamp.pl that adds the current Git 
> commit information from "git describe --tags --long" to the current 
> version string if "git" is specified on the command line.

Salesforce did something similar in their internal build, and TBH I do not
find it a good idea.  The basic problem is it's completely misleading to
equate the last commit with the source you actually built from, because
that might not have been an unmodified file set.  It's particularly a bad
idea to bind the data in as early as version_stamp.pl, which even precedes
autoconf let alone configure+build.  There are too many opportunities to
modify the source and then later think that what you built exactly matches
the stamped commit hash.  (In the Salesforce hack, we actually captured
the commit hash at the time of running configure, two steps later than you
have here --- but that's still too early to have any confidence that the
hash matches the built files.)

As a more concrete complaint: it's physically impossible for something
done this way to match an official git commit, let alone an official
tarball, because the commit hash you're starting from would have to
predate a commit of the files that version_stamp.pl stamps.
        regards, tom lane

Michael Paquier <michael.paquier@gmail.com> writes:
> ... you can already append
> a custom string after the version string with --with-extra-version in
> configure. Here is for example one I use for development:
> GIT_CURRENT=`cd $PGSOURCE && git rev-parse --short HEAD`
> ./configure --with-extra-version=-$GIT_CURRENT

Yeah.  To clarify my earlier comment: what Salesforce did[1] was basically
to modify the configure script to do this automatically.  That meant that
even a heavily-hacked development build would still advertise itself as
having an identifiable commit hash.  I think that leads to at least as
much confusion as value added.

The only way that something like this can have any integrity is if the
hash is added in an automated, hands-off build process that works only
from clean git pulls.  The approach Michael suggests works just fine as
part of a build script that's used that way.  But I doubt that it's wise
to put it somewhere where the hash could end up in hand-modified builds.
        regards, tom lane

[1]: dept of full disclosure: said patch was actually my idea.  It was
a bad one.

On 28 August 2015 at 06:17, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Michael Paquier <michael.paquier@gmail.com> writes:
> > ... you can already append
> > a custom string after the version string with --with-extra-version
> > in configure. Here is for example one I use for development:
> > GIT_CURRENT=`cd $PGSOURCE && git rev-parse --short HEAD`
> > ./configure --with-extra-version=-$GIT_CURRENT
>
> Yeah.  To clarify my earlier comment: what Salesforce did[1] was
> basically to modify the configure script to do this automatically.
> That meant that even a heavily-hacked development build would still
> advertise itself as having an identifiable commit hash.  I think that
> leads to at least as much confusion as value added.
>
> The only way that something like this can have any integrity is if the
> hash is added in an automated, hands-off build process that works only
> from clean git pulls.  The approach Michael suggests works just fine
> as part of a build script that's used that way.  But I doubt that it's
> wise to put it somewhere where the hash could end up in hand-modified
> builds.

Thanks for the quick reponses. Yes, using --with-extra-version as
Michael mentioned is clearly a better solution than running
version_stamp.pl . I didn't know about that option, and executing
version_stamp.pl has its drawbacks, for example introducing local
changes to the working copy during compilation. It was solely meant as a
local workaround to identify which commit it came from.

Oh well, at least I learned how the patch delivery process works here,
that's at least something. :)

Thanks to you both for the info,
Øyvind

> Salesforce did something similar in their internal build, and TBH I do 
> not find it a good idea.  The basic problem is it's completely 
> misleading to equate the last commit with the source you actually built 
> from, because that might not have been an unmodified file set.

Indeed. What I've done in an svn-based project is to build the stamp from 
the Makefile basically when linking, that is really as late as possible. 
The other good point is that svnversion adds 'M' for modified if the 
source tree has uncommitted changes.

Maybe such an approach could be used with git to have something reliable.

-- 
Fabien.

On 2015-08-28 07:48:28 +0200, Fabien COELHO wrote:
> >Salesforce did something similar in their internal build, and TBH I do not
> >find it a good idea.  The basic problem is it's completely misleading to
> >equate the last commit with the source you actually built from, because
> >that might not have been an unmodified file set.
> 
> Indeed. What I've done in an svn-based project is to build the stamp from
> the Makefile basically when linking, that is really as late as possible. The
> other good point is that svnversion adds 'M' for modified if the source tree
> has uncommitted changes.
> 
> Maybe such an approach could be used with git to have something reliable.

I've done the same using the output $(git describe --tags --dirty) -
which will return something like REL9_5_ALPHA1-330-g8a7d070-dirty. That
is, the last tag, the number of commits since, the commit hash, and
whether the current build tree is dirty.

That's still not perfect considering plpgsql and such, but it's pretty
helpful.

Greetings,

Andres Freund

> I've done the same using the output $(git describe --tags --dirty) -
> which will return something like REL9_5_ALPHA1-330-g8a7d070-dirty.

Looks good!

> That's still not perfect considering plpgsql and such,

ISTM That even for plpgsql it could be done, the stamp can be generated 
when the shared object/dll is generated, and could be made available from 
some inspection function.

> but it's pretty helpful.

Yep.

-- 
Fabien.

On 2015-08-28 09:13:59 -0700, Jeff Janes wrote:
> On Fri, Aug 28, 2015 at 1:44 AM, Andres Freund <andres@anarazel.de> wrote:
> > I've done the same using the output $(git describe --tags --dirty) -
> > which will return something like REL9_5_ALPHA1-330-g8a7d070-dirty. That
> > is, the last tag, the number of commits since, the commit hash, and
> > whether the current build tree is dirty.
> >
> 
> That looks handy. But, why isn't it alpha2 rather than alpha1 ?

I was on master, and master branched after ALPHA1, not ALPHA2.

Andres