threads / discuss / 30931

git blame gives an ambiguous short revision

Subject: git blame gives an ambiguous short revision

## tl;dr

5 messages between Jul 1, 2012 and Jul 2, 2012.

replies: 4people: 2as markdown or json

Julia Lawall· Jul 1, 2012, 12:30 UTC · lore

Using linux-next cloned today (July 1), I then checkout out the revision 60d5c9f5b. The command

git blame drivers/staging/brcm80211/brcmfmac/wl_iw.c -L3675,3675
then gives:
60d5c9f5 (Julia Lawall 2011-04-01 16:23:42 +0200 3675)  if (!iscan->iscan_ex_params_p) {
Then I try:
git show 60d5c9f5
which gives:

error: short SHA1 60d5c9f5 is ambiguous. error: short SHA1 60d5c9f5 is ambiguous. fatal: ambiguous argument '60d5c9f5': unknown revision or path not in the working tree. Use '--' to separate paths from revisions

If I give git blame the -l option, every thing is fine.

thanks, julia

Julia Lawall· Jul 1, 2012, 12:31 UTC · re: Julia Lawall · lore

Re: git blame gives an ambiguous short revision

Sorry, I forgot the git version number:
git version 1.7.3.4
on Ubuntu.
julia
On Sun, 1 Jul 2012, Julia Lawall wrote:
Show 26 quoted lines
> Using linux-next cloned today (July 1), I then checkout out the
> revision 60d5c9f5b.  The command
>
> git blame drivers/staging/brcm80211/brcmfmac/wl_iw.c -L3675,3675
>
> then gives:
>
> 60d5c9f5 (Julia Lawall 2011-04-01 16:23:42 +0200 3675)  if (!iscan->iscan_ex_params_p) {
>
> Then I try:
>
> git show 60d5c9f5
>
> which gives:
>
> error: short SHA1 60d5c9f5 is ambiguous.
> error: short SHA1 60d5c9f5 is ambiguous.
> fatal: ambiguous argument '60d5c9f5': unknown revision or path not in the
> working tree.
> Use '--' to separate paths from revisions
>
> If I give git blame the -l option, every thing is fine.
>
> thanks,
> julia
>
Junio C Hamano· Jul 2, 2012, 07:17 UTC · re: Julia Lawall · lore

Re: git blame gives an ambiguous short revision

Julia Lawall <julia.lawall@lip6.fr> writes:
Show 22 quoted lines
> Using linux-next cloned today (July 1), I then checkout out the
> revision 60d5c9f5b.  The command
>
> git blame drivers/staging/brcm80211/brcmfmac/wl_iw.c -L3675,3675
>
> then gives:
>
> 60d5c9f5 (Julia Lawall 2011-04-01 16:23:42 +0200 3675)  if (!iscan->iscan_ex_params_p) {
>
> Then I try:
>
> git show 60d5c9f5
>
> which gives:
>
> error: short SHA1 60d5c9f5 is ambiguous.
> error: short SHA1 60d5c9f5 is ambiguous.
> fatal: ambiguous argument '60d5c9f5': unknown revision or path not in the
> working tree.
> Use '--' to separate paths from revisions
>
> If I give git blame the -l option, every thing is fine.

Or you can use --abbrev to explicitly set the width; as the length necessary to make the abbreviated object names depends on the project, there is no good default.

I think it should be an easy patch to add a post-processing phase after all lines are blamed to automatically compute an appropriate value of abbreviation to ensure the uniqueness, but the current blame output does not bother to do so.

Junio C Hamano· Jul 2, 2012, 07:28 UTC · re: Junio C Hamano · lore

Re: git blame gives an ambiguous short revision

Junio C Hamano <gitster@pobox.com> writes:
> I think it should be an easy patch to add a post-processing phase
> after all lines are blamed to automatically compute an appropriate
> value of abbreviation to ensure the uniqueness, but the current
> blame output does not bother to do so.

Something like this, perhaps, but I didn't test the patch beyond "it compiles".

 builtin/blame.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 24d3dd5..960c58d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1837,6 +1837,16 @@ static int read_ancestry(const char *graft_file)
 	return 0;
 }
 
+static int update_auto_abbrev(int auto_abbrev, struct origin *suspect)
+{
+	const char *uniq = find_unique_abbrev(suspect->commit->object.sha1,
+					      auto_abbrev);
+	int len = strlen(uniq);
+	if (auto_abbrev < len)
+		return len;
+	return auto_abbrev;
+}
+
 /*
  * How many columns do we need to show line numbers, authors,
  * and filenames?
@@ -1847,12 +1857,16 @@ static void find_alignment(struct scoreboard *sb, int *option)
 	int longest_dst_lines = 0;
 	unsigned largest_score = 0;
 	struct blame_entry *e;
+	int compute_auto_abbrev = (abbrev < 0);
+	int auto_abbrev = default_abbrev;
 
 	for (e = sb->ent; e; e = e->next) {
 		struct origin *suspect = e->suspect;
 		struct commit_info ci;
 		int num;
 
+		if (compute_auto_abbrev)
+			auto_abbrev = update_auto_abbrev(auto_abbrev, suspect);
 		if (strcmp(suspect->path, sb->path))
 			*option |= OUTPUT_SHOW_NAME;
 		num = strlen(suspect->path);
@@ -1880,6 +1894,10 @@ static void find_alignment(struct scoreboard *sb, int *option)
 	max_orig_digits = decimal_width(longest_src_lines);
 	max_digits = decimal_width(longest_dst_lines);
 	max_score_digits = decimal_width(largest_score);
+
+	if (compute_auto_abbrev)
+		/* one more abbrev length is needed for the boundary commit */
+		abbrev = auto_abbrev + 1;
 }
 
 /*
@@ -2353,10 +2371,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 parse_done:
 	argc = parse_options_end(&ctx);
 
-	if (abbrev == -1)
-		abbrev = default_abbrev;
-	/* one more abbrev length is needed for the boundary commit */
-	abbrev++;
+	if (0 < abbrev)
+		/* one more abbrev length is needed for the boundary commit */
+		abbrev++;
 
 	if (revs_file && read_ancestry(revs_file))
 		die_errno("reading graft file '%s' failed", revs_file);
Junio C Hamano· Jul 2, 2012, 07:54 UTC · re: Junio C Hamano · lore

Re: git blame gives an ambiguous short revision

Junio C Hamano <gitster@pobox.com> writes:
> Something like this, perhaps, but I didn't test the patch beyond "it
> compiles".

OK, now I tested it ;-) I'll queue this and eventually merge down to older maintenance releases.

