Re: [PATCH] git-version-gen: Fix for tags containing '-'

[Markus Armbruster]

Paul Eggert  writes:

> Markus Armbruster wrote:
>> Should we replace /-([^-]+)-g([^-]*)$/ by /.\1-\2/?  Extended regexp
>> for clarity.  Tighter matching would be possible, say
>> /-([0-9]+)-g[0-9a-f]{4,}$/.
>
> Sounds good, I installed the attached.

Works for me, thanks!

Re: [PATCH] git-version-gen: Fix for tags containing '-'

[Paul Eggert]

Markus Armbruster wrote:

Should we replace /-([^-]+)-g([^-]*)$/ by /.\1-\2/?  Extended regexp
for clarity.  Tighter matching would be possible, say
/-([0-9]+)-g[0-9a-f]{4,}$/.


Sounds good, I installed the attached.
>From 45a1331864ed37db2cd058a99eb9934648d4c342 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sun, 6 Aug 2017 23:24:10 -0700
Subject: [PATCH] git-version-gen: another fix for tags with "-"

* build-aux/git-version-gen: Improve fix for tags containing "-".
Suggested by Markus Armbruster in:
http://lists.gnu.org/archive/html/bug-gnulib/2017-08/msg00034.html
---
 ChangeLog | 7 +++
 build-aux/git-version-gen | 8 
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8282f2e..37daf4c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2017-08-06  Paul Eggert  
+
+	git-version-gen: another fix for tags with "-"
+	* build-aux/git-version-gen: Improve fix for tags containing "-".
+	Suggested by Markus Armbruster in:
+	http://lists.gnu.org/archive/html/bug-gnulib/2017-08/msg00034.html
+
 2017-08-06  Bruno Haible  
 
 	warnings, manywarnings: Add support for multiple languages, not just C.
diff --git a/build-aux/git-version-gen b/build-aux/git-version-gen
index e6c329d..a8818b2 100755
--- a/build-aux/git-version-gen
+++ b/build-aux/git-version-gen
@@ -1,6 +1,6 @@
 #!/bin/sh
 # Print a version string.
-scriptversion=2017-08-06.16; # UTC
+scriptversion=2017-08-07.06; # UTC
 
 # Copyright (C) 2007-2017 Free Software Foundation, Inc.
 #
@@ -184,9 +184,9 @@ then
 ;;
 esac
 
-# Change the first '-' to a '.', so version-comparing tools work properly.
-# Remove the "g" in git describe's output string, to save a byte.
-v=`echo "$v" | sed 's/-/./;s/\(.*\)-g/\1-/'`;
+# Change the penultimate "-" to ".", for version-comparing tools.
+# Remove the "g" to save a byte.
+v=`echo "$v" | sed 's/-\([^-]*\)-g\([^-]*\)$/.\1-\2/'`;
 v_from_git=1
 elif test "x$fallback" = x || git --version >/dev/null 2>&1; then
 v=UNKNOWN
-- 
2.7.4

Re: [PATCH] git-version-gen: Fix for tags containing '-'

[Markus Armbruster]

Paul Eggert  writes:

> Markus Armbruster wrote:
>> there's an additional problem my patch fails to address:
>>
>>  # Change the first '-' to a '.', so version-comparing tools work 
>> properly.
>>  # Remove the "g" in git describe's output string, to save a byte.
>>  v=`echo "$v" | sed 's/-/./;s/\(.*\)-g/\1-/'`;
>>
>> Messes with '-' in tags.  I think it should replace the first '-g'
>> instead.  What do you think?
>
> Sorry, I'm not following. Could you give an example of the problem?
> Evidently the code is messing with the first "-" and the last "-g"
> deliberately.

I was too terse, and not entirely accurate, sorry.  Let me try again.

If I understand this code's intent correctly, it tries to change the "-"
separating the tag from the number of additional commits to ".", and the
"-g" separating the number of additional commits from the abbreviated
object name to just "-".  Example:

v0.1-1496-gcbc7002

becomes

