Skip to contentSkip to conversation

Teach git-describe to display distances from tags.

PATCH In review Shawn O. Pearce 7 participants 15 messages Jun 15, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 Teach git-describe to display distances from tags.

Junio C Hamano <[email protected]>:

Quoted 8 lines

However, I suspect that we could do better with Shawn's new fangled describe implementation that actually counts the distance between what is described and the tag. We could add "number of commits since the tag" somewhere, to describe:

v2.6.20-rc5-256-g419dd83
v2.6.20-rc5-217-gde14569

Quoted 6 lines

to say that the first one has 256 commits accumulated since the given tag "v2.6.20-rc5" and the second one has only 217 commits, to get the sense of how busy the development activity is.

Is it useful? That is something I am not sure.

Yes, its very useful. If you get two different describes at different times from a non-rewinding branch and they both come up with the same tag name, you can tell which is the 'newer' one by distance. This is rather common in practice, so its incredibly useful.

---
 builtin-describe.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
builtin-describe.c +2 -1
index e7b8f95..d8ff621 100644
@@ -189,7 +189,8 @@ static void describe(const char *arg, int last_one)
 				sha1_to_hex(gave_up_on->object.sha1));
 		}
 	}
-	printf("%s-g%s\n", all_matches[0].name->path,
+	printf("%s-%i-g%s\n", all_matches[0].name->path,
+		   all_matches[0].depth,
 		   find_unique_abbrev(cmit->object.sha1, abbrev));
 
 	if (!last_one)
Signature · 1 line
1.5.0.rc2.g18af
#1 lore raw
Quoted 15 lines · Shawn O. Pearce

Yes, its very useful. If you get two different describes at different times from a non-rewinding branch and they both come up with the same tag name, you can tell which is the 'newer' one by distance. This is rather common in practice, so its incredibly useful.

---
 builtin-describe.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
builtin-describe.c +0 -0
index e7b8f95..d8ff621 100644

builtin-describe.c static void describe(const char *arg, int last_one) · line 196

Quoted 9 lines
builtin-describe.c +2 -1
@@ -189,7 +189,8 @@ static void describe(const char *arg, int last_one)
 				sha1_to_hex(gave_up_on->object.sha1));
 		}
 	}
-	printf("%s-g%s\n", all_matches[0].name->path,
+	printf("%s-%i-g%s\n", all_matches[0].name->path,
+		   all_matches[0].depth,
 		   find_unique_abbrev(cmit->object.sha1, abbrev));
 
 	if (!last_one)

Two comments.

- This is purely style, but we seem to prefer %d instead of %i elsewhere in the code (three existing offenders are builtin-describe.c, receive-pack.c and sha1_file which we may want to clean up for consistency).

- How much damage are we talking about with this patch to People's existing scripts? I expect they all extract the hash from last -g (because they cannot rely on particular convention in tagnames), but I am also worried if people are expecting everything that comes before the last -g is the whole tag.

#2 lore raw
Quoted 4 lines · Junio C Hamano

- This is purely style, but we seem to prefer %d instead of %i elsewhere in the code (three existing offenders are builtin-describe.c, receive-pack.c and sha1_file which we may want to clean up for consistency).

I'll send a cleanup patch then for these locations.

Quoted 6 lines

- How much damage are we talking about with this patch to People's existing scripts? I expect they all extract the hash from last -g (because they cannot rely on particular convention in tagnames), but I am also worried if people are expecting everything that comes before the last -g is the whole tag.

No way to know for sure. I almost put this as an option (e.g. "--include-distance") but didn't want everyone to need to update their existing git-describe callers to obtain this new data. I would hope that people split on last -g to get at or remove the commit SHA-1, but they probably are also assiming te tag name is everything to the left. Including the distance would certainly violate that.

Signature · 1 line
Shawn.
#3 lore raw
In reply to Junio C Hamano
Quoted 26 lines · Junio C Hamano
Quoted 9 lines
---
 builtin-describe.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
builtin-describe.c +0 -0
index e7b8f95..d8ff621 100644

builtin-describe.c static void describe(const char *arg, int last_one) · line 196

Quoted 9 lines
builtin-describe.c +2 -1
@@ -189,7 +189,8 @@ static void describe(const char *arg, int last_one)
 				sha1_to_hex(gave_up_on->object.sha1));
 		}
 	}
