[RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling

[RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling

[Ramsay Jones]

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
---

Hi Junio,

I recently noticed that:

    $ make >pout 2>&1
    $ ./git version
    git version 2.11.0.286.g109e8a9
    $ git describe
    v2.11.0-286-g109e8a99d
    $

... for non-release builds, the commit part of the version
string was still using an --abbrev=7.

I don't know that it actually matters too much (since it will
show as many as necessary, thus the RFC), but it caused me to
look twice. ;-)

ATB,
Ramsay Jones

 GIT-VERSION-GEN | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 520d6e66e..05601f753 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -12,7 +12,7 @@ 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=9 HEAD 2>/dev/null) &&
 	case "$VN" in
 	*$LF*) (exit 1) ;;
 	v[0-9]*)
-- 
2.11.0

Re: [RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling

[Jeff King]

On Sun, Dec 04, 2016 at 08:45:59PM +0000, Ramsay Jones wrote:

> 
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
> 
> Hi Junio,
> 
> I recently noticed that:
> 
>     $ make >pout 2>&1
>     $ ./git version
>     git version 2.11.0.286.g109e8a9
>     $ git describe
>     v2.11.0-286-g109e8a99d
>     $
> 
> ... for non-release builds, the commit part of the version
> string was still using an --abbrev=7.

It seems like this kind of discussion ought to go in the commit message.
:)

That said, I think the right patch may be to just drop --abbrev
entirely. Its use goes all the way back to 9b88fcef7 (Makefile: use
git-describe to mark the git version., 2005-12-27), where it was
--abbrev=4. That became --abbrev=7 in bf505158d (Git 1.7.10.1,
2012-05-01) without further comment.

I think at that point it was a noop, as 7 should have been the default.
And now we probably ought to drop it, so that we can use the
auto-scaling default.

-Peff

Re: [RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling

[Ramsay Jones]

On 05/12/16 05:32, Jeff King wrote:
> On Sun, Dec 04, 2016 at 08:45:59PM +0000, Ramsay Jones wrote:
>> I recently noticed that:
>>
>>     $ make >pout 2>&1
>>     $ ./git version
>>     git version 2.11.0.286.g109e8a9
>>     $ git describe
>>     v2.11.0-286-g109e8a99d
>>     $
>>
>> ... for non-release builds, the commit part of the version
>> string was still using an --abbrev=7.
> 
> It seems like this kind of discussion ought to go in the commit message.
> :)
> 
> That said, I think the right patch may be to just drop --abbrev
> entirely.

Heh, that was the first version of the patch. However, I got to thinking
about why --abbrev=7 was there in the first place; the only reason I
could think of was to defeat local configuration to get a measure of
reproducibility.

Unfortunately, you can't get the 'auto' behaviour from --abbrev
(on the pu branch):

    $ ./git describe --abbrev=-1
    v2.11.0-286-g109e8
    $ ./git describe --abbrev=0
    v2.11.0
    $ ./git describe
    v2.11.0-286-g109e8a99d
    $

I did think about using '-c core.abbrev=auto', but that would
depend on Junio's patch (nothing wrong with that, of course):

    $ git version
    git version 2.11.0
    $ git -c core.abbrev=auto describe
    fatal: bad numeric config value 'auto' for 'core.abbrev': invalid unit
    $ ./git -c core.abbrev=auto describe
    v2.11.0-286-g109e8a99d
    $ 

What do you think?

ATB,
Ramsay Jones

Re: [RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Sun, Dec 04, 2016 at 08:45:59PM +0000, Ramsay Jones wrote:
>
>> 
>> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
>> ---
>> 
>> Hi Junio,
>> 
>> I recently noticed that:
>> 
>>     $ make >pout 2>&1
>>     $ ./git version
>>     git version 2.11.0.286.g109e8a9
>>     $ git describe
>>     v2.11.0-286-g109e8a99d
>>     $
>> 
>> ... for non-release builds, the commit part of the version
>> string was still using an --abbrev=7.
>
> It seems like this kind of discussion ought to go in the commit message.
> :)
>
> That said, I think the right patch may be to just drop --abbrev
> entirely.
> ...
> I think at that point it was a noop, as 7 should have been the default.
> And now we probably ought to drop it, so that we can use the
> auto-scaling default.

Yeah, I agree.

It does mean that snapshot binaries built out of the same commit in
the same repository before and after a repack have higher chances of
getting named differently, which may surprise people, but that
already is possible with a fixed length if the repacking involves
pruning (albeit with lower probabilities), and I do not think it is
a problem.

