# [PATCH 00/18] version-gen: complete revamp/rewrite

24 messages from 2023-04-14 to 2023-04-14. Participants: Felipe Contreras, Todd Zullinger.
Thread: https://gitlist.dev/t/59597

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 00/18] version-gen: complete revamp/rewrite
Message-ID: <20230414121841.373980-1-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-1-felipe.contreras%40gmail.com

```
The version generation script needs some love, as the last true change was done in 2008.

This series step by step revamps the whole script ending in only a few lines of code:

    get_version () {
        test -f version && cat version && return
        git describe --match "v[0-9]*" --dirty 2>/dev/null | sed -e 's/-/+/' -e 's/^v//'
    }

    NEW="GIT_VERSION = $(get_version)"

    test -r GIT-VERSION-FILE && test "$NEW" = "$(cat GIT-VERSION-FILE)" && exit
    echo "$NEW" | tee GIT-VERSION-FILE >&2

There should be no functional changes except for the last patch that changes
interim version from `2.40.0.$n.g${oid}` to `2.40.0+$n-g${oid}` as that causes
the proper sorting.

It's hard to see the actual changes to this script as 99% of the commits are
just to bump the default version (which I don't know why it even exists).

For reference, cleaning the history I came up with these actual changes in case
anyone is interested:

c48799e560 (Teach GIT-VERSION-GEN about the .git file, 2008-02-20)
1100ac81a9 (Change GIT-VERSION-GEN to call git commands with "git" not "git-"., 2006-05-22)
374dfaa2e3 (Make GIT-VERSION-GEN tolerate missing git describe command again, 2006-01-26)
5c7d3c9507 (Allow building of RPM from interim snapshot., 2006-01-16)
181129d24c (For release tarballs, include the proper version, 2006-01-09)
026351a035 (Make GIT-VERSION-GEN tolerate missing git describe command, 2005-12-30)
9b88fcef7d (Makefile: use git-describe to mark the git version., 2005-12-27)

Cheers.

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 9a1111af9b..99584bf86d 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -1,40 +1,11 @@
 #!/bin/sh
 
-GVF=GIT-VERSION-FILE
-DEF_VER=v2.40.GIT
-
-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]*" HEAD 2>/dev/null) &&
-	case "$VN" in
-	*$LF*) (exit 1) ;;
-	v[0-9]*)
-		git update-index -q --refresh
-		test -z "$(git diff-index --name-only HEAD --)" ||
-		VN="$VN-dirty" ;;
-	esac
-then
-	VN=$(echo "$VN" | sed -e 's/-/./g');
-else
-	VN="$DEF_VER"
-fi
-
-VN=$(expr "$VN" : v*'\(.*\)')
-
-if test -r $GVF
-then
-	VC=$(sed -e 's/^GIT_VERSION = //' <$GVF)
-else
-	VC=unset
-fi
-test "$VN" = "$VC" || {
-	echo >&2 "GIT_VERSION = $VN"
-	echo "GIT_VERSION = $VN" >$GVF
+get_version () {
+	test -f version && cat version && return
+	git describe --match "v[0-9]*" --dirty 2>/dev/null | sed -e 's/-/+/' -e 's/^v//'
 }
+
+NEW="GIT_VERSION = $(get_version)"
+
+test -r GIT-VERSION-FILE && test "$NEW" = "$(cat GIT-VERSION-FILE)" && exit
+echo "$NEW" | tee GIT-VERSION-FILE >&2

Felipe Contreras (18):
  version-gen: reorganize
  version-gen: trivial cleanup
  version-gen: refactor default version
  version-gen: simplify v prefix removal
  version-gen: simplify update check
  version-gen: remove redundant check
  version-gen: simplify `git describe` checks
  version-gen: simplify dirty check
  version-gen: move describe fix into function
  version-gen: describe and sed in one go
  version-gen: refactor describe function
  version-gen: do v fix only when necessary
  version-gen: move v fix into sed
  version-gen: refactor main functionality
  version-gen: remove default version
  version-gen: refactor GIT_VERSION string
  version-gen: get rid of GVF variable
  version-gen: generate proper interim versions

 GIT-VERSION-GEN | 45 ++++++++-------------------------------------
 1 file changed, 8 insertions(+), 37 deletions(-)

-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 01/18] version-gen: reorganize
Message-ID: <20230414121841.373980-2-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-2-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
Simply move some code into a `describe` function so it's clear what code
is related to dealing with `git describe`, and what code is the main
functionality.

No functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 9a1111af9b..29d634a30b 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -6,20 +6,27 @@ DEF_VER=v2.40.GIT
 LF='
 '
 
+describe () {
+	VN=$(git describe --match "v[0-9]*" HEAD 2>/dev/null) || return 1
+	case "$VN" in
+	*$LF*)
+		return 1
+		;;
+	v[0-9]*)
+		git update-index -q --refresh
+		test -z "$(git diff-index --name-only HEAD --)" ||
+		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 &&
-	VN=$(git describe --match "v[0-9]*" HEAD 2>/dev/null) &&
-	case "$VN" in
-	*$LF*) (exit 1) ;;
-	v[0-9]*)
-		git update-index -q --refresh
-		test -z "$(git diff-index --name-only HEAD --)" ||
-		VN="$VN-dirty" ;;
-	esac
+elif test -d ${GIT_DIR:-.git} -o -f .git && describe
 then
 	VN=$(echo "$VN" | sed -e 's/-/./g');
 else
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 02/18] version-gen: trivial cleanup
Message-ID: <20230414121841.373980-3-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-3-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
We don't use `git-foo` since git 1.6.

HEAD is the default of `git describe`.

Also, deal with a bunch of shellcheck warnings.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 29d634a30b..6dd7683ee7 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -7,7 +7,7 @@ LF='
 '
 
 describe () {
-	VN=$(git describe --match "v[0-9]*" HEAD 2>/dev/null) || return 1
+	VN=$(git describe --match "v[0-9]*" 2>/dev/null) || return 1
 	case "$VN" in
 	*$LF*)
 		return 1
@@ -22,26 +22,24 @@ describe () {
 }
 
 # First see if there is a version file (included in release tarballs),
-# then try git-describe, then default.
+# 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
+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
 
-VN=$(expr "$VN" : v*'\(.*\)')
+VN=$(expr "$VN" : 'v*\(.*\)')
 
 if test -r $GVF
 then
 	VC=$(sed -e 's/^GIT_VERSION = //' <$GVF)
 else
-	VC=unset
+	VC='unset'
 fi
-test "$VN" = "$VC" || {
-	echo >&2 "GIT_VERSION = $VN"
-	echo "GIT_VERSION = $VN" >$GVF
-}
+test "$VN" = "$VC" && exit
+echo "GIT_VERSION = $VN" | tee $GVF >&2
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 03/18] version-gen: refactor default version
Message-ID: <20230414121841.373980-4-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-4-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
It's not clear how `cat version` might fail, but either way if VN is
empty (or unset), assign the default value.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 6dd7683ee7..0d00fa3d9a 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -25,14 +25,14 @@ describe () {
 # then try `git describe`, then default.
 if test -f version
 then
-	VN=$(cat version) || VN="$DEF_VER"
+	VN=$(cat version)
 elif test -d "${GIT_DIR:-.git}" -o -f .git && describe
 then
 	VN=$(echo "$VN" | sed -e 's/-/./g')
-else
-	VN="$DEF_VER"
 fi
 
+: "${VN:=$DEF_VER}"
+
 VN=$(expr "$VN" : 'v*\(.*\)')
 
 if test -r $GVF
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 04/18] version-gen: simplify v prefix removal
Message-ID: <20230414121841.373980-5-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-5-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
There is a much simpler way.

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 0d00fa3d9a..c0f6bb242f 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -33,7 +33,7 @@ fi
 
 : "${VN:=$DEF_VER}"
 
-VN=$(expr "$VN" : 'v*\(.*\)')
+VN=${VN#v}
 
 if test -r $GVF
 then
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 05/18] version-gen: simplify update check
Message-ID: <20230414121841.373980-6-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-6-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
We don't need to extract the version when we can compare the whole
contents.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index c0f6bb242f..34f561752b 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -35,11 +35,5 @@ fi
 
 VN=${VN#v}
 
-if test -r $GVF
-then
-	VC=$(sed -e 's/^GIT_VERSION = //' <$GVF)
-else
-	VC='unset'
-fi
-test "$VN" = "$VC" && exit
+test -r $GVF && test "GIT_VERSION = $VN" = "$(cat $GVF)" && exit
 echo "GIT_VERSION = $VN" | tee $GVF >&2
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 07/18] version-gen: simplify `git describe` checks
Message-ID: <20230414121841.373980-8-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-8-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
How can `git describe --match 'foo*'` return something that doesn't
contain 'foo' and without error?

It can't, so no need for check for the impossible.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index cd94a7902e..0021e88cdc 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -3,22 +3,11 @@
 GVF=GIT-VERSION-FILE
 DEF_VER=v2.40.GIT
 
-LF='
-'
-
 describe () {
 	VN=$(git describe --match "v[0-9]*" 2>/dev/null) || return 1
-	case "$VN" in
-	*$LF*)
-		return 1
-		;;
-	v[0-9]*)
-		git update-index -q --refresh
-		test -z "$(git diff-index --name-only HEAD --)" ||
-		VN="$VN-dirty"
-		return 0
-		;;
-	esac
+	git update-index -q --refresh
+	test -z "$(git diff-index --name-only HEAD --)" ||
+	VN="$VN-dirty"
 }
 
 # First see if there is a version file (included in release tarballs),
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 06/18] version-gen: remove redundant check
Message-ID: <20230414121841.373980-7-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-7-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
If we are not in a git repository `git describe` will fail anyway.

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 34f561752b..cd94a7902e 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -26,7 +26,7 @@ describe () {
 if test -f version
 then
 	VN=$(cat version)
-elif test -d "${GIT_DIR:-.git}" -o -f .git && describe
+elif describe
 then
 	VN=$(echo "$VN" | sed -e 's/-/./g')
 fi
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 08/18] version-gen: simplify dirty check
Message-ID: <20230414121841.373980-9-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-9-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 0021e88cdc..9be9e14204 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -4,10 +4,7 @@ GVF=GIT-VERSION-FILE
 DEF_VER=v2.40.GIT
 
 describe () {
-	VN=$(git describe --match "v[0-9]*" 2>/dev/null) || return 1
-	git update-index -q --refresh
-	test -z "$(git diff-index --name-only HEAD --)" ||
-	VN="$VN-dirty"
+	VN=$(git describe --match "v[0-9]*" --dirty 2>/dev/null) || return 1
 }
 
 # First see if there is a version file (included in release tarballs),
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 09/18] version-gen: move describe fix into function
Message-ID: <20230414121841.373980-10-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-10-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 9be9e14204..b7d2b8758e 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -5,6 +5,7 @@ DEF_VER=v2.40.GIT
 
 describe () {
 	VN=$(git describe --match "v[0-9]*" --dirty 2>/dev/null) || return 1
+	VN=$(echo "$VN" | sed -e 's/-/./g')
 }
 
 # First see if there is a version file (included in release tarballs),
@@ -12,9 +13,8 @@ describe () {
 if test -f version
 then
 	VN=$(cat version)
-elif describe
-then
-	VN=$(echo "$VN" | sed -e 's/-/./g')
+else
+	describe
 fi
 
 : "${VN:=$DEF_VER}"
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 11/18] version-gen: refactor describe function
Message-ID: <20230414121841.373980-12-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-12-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
No functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 962b9441e4..d4de540249 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -4,7 +4,7 @@ GVF=GIT-VERSION-FILE
 DEF_VER=v2.40.GIT
 
 describe () {
-	VN=$(git describe --match "v[0-9]*" --dirty 2>/dev/null | sed -e 's/-/./g') || return 1
+	git describe --match "v[0-9]*" --dirty 2>/dev/null | sed -e 's/-/./g'
 }
 
 # First see if there is a version file (included in release tarballs),
@@ -13,7 +13,7 @@ if test -f version
 then
 	VN=$(cat version)
 else
-	describe
+	VN=$(describe)
 fi
 
 : "${VN:=$DEF_VER}"
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 10/18] version-gen: describe and sed in one go
Message-ID: <20230414121841.373980-11-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-11-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index b7d2b8758e..962b9441e4 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -4,8 +4,7 @@ GVF=GIT-VERSION-FILE
 DEF_VER=v2.40.GIT
 
 describe () {
-	VN=$(git describe --match "v[0-9]*" --dirty 2>/dev/null) || return 1
-	VN=$(echo "$VN" | sed -e 's/-/./g')
+	VN=$(git describe --match "v[0-9]*" --dirty 2>/dev/null | sed -e 's/-/./g') || return 1
 }
 
 # First see if there is a version file (included in release tarballs),
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 13/18] version-gen: move v fix into sed
Message-ID: <20230414121841.373980-14-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-14-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
We are already using sed, might as well take advantage of it.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 0691f481e4..fa0e72a32c 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -4,7 +4,7 @@ GVF=GIT-VERSION-FILE
 DEF_VER=2.40.GIT
 
 describe () {
-	git describe --match "v[0-9]*" --dirty 2>/dev/null | sed -e 's/-/./g'
+	git describe --match "v[0-9]*" --dirty 2>/dev/null | sed -e 's/-/./g' -e 's/^v//'
 }
 
 # First see if there is a version file (included in release tarballs),
@@ -14,7 +14,6 @@ then
 	VN=$(cat version)
 else
 	VN=$(describe)
-	VN=${VN#v}
 fi
 
 : "${VN:=$DEF_VER}"
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 15/18] version-gen: remove default version
Message-ID: <20230414121841.373980-16-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-16-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
There's two use-cases:

 1) We are in a git repository

 2) We are in a dir extracted from a tarball

Is there any other case?

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 53b3d64131..2b0973f4b5 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -1,7 +1,6 @@
 #!/bin/sh
 
 GVF=GIT-VERSION-FILE
-DEF_VER=2.40.GIT
 
 get_version () {
 	test -f version && cat version && return
@@ -10,7 +9,5 @@ get_version () {
 
 VN=$(get_version)
 
-: "${VN:=$DEF_VER}"
-
 test -r $GVF && test "GIT_VERSION = $VN" = "$(cat $GVF)" && exit
 echo "GIT_VERSION = $VN" | tee $GVF >&2
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 12/18] version-gen: do v fix only when necessary
Message-ID: <20230414121841.373980-13-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-13-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
There's no point in having a v in the default version only to be
removed.

The only time we need to remove the v is from `git describe`.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index d4de540249..0691f481e4 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -1,7 +1,7 @@
 #!/bin/sh
 
 GVF=GIT-VERSION-FILE
-DEF_VER=v2.40.GIT
+DEF_VER=2.40.GIT
 
 describe () {
 	git describe --match "v[0-9]*" --dirty 2>/dev/null | sed -e 's/-/./g'
@@ -14,11 +14,10 @@ then
 	VN=$(cat version)
 else
 	VN=$(describe)
+	VN=${VN#v}
 fi
 
 : "${VN:=$DEF_VER}"
 
-VN=${VN#v}
-
 test -r $GVF && test "GIT_VERSION = $VN" = "$(cat $GVF)" && exit
 echo "GIT_VERSION = $VN" | tee $GVF >&2
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 14/18] version-gen: refactor main functionality
Message-ID: <20230414121841.373980-15-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-15-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
It's pretty clear that the `version` file overrides `describe`, so do it
in one function.

There's no need for the comment as the code is self-describing.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index fa0e72a32c..53b3d64131 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -3,18 +3,12 @@
 GVF=GIT-VERSION-FILE
 DEF_VER=2.40.GIT
 
-describe () {
+get_version () {
+	test -f version && cat version && return
 	git describe --match "v[0-9]*" --dirty 2>/dev/null | sed -e 's/-/./g' -e 's/^v//'
 }
 
-# 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)
-else
-	VN=$(describe)
-fi
+VN=$(get_version)
 
 : "${VN:=$DEF_VER}"
 
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 17/18] version-gen: get rid of GVF variable
Message-ID: <20230414121841.373980-18-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-18-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
There's not much point in a variable which is never going to change and
doesn't really add any readability.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 8f2250bb1b..161fcdf1ab 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -1,7 +1,5 @@
 #!/bin/sh
 
-GVF=GIT-VERSION-FILE
-
 get_version () {
 	test -f version && cat version && return
 	git describe --match "v[0-9]*" --dirty 2>/dev/null | sed -e 's/-/./g' -e 's/^v//'
@@ -9,5 +7,5 @@ get_version () {
 
 NEW="GIT_VERSION = $(get_version)"
 
-test -r $GVF && test "$NEW" = "$(cat $GVF)" && exit
-echo "$NEW" | tee $GVF >&2
+test -r GIT-VERSION-FILE && test "$NEW" = "$(cat GIT-VERSION-FILE)" && exit
+echo "$NEW" | tee GIT-VERSION-FILE >&2
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 16/18] version-gen: refactor GIT_VERSION string
Message-ID: <20230414121841.373980-17-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-17-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 GIT-VERSION-GEN | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 2b0973f4b5..8f2250bb1b 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -7,7 +7,7 @@ get_version () {
 	git describe --match "v[0-9]*" --dirty 2>/dev/null | sed -e 's/-/./g' -e 's/^v//'
 }
 
-VN=$(get_version)
+NEW="GIT_VERSION = $(get_version)"
 
-test -r $GVF && test "GIT_VERSION = $VN" = "$(cat $GVF)" && exit
-echo "GIT_VERSION = $VN" | tee $GVF >&2
+test -r $GVF && test "$NEW" = "$(cat $GVF)" && exit
+echo "$NEW" | tee $GVF >&2
-- 
2.40.0+fc1


```