-	printf("%s-g%s\n", all_matches[0].name->path,
+	printf("%s-%i-g%s\n", all_matches[0].name->path,
+		   all_matches[0].depth,
 		   find_unique_abbrev(cmit->object.sha1, abbrev));
 
 	if (!last_one)

Two comments.

- This is purely style, but we seem to prefer %d instead of %i elsewhere in the code (three existing offenders are builtin-describe.c, receive-pack.c and sha1_file which we may want to clean up for consistency).

Quoted 6 lines

- How much damage are we talking about with this patch to People's existing scripts? I expect they all extract the hash from last -g (because they cannot rely on particular convention in tagnames), but I am also worried if people are expecting everything that comes before the last -g is the whole tag.

Actually there is a third one and a half.

- We need to update the documentation to say what this new number means.

It's some number close to the number of revs since the named tag, but not exactly.

    $ git describe --debug 65ebe634 2>&1 | head -4
    searching to describe 65ebe634
     annotated        251 v2.6.20-rc5
     annotated        427 v2.6.20-rc4
     annotated        594 v2.6.20-rc3
    $ git rev-list v2.6.20-rc5..65ebe634 | wc -l
    254
    $ git rev-list v2.6.20-rc4..65ebe634 | wc -l
    430

And it does not seem to be always "minus 3" either.

#4 lore raw
Quoted 2 lines · Junio C Hamano

- We need to update the documentation to say what this new number means.

Indeed; I apologize for forgetting to do that. At this point I'll wind up resending the patch to address everything so toss what you have.

Quoted 13 lines

It's some number close to the number of revs since the named tag, but not exactly.

    $ git describe --debug 65ebe634 2>&1 | head -4
    searching to describe 65ebe634
     annotated        251 v2.6.20-rc5
     annotated        427 v2.6.20-rc4
     annotated        594 v2.6.20-rc3
    $ git rev-list v2.6.20-rc5..65ebe634 | wc -l
    254
    $ git rev-list v2.6.20-rc4..65ebe634 | wc -l
    430
Quoted 1 line

And it does not seem to be always "minus 3" either.

What you skipped in the --debug output above was that we hit our internal possible tag limit (10 by default) and aborted walking the revision chain. That's why we missed 3 revs in our counting. :)

We probably should make an option to enable the count, and if the count is enabled then we'll have to pickup counting where we left off and finish it out for the chosen tag so the count is correct.

Signature · 1 line
Shawn.
#5 lore raw
Quoted 4 lines · Shawn O. Pearce

We probably should make an option to enable the count, and if the count is enabled then we'll have to pickup counting where we left off and finish it out for the chosen tag so the count is correct.

Please don't make it an option. This is too useful to require an additional switch all the time.

Nicolas

#6 lore raw
Quoted 9 lines · Nicolas Pitre
Quoted 4 lines · Shawn O. Pearce

We probably should make an option to enable the count, and if the count is enabled then we'll have to pickup counting where we left off and finish it out for the chosen tag so the count is correct.

Please don't make it an option. This is too useful to require an additional switch all the time.

Hmph. And an option --no-number to disable this is probably not worth it, as it forces the existing scripts that wants the tagname to be updated to pass that new option. If the users need to update their scripts anyway, they can sed/expr the number out just as easily as passing the new option.

Another backward incompatibility to mention in the release notes. I guess it is not too big a deal but makes me feel uneasy.

#7 lore raw
Quoted 5 lines · Junio C Hamano

Hmph. And an option --no-number to disable this is probably not worth it, as it forces the existing scripts that wants the tagname to be updated to pass that new option. If the users need to update their scripts anyway, they can sed/expr the number out just as easily as passing the new option.

Actually, the more useful switch would be "--tag-only"; then

x=$(git-describe HEAD | sed 's/-g*//')

In scripts could become

x=$(git-describe --tag-only HEAD)

Then, the suffix can be anything we want, and can change in the future without affecting these scripts.

Andy

Signature · 2 lines
Dr Andy Parkins, M Eng (hons), MIEE
[email protected]
#8 lore raw

PATCH 1/1 If abbrev is set to zero in git-describe, don't add the unique suffix

When on a non-tag commit, git-describe normally outputs descriptions of the form
v1.0.0-g1234567890
Some scripts (for example the update hook script) might just want to know the name of the nearest tag, so they then have to do
x=$(git-describe HEAD | sed 's/-g*//')
This is costly, but more importantly is fragile as it is relying on the output format of git-describe, which we would then have to maintain forever.

This patch adds support for setting the --abbrev option to zero. In that case git-describe does as it always has, but outputs only the nearest found tag instead of a completely unique name. This means that scripts would not have to parse the output format and won't need changing if the git-describe suffix is ever changed.

---
 builtin-describe.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)
