threads / patch / 35125

v4, 2 partsversion-gen: cleanup

Subject: [PATCH v4 1/2] version-gen: cleanup

## tl;dr

7 messages between Oct 12, 2013 and Oct 14, 2013. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Felipe Contreras· Oct 12, 2013, 07:07 UTC · lore

[PATCH v4 0/2] Version fixes and cleanups

Chances since v3:
Show changes to 2 files +4 −2

GIT-VERSION-GEN, Makefile

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 19902e9..c04c4de 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -26,8 +26,10 @@ describe () {
 if test -f version
 then
        VN=$(cat version) || VN="$DEF_VER"
-elif test ! -d ${GIT_DIR:-.git} -a ! -f .git || ! describe
+elif test -d ${GIT_DIR:-.git} -o -f .git && describe
 then
+       VN=$(echo "$VN" | sed -e 's/-/~/g')
+else
        VN="$DEF_VER"
 fi
 
diff --git a/Makefile b/Makefile
index 7a8bee7..3588ca1 100644
--- a/Makefile
+++ b/Makefile
@@ -2426,7 +2426,7 @@ quick-install-html:
 ### Maintainer's dist rules
 
 git.spec: git.spec.in GIT-VERSION-FILE
-       sed -e 's/@@VERSION@@/$(subst -,~,$(GIT_VERSION))/g' < $< > $@+
+       sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+
        mv $@+ $@
 
 GIT_TARNAME = git-$(GIT_VERSION)

Felipe Contreras (2):
  version-gen: cleanup
  version-gen: fix versions

 GIT-VERSION-GEN | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

-- 
1.8.4-fc
Felipe Contreras· Oct 12, 2013, 07:07 UTC · re: Felipe Contreras · lore
No functional changes.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)
Show changes to GIT-VERSION-GEN +20 −16
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 06026ea..e96538d 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -6,22 +6,29 @@ DEF_VER=v1.8.4
 LF='
 '
 
-# First see if there is a version file (included in release tarballs),
-# then try git-describe, then default.
-if test -f version
-then
-	VN=$(cat version) || VN="$DEF_VER"
-elif test -d ${GIT_DIR:-.git} -o -f .git &&
-	VN=$(git describe --match "v[0-9]*" --abbrev=7 HEAD 2>/dev/null) &&
+describe () {
+	VN=$(git describe --match "v[0-9]*" --abbrev=7 HEAD 2>/dev/null) || return 1
 	case "$VN" in
-	*$LF*) (exit 1) ;;
+	*$LF*)
+		return 1
+		;;
 	v[0-9]*)
 		git update-index -q --refresh
 		test -z "$(git diff-index --name-only HEAD --)" ||
-		VN="$VN-dirty" ;;
+		VN="$VN-dirty"
+		return 0
+		;;
 	esac
+}
+
+# First see if there is a version file (included in release tarballs),
+# then try 'git describe', then default.
+if test -f version
+then
+	VN=$(cat version) || VN="$DEF_VER"
+elif test -d ${GIT_DIR:-.git} -o -f .git && describe
 then
-	VN=$(echo "$VN" | sed -e 's/-/./g');
+	VN=$(echo "$VN" | sed -e 's/-/./g')
 else
 	VN="$DEF_VER"
 fi
@@ -34,9 +41,6 @@ then
 else
 	VC=unset
 fi
-test "$VN" = "$VC" || {
-	echo >&2 "GIT_VERSION = $VN"
-	echo "GIT_VERSION = $VN" >$GVF
-}
-
-
+test "$VN" = "$VC" && exit
+echo >&2 "GIT_VERSION = $VN"
+echo "GIT_VERSION = $VN" >$GVF

-- 
1.8.4-fc
Felipe Contreras· Oct 12, 2013, 07:07 UTC · re: Felipe Contreras · lore

[PATCH v4 2/2] version-gen: fix versions

Virtually all packaging guidelines would prefer 1.8.4~rc1, over 1.8.4.rc1 or 1.8.4-rc1, so it makes sense to use that instead.

In particular, the only packaging we provide, git.spec, generates a wrong version, because git-1.8.4 < git-1.8.4.rc1, changing to ~rc1 fixes the problem as it's considered newer.

The same happens in dpkg.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to GIT-VERSION-GEN +1 −1
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index e96538d..c04c4de 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -28,7 +28,7 @@ then
 	VN=$(cat version) || VN="$DEF_VER"
 elif test -d ${GIT_DIR:-.git} -o -f .git && describe
 then
-	VN=$(echo "$VN" | sed -e 's/-/./g')
+	VN=$(echo "$VN" | sed -e 's/-/~/g')
 else
 	VN="$DEF_VER"
 fi

-- 
1.8.4-fc
David Aguilar· Oct 13, 2013, 21:56 UTC · re: Felipe Contreras · lore

Re: [PATCH v4 2/2] version-gen: fix versions