## Felipe Contreras, 2023-04-14 12:18

Subject: [PATCH 18/18] version-gen: generate proper interim versions
Message-ID: <20230414121841.373980-19-felipe.contreras@gmail.com>
URL: https://gitlist.dev/e/20230414121841.373980-19-felipe.contreras%40gmail.com
In-Reply-To: <20230414121841.373980-1-felipe.contreras@gmail.com>

```
The change from dashes to dots was done in 2006 to satisfy some RPM
 requirements: 5c7d3c9507 (Allow building of RPM from interim snapshot.,
2006-01-16).

This probably was done because `2.40.0-100-g000` would be interpreted as
version `2.40.0` release `100`.

It isn't clear because the commit message doesn't explain.

But using a dot makes it worse because `2.40.0.n` will always be newer
than `2.40.0-n`.

What we want is an ordering such as:

 * 2.40.0         # git release
 * 2.40.0+100-g00 # interim version
 * 2.40.0-1       # Fedora release
 * 2.40.0.1       # hypothetical git release

So we should use a single `+` sign for interim versions, and for the
record that's what Mercurial does `6.3.3+hg591.dd42156b6441`.

[1] https://rpm-software-management.github.io/rpm/manual/dependencies.html

Cc: Junio C Hamano <gitster@pobox.com>
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 161fcdf1ab..99584bf86d 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -2,7 +2,7 @@
 
 get_version () {
 	test -f version && cat version && return
-	git describe --match "v[0-9]*" --dirty 2>/dev/null | sed -e 's/-/./g' -e 's/^v//'
+	git describe --match "v[0-9]*" --dirty 2>/dev/null | sed -e 's/-/+/' -e 's/^v//'
 }
 
 NEW="GIT_VERSION = $(get_version)"
-- 
2.40.0+fc1


```

