git blame gives an ambiguous short revision

git blame gives an ambiguous short revision

[Julia Lawall]

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

Re: git blame gives an ambiguous short revision

[Julia Lawall]

Sorry, I forgot the git version number:

git version 1.7.3.4

on Ubuntu.

julia

On Sun, 1 Jul 2012, Julia Lawall wrote:

> 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
>

Re: git blame gives an ambiguous short revision

[Junio C Hamano]

Julia Lawall <julia.lawall@lip6.fr> writes:

> 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.

Re: git blame gives an ambiguous short revision

[Junio C Hamano]

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);

Re: git blame gives an ambiguous short revision

[Junio C Hamano]

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