v0.1.1496-cbc7002

The problem is once again tag names containing "-", because then the
first "-" is *not* the one we want to change.

Example:

$ git-tag -m test v0.2-rc1 HEAD^
$ git-describe 
v0.2-rc1-1-gcbc7002
$ build-aux/git-version-gen .tarball-version; echo
0.2.rc1-1-cbc700

We change the tag name instead of the "-" seperating it from the number
of commits.

After the next commit, we'll get "0.2.rc1-2-cbc700".  Will those two
version-compare correctly?

Example:

$ git-tag -fm test v0.2-rc1
Updated tag 'v0.2-rc1' (was 1657134)
$ git-describe 
v0.2-rc1
$ build-aux/git-version-gen .tarball-version; echo
0.2.rc1

There is no number of commits to change.  We change the tag name
instead.

Should we replace /-([^-]+)-g([^-]*)$/ by /.\1-\2/?  Extended regexp
for clarity.  Tighter matching would be possible, say
/-([0-9]+)-g[0-9a-f]{4,}$/.

Re: [PATCH] git-version-gen: Fix for tags containing '-'

[Paul Eggert]

Markus Armbruster wrote:

there's an additional problem my patch fails to address:

 # Change the first '-' to a '.', so version-comparing tools work properly.
 # Remove the "g" in git describe's output string, to save a byte.
 v=`echo "$v" | sed 's/-/./;s/\(.*\)-g/\1-/'`;

Messes with '-' in tags.  I think it should replace the first '-g'
instead.  What do you think?


Sorry, I'm not following. Could you give an example of the problem? Evidently 
the code is messing with the first "-" and the last "-g" deliberately.

Re: [PATCH] git-version-gen: Fix for tags containing '-'

[Markus Armbruster]

Paul Eggert  writes:

> Thanks for reporting the problem. Unfortunately that patch didn't work
> for me, since Git sometimes outputs more than 4 hex digits after the
> "g", if needed to avoid ambiguity. Please try the attached further
> patch, which I installed.

You're right.  Your incremental patch works for me.  The pattern could
be tightened, but I figure it's good enough as is.

Hmm, there's an additional problem my patch fails to address: 

# Change the first '-' to a '.', so version-comparing tools work properly.
# Remove the "g" in git describe's output string, to save a byte.
v=`echo "$v" | sed 's/-/./;s/\(.*\)-g/\1-/'`;

Messes with '-' in tags.  I think it should replace the first '-g'
instead.  What do you think?

Re: [PATCH] git-version-gen: Fix for tags containing '-'

[Jim Meyering]

On Sun, Aug 6, 2017 at 9:22 AM, Paul Eggert  wrote:
> Thanks for reporting the problem. Unfortunately that patch didn't work for
> me, since Git sometimes outputs more than 4 hex digits after the "g", if
> needed to avoid ambiguity. Please try the attached further patch, which I
> installed.

Thanks for the quick fix.

Re: [PATCH] git-version-gen: Fix for tags containing '-'

[Paul Eggert]

Thanks for reporting the problem. Unfortunately that patch didn't work for me, 
since Git sometimes outputs more than 4 hex digits after the "g", if needed to 
avoid ambiguity. Please try the attached further patch, which I installed.
From 61c1c092bda1ff5dd0f31f8807795ee7993f411a Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sun, 6 Aug 2017 09:21:05 -0700
Subject: [PATCH] git-version-gen: port better to older Git

Work even if the hash contains more than 4 digits.
---
 build-aux/git-version-gen | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/build-aux/git-version-gen b/build-aux/git-version-gen
index 6c054b4..e6c329d 100755
--- a/build-aux/git-version-gen
+++ b/build-aux/git-version-gen
@@ -1,6 +1,6 @@
 #!/bin/sh
 # Print a version string.
-scriptversion=2017-08-06.05; # UTC
+scriptversion=2017-08-06.16; # UTC
 
 # Copyright (C) 2007-2017 Free Software Foundation, Inc.
 #
