threads / rfc / 31218

RFC patchrebase -i: use full onto sha1 in reflog

Subject: [RFC/PATCH] rebase -i: use full onto sha1 in reflog

## tl;dr

5 messages between Aug 9, 2012 and Aug 10, 2012. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Michael J Gruber· Aug 9, 2012, 16:05 UTC · lore

'git rebase' uses the full onto sha1 for the reflog message whereas 'git rebase -i' uses the short sha1. This is not only inconsistent, but can lead to problems when the reflog is inspected at a later time at which that abbreviation may have become ambiguous.

Make 'rebase -i' use the full onto sha1, as well.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
I don't remember having sent this out before but have been running with
it since (including tests). I don't see it on the list either.

While not for 1.7.12 obviously, it might still be good to have.

 git-rebase--interactive.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Show changes to git-rebase--interactive.sh +1 −1

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0d2056f..dbc9de6 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -573,7 +573,7 @@ do_next () {
 	newhead=$(git rev-parse HEAD) &&
 	case $head_name in
 	refs/*)
-		message="$GIT_REFLOG_ACTION: $head_name onto $shortonto" &&
+		message="$GIT_REFLOG_ACTION: $head_name onto $onto" &&
 		git update-ref -m "$message" $head_name $newhead $orig_head &&
 		git symbolic-ref \
 		  -m "$GIT_REFLOG_ACTION: returning to $head_name" \

-- 
1.7.12.rc2.245.g4f94e02

Martin von Zweigbergk· Aug 9, 2012, 17:04 UTC · re: Michael J Gruber · lore

Re: [RFC/PATCH] rebase -i: use full onto sha1 in reflog

On Thu, Aug 9, 2012 at 9:05 AM, Michael J Gruber <git@drmicha.warpmail.net> wrote:

Show 10 quoted lines

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0d2056f..dbc9de6 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -573,7 +573,7 @@ do_next () {
>         newhead=$(git rev-parse HEAD) &&
>         case $head_name in
>         refs/*)
> -               message="$GIT_REFLOG_ACTION: $head_name onto $shortonto" &&
> +               message="$GIT_REFLOG_ACTION: $head_name onto $onto" &&

After this patch, is there any point in the

 shortonto=$(git rev-parse --short $onto) &&

line just before the context? (I can't see any.)

Junio C Hamano· Aug 9, 2012, 17:48 UTC · re: Martin von Zweigbergk · lore

Re: [RFC/PATCH] rebase -i: use full onto sha1 in reflog

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:

Show 18 quoted lines

> On Thu, Aug 9, 2012 at 9:05 AM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> index 0d2056f..dbc9de6 100644
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -573,7 +573,7 @@ do_next () {
>>         newhead=$(git rev-parse HEAD) &&
>>         case $head_name in
>>         refs/*)
>> -               message="$GIT_REFLOG_ACTION: $head_name onto $shortonto" &&
>> +               message="$GIT_REFLOG_ACTION: $head_name onto $onto" &&
>
> After this patch, is there any point in the
>
>  shortonto=$(git rev-parse --short $onto) &&
>
> line just before the context? (I can't see any.)

Good point; the function always exits after reaching this point, so it should be safe to omit it.

Michael J Gruber· Aug 10, 2012, 06:51 UTC · re: Junio C Hamano · lore

[PATCHv2] rebase -i: use full onto sha1 in reflog

Make 'rebase -i' use the full onto sha1, as well.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
So, this is without the now superfluous shortonto definition.

The other place where shortonto is defined is still needed.

 git-rebase--interactive.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Show changes to git-rebase--interactive.sh +1 −2

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0d2056f..a09e842 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -569,11 +569,10 @@ do_next () {
 	test -s "$todo" && return
 
 	comment_for_reflog finish &&
-	shortonto=$(git rev-parse --short $onto) &&
 	newhead=$(git rev-parse HEAD) &&
 	case $head_name in
 	refs/*)
-		message="$GIT_REFLOG_ACTION: $head_name onto $shortonto" &&
+		message="$GIT_REFLOG_ACTION: $head_name onto $onto" &&
 		git update-ref -m "$message" $head_name $newhead $orig_head &&
 		git symbolic-ref \
 		  -m "$GIT_REFLOG_ACTION: returning to $head_name" \

-- 
1.7.12.rc2.245.g4f94e02

Junio C Hamano· Aug 9, 2012, 17:08 UTC · re: Michael J Gruber · lore

Re: [RFC/PATCH] rebase -i: use full onto sha1 in reflog

Michael J Gruber <git@drmicha.warpmail.net> writes:

Show 13 quoted lines

> 'git rebase' uses the full onto sha1 for the reflog message whereas 'git
> rebase -i' uses the short sha1. This is not only inconsistent, but can
> lead to problems when the reflog is inspected at a later time at which
> that abbreviation may have become ambiguous.
>
> Make 'rebase -i' use the full onto sha1, as well.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---
> I don't remember having sent this out before but have been running with
> it since (including tests). I don't see it on the list either.
>
> While not for 1.7.12 obviously, it might still be good to have.

Makes sense, I agree it is not that urgent to include it in 1.7.12, and I think it is not just "good to have" but "we should have done so from day one and it cannot break anybody who is reading the reflog".

Thanks.

Show 17 quoted lines

>
>  git-rebase--interactive.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 0d2056f..dbc9de6 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -573,7 +573,7 @@ do_next () {
>  	newhead=$(git rev-parse HEAD) &&
>  	case $head_name in
>  	refs/*)
> -		message="$GIT_REFLOG_ACTION: $head_name onto $shortonto" &&
> +		message="$GIT_REFLOG_ACTION: $head_name onto $onto" &&
>  		git update-ref -m "$message" $head_name $newhead $orig_head &&
>  		git symbolic-ref \
>  		  -m "$GIT_REFLOG_ACTION: returning to $head_name" \