From: "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Phillip Wood" <phillip.wood123@gmail.com>,
"Christian Couder" <christian.couder@gmail.com>,
"Elijah Newren" <newren@gmail.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Calvin Wan" <calvinwan@google.com>,
"Emily Shaffer" <emilyshaffer@google.com>,
"Glen Choo" <chooglen@google.com>,
"Victoria Dye" <vdye@github.com>,
"Phillip Wood" <phillip.wood@dunelm.org.uk>
Subject: [PATCH v4 0/8] rebase: make reflog messages independent of the backend
Date: Fri, 21 Oct 2022 09:21:40 +0000 [thread overview]
Message-ID: <pull.1150.v4.git.1666344108.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1150.v3.git.1665567312.gitgitgadget@gmail.com>
This series fixes some bugs in the reflog messages when rebasing and changes
the reflog messages of "rebase --apply" to match "rebase --merge" with the
aim of making the reflog easier to parse.
Thanks to Junio for his comments, here are the changes since V3:
* Patch 1: Swapped with patch 2. Small change to fast-forward tests to make
them pass
* Patch 2: Swapped with patch 1. Reworded commit message and added a change
to the fast-forward test to reflect the change in reflog message
introduced by this patch
The rest of the patches are unchanged.
V2 Cover Letter
Thanks to everyone who commented on V2, I've added the review club
participants that I've got address for to the cc list. I've rebased onto
pw/rebase-keep-base-fixes.
Change since V2:
* Patch 1: Reworded the commit message to address the concerns in [1,2]
about the behavior when head_name is NULL. There is also a small change
due to being rebased.
* Patch 2: Unchanged. There wasn't much love for parameterized tests in
review club but we want to ensure both backends produce the same messages
I think this is the safest way to achieve that. Using separate tests
makes it too easy to introduce subtle differences in the testing of the
two backends.
* Patch 3: Added a note to the commit message to address the concerns in
[1] about not resetting GIT_REFLOG_ACTION when we return early.
* Patches 4 & 5: Unchanged.
* Patch 6: Reworded the commit message to make a stronger argument for this
change. There are concerns about backwards compatibility in [1,3,4] but
(i) we have made similar changes in the past without complaints and (ii)
we're changing the message to an existing format. There is also a small
change due to being rebased.
* Patches 7 & 8: Small changes due to rebase.
[1]
https://docs.google.com/document/d/14L8BAumGTpsXpjDY8VzZ4rRtpAjuGrFSRqn3stCuS_w/edit?pli=1#heading=h.t6g5l2t5ibzw
[2] https://lore.kernel.org/git/xmqq35i7r4rj.fsf@gitster.g/ [3]
https://lore.kernel.org/git/xmqq4k2nmmeg.fsf@gitster.g/ [4]
https://lore.kernel.org/git/220420.865yn4833u.gmgdl@evledraar.gmail.com/
V2 Cover Letter:
Thanks to Christian and Elijah for their comments on V1.
I've updated commit message for patch 1 to try and be clearer about the
removal of a call to strbuf_release() and spilt out the test changes from
the old patch 2 into a separate preparatory patch.
V1 Cover Letter:
This is a series of rebase reflog related patches with the aim of unifying
the reflog messages from the two rebase backends.
* improve rebase reflog test coverage
* rebase --merge: fix reflog messages for --continue and --skip
* rebase --apply: respect GIT_REFLOG_ACTION
* rebase --abort: improve reflog message
* unify reflog messages between the two rebase backends
This series is based on pw/use-inprocess-checkout-in-rebase
Phillip Wood (8):
t3406: rework rebase reflog tests
rebase --apply: improve fast-forward reflog message
rebase --merge: fix reflog when continuing
rebase --merge: fix reflog message after skipping
rebase --apply: respect GIT_REFLOG_ACTION
rebase --apply: make reflog messages match rebase --merge
rebase --abort: improve reflog message
rebase: cleanup action handling
builtin/rebase.c | 146 ++++++++++++------------------
sequencer.c | 5 ++
t/t3406-rebase-message.sh | 185 +++++++++++++++++++++++++++++++-------
3 files changed, 215 insertions(+), 121 deletions(-)
base-commit: aa1df8146d70bb85c63b0999868fe29aebc1173e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1150%2Fphillipwood%2Fwip%2Frebase-reflog-fixes-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1150/phillipwood/wip/rebase-reflog-fixes-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1150
Range-diff vs v3:
2: b9255ad35d2 ! 1: 8d5ae067ce3 t3406: rework rebase reflog tests
@@ Commit message
the "merge" and "apply" backends. The test coverage for the "apply"
backend now includes setting GIT_REFLOG_ACTION.
- Note that rebasing the "conflicts" branch does not create any
- conflicts yet. A commit to do that will be added in the next commit
- and the diff ends up smaller if we have don't rename the branch when
- it is added.
+ Note that rebasing the "conflicts" branch does not create any conflicts
+ yet. A commit to do that will be added shortly and the diff ends up
+ smaller if we have don't rename the branch when it is added.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
@@ t/t3406-rebase-message.sh: test_expect_success 'error out early upon -C<n> or --
+ ) &&
+
+ git log -g --format=%gs -2 >actual &&
++ if test "$mode" = "--apply"
++ then
++ finish_msg="refs/heads/fast-forward onto $(git rev-parse main)"
++ else
++ finish_msg="returning to refs/heads/fast-forward"
++ fi &&
+ write_reflog_expect <<-EOF &&
-+ ${reflog_action:-rebase} (finish): returning to refs/heads/fast-forward
++ ${reflog_action:-rebase} (finish): $finish_msg
+ ${reflog_action:-rebase} (start): checkout main
EOF
test_cmp expect actual &&
1: a84cf971a75 ! 2: 12701ef7c7c rebase --apply: remove duplicated code
@@ Metadata
Author: Phillip Wood <phillip.wood@dunelm.org.uk>
## Commit message ##
- rebase --apply: remove duplicated code
+ rebase --apply: improve fast-forward reflog message
- Use move_to_original_branch() when reattaching HEAD after a fast-forward
- rather than open coding a copy of that code. move_to_original_branch()
- does not call reset_head() if head_name is NULL but there should be no
- user visible changes even though we currently call reset_head() in that
- case. The reason for this is that the reset_head() call does not add a
- message to the reflog because we're not changing the commit that HEAD
- points to and so lock_ref_for_update() elides the update. When head_name
- is not NULL then reset_head() behaves like "git symbolic-ref" and so the
- reflog is updated.
+ Using move_to_original_branch() when reattaching HEAD after a
+ fast-forward improves HEAD's reflog message when rebasing a branch. This
+ is because it uses separate reflog messages for "HEAD" and
+ "branch". HEAD's reflog will now record which branch we're returning to.
- Note that the removal of "strbuf_release(&msg)" is safe as there is an
- identical call just above this hunk which can be seen by viewing the
- diff with -U6.
+ When rebasing a detached HEAD there is no change in behavior. We
+ currently call reset_head() when head_name is NULL but
+ move_to_original_branch() does not. However the existing call does not
+ add a message to the reflog because we're not changing the commit that
+ HEAD points to and so lock_ref_for_update() skips the update.
+ Note that the removal of "strbuf_reset(&msg)" is safe as there is a call
+ to strbuf_release(&msf) just above this hunk which can be seen by
+ viewing the diff with -U6.
+
+ Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
## builtin/rebase.c ##
@@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix
ret = finish_rebase(&options);
goto cleanup;
}
+
+ ## t/t3406-rebase-message.sh ##
+@@ t/t3406-rebase-message.sh: test_reflog () {
+ ) &&
+
+ git log -g --format=%gs -2 >actual &&
+- if test "$mode" = "--apply"
+- then
+- finish_msg="refs/heads/fast-forward onto $(git rev-parse main)"
+- else
+- finish_msg="returning to refs/heads/fast-forward"
+- fi &&
+ write_reflog_expect <<-EOF &&
+- ${reflog_action:-rebase} (finish): $finish_msg
++ ${reflog_action:-rebase} (finish): returning to refs/heads/fast-forward
+ ${reflog_action:-rebase} (start): checkout main
+ EOF
+ test_cmp expect actual &&
3: ea4da25a19c = 3: 2c965f4b97c rebase --merge: fix reflog when continuing
4: 225ff4baef7 = 4: 17138d910f0 rebase --merge: fix reflog message after skipping
5: 1094681eb11 = 5: 0c71c732904 rebase --apply: respect GIT_REFLOG_ACTION
6: a5338e6bdd8 = 6: 3f6b2e39f40 rebase --apply: make reflog messages match rebase --merge
7: aa808725fb8 = 7: c8fa57f129d rebase --abort: improve reflog message
8: f9c8664b883 = 8: ed800844ba1 rebase: cleanup action handling
--
gitgitgadget
next prev parent reply other threads:[~2022-10-21 9:21 UTC|newest]
Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-21 11:10 [PATCH 0/7] rebase: make reflog messages independent of the backend Phillip Wood via GitGitGadget
2022-02-21 11:10 ` [PATCH 1/7] rebase --apply: remove duplicated code Phillip Wood via GitGitGadget
2022-04-07 13:35 ` Christian Couder
2022-04-17 1:56 ` Elijah Newren
2022-02-21 11:10 ` [PATCH 2/7] rebase --merge: fix reflog when continuing Phillip Wood via GitGitGadget
2022-04-07 13:49 ` Christian Couder
2022-04-15 14:00 ` Phillip Wood
2022-04-17 1:57 ` Elijah Newren
2022-02-21 11:10 ` [PATCH 3/7] rebase --merge: fix reflog message after skipping Phillip Wood via GitGitGadget
2022-04-17 1:58 ` Elijah Newren
2022-02-21 11:10 ` [PATCH 4/7] rebase --apply: respect GIT_REFLOG_ACTION Phillip Wood via GitGitGadget
2022-04-07 13:59 ` Christian Couder
2022-04-15 14:03 ` Phillip Wood
2022-02-21 11:10 ` [PATCH 5/7] rebase --apply: make reflog messages match rebase --merge Phillip Wood via GitGitGadget
2022-04-17 2:03 ` Elijah Newren
2022-02-21 11:10 ` [PATCH 6/7] rebase --abort: improve reflog message Phillip Wood via GitGitGadget
2022-04-17 2:07 ` Elijah Newren
2022-02-21 11:10 ` [PATCH 7/7] rebase: cleanup action handling Phillip Wood via GitGitGadget
2022-04-17 2:07 ` Elijah Newren
2022-04-04 15:34 ` Review Request (was Re: [PATCH 0/7] rebase: make reflog messages independent of the backend) Phillip Wood
2022-04-17 2:13 ` Elijah Newren
2022-04-18 18:56 ` Phillip Wood
2022-04-07 14:15 ` [PATCH 0/7] rebase: make reflog messages independent of the backend Christian Couder
2022-04-17 2:09 ` Elijah Newren
2022-04-20 9:56 ` [PATCH v2 0/8] " Phillip Wood via GitGitGadget
2022-04-20 9:56 ` [PATCH v2 1/8] rebase --apply: remove duplicated code Phillip Wood via GitGitGadget
2022-04-20 10:10 ` Ævar Arnfjörð Bjarmason
2022-04-20 18:25 ` Junio C Hamano
2022-04-20 18:39 ` Junio C Hamano
2022-04-20 9:56 ` [PATCH v2 2/8] t3406: rework rebase reflog tests Phillip Wood via GitGitGadget
2022-04-20 20:17 ` Junio C Hamano
2022-04-20 9:56 ` [PATCH v2 3/8] rebase --merge: fix reflog when continuing Phillip Wood via GitGitGadget
2022-04-20 9:56 ` [PATCH v2 4/8] rebase --merge: fix reflog message after skipping Phillip Wood via GitGitGadget
2022-04-20 9:56 ` [PATCH v2 5/8] rebase --apply: respect GIT_REFLOG_ACTION Phillip Wood via GitGitGadget
2022-04-20 9:56 ` [PATCH v2 6/8] rebase --apply: make reflog messages match rebase --merge Phillip Wood via GitGitGadget
2022-04-20 10:22 ` Ævar Arnfjörð Bjarmason
2022-04-20 22:15 ` Junio C Hamano
2022-04-20 9:56 ` [PATCH v2 7/8] rebase --abort: improve reflog message Phillip Wood via GitGitGadget
2022-04-20 9:56 ` [PATCH v2 8/8] rebase: cleanup action handling Phillip Wood via GitGitGadget
2022-04-20 10:34 ` Ævar Arnfjörð Bjarmason
2022-10-12 9:35 ` [PATCH v3 0/8] rebase: make reflog messages independent of the backend Phillip Wood via GitGitGadget
2022-10-12 9:35 ` [PATCH v3 1/8] rebase --apply: remove duplicated code Phillip Wood via GitGitGadget
2022-10-12 20:36 ` Junio C Hamano
2022-10-13 18:13 ` Junio C Hamano
2022-10-20 9:50 ` Phillip Wood
2022-10-12 9:35 ` [PATCH v3 2/8] t3406: rework rebase reflog tests Phillip Wood via GitGitGadget
2022-10-12 9:35 ` [PATCH v3 3/8] rebase --merge: fix reflog when continuing Phillip Wood via GitGitGadget
2022-10-12 9:35 ` [PATCH v3 4/8] rebase --merge: fix reflog message after skipping Phillip Wood via GitGitGadget
2022-10-12 9:35 ` [PATCH v3 5/8] rebase --apply: respect GIT_REFLOG_ACTION Phillip Wood via GitGitGadget
2022-10-12 9:35 ` [PATCH v3 6/8] rebase --apply: make reflog messages match rebase --merge Phillip Wood via GitGitGadget
2022-10-12 9:35 ` [PATCH v3 7/8] rebase --abort: improve reflog message Phillip Wood via GitGitGadget
2022-10-12 9:35 ` [PATCH v3 8/8] rebase: cleanup action handling Phillip Wood via GitGitGadget
2022-10-12 20:37 ` [PATCH v3 0/8] rebase: make reflog messages independent of the backend Junio C Hamano
2022-10-13 8:44 ` Phillip Wood
2022-10-13 15:31 ` Junio C Hamano
2022-10-21 9:21 ` Phillip Wood via GitGitGadget [this message]
2022-10-21 9:21 ` [PATCH v4 1/8] t3406: rework rebase reflog tests Phillip Wood via GitGitGadget
2022-10-21 9:21 ` [PATCH v4 2/8] rebase --apply: improve fast-forward reflog message Phillip Wood via GitGitGadget
2022-10-21 9:21 ` [PATCH v4 3/8] rebase --merge: fix reflog when continuing Phillip Wood via GitGitGadget
2022-10-21 17:37 ` Junio C Hamano
2022-10-25 10:08 ` Phillip Wood
2022-10-25 16:11 ` Junio C Hamano
2022-10-26 15:17 ` Phillip Wood
2022-10-26 16:55 ` Junio C Hamano
2022-10-21 9:21 ` [PATCH v4 4/8] rebase --merge: fix reflog message after skipping Phillip Wood via GitGitGadget
2022-10-21 9:21 ` [PATCH v4 5/8] rebase --apply: respect GIT_REFLOG_ACTION Phillip Wood via GitGitGadget
2022-10-21 17:38 ` Junio C Hamano
2022-10-21 9:21 ` [PATCH v4 6/8] rebase --apply: make reflog messages match rebase --merge Phillip Wood via GitGitGadget
2022-10-21 17:39 ` Junio C Hamano
2022-10-21 9:21 ` [PATCH v4 7/8] rebase --abort: improve reflog message Phillip Wood via GitGitGadget
2022-10-21 17:44 ` Junio C Hamano
2022-10-21 9:21 ` [PATCH v4 8/8] rebase: cleanup action handling Phillip Wood via GitGitGadget
2022-10-21 17:54 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1150.v4.git.1666344108.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=avarab@gmail.com \
--cc=calvinwan@google.com \
--cc=chooglen@google.com \
--cc=christian.couder@gmail.com \
--cc=emilyshaffer@google.com \
--cc=git@vger.kernel.org \
--cc=newren@gmail.com \
--cc=phillip.wood123@gmail.com \
--cc=phillip.wood@dunelm.org.uk \
--cc=vdye@github.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.