@@ -167,9 +167,9 @@ then
 # tag or the previous older version that did not?
 #   Newer: v6.10-77-g0f8faeb
 #   Older: v6.10-g0f8faeb
-case $v in
-*-*-g) : git describe is okay three part flavor ;;
-*-g)
+case ${v#-g*} in
+*-*) : git describe is probably okay three part flavor ;;
+*)
 : git describe is older two part flavor
 # Recreate the number of commits and rewrite such that the
 # result is the same as if we were using the newer version
-- 
2.7.4

Re: [PATCH] git-version-gen: Fix for tags containing '-'

[Jim Meyering]

On Sat, Aug 5, 2017 at 11:26 PM, Markus Armbruster  wrote:
> Really old versions of git-describe (before v1.5.0, Feb 2007) don't
> have the number of commits in their long format output, i.e. where
> modern 'git describe --abbrev=4 --match="v*"' prints
> "v0.1-1494-g124b9", they print "v0.1-1494-g124b9".  git-version-gen
> recognizes both patterns, and normalizes the old format to the new
> one.
>
> Unfortunately, this normalization code gets confused when the tag
> contains '-'.  Reproducer:
>
> $ git-tag -m test v0.2-rc1
> $ build-aux/git-version-gen .tarball-version; echo
> build-aux/git-version-gen: WARNING: git rev-list failed
> UNKNOWN
>
> We take exact tag "v0.2-rc1" for the old format, extract the presumed
> tag "v0.2" from it, then run "git rev-list v0.2..HEAD" to count
> commits since tha tag.  Fails, because tag "v0.2" does not exist.
>
> We could perhaps drop support for versions from more than a decade
> ago.  But tightening the pattern match is easy enough, so do that.
> Still breaks when you use version tags ending in something matching
> -g, but you arguably get what you deserve then.

Hi Markus!

Thank you for that patch.
I've tweaked the commit log, copied it into the ChangeLog file and pushed.

[PATCH] git-version-gen: Fix for tags containing '-'

[Markus Armbruster]

Really old versions of git-describe (before v1.5.0, Feb 2007) don't
have the number of commits in their long format output, i.e. where
modern 'git describe --abbrev=4 --match="v*"' prints
"v0.1-1494-g124b9", they print "v0.1-1494-g124b9".  git-version-gen
recognizes both patterns, and normalizes the old format to the new
one.

Unfortunately, this normalization code gets confused when the tag
contains '-'.  Reproducer:

$ git-tag -m test v0.2-rc1
$ build-aux/git-version-gen .tarball-version; echo
build-aux/git-version-gen: WARNING: git rev-list failed
UNKNOWN

We take exact tag "v0.2-rc1" for the old format, extract the presumed
tag "v0.2" from it, then run "git rev-list v0.2..HEAD" to count
commits since tha tag.  Fails, because tag "v0.2" does not exist.

We could perhaps drop support for versions from more than a decade
ago.  But tightening the pattern match is easy enough, so do that.
Still breaks when you use version tags ending in something matching
-g, but you arguably get what you deserve then.

Signed-off-by: Markus Armbruster 
---
 build-aux/git-version-gen | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/build-aux/git-version-gen b/build-aux/git-version-gen
index 079849d..6c054b4 100755
--- a/build-aux/git-version-gen
+++ b/build-aux/git-version-gen
@@ -1,6 +1,6 @@
 #!/bin/sh
 # Print a version string.
-scriptversion=2017-01-09.19; # UTC
+scriptversion=2017-08-06.05; # UTC
 
 # Copyright (C) 2007-2017 Free Software Foundation, Inc.
 #
@@ -168,8 +168,8 @@ then
 #   Newer: v6.10-77-g0f8faeb
 #   Older: v6.10-g0f8faeb
 case $v in
-*-*-*) : git describe is okay three part flavor ;;
-*-*)
+*-*-g) : git describe is okay three part flavor ;;
+*-g)
 : git describe is older two part flavor
 # Recreate the number of commits and rewrite such that the
 # result is the same as if we were using the newer version
-- 
2.7.5