[BUG] rebase should desambiguate abbreviated hashes before starting

[BUG] rebase should desambiguate abbreviated hashes before starting

[Yann Dirson]

I just ran into a funny edge-case when doing a long rebase: one of
the rewritten commits got a sha1 starting with one of the abbreviated
sha1's of a commit still to be applied.

As a result, the rebase stopped with a funny-looking "short SHA1 ... was
ambiguous", which would not have occured if the shortened sha1's presented
to the user were expanded to full sha1's before starting the rebase.

-- 
Yann Dirson - Bertin Technologies

Re: [BUG] rebase should desambiguate abbreviated hashes before starting

[David]

On 27 June 2013 18:55, Yann Dirson <dirson@bertin.fr> wrote:
> I just ran into a funny edge-case when doing a long rebase: one of
> the rewritten commits got a sha1 starting with one of the abbreviated
> sha1's of a commit still to be applied.
>
> As a result, the rebase stopped with a funny-looking "short SHA1 ... was
> ambiguous", which would not have occured if the shortened sha1's presented
> to the user were expanded to full sha1's before starting the rebase.

I do many large rebases, and I have experienced this about a dozen times
in the last few years.

I'm not sure that rebase could predict the new hashes without actually creating
the prior commits? So maybe the "short" SHA1 is "too short"?

When the rebase stops with this message, my workaround is:

The last (failed) entry is in .git/rebase-merge/done
The next todo is the top entry in .git/rebase-merge/git-rebase-todo
I enter the short SHA1 in gitk to find the long SHA1.
I edit both the above files to move the failed entry back into the top
of the todo file, with the long SHA1
And then git rebase --continue

Re: [BUG] rebase should desambiguate abbreviated hashes before starting

[Matthieu Moy]

David <bouncingcats@gmail.com> writes:

> I'm not sure that rebase could predict the new hashes without actually creating
> the prior commits? So maybe the "short" SHA1 is "too short"?

It's OK to show the short sha1 to the user, but "git rebase" could and
should expand these to complete sha1 right after the editor is closed. I
think that's what Yann means.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [BUG] rebase should desambiguate abbreviated hashes before starting

[David]

On 27 June 2013 21:04, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> David <bouncingcats@gmail.com> writes:
>
>> I'm not sure that rebase could predict the new hashes without actually creating
>> the prior commits? So maybe the "short" SHA1 is "too short"?
>
> It's OK to show the short sha1 to the user, but "git rebase" could and
> should expand these to complete sha1 right after the editor is closed. I
> think that's what Yann means.

Yes. I realised that just after clicking "send". Thanks :)

Re: [BUG] rebase should desambiguate abbreviated hashes before starting

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> David <bouncingcats@gmail.com> writes:
>
>> I'm not sure that rebase could predict the new hashes without actually creating
>> the prior commits? So maybe the "short" SHA1 is "too short"?
>
> It's OK to show the short sha1 to the user, but "git rebase" could and
> should expand these to complete sha1 right after the editor is closed. I
> think that's what Yann means.

Yes, any "short" is by definition "too short".  I agree that it is
OK to show short one in "rebase -i" instruction sheet, as they are
uniquely generated before the actual replaying of commits begins,
and it is a sensible thing to do to convert them to the full form
before starting to do the real work.

It could be something as simple like this (not tested).

 git-rebase--interactive.sh | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f953d8d..6766b44 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -678,6 +678,23 @@ skip_unnecessary_picks () {
 	die "Could not skip unnecessary pick commands"
 }
 
+# expand shortened commit object name to the full form
+expand_todo_commit_names () {
+	while read -r command rest
+	do
+		case "$command" in
+		'#'*)
+			;;
+		*)
+			sha1=$(git rev-parse --verify --quiet ${rest%% *})
+			rest="$sha1 ${rest#* }"
+			;;
+		esac
+		printf '%s\n' "$command${rest:+ }$rest"
+	done <"$todo" >"$todo.new" &&
+	mv -f "$todo.new" "$todo"
+}
+
 # Rearrange the todo list that has both "pick sha1 msg" and
 # "pick sha1 fixup!/squash! msg" appears in it so that the latter
 # comes immediately after the former, and change "pick" to
@@ -979,6 +996,8 @@ git_sequence_editor "$todo" ||
 has_action "$todo" ||
 	die_abort "Nothing to do"
 
+expand_todo_commit_names
+
 test -d "$rewritten" || test -n "$force_rebase" || skip_unnecessary_picks
 
 output git checkout $onto || die_abort "could not detach HEAD"

Re: [BUG] rebase should desambiguate abbreviated hashes before starting

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> It could be something as simple like this (not tested).
>
>  git-rebase--interactive.sh | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index f953d8d..6766b44 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -678,6 +678,23 @@ skip_unnecessary_picks () {
>  	die "Could not skip unnecessary pick commands"
>  }
>  
> +# expand shortened commit object name to the full form
> +expand_todo_commit_names () {
> +	while read -r command rest
> +	do
> +		case "$command" in
> +		'#'*)
> +			;;
> +		*)
> +			sha1=$(git rev-parse --verify --quiet ${rest%% *})
> +			rest="$sha1 ${rest#* }"
> +			;;

In case somebody wants to polish it to a real patch, this part
should at least be:

		case "$command" in
		'#'* | exec)
			# Be careful for oddball commands like 'exec'
			# that do not have a short-SHA-1 at the beginning
			# of $rest.
			;;
		*)
			sha1=$(git rev-parse --verify --quiet ${rest%% *}) &&
			rest="$sha1 ${rest#* }"
			;;