Re: [RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling

[Junio C Hamano]

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

> Heh, that was the first version of the patch. However, I got to thinking
> about why --abbrev=7 was there in the first place; the only reason I
> could think of was to defeat local configuration to get a measure of
> reproducibility.
>
> Unfortunately, you can't get the 'auto' behaviour from --abbrev
> (on the pu branch):
>
>     $ ./git describe --abbrev=-1
>     v2.11.0-286-g109e8
>     $ ./git describe --abbrev=0
>     v2.11.0
>     $ ./git describe
>     v2.11.0-286-g109e8a99d
>     $

What is the reason why the last one is undesirable?  Is it because
the user may have core.abbrev set to some value in the configuration
and you want to override it to force "auto"?

I am not sure how rigid GIT-VERSION-GEN wants to be to countermand
such an explicit user preference (i.e. existing configuration).

> I did think about using '-c core.abbrev=auto', 

Having said that, if countermanding end-user's configuration is
desireble, I agree that "-c core.abbrev=auto" is the way to do so.

> but that would depend on Junio's patch (nothing wrong with that,
> of course):

You caught me.  I'll need to polish that into a usable shape soon
then.  And that is orthogonal to the "does it make sense to force
'auto' in this context?" question.

Re: [RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling

[Ramsay Jones]

On 05/12/16 18:10, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
>> Heh, that was the first version of the patch. However, I got to thinking
>> about why --abbrev=7 was there in the first place; the only reason I
>> could think of was to defeat local configuration to get a measure of
>> reproducibility.
>>
>> Unfortunately, you can't get the 'auto' behaviour from --abbrev
>> (on the pu branch):
>>
>>     $ ./git describe --abbrev=-1
>>     v2.11.0-286-g109e8
>>     $ ./git describe --abbrev=0
>>     v2.11.0
>>     $ ./git describe
>>     v2.11.0-286-g109e8a99d
>>     $
> 
> What is the reason why the last one is undesirable?  Is it because
> the user may have core.abbrev set to some value in the configuration
> and you want to override it to force "auto"?

As I said, the original version of the patch just removed the
--abbrev=7, but then I started to think about why you might have
used --abbrev in the first place (first in commit 9b88fcef7 and
again in commit bf505158d). Making sure to override the configuration
was the only thing I could come up with. So, I was hoping you could
remember why! :-P

(I assumed it was to force a measure of uniformity/reproducibility).

ATB,
Ramsay Jones

Re: [RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling

[Junio C Hamano]

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

> As I said, the original version of the patch just removed the
> --abbrev=7, but then I started to think about why you might have
> used --abbrev in the first place (first in commit 9b88fcef7 and
> again in commit bf505158d). Making sure to override the configuration
> was the only thing I could come up with. So, I was hoping you could
> remember why! :-P

Nope.  As a maintainer support script, the only thing I cared about
it is that there is no -gXXXX at the end for anything I release ;-)

> (I assumed it was to force a measure of uniformity/reproducibility).

You cannot force uniformity/reproducibility with fixed abbrev,
unless you set abbreviation length to 40, so you are correct to add
"a measure of" there ;-) The first choice (i.e. 4) may have had a
justification to force absolute minimum, and the second one (i.e. 7)
may have had a justifiation to make it clear that we are using the
same setting as the default, so in post-1.7.10 era, I think it is
fine for us to just say "we have been using the same as default, so
let's not specify anything explicitly".

Re: [RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling

[Ramsay Jones]

On 05/12/16 22:24, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
>> As I said, the original version of the patch just removed the
>> --abbrev=7, but then I started to think about why you might have
>> used --abbrev in the first place (first in commit 9b88fcef7 and
>> again in commit bf505158d). Making sure to override the configuration
>> was the only thing I could come up with. So, I was hoping you could
>> remember why! :-P
> 
> Nope.  As a maintainer support script, the only thing I cared about
> it is that there is no -gXXXX at the end for anything I release ;-)
> 
>> (I assumed it was to force a measure of uniformity/reproducibility).
> 
> You cannot force uniformity/reproducibility with fixed abbrev,
> unless you set abbreviation length to 40, so you are correct to add
> "a measure of" there ;-)

Indeed. ;-)

>                            The first choice (i.e. 4) may have had a
> justification to force absolute minimum, and the second one (i.e. 7)
> may have had a justifiation to make it clear that we are using the
> same setting as the default, so in post-1.7.10 era, I think it is
> fine for us to just say "we have been using the same as default, so
> let's not specify anything explicitly".

So, you would be happy with just removing the '--abbrev' argument?

(That's fine by me; I don't set core.abbrev!)

ATB,
Ramsay Jones

