Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes:
> Junio C Hamano wrote:
>> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
>> index a58406d..c175ef1 100755
>> --- a/t/t3404-rebase-interactive.sh
>> +++ b/t/t3404-rebase-interactive.sh
>> @@ -934,6 +934,21 @@ test_expect_success 'rebase --edit-todo can be used to modify todo' '
>> test L = $(git cat-file commit HEAD | sed -ne \$p)
>> '
>>
>> +test_expect_success 'rebase -i produces readable reflog' '
>
> I don't know if this test is a good idea at all.
It is.
This catches "a change to GIT_REFLOG_ACTION that is meant to only
apply to checkout done internally in git-rebase leaks to the later
codepath and affects the reflog message left by rebase--interactive".
Apply the test change without the "do not leak" part in the fix-up
(queued as a single "SQUASH???" commit on 'pu') to what you posted
earlier and see how it breaks.
# rr/rebase-checkout-reflog with fix-up
$ git checkout 33b1cdeb
# revert the fix but keep the test
$ git checkout HEAD^ -- git-rebase--interactive.sh git-rebase.sh
# run the test
$ make && cd t && sh ./t3404-*.sh -v -i
The test fails with this:
--- expect 2013-06-18 16:09:21.000000000 +0000
+++ actual 2013-06-18 16:09:21.000000000 +0000
@@ -1,4 +1,4 @@
-rebase -i (start): checkout I
+rebase -i (start): checkout branch-reflog-test: checkout I
rebase -i (pick): G
rebase -i (pick): H
rebase -i (finish): returning to refs/heads/branch-reflog-test
"checkout branch-reflog-test" part is leaking from your setting of
GIT_REFLOG_ACTION for "git checkout" in "git rebase"; it is only
necessary to affect that "checkout" and it should not affect later
commands that write reflog entries, but we see the breakage.
And that is why I did the fix-up to make sure your changes only
apply to "git checkout".
If you apply the test part of that fixup to what you posted today,
what would you see? I didn't look at the patches very closely, but
I wouldn't be surprised if they are still leaking the change meant
only to "checkout" to the later stages and breaking that test (the
reason why I would not be surprised is because the impression I am
getting from reading your responses is that you do not understand
why it is bad not to limit the setting only to checkout).
> Why make an exception in the case of rebase -i?
Because the whole point of last two patches are "word reflog
messages better for rebase", the improvements made by these two
patches are better protected from future changes; it also makes sure
that deliberate improvements will show up as changes to the tests.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html