Felipe Contreras[PATCH 0/2] version-gen: fixes and cleanups
Felipe Contreras (2): version-gen: cleanup version-gen: avoid messing the version
GIT-VERSION-GEN | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-)
-- 1.8.4-338-gefd7fa6
Felipe ContrerasNo functional changes.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- GIT-VERSION-GEN | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 06026ea..b0db139 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -7,21 +7,24 @@ LF=' ' # 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 && - VN=$(git describe --match "v[0-9]*" --abbrev=7 HEAD 2>/dev/null) && + VN=$(git describe --match "v[0-9]*" --abbrev=7 HEAD 2>/dev/null) +then case "$VN" in - *$LF*) (exit 1) ;; + *$LF*) + exit 1 + ;; v[0-9]*) git update-index -q --refresh test -z "$(git diff-index --name-only HEAD --)" || - VN="$VN-dirty" ;; + VN="$VN-dirty" + ;; esac -then - VN=$(echo "$VN" | sed -e 's/-/./g'); + VN=$(echo "$VN" | sed -e 's/-/./g') else VN="$DEF_VER" fi @@ -34,9 +37,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-338-gefd7fa6
Junio C HamanoRe: [PATCH 1/2] version-gen: cleanup
Felipe Contreras <felipe.contreras@gmail.com> writes:
> No functional changes.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> GIT-VERSION-GEN | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
> index 06026ea..b0db139 100755
> --- a/GIT-VERSION-GEN
> +++ b/GIT-VERSION-GEN
> @@ -7,21 +7,24 @@ LF='
> '
>
> # 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 &&
> - VN=$(git describe --match "v[0-9]*" --abbrev=7 HEAD 2>/dev/null) &&
> + VN=$(git describe --match "v[0-9]*" --abbrev=7 HEAD 2>/dev/null)
> +then
> case "$VN" in
> - *$LF*) (exit 1) ;;
> + *$LF*)
> + exit 1The funnily written "false" is misleading and a clean-up may be a good idea to turn it to a straight "false".
I however think this actually changes the behaviour.
In any case, if you want to keep this step a "no functional change" rewrite, this section has to be part of the condition of this "elif" (because it logically is). If describe couldn't describe HEAD, or even if it could, if its output was multi-line for any reason, we wanted to punt and let the DEF_VER in the last "else" clause kick in.
After the update, describe output that is not a single-liner will fail the entire script, instead of falling back to DEF_VER. I actually think it is a good change to fail it (even though it is unlikely that the "describe" command give above would give more than one line).
> + ;; > v[0-9]*) > git update-index -q --refresh > test -z "$(git diff-index --name-only HEAD --)" || > - VN="$VN-dirty" ;; > + VN="$VN-dirty" > + ;; > esac > -then > - VN=$(echo "$VN" | sed -e 's/-/./g'); > + VN=$(echo "$VN" | sed -e 's/-/./g') > else > VN="$DEF_VER" > fi
Felipe Contreras[PATCH 2/2] version-gen: avoid messing the version
If the version is 'v1.8.4-rc1' that is the version, and there's no need to change it to anything else, like 'v1.8.4.rc1'.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- GIT-VERSION-GEN | 1 - 1 file changed, 1 deletion(-)
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index b0db139..2b9fd2f 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -24,7 +24,6 @@ then VN="$VN-dirty" ;; esac - VN=$(echo "$VN" | sed -e 's/-/./g') else VN="$DEF_VER" fi
-- 1.8.4-338-gefd7fa6
Junio C HamanoRe: [PATCH 2/2] version-gen: avoid messing the version
Felipe Contreras <felipe.contreras@gmail.com> writes:
> If the version is 'v1.8.4-rc1' that is the version, and there's no need > to change it to anything else, like 'v1.8.4.rc1'. > > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> > --- > GIT-VERSION-GEN | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN > index b0db139..2b9fd2f 100755 > --- a/GIT-VERSION-GEN > +++ b/GIT-VERSION-GEN > @@ -24,7 +24,6 @@ then > VN="$VN-dirty" > ;; > esac > - VN=$(echo "$VN" | sed -e 's/-/./g') > else > VN="$DEF_VER" > fi
The log message of 5c7d3c95 (Allow building of RPM from interim snapshot., 2006-01-16) leaves a lot to be desired, but I do remember that this was added due to user request. Before that commit, we did not strip dashes, and RPM packaging did not like dash in the version name, hence folks on RH and derived distributions could not package the software out of the box themselves.
I do not offhand know if that RPM limitation was lifted. I do not know if there are other packagers with a similar limitation that have been relying on this version mangling, either. This change may end up being a regression for these users.
The mangling is done only for auto-generated names, so I suspect that anybody who wanted the dash in their version names for whatever reason has already been using the "version file" escape hatch (or nobody had a desparate need to use dash instead of dot).
Felipe ContrerasRe: [PATCH 2/2] version-gen: avoid messing the version
On Mon, Sep 9, 2013 at 12:51 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes: > >> If the version is 'v1.8.4-rc1' that is the version, and there's no need >> to change it to anything else, like 'v1.8.4.rc1'. >> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> >> --- >> GIT-VERSION-GEN | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN >> index b0db139..2b9fd2f 100755 >> --- a/GIT-VERSION-GEN >> +++ b/GIT-VERSION-GEN >> @@ -24,7 +24,6 @@ then >> VN="$VN-dirty" >> ;; >> esac >> - VN=$(echo "$VN" | sed -e 's/-/./g') >> else >> VN="$DEF_VER" >> fi > > The log message of 5c7d3c95 (Allow building of RPM from interim > snapshot., 2006-01-16) leaves a lot to be desired, but I do remember > that this was added due to user request. Before that commit, we did > not strip dashes, and RPM packaging did not like dash in the version > name, hence folks on RH and derived distributions could not package > the software out of the box themselves.
Of course they could, by doing the replace themselves, or by simply using a 'version' file. Didn't you just recently suggested somebody to do exactly that?
> I do not offhand know if that RPM limitation was lifted. I do not > know if there are other packagers with a similar limitation that > have been relying on this version mangling, either. This change may > end up being a regression for these users.
Of course not. First of all, who exactly is packaging release candidates nowadays? And second, why they can't they use the existing tools, like the 'version' file?
The information behind the commit 5c7d3c95 is basically non-existant, as I cannot even find a patch for that change in the mailing list. I guess you decided the commit was so trivial it didn't merit a patch for review and discussion. Anyway, it happened before the 'version' file option was present, so it's quite likely they (whatever "they" might be) would not have any issue.
> The mangling is done only for auto-generated names, so I suspect > that anybody who wanted the dash in their version names for whatever > reason has already been using the "version file" escape hatch (or > nobody had a desparate need to use dash instead of dot).
So, let me get this straight; the nominal version of Git has a dash (v1.8.3-rc1), but the internal version of doesn't (1.8.3.rc1), there is no real reason for that, except that you say "some people" need it that way, but then you say the people that need it in the nominal way should use the 'version' file. Why couldn't the people that want the non-nominal form use the 'version' file instead?
It doesn't follow.
-- Felipe Contreras
brian m. carlsonRe: [PATCH 2/2] version-gen: avoid messing the version
On Mon, Sep 09, 2013 at 02:00:55AM -0500, Felipe Contreras wrote:
> Of course not. First of all, who exactly is packaging release > candidates nowadays? And second, why they can't they use the existing > tools, like the 'version' file?
Debian unstable, for one. However, they don't use RPMs and the version would be rewritten to use ~ anyway. Fedora rawhide might.
-- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187
Felipe ContrerasRe: [PATCH 2/2] version-gen: avoid messing the version
On Mon, Sep 9, 2013 at 6:30 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:
> On Mon, Sep 09, 2013 at 02:00:55AM -0500, Felipe Contreras wrote: >> Of course not. First of all, who exactly is packaging release >> candidates nowadays? And second, why they can't they use the existing >> tools, like the 'version' file? > > Debian unstable, for one. However, they don't use RPMs and the version > would be rewritten to use ~ anyway. Fedora rawhide might.
Debian uses their own version; 1:1.8.4~rc3-1, it doesn't use Git's version 1.8.4.rc3, so if we change it to 1.8.4-rc3, that doesn't affect them one bit because they are not using it anyway.
As for Fedora rawhide, they don't even use rc versions:
http://koji.fedoraproject.org/koji/packageinfo?packageID=1864
-- Felipe Contreras