On Tue, Jun 28, 2022 at 6:26 AM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
>
> When working on a large feature, it can be helpful to break that feature
> into multiple smaller parts that become reviewed in sequence. During
> development or during review, a change to one part of the feature could
> affect multiple of these parts. An interactive rebase can help adjust
> the multi-part "story" of the branch.
>
> However, if there are branches tracking the different parts of the
> feature, then rebasing the entire list of commits can create commits not
> reachable from those "sub branches". It can take a manual step to update
> those branches.
>
> Add a new --update-refs option to 'git rebase -i' that adds 'update-ref
> <ref>' steps to the todo file whenever a commit that is being rebased is
> decorated with that <ref>. At the very end, the rebase process updates
> all of the listed refs to the values stored during the rebase operation.
>
> Be sure to iterate after any squashing or fixups are placed. Update the
> branch only after those squashes and fixups are complete. This allows a
> --fixup commit at the tip of the feature to apply correctly to the sub
> branch, even if it is fixing up the most-recent commit in that part.
>
> One potential problem here is that refs decorating commits that are
> already marked as "fixup!" or "squash!" will not be included in this
> list. Generally, the reordering of the "fixup!" and "squash!" is likely
> to change the relative order of these refs, so it is not recommended.
> The workflow here is intended to allow these kinds of commits at the tip
> of the rebased branch while the other sub branches come along for the
> ride without intervention.
>
> This change update the documentation and builtin to accept the
> --update-refs option as well as updating the todo file with the
> 'update-ref' commands. Tests are added to ensure that these todo
> commands are added in the correct locations.
>
> A future change will update the behavior to actually update the refs
> at the end of the rebase sequence.
>
> Signed-off-by: Derrick Stolee <derrickstolee@xxxxxxxxxx>
> ---
> Documentation/git-rebase.txt | 8 +++
> builtin/rebase.c | 5 ++
> sequencer.c | 107 ++++++++++++++++++++++++++++++++++
> sequencer.h | 1 +
> t/t2407-worktree-heads.sh | 17 ++++++
> t/t3404-rebase-interactive.sh | 70 ++++++++++++++++++++++
> 6 files changed, 208 insertions(+)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 262fb01aec0..e7611b4089c 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -609,6 +609,13 @@ provided. Otherwise an explicit `--no-reschedule-failed-exec` at the
> start would be overridden by the presence of
> `rebase.rescheduleFailedExec=true` configuration.
>
> +--update-refs::
> +--no-update-refs::
> + Automatically force-update any branches that point to commits that
> + are being rebased. Any branches that are checked out in a worktree
> + or point to a `squash! ...` or `fixup! ...` commit are not updated
> + in this way.
> +
> INCOMPATIBLE OPTIONS
> --------------------
>
> @@ -632,6 +639,7 @@ are incompatible with the following options:
> * --empty=
> * --reapply-cherry-picks
> * --edit-todo
> + * --update-refs
> * --root when used in combination with --onto
>
> In addition, the following pairs of options are incompatible:
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 7ab50cda2ad..56d82a52106 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -102,6 +102,7 @@ struct rebase_options {
> int reschedule_failed_exec;
> int reapply_cherry_picks;
> int fork_point;
> + int update_refs;
> };
>
> #define REBASE_OPTIONS_INIT { \
> @@ -298,6 +299,7 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
> ret = complete_action(the_repository, &replay, flags,
> shortrevisions, opts->onto_name, opts->onto,
> &opts->orig_head, &commands, opts->autosquash,
> + opts->update_refs,
> &todo_list);
> }
>
> @@ -1124,6 +1126,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> OPT_BOOL(0, "autosquash", &options.autosquash,
> N_("move commits that begin with "
> "squash!/fixup! under -i")),
> + OPT_BOOL(0, "update-refs", &options.update_refs,
> + N_("update local refs that point to commits "
> + "that are being rebased")),
> { OPTION_STRING, 'S', "gpg-sign", &gpg_sign, N_("key-id"),
> N_("GPG-sign commits"),
> PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
> diff --git a/sequencer.c b/sequencer.c
> index 0b61835d441..915d87a0336 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -35,6 +35,7 @@
> #include "commit-reach.h"
> #include "rebase-interactive.h"
> #include "reset.h"
> +#include "branch.h"
>
> #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
>
> @@ -5615,10 +5616,113 @@ static int skip_unnecessary_picks(struct repository *r,
> return 0;
> }
>
> +struct todo_add_branch_context {
> + struct todo_item *items;
> + size_t items_nr;
> + size_t items_alloc;
> + struct strbuf *buf;
> + struct commit *commit;
> + struct string_list refs_to_oids;
> +};
> +
> +static int add_decorations_to_list(const struct commit *commit,
> + struct todo_add_branch_context *ctx)
> +{
> + const struct name_decoration *decoration = get_name_decoration(&commit->object);
> +
> + while (decoration) {
> + struct todo_item *item;
> + const char *path;
> + size_t base_offset = ctx->buf->len;
> +
> + ALLOC_GROW(ctx->items,
> + ctx->items_nr + 1,
> + ctx->items_alloc);
> + item = &ctx->items[ctx->items_nr];
> + memset(item, 0, sizeof(*item));
> +
> + /* If the branch is checked out, then leave a comment instead. */
> + if ((path = branch_checked_out(decoration->name))) {
> + item->command = TODO_COMMENT;
> + strbuf_addf(ctx->buf, "# Ref %s checked out at '%s'\n",
> + decoration->name, path);
> + } else {
> + struct string_list_item *sti;
> + item->command = TODO_UPDATE_REF;
> + strbuf_addf(ctx->buf, "%s\n", decoration->name);
> +
> + sti = string_list_append(&ctx->refs_to_oids,
> + decoration->name);
> + sti->util = oiddup(the_hash_algo->null_oid);
> + }
> +
> + item->offset_in_buf = base_offset;
> + item->arg_offset = base_offset;
> + item->arg_len = ctx->buf->len - base_offset;
> + ctx->items_nr++;
> +
> + decoration = decoration->next;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * For each 'pick' command, find out if the commit has a decoration in
Is this really limited to picks? If someone uses --autosquash and has
a fixup or squash in the list, wouldn't this apply as well, or does
all of this apply before the transformations to fixup/squash? Also,
what if the user is doing --rebase-merges and there's a merge commit
with a branch pointing at the merge. Would that be included?
> + * refs/heads/. If so, then add a 'label for-update-refs/' command.
> + */
> +static int todo_list_add_update_ref_commands(struct todo_list *todo_list)
> +{
> + int i;
> + static struct string_list decorate_refs_exclude = STRING_LIST_INIT_NODUP;
> + static struct string_list decorate_refs_exclude_config = STRING_LIST_INIT_NODUP;
> + static struct string_list decorate_refs_include = STRING_LIST_INIT_NODUP;
> + struct decoration_filter decoration_filter = {
> + .include_ref_pattern = &decorate_refs_include,
> + .exclude_ref_pattern = &decorate_refs_exclude,
> + .exclude_ref_config_pattern = &decorate_refs_exclude_config,
> + };
> + struct todo_add_branch_context ctx = {
> + .buf = &todo_list->buf,
> + .refs_to_oids = STRING_LIST_INIT_DUP,
> + };
> +
> + ctx.items_alloc = 2 * todo_list->nr + 1;
> + ALLOC_ARRAY(ctx.items, ctx.items_alloc);
> +
> + string_list_append(&decorate_refs_include, "refs/heads/");
> + load_ref_decorations(&decoration_filter, 0);
> +
> + for (i = 0; i < todo_list->nr; ) {
> + struct todo_item *item = &todo_list->items[i];
> +
> + /* insert ith item into new list */
> + ALLOC_GROW(ctx.items,
> + ctx.items_nr + 1,
> + ctx.items_alloc);
> +
> + ctx.items[ctx.items_nr++] = todo_list->items[i++];
> +
> + if (item->commit) {
> + ctx.commit = item->commit;
> + add_decorations_to_list(item->commit, &ctx);
> + }
> + }
> +
> + string_list_clear(&ctx.refs_to_oids, 1);
> + free(todo_list->items);
> + todo_list->items = ctx.items;
> + todo_list->nr = ctx.items_nr;
> + todo_list->alloc = ctx.items_alloc;
> +
> + return 0;
> +}
> +
> int complete_action(struct repository *r, struct replay_opts *opts, unsigned flags,
> const char *shortrevisions, const char *onto_name,
> struct commit *onto, const struct object_id *orig_head,
> struct string_list *commands, unsigned autosquash,
> + unsigned update_refs,
> struct todo_list *todo_list)
> {
> char shortonto[GIT_MAX_HEXSZ + 1];
> @@ -5637,6 +5741,9 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
> item->arg_len = item->arg_offset = item->flags = item->offset_in_buf = 0;
> }
>
> + if (update_refs && todo_list_add_update_ref_commands(todo_list))
> + return -1;
> +
> if (autosquash && todo_list_rearrange_squash(todo_list))
> return -1;
>
> diff --git a/sequencer.h b/sequencer.h
> index 2cf5c1b9a38..e671d7e0743 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -167,6 +167,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
> const char *shortrevisions, const char *onto_name,
> struct commit *onto, const struct object_id *orig_head,
> struct string_list *commands, unsigned autosquash,
> + unsigned update_refs,
> struct todo_list *todo_list);
> int todo_list_rearrange_squash(struct todo_list *todo_list);
>
> diff --git a/t/t2407-worktree-heads.sh b/t/t2407-worktree-heads.sh
> index f1e9e172a0c..203997d92c6 100755
> --- a/t/t2407-worktree-heads.sh
> +++ b/t/t2407-worktree-heads.sh
> @@ -151,4 +151,21 @@ test_expect_success 'refuse to overwrite when in error states' '
> done
> '
>
> +. "$TEST_DIRECTORY"/lib-rebase.sh
> +
> +test_expect_success !SANITIZE_LEAK 'refuse to overwrite during rebase with --update-refs' '
> + git commit --fixup HEAD~2 --allow-empty &&
> + (
> + set_cat_todo_editor &&
> + test_must_fail git rebase -i --update-refs HEAD~3 >todo &&
> + ! grep "update-refs" todo
> + ) &&
> + git branch -f allow-update HEAD~2 &&
> + (
> + set_cat_todo_editor &&
> + test_must_fail git rebase -i --update-refs HEAD~3 >todo &&
> + grep "update-ref refs/heads/allow-update" todo
> + )
> +'
> +
> test_done
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index f31afd4a547..3cd20733bc8 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1743,6 +1743,76 @@ test_expect_success 'ORIG_HEAD is updated correctly' '
> test_cmp_rev ORIG_HEAD test-orig-head@{1}
> '
>
> +test_expect_success '--update-refs adds label and update-ref commands' '
> + git checkout -b update-refs no-conflict-branch &&
> + git branch -f base HEAD~4 &&
> + git branch -f first HEAD~3 &&
> + git branch -f second HEAD~3 &&
> + git branch -f third HEAD~1 &&
> + git commit --allow-empty --fixup=third &&
> + git branch -f shared-tip &&
> + (
> + set_cat_todo_editor &&
> +
> + cat >expect <<-EOF &&
> + pick $(git log -1 --format=%h J) J
> + update-ref refs/heads/second
> + update-ref refs/heads/first
> + pick $(git log -1 --format=%h K) K
> + pick $(git log -1 --format=%h L) L
> + fixup $(git log -1 --format=%h update-refs) fixup! L # empty
> + update-ref refs/heads/third
> + pick $(git log -1 --format=%h M) M
> + update-ref refs/heads/no-conflict-branch
> + update-ref refs/heads/shared-tip
> + EOF
> +
> + test_must_fail git rebase -i --autosquash --update-refs primary >todo &&
> + test_cmp expect todo
> + )
> +'
> +
> +test_expect_success '--update-refs adds commands with --rebase-merges' '
> + git checkout -b update-refs-with-merge no-conflict-branch &&
> + git branch -f base HEAD~4 &&
> + git branch -f first HEAD~3 &&
> + git branch -f second HEAD~3 &&
> + git branch -f third HEAD~1 &&
> + git merge -m merge branch2 &&
> + git branch -f merge-branch &&
> + git commit --fixup=third --allow-empty &&
> + (
> + set_cat_todo_editor &&
> +
> + cat >expect <<-EOF &&
> + label onto
> + reset onto
> + pick $(git log -1 --format=%h branch2~1) F
> + pick $(git log -1 --format=%h branch2) I
> + update-ref refs/heads/branch2
> + label merge
> + reset onto
> + pick $(git log -1 --format=%h refs/heads/second) J
> + update-ref refs/heads/second
> + update-ref refs/heads/first
> + pick $(git log -1 --format=%h refs/heads/third~1) K
> + pick $(git log -1 --format=%h refs/heads/third) L
> + fixup $(git log -1 --format=%h update-refs-with-merge) fixup! L # empty
> + update-ref refs/heads/third
> + pick $(git log -1 --format=%h HEAD~2) M
> + update-ref refs/heads/no-conflict-branch
> + merge -C $(git log -1 --format=%h HEAD~1) merge # merge
> + update-ref refs/heads/merge-branch
> + EOF
> +
> + test_must_fail git rebase -i --autosquash \
> + --rebase-merges=rebase-cousins \
> + --update-refs primary >todo &&
> +
> + test_cmp expect todo
> + )
> +'
> +
> # This must be the last test in this file
> test_expect_success '$EDITOR and friends are unchanged' '
> test_editor_unchanged
> --
> gitgitgadget
>
