threads / patch / 5624

patchsha1_name: accept output of git-describe

Subject: [PATCH] sha1_name: accept output of git-describe

## tl;dr

4 messages between Sep 20, 2006 and Sep 20, 2006. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Johannes Schindelin· Sep 20, 2006, 20:56 UTC · lore
Now, 'git log $(git describe)' does the same as 'git log'.
NOTE: it might be possible that the unique abbreviation generated by
git-describe is no longer unique at a later stage. Evidently, in this
case the sha1 resolution fails.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 sha1_name.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
Show changes to sha1_name.c +12 −0
diff --git a/sha1_name.c b/sha1_name.c
index b497528..c1f6cca 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -159,6 +159,18 @@ static int get_short_sha1(const char *na
 
 	if (len < MINIMUM_ABBREV)
 		return -1;
+
+	/* check for output of git-describe */
+	if (len > MINIMUM_ABBREV + 2) {
+		int i;
+		for (i = len - MINIMUM_ABBREV - 2; i > 0 && name[i] != '-'; i--)
+			; /* do nothing */
+		if (i > 0 && name[i + 1] == 'g') {
+			name += i + 2;
+			len -= i + 2;
+		}
+	}
+
 	hashclr(res);
 	memset(canonical, 'x', 40);
 	for (i = 0; i < len ;i++) {

-- 
1.4.2.1.ga0df5-dirty
Junio C Hamano· Sep 20, 2006, 21:25 UTC · re: Johannes Schindelin · lore

Re: [PATCH] sha1_name: accept output of git-describe

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 31 quoted lines
> Now, 'git log $(git describe)' does the same as 'git log'.
>
> NOTE: it might be possible that the unique abbreviation generated by
> git-describe is no longer unique at a later stage. Evidently, in this
> case the sha1 resolution fails.
>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> ---
>  sha1_name.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/sha1_name.c b/sha1_name.c
> index b497528..c1f6cca 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -159,6 +159,18 @@ static int get_short_sha1(const char *na
>  
>  	if (len < MINIMUM_ABBREV)
>  		return -1;
> +
> +	/* check for output of git-describe */
> +	if (len > MINIMUM_ABBREV + 2) {
> +		int i;
> +		for (i = len - MINIMUM_ABBREV - 2; i > 0 && name[i] != '-'; i--)
> +			; /* do nothing */
> +		if (i > 0 && name[i + 1] == 'g') {
> +			name += i + 2;
> +			len -= i + 2;
> +		}
> +	}
> +

Shouldn't you also verify the leading ref (most likely a tag) exists (and optionally check that it precedes the commit you decoded into, but that is probably more expensive than it's worth)?

Johannes Schindelin· Sep 20, 2006, 21:31 UTC · re: Junio C Hamano · lore

Re: [PATCH] sha1_name: accept output of git-describe

Hi,
On Wed, 20 Sep 2006, Junio C Hamano wrote:
> Shouldn't you also verify the leading ref (most likely a tag)
> exists (and optionally check that it precedes the commit you
> decoded into, but that is probably more expensive than it's
> worth)?

I debated this with myself. But there is the cost, and more importantly the possibility that your victim^H^H^H^H^H^Hother side does not have the tag at all (for whatever reasons).

Ciao, Dscho

Shawn Pearce· Sep 20, 2006, 21:34 UTC · re: Junio C Hamano · lore

Re: [PATCH] sha1_name: accept output of git-describe

Junio C Hamano <junkio@cox.net> wrote:
Show 7 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Now, 'git log $(git describe)' does the same as 'git log'.
> >
> > NOTE: it might be possible that the unique abbreviation generated by
> > git-describe is no longer unique at a later stage. Evidently, in this
> > case the sha1 resolution fails.
[snip]
> Shouldn't you also verify the leading ref (most likely a tag)
> exists (and optionally check that it precedes the commit you
> decoded into, but that is probably more expensive than it's
> worth)?

I thought the same thing when I first posted that it was probably an easy change to this code (and then didn't do it myself) and again when I read Johannes' implementation. Both times I agreed with you Junio that it was probably too expensive and not worth doing.

Most of the time we assume not only that the SHA1 itself is unique but also that it some hex character prefix is unique (such as 6 or 8 digits). One might be able to argue that if we have duplicate commits (when abbreviated) that you might be able to break the tie by looking at the tag but even then you may not always be able to do so.

So no, probably not worth it...
-- 
Shawn.

← back to recent threads