-- >8 --
Subject: [PATCH] blame: compute abbreviation width that ensures uniqueness

Julia Lawall noticed that in linux-next repository the commit object 60d5c9f5 (shown with the default abbreviation width baked into "git blame") in output from

  $ git blame -L 3675,3675 60d5c9f5b -- \
      drivers/staging/brcm80211/brcmfmac/wl_iw.c

is no longer unique in the repository, which results in "short SHA1 60d5c9f5 is ambiguous".

Compute the minimum abbreviation width that ensures uniqueness when the user did not specify the --abbrev option to avoid this.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

Incidentally, "git show v2.6.39-rc4-181-g60d5c9f5" is resolved correctly with the recent "prolong the shelf-life of decribed name" topic that will hopefully be ready by the next release, and it resolves even shorter "v2.6.39-rc4-181-g60d5c" just fine ;-).

 builtin/blame.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 3e1f7e1..f13ec32 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1828,6 +1828,16 @@ static int read_ancestry(const char *graft_file)
 	return 0;
 }
 
+static int update_auto_abbrev(int auto_abbrev, struct origin *suspect)
+{
+	const char *uniq = find_unique_abbrev(suspect->commit->object.sha1,
+					      auto_abbrev);
+	int len = strlen(uniq);
+	if (auto_abbrev < len)
+		return len;
+	return auto_abbrev;
+}
+
 /*
  * How many columns do we need to show line numbers in decimal?
  */
@@ -1850,12 +1860,16 @@ static void find_alignment(struct scoreboard *sb, int *option)
 	int longest_dst_lines = 0;
 	unsigned largest_score = 0;
 	struct blame_entry *e;
+	int compute_auto_abbrev = (abbrev < 0);
+	int auto_abbrev = default_abbrev;
 
 	for (e = sb->ent; e; e = e->next) {
 		struct origin *suspect = e->suspect;
 		struct commit_info ci;
 		int num;
 
+		if (compute_auto_abbrev)
+			auto_abbrev = update_auto_abbrev(auto_abbrev, suspect);
 		if (strcmp(suspect->path, sb->path))
 			*option |= OUTPUT_SHOW_NAME;
 		num = strlen(suspect->path);
@@ -1883,6 +1897,10 @@ static void find_alignment(struct scoreboard *sb, int *option)
 	max_orig_digits = lineno_width(longest_src_lines);
 	max_digits = lineno_width(longest_dst_lines);
 	max_score_digits = lineno_width(largest_score);
+
+	if (compute_auto_abbrev)
+		/* one more abbrev length is needed for the boundary commit */
+		abbrev = auto_abbrev + 1;
 }
 
 /*
@@ -2360,10 +2378,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 parse_done:
 	argc = parse_options_end(&ctx);
 
-	if (abbrev == -1)
-		abbrev = default_abbrev;
-	/* one more abbrev length is needed for the boundary commit */
-	abbrev++;
+	if (0 < abbrev)
+		/* one more abbrev length is needed for the boundary commit */
+		abbrev++;
 
 	if (revs_file && read_ancestry(revs_file))
 		die_errno("reading graft file '%s' failed", revs_file);

-- 
1.7.11.1.212.g52fe12e

← back to recent threads