On Sat, Oct 12, 2013 at 12:07 AM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 28 quoted lines
> Virtually all packaging guidelines would prefer 1.8.4~rc1, over
> 1.8.4.rc1 or 1.8.4-rc1, so it makes sense to use that instead.
>
> In particular, the only packaging we provide, git.spec, generates a
> wrong version, because git-1.8.4 < git-1.8.4.rc1, changing to ~rc1 fixes
> the problem as it's considered newer.
>
> The same happens in dpkg.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  GIT-VERSION-GEN | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
> index e96538d..c04c4de 100755
> --- a/GIT-VERSION-GEN
> +++ b/GIT-VERSION-GEN
> @@ -28,7 +28,7 @@ then
>         VN=$(cat version) || VN="$DEF_VER"
>  elif test -d ${GIT_DIR:-.git} -o -f .git && describe
>  then
> -       VN=$(echo "$VN" | sed -e 's/-/./g')
> +       VN=$(echo "$VN" | sed -e 's/-/~/g')
>  else
>         VN="$DEF_VER"
>  fi
> --
This seems related:
http://lintian.debian.org/tags/rc-version-greater-than-expected-version.html

Should the RC tags in the Git repo be named v1.2.3~rc4 (tilde-rc#) instead of dash-rc#, or does that not matter?

If so, would that change anything about this patch, or is it better to normalize it all here?

The input is subtly different sometimes so I'm curious whether whether
"~" is preferred in all cases (particularly, by all package managers).
 e.g.

$ git describe v1.5.0^ v1.5.0-rc4-372-g26cfcfb

$ git describe v1.5.0.1^ v1.5.0-27-g38eb932

-- 
David
Felipe Contreras· Oct 14, 2013, 02:42 UTC · re: David Aguilar · lore

Re: [PATCH v4 2/2] version-gen: fix versions

On Sun, Oct 13, 2013 at 4:56 PM, David Aguilar <davvid@gmail.com> wrote:
Show 37 quoted lines
> On Sat, Oct 12, 2013 at 12:07 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> Virtually all packaging guidelines would prefer 1.8.4~rc1, over
>> 1.8.4.rc1 or 1.8.4-rc1, so it makes sense to use that instead.
>>
>> In particular, the only packaging we provide, git.spec, generates a
>> wrong version, because git-1.8.4 < git-1.8.4.rc1, changing to ~rc1 fixes
>> the problem as it's considered newer.
>>
>> The same happens in dpkg.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>  GIT-VERSION-GEN | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
>> index e96538d..c04c4de 100755
>> --- a/GIT-VERSION-GEN
>> +++ b/GIT-VERSION-GEN
>> @@ -28,7 +28,7 @@ then
>>         VN=$(cat version) || VN="$DEF_VER"
>>  elif test -d ${GIT_DIR:-.git} -o -f .git && describe
>>  then
>> -       VN=$(echo "$VN" | sed -e 's/-/./g')
>> +       VN=$(echo "$VN" | sed -e 's/-/~/g')
>>  else
>>         VN="$DEF_VER"
>>  fi
>> --
>
> This seems related:
>
> http://lintian.debian.org/tags/rc-version-greater-than-expected-version.html
>
> Should the RC tags in the Git repo be named v1.2.3~rc4 (tilde-rc#)
> instead of dash-rc#, or does that not matter?
I thought so first, but then I realized ~ is not allowed in a ref.
Show 6 quoted lines
> If so, would that change anything about this patch, or is it better to
> normalize it all here?
>
> The input is subtly different sometimes so I'm curious whether whether
> "~" is preferred in all cases (particularly, by all package managers).
>  e.g.

All package managers I investigated do handle ~ specially, and thus recommend it for rc versioning, except pacman. So in pacman, v1.5.0~rc4 would remain newer than v1.5.0, but that's not different from the current situation, and there isn't much we can do about that.

Show 5 quoted lines
> $ git describe v1.5.0^
> v1.5.0-rc4-372-g26cfcfb
>
> $ git describe v1.5.0.1^
> v1.5.0-27-g38eb932
At least both in RPM and dpkg, 1.5.0~27 is newer than 1.5.0~rc4.
-- 
Felipe Contreras
Jonathan Nieder· Oct 14, 2013, 05:01 UTC · re: David Aguilar · lore

Re: [PATCH v4 2/2] version-gen: fix versions

Hi,
David Aguilar wrote:
> Felipe Contreras <felipe.contreras@gmail.com> wrote:
Show 6 quoted lines
>> Virtually all packaging guidelines would prefer 1.8.4~rc1, over
>> 1.8.4.rc1 or 1.8.4-rc1, so it makes sense to use that instead.
>>
>> In particular, the only packaging we provide, git.spec, generates a
>> wrong version, because git-1.8.4 < git-1.8.4.rc1, changing to ~rc1 fixes
>> the problem as it's considered newer.

A more conservative fix would be to tweak the .spec generation in the Makefile to follow whatever the appropriate Red Hat convention is. For example, something like this:

-- >8 --
Show changes to diff +2 −2
diff --git i/Makefile w/Makefile
index 0f931a2..73bd89d 100644
--- i/Makefile
+++ w/Makefile
@@ -2385,8 +2385,9 @@ quick-install-html:
 
 ### Maintainer's dist rules
 
+GIT_VERSION_RPM = $(subst -rc,~rc,$(GIT_VERSION))
 git.spec: git.spec.in GIT-VERSION-FILE
-	sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+
+	sed -e 's/@@VERSION@@/$(GIT_VERSION_RPM)/g' < $< > $@+
 	mv $@+ $@
 
 GIT_TARNAME = git-$(GIT_VERSION)
-- 8< --

That way, programs that parse the git version by splitting at '.'
(there are more than a few, unfortunately) would continue to work, but
the packaging system would get the benefit of the proposed versioning
style change.

>> The same happens in dpkg.

Have you tested this?  I thought the Debian packaging did not use the
GIT-VERSION-GEN generated version in this way.

[...]
> This seems related:
>
> http://lintian.debian.org/tags/rc-version-greater-than-expected-version.html

If I understand correctly, that page has an exhaustive list of affected
packages in the Debian archive and doesn't include git.

Thanks and hope that helps,
Jonathan
Felipe Contreras· Oct 14, 2013, 05:29 UTC · re: Jonathan Nieder · lore

Re: [PATCH v4 2/2] version-gen: fix versions

On Mon, Oct 14, 2013 at 12:01 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 15 quoted lines
> Hi,
>
> David Aguilar wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> wrote:
>
>>> Virtually all packaging guidelines would prefer 1.8.4~rc1, over
>>> 1.8.4.rc1 or 1.8.4-rc1, so it makes sense to use that instead.
>>>
>>> In particular, the only packaging we provide, git.spec, generates a
>>> wrong version, because git-1.8.4 < git-1.8.4.rc1, changing to ~rc1 fixes
>>> the problem as it's considered newer.
>
> A more conservative fix would be to tweak the .spec generation in the
> Makefile to follow whatever the appropriate Red Hat convention is.
> For example, something like this:

It's not Red Hat's convention, it's RPM, and dpkg, so basically every package manager that most distributions out there use.

And I already sent a patch for that which was ignored:
http://article.gmane.org/gmane.comp.version-control.git/234794
Show 10 quoted lines
> -- >8 --
> diff --git i/Makefile w/Makefile
> index 0f931a2..73bd89d 100644
> --- i/Makefile
> +++ w/Makefile
> @@ -2385,8 +2385,9 @@ quick-install-html:
>
>  ### Maintainer's dist rules
>
> +GIT_VERSION_RPM = $(subst -rc,~rc,$(GIT_VERSION))

That wouldn't work; VERSION doesn't have '-rc', it has '.rc'. and what's the point of creating a new variable? It's a was of space.

Show 10 quoted lines
>  git.spec: git.spec.in GIT-VERSION-FILE
> -       sed -e 's/@@VERSION@@/$(GIT_VERSION)/g' < $< > $@+
> +       sed -e 's/@@VERSION@@/$(GIT_VERSION_RPM)/g' < $< > $@+
>         mv $@+ $@
>
>  GIT_TARNAME = git-$(GIT_VERSION)
> -- 8< --
>
> That way, programs that parse the git version by splitting at '.'
> (there are more than a few, unfortunately) would continue to work,

Do you have any evidence that such programs exist? Specifically, programs that work with 1.8.4.rc1, but not 1.8.4-rc1 or 1.8.4~rc1. I find that very very very unlikely.

Anyway, in the very very very unlikely scenario that somebody's script does break they can report that, and we can revert. What's the problem?

Show 7 quoted lines
> but
> the packaging system would get the benefit of the proposed versioning
> style change.
>
>>> The same happens in dpkg.
>
> Have you tested this?
% dpkg --compare-versions 1.8.4 gt 1.8.4~rc1 && echo yes || echo no
yes
 % dpkg --compare-versions 1.8.4 gt 1.8.4-rc1 && echo yes || echo no
no
> I thought the Debian packaging did not use the
> GIT-VERSION-GEN generated version in this way.

It doesn't matter. The name of the package would be git-1.8.4~rc1, and 'git --version' would return 1.8.4.rc1, that's inconsistent.

Why be inconsistent when we can be consistent?
Show 7 quoted lines
> [...]
>> This seems related:
>>
>> http://lintian.debian.org/tags/rc-version-greater-than-expected-version.html
>
> If I understand correctly, that page has an exhaustive list of affected
> packages in the Debian archive and doesn't include git.

Because a) they don't package release candidates, and b) they use a different version (s/\./~/).

-- 
Felipe Contreras

← back to recent threads