## Todd Zullinger, 2023-04-14 15:11

Subject: Re: [PATCH 06/18] version-gen: remove redundant check
Message-ID: <ZDltLSTxBCWRoMjK@pobox.com>
URL: https://gitlist.dev/e/ZDltLSTxBCWRoMjK%40pobox.com
In-Reply-To: <20230414121841.373980-7-felipe.contreras@gmail.com>

```
Felipe Contreras wrote:
> If we are not in a git repository `git describe` will fail anyway.

The parent directory may be a git repository though.  The
current code ensures that we're running `git describe` in
the proper repository.

If we drop this, aren't we breaking things for someone
building a git tarball which is in a subdirectory of a git
repository?

> 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 34f561752b..cd94a7902e 100755
> --- a/GIT-VERSION-GEN
> +++ b/GIT-VERSION-GEN
> @@ -26,7 +26,7 @@ describe () {
>  if test -f version
>  then
>  	VN=$(cat version)
> -elif test -d "${GIT_DIR:-.git}" -o -f .git && describe
> +elif describe
>  then
>  	VN=$(echo "$VN" | sed -e 's/-/./g')
>  fi

-- 
Todd

```

## Felipe Contreras, 2023-04-14 17:47

Subject: Re: [PATCH 06/18] version-gen: remove redundant check
Message-ID: <643991c58aef_5ecfe29433@chronos.notmuch>
URL: https://gitlist.dev/e/643991c58aef_5ecfe29433%40chronos.notmuch
In-Reply-To: <ZDltLSTxBCWRoMjK@pobox.com>

