| Message ID | 20240202091850.160203-1-vegard.nossum@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | t/t3515-cherry-pick-rebase.sh: new testcase demonstrating broken behavior | expand |
Hi Vegard On 02/02/2024 09:18, Vegard Nossum wrote: > Running "git cherry-pick" as an x-command in the rebase plan loses the > original authorship information. > > Write a known-broken test case for this: > > $ (cd t && ./t3515-cherry-pick-rebase.sh) > ok 1 - setup > ok 2 - cherry-pick preserves authorship information > not ok 3 - cherry-pick inside rebase preserves authorship information # TODO known breakage > # still have 1 known breakage(s) > # passed all remaining 2 test(s) > 1..3 > > Running with --verbose we see the diff between expected and actual: > > --- expected 2024-02-02 08:54:48.954753285 +0000 > +++ actual 2024-02-02 08:54:48.966753294 +0000 > @@ -1 +1 @@ > -Original Author > +A U Thor > > As far as I can tell, this is due to the check in print_advice() > which deletes CHERRY_PICK_HEAD when GIT_CHERRY_PICK_HELP is set, > but I'm not sure what a good fix would be. Thanks for reporting this and for the test case. I agree with your diagnosis. I think the simplest fix would be to unset GIT_CHERRY_PICK_HELP in the child environment in sequencer.c:do_exec(). Long term we should stop setting GIT_CHERRY_PICK_HELP when rebasing and hard code the rebase conflicts message in sequencer.c as the environment variable is a vestige of the scripted rebase implementation. To work around the bug I think you can change the exec lines in the todo list to exec unset GIT_CHERRY_PICK_HELP; git cherry-pick ... Best Wishes Phillip > Cc: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> > Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> > --- > t/t3515-cherry-pick-rebase.sh | 37 +++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > create mode 100755 t/t3515-cherry-pick-rebase.sh > > diff --git a/t/t3515-cherry-pick-rebase.sh b/t/t3515-cherry-pick-rebase.sh > new file mode 100755 > index 0000000000..ffe6f5fe2a > --- /dev/null > +++ b/t/t3515-cherry-pick-rebase.sh > @@ -0,0 +1,37 @@ > +#!/bin/sh > + > +test_description='test cherry-pick during a rebase' > + > +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main > +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > + > +. ./test-lib.sh > + > +test_expect_success setup ' > + test_commit --author "Original Author <original.author@example.com>" foo file contents1 && > + git checkout -b feature && > + test_commit --author "Another Author <another.author@example.com>" bar file contents2 > +' > + > +test_expect_success 'cherry-pick preserves authorship information' ' > + git checkout -B tmp feature && > + test_must_fail git cherry-pick foo && > + git add file && > + git commit --no-edit && > + git log -1 --format='%an' foo >expected && > + git log -1 --format='%an' >actual && > + test_cmp expected actual > +' > + > +test_expect_failure 'cherry-pick inside rebase preserves authorship information' ' > + git checkout -B tmp feature && > + echo "x git cherry-pick -x foo" >rebase-plan && > + test_must_fail env GIT_SEQUENCE_EDITOR="cp rebase-plan" git rebase -i feature && > + git add file && > + git commit --no-edit && > + git log -1 --format='%an' foo >expected && > + git log -1 --format='%an' >actual && > + test_cmp expected actual > +' > + > +test_done
diff --git a/t/t3515-cherry-pick-rebase.sh b/t/t3515-cherry-pick-rebase.sh new file mode 100755 index 0000000000..ffe6f5fe2a --- /dev/null +++ b/t/t3515-cherry-pick-rebase.sh @@ -0,0 +1,37 @@ +#!/bin/sh + +test_description='test cherry-pick during a rebase' + +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME + +. ./test-lib.sh + +test_expect_success setup ' + test_commit --author "Original Author <original.author@example.com>" foo file contents1 && + git checkout -b feature && + test_commit --author "Another Author <another.author@example.com>" bar file contents2 +' + +test_expect_success 'cherry-pick preserves authorship information' ' + git checkout -B tmp feature && + test_must_fail git cherry-pick foo && + git add file && + git commit --no-edit && + git log -1 --format='%an' foo >expected && + git log -1 --format='%an' >actual && + test_cmp expected actual +' + +test_expect_failure 'cherry-pick inside rebase preserves authorship information' ' + git checkout -B tmp feature && + echo "x git cherry-pick -x foo" >rebase-plan && + test_must_fail env GIT_SEQUENCE_EDITOR="cp rebase-plan" git rebase -i feature && + git add file && + git commit --no-edit && + git log -1 --format='%an' foo >expected && + git log -1 --format='%an' >actual && + test_cmp expected actual +' + +test_done
Running "git cherry-pick" as an x-command in the rebase plan loses the original authorship information. Write a known-broken test case for this: $ (cd t && ./t3515-cherry-pick-rebase.sh) ok 1 - setup ok 2 - cherry-pick preserves authorship information not ok 3 - cherry-pick inside rebase preserves authorship information # TODO known breakage # still have 1 known breakage(s) # passed all remaining 2 test(s) 1..3 Running with --verbose we see the diff between expected and actual: --- expected 2024-02-02 08:54:48.954753285 +0000 +++ actual 2024-02-02 08:54:48.966753294 +0000 @@ -1 +1 @@ -Original Author +A U Thor As far as I can tell, this is due to the check in print_advice() which deletes CHERRY_PICK_HEAD when GIT_CHERRY_PICK_HELP is set, but I'm not sure what a good fix would be. Cc: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> --- t/t3515-cherry-pick-rebase.sh | 37 +++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100755 t/t3515-cherry-pick-rebase.sh