Re: [RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling

[Jeff King]

On Mon, Dec 05, 2016 at 10:01:19AM -0800, Junio C Hamano wrote:

> > That said, I think the right patch may be to just drop --abbrev
> > entirely.
> > ...
> > I think at that point it was a noop, as 7 should have been the default.
> > And now we probably ought to drop it, so that we can use the
> > auto-scaling default.
> 
> Yeah, I agree.
> 
> It does mean that snapshot binaries built out of the same commit in
> the same repository before and after a repack have higher chances of
> getting named differently, which may surprise people, but that
> already is possible with a fixed length if the repacking involves
> pruning (albeit with lower probabilities), and I do not think it is
> a problem.

I think that the number is already unstable, even with --abbrev, as it
just specifies a minimum. So any operation that creates objects has a
possibility of increasing the length. Or more likely, two people
describing the same version may end up with different strings because
they have different objects in their repositories (e.g., I used to
carry's trast's git-notes archive of the mailing list which added quite
a few objects).

I agree that having it change over the course of a repack is slightly
_more_ surprising than those cases, but ultimately the value just isn't
stable.

-Peff

Re: [RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Dec 05, 2016 at 10:01:19AM -0800, Junio C Hamano wrote:
>
>> > That said, I think the right patch may be to just drop --abbrev
>> > entirely.
>> > ...
>> > I think at that point it was a noop, as 7 should have been the default.
>> > And now we probably ought to drop it, so that we can use the
>> > auto-scaling default.
>> 
>> Yeah, I agree.
>> 
>> It does mean that snapshot binaries built out of the same commit in
>> the same repository before and after a repack have higher chances of
>> getting named differently, which may surprise people, but that
>> already is possible with a fixed length if the repacking involves
>> pruning (albeit with lower probabilities), and I do not think it is
>> a problem.
>
> I think that the number is already unstable, even with --abbrev, as it
> just specifies a minimum. So any operation that creates objects has a
> possibility of increasing the length. Or more likely, two people
> describing the same version may end up with different strings because
> they have different objects in their repositories (e.g., I used to
> carry's trast's git-notes archive of the mailing list which added quite
> a few objects).
>
> I agree that having it change over the course of a repack is slightly
> _more_ surprising than those cases, but ultimately the value just isn't
> stable.

Yup, that is what I meant to say with "that is already possible" and
we are on the same page.  As all three of us seem to be happy with
just dropping --abbrev and letting describe do its default thing (as
configured by whoever is doing the build), let's do so.

-- >8 --
From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Sun, 4 Dec 2016 20:45:59 +0000
Subject: [PATCH] GIT-VERSION-GEN: do not force abbreviation length used by 'describe'

The default version name for a Git binary is computed by running
"git describe" on the commit the binary is made out of, basing on a
tag whose name matches "v[0-9]*", e.g. v2.11.0-rc2-2-g7f1dc9.

In the very early days, with 9b88fcef7d ("Makefile: use git-describe
to mark the git version.", 2005-12-27), we used "--abbrev=4" to get
absolute minimum number of abbreviated commit object name.  This was
later changed to match the default minimum of 7 with bf505158d0
("Git 1.7.10.1", 2012-05-01).

These days, the "default minimum" scales automatically depending on
the size of the repository, and there is no point in specifying a
particular abbreviation length; all we wanted since Git 1.7.10.1
days was to get "something reasonable we would use by default".

Just drop "--abbrev=<number>" from the invocation of "git describe"
and let the command pick what it thinks is appropriate, taking the
end user's configuration and the repository contents into account.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 GIT-VERSION-GEN | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 556fbfc104..f95b04bb36 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -12,7 +12,7 @@ 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]*" HEAD 2>/dev/null) &&
 	case "$VN" in
 	*$LF*) (exit 1) ;;
 	v[0-9]*)
-- 
2.11.0-263-gd89495280e

Re: [RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling

[Jeff King]

On Tue, Dec 06, 2016 at 10:17:55AM -0800, Junio C Hamano wrote:

> Yup, that is what I meant to say with "that is already possible" and
> we are on the same page.  As all three of us seem to be happy with
> just dropping --abbrev and letting describe do its default thing (as
> configured by whoever is doing the build), let's do so.
> 
> -- >8 --
> From: Ramsay Jones <ramsay@ramsayjones.plus.com>
> Date: Sun, 4 Dec 2016 20:45:59 +0000
> Subject: [PATCH] GIT-VERSION-GEN: do not force abbreviation length used by 'describe'

Thanks, this is a nice summary of the discussion, and the patch is
obviously correct.

-Peff

Re: [RFC PATCH] GIT-VERSION-GEN: set --abbrev=9 to match auto-scaling

[Ramsay Jones]

On 06/12/16 18:26, Jeff King wrote:
> On Tue, Dec 06, 2016 at 10:17:55AM -0800, Junio C Hamano wrote:
> 
>> Yup, that is what I meant to say with "that is already possible" and
>> we are on the same page.  As all three of us seem to be happy with
>> just dropping --abbrev and letting describe do its default thing (as
>> configured by whoever is doing the build), let's do so.
>>
>> -- >8 --
>> From: Ramsay Jones <ramsay@ramsayjones.plus.com>
>> Date: Sun, 4 Dec 2016 20:45:59 +0000
>> Subject: [PATCH] GIT-VERSION-GEN: do not force abbreviation length used by 'describe'
> 
> Thanks, this is a nice summary of the discussion, and the patch is
> obviously correct.

Yep, looks very good to me! (I had started to write a commit
message for this patch, when your email arrived. Needless to
say, but your message is much better than mine!)

Thanks!

ATB,
Ramsay Jones