Re: [PATCH v2 2/2] version-gen: avoid messing the version

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 13, 2013, 18:16 UTC
Message-ID: <xmqqbo3wk2ok.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1379035046-6688-3-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

Show 8 quoted lines

> 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'.
>
> If RedHat, or somebody else, needs a specific version, they can use the
> 'version' file, like everybody else.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---

I already explained to you why this is a bad change.

When we say "we try to avoid regressions", we really mean it. Before coming up with a change to pay Paul by robbing Peter, we must make an honest effort to see if there is a way to pay Paul without robbing anybody.

This change forces existing users who depend on how dashes are mangled into dots to change their tooling.

We do occasionally make deliberate regressions that force existing users to change the way they work, but only when there is no other way, and when the benefit of the change far outweighs the cost of such an adjustment, and with careful planning to ease the pain of transition. The updates to "git add" and "git push" planned for 2.0 fall into that category.

There has to be a benefit that far outweighs the inconvenience this patch imposes on existing users, but I do not see there is any. "If somebody needs a specific version, they can use the 'version' file" does not justify it at all; it equally applies to those who want to use a version name with a dash.

Besides, the patch does not even do what it claims to do; if the version is "v1.8.4-rc1", what you get out of the updated code is "1.8.4-rc1", still losing the leading "v".

I'd be more receptive if the patch were like this instead, though.

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index b444c18..c6d78ec 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -3,12 +3,16 @@
 GVF=GIT-VERSION-FILE
 DEF_VER=v1.8.4.GIT
 
+READ_RAW_VERSION=
 LF='
 '
 
 # First see if there is a version file (included in release tarballs),
 # then try git-describe, then default.
-if test -f version
+if test -f raw-version && VN=$(cat raw-version)
+then
+	READ_RAW_VERSION=yes
+elif test -f version
 then
 	VN=$(cat version) || VN="$DEF_VER"
 elif test -d ${GIT_DIR:-.git} -o -f .git &&
@@ -26,7 +30,10 @@ else
 	VN="$DEF_VER"
 fi
 
-VN=$(expr "$VN" : v*'\(.*\)')
+if test -z "$READ_RAW_VERSION"
+then
+	VN=$(expr "$VN" : v*'\(.*\)')
+fi
 
 if test -r $GVF
 then