builtin-describe.c +6 -3
index 4921eee..f3ac2d5 100644
@@ -188,8 +188,11 @@ static void describe(const char *arg, int last_one)
 				sha1_to_hex(gave_up_on->object.sha1));
 		}
 	}
-	printf("%s-g%s\n", all_matches[0].name->path,
-		   find_unique_abbrev(cmit->object.sha1, abbrev));
+	if (abbrev == 0)
+		printf("%s\n", all_matches[0].name->path );
+	else
+		printf("%s-g%s\n", all_matches[0].name->path,
+			   find_unique_abbrev(cmit->object.sha1, abbrev));
 
 	if (!last_one)
 		clear_commit_marks(cmit, -1);
@@ -212,7 +215,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 			tags = 1;
 		else if (!strncmp(arg, "--abbrev=", 9)) {
 			abbrev = strtoul(arg + 9, NULL, 10);
-			if (abbrev < MINIMUM_ABBREV || 40 < abbrev)
+			if (abbrev != 0 && (abbrev < MINIMUM_ABBREV || 40 < abbrev))
 				abbrev = DEFAULT_ABBREV;
 		}
 		else if (!strncmp(arg, "--candidates=", 13)) {
Signature · 1 line
1.5.0.rc2.gc3537-dirty
#9 lore raw
In reply to Junio C Hamano
Quoted 4 lines · Junio C Hamano

Another backward incompatibility to mention in the release notes. I guess it is not too big a deal but makes me feel uneasy.

Sorry if I missed any discussions about that point but why not calling the coming release 2.0 instead of 1.5 if this one implies some backward incompabilities and a lot of change in the UI ?

thanks

Signature · 1 line
Francis
#10 lore raw
Quoted 9 lines · Francis Moreau
Quoted 4 lines · Junio C Hamano

Another backward incompatibility to mention in the release notes. I guess it is not too big a deal but makes me feel uneasy.

Sorry if I missed any discussions about that point but why not calling the coming release 2.0 instead of 1.5 if this one implies some backward incompabilities and a lot of change in the UI ?

Because with the usual numbering the next normal feature release would have been called 1.4.5, we said we would bump to 1.5 some time ago.

There is no fundamental change between 1.4.4 series and the upcoming 1.5.0 -- only superficial ones. I do not think it deserves a bump in the major version; well, at least I did not think it would, when we first started talking about "the new release whose main theme was usability enhancements".

#11 lore raw
Quoted 2 lines · Junio C Hamano

There is no fundamental change between 1.4.4 series and the upcoming 1.5.0 -- only superficial ones. I do not think it

But there are incompatibilities.

Another example, which may be a bad one since I unfortunately don't have time to follow closely git's development: the internal of 'git pull' has completely changed. If I clone a repo with git v1.4.x and I pull in this repo with git v1.5.x, does it still works ?

thanks

Signature · 1 line
Francis
#12 lore raw
Quoted 1 line · Francis Moreau

But there are incompatibilities.

Nothing really *that* major. Heck, that git-describe change is probably one of the biggest, if not the biggest.

Quoted 4 lines

Another example, which may be a bad one since I unfortunately don't have time to follow closely git's development: the internal of 'git pull' has completely changed. If I clone a repo with git v1.4.x and I pull in this repo with git v1.5.x, does it still works ?

Yes, of course. Just like before too.

What's different is if you clone with 1.5.x, as we now default to the seperate remotes layout. This was an option in 1.4.x and earlier, its now the only way its done in 1.5.x.

Signature · 1 line
Shawn.
#14 lore raw
Quoted 1 line · Jakub Narebski

I'd rather use "%s+%i-g%s\n" here...

Ack, this makes it much easier to recognise what this number is.