```
Todd Zullinger wrote:
> Felipe Contreras wrote:
> > If we are not in a git repository `git describe` will fail anyway.
> 
> The parent directory may be a git repository though.  The
> current code ensures that we're running `git describe` in
> the proper repository.

How exactly does it do that?

The current code expects the cwd to be the git repo, run it in any other
directory and it will generate GIT-VERSION-FILE in that directory, which is
clearly not intended.

> If we drop this, aren't we breaking things for someone
> building a git tarball which is in a subdirectory of a git
> repository?

How exactly would this hypothetical person build such a tarball?

  git init /tmp/foo
  mkdir -p /tmp/foo/bar
  cd /tmp/foo/bar
  make -C ~/dev/git dist

Works fine.

-- 
Felipe Contreras

```

## Todd Zullinger, 2023-04-14 19:01

Subject: Re: [PATCH 06/18] version-gen: remove redundant check
Message-ID: <ZDmjIqNwVMESgHnn@pobox.com>
URL: https://gitlist.dev/e/ZDmjIqNwVMESgHnn%40pobox.com
In-Reply-To: <643991c58aef_5ecfe29433@chronos.notmuch>

```
Felipe Contreras wrote:
> Todd Zullinger wrote:
>> Felipe Contreras wrote:
>>> If we are not in a git repository `git describe` will fail anyway.
>> 
>> The parent directory may be a git repository though.  The
>> current code ensures that we're running `git describe` in
>> the proper repository.
> 
> How exactly does it do that?
> 
> The current code expects the cwd to be the git repo, run it in any other
> directory and it will generate GIT-VERSION-FILE in that directory, which is
> clearly not intended.

Whether it's fool proof isn't really my point.  It did
attempt to check that .git was a file or directory.  Not
checking at all isn't necessarily an improvement, was my
concern.

>> If we drop this, aren't we breaking things for someone
>> building a git tarball which is in a subdirectory of a git
>> repository?
> 
> How exactly would this hypothetical person build such a tarball?
> 
>   git init /tmp/foo
>   mkdir -p /tmp/foo/bar
>   cd /tmp/foo/bar
>   make -C ~/dev/git dist

If I have a git repo, say ~/fedora/git which contains the
fedora packaging (spec file, etc.) and extract a git archive
in this directory, the describe will now pick up the data
from the parent git directory, won't it?

    $ git -C ~/fedora clone https://src.fedoraproject.org/rpms/git.git
    $ cd ~/fedora/git
    $ git -C ~/upstream/git archive --format=tar --prefix=git/ HEAD | tar xf -
    $ cd git
    $ make GIT-VERSION-GEN
    $ cat GIT-VERSION-FILE 
    GIT_VERSION = 

The version file in the tarballs prevents this from
happening in the most common case, but it still feels like
this is loosening things a little more than it should.

-- 
Todd

```

## Felipe Contreras, 2023-04-14 19:32

Subject: Re: [PATCH 06/18] version-gen: remove redundant check
Message-ID: <6439aa5b2c776_6058729489@chronos.notmuch>
URL: https://gitlist.dev/e/6439aa5b2c776_6058729489%40chronos.notmuch
In-Reply-To: <ZDmjIqNwVMESgHnn@pobox.com>

```
Todd Zullinger wrote:
> Felipe Contreras wrote:
> > Todd Zullinger wrote:
> >> Felipe Contreras wrote:
> >>> If we are not in a git repository `git describe` will fail anyway.
> >> 
> >> The parent directory may be a git repository though.  The
> >> current code ensures that we're running `git describe` in
> >> the proper repository.
> > 
> > How exactly does it do that?
> > 
> > The current code expects the cwd to be the git repo, run it in any other
> > directory and it will generate GIT-VERSION-FILE in that directory, which is
> > clearly not intended.
> 
> Whether it's fool proof isn't really my point.  It did
> attempt to check that .git was a file or directory.  Not
> checking at all isn't necessarily an improvement, was my
> concern.
> 
> >> If we drop this, aren't we breaking things for someone
> >> building a git tarball which is in a subdirectory of a git
> >> repository?
> > 
> > How exactly would this hypothetical person build such a tarball?
> > 
> >   git init /tmp/foo
> >   mkdir -p /tmp/foo/bar
> >   cd /tmp/foo/bar
> >   make -C ~/dev/git dist
> 
> If I have a git repo, say ~/fedora/git which contains the
> fedora packaging (spec file, etc.) and extract a git archive
> in this directory, the describe will now pick up the data
> from the parent git directory, won't it?
> 
>     $ git -C ~/fedora clone https://src.fedoraproject.org/rpms/git.git
>     $ cd ~/fedora/git
>     $ git -C ~/upstream/git archive --format=tar --prefix=git/ HEAD | tar xf -
>     $ cd git
>     $ make GIT-VERSION-GEN
>     $ cat GIT-VERSION-FILE 
>     GIT_VERSION = 

I don't think this is a realistic use-case, but supposing it is, what would be
the desired outcome in this case?

 GIT_VERSION = 2.40.GIT

?

> The version file in the tarballs prevents this from
> happening in the most common case, but it still feels like
> this is loosening things a little more than it should.

If we care about this, the same behavior can be achieved with GIT_CEILING_DIRECTORIES:

 GIT_CEILING_DIRECTORIES=$(cd .. && pwd) git describe ...

-- 
Felipe Contreras

```

## Todd Zullinger, 2023-04-14 19:41

Subject: Re: [PATCH 06/18] version-gen: remove redundant check
Message-ID: <ZDmsg0njhtv_qM9a@pobox.com>
URL: https://gitlist.dev/e/ZDmsg0njhtv_qM9a%40pobox.com
In-Reply-To: <6439aa5b2c776_6058729489@chronos.notmuch>

```
Felipe Contreras wrote:
> Todd Zullinger wrote:
>> If I have a git repo, say ~/fedora/git which contains the
>> fedora packaging (spec file, etc.) and extract a git archive
>> in this directory, the describe will now pick up the data
>> from the parent git directory, won't it?
>> 
>>     $ git -C ~/fedora clone https://src.fedoraproject.org/rpms/git.git
>>     $ cd ~/fedora/git
>>     $ git -C ~/upstream/git archive --format=tar --prefix=git/ HEAD | tar xf -
>>     $ cd git
>>     $ make GIT-VERSION-GEN
>>     $ cat GIT-VERSION-FILE 
>>     GIT_VERSION = 
> 
> I don't think this is a realistic use-case, but supposing it is, what would be
> the desired outcome in this case?
> 
>  GIT_VERSION = 2.40.GIT
> 
> ?

That's what we get before this change.  I don't have an
opinion on whether that could/should change, but if it does,
it should be deliberate I think.

>> The version file in the tarballs prevents this from
>> happening in the most common case, but it still feels like
>> this is loosening things a little more than it should.
> 
> If we care about this, the same behavior can be achieved with GIT_CEILING_DIRECTORIES:
> 
>  GIT_CEILING_DIRECTORIES=$(cd .. && pwd) git describe ...

I only mention it because it seemed like a change in
behavior when the series aimed to not change any behavior.
I'm sure there are plenty of things which could be changed
while making imrpveoments here.  I think we'll be better off
if those changes are all noted and deliberate.

Thanks for working on this stuff, it is good to see
improvements to this sort of plumbing.

-- 
Todd

```