Re: [PATCH v3 0/8] rebase: update branches in multi-part topic
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Jul 1, 2022, 13:43 UTC
- Message-ID
- <799939ac-e54d-b156-efe1-5f2d7edf2312@gmail.com>
- In-Reply-To
- <pull.1247.v3.git.1656422759.gitgitgadget@gmail.com>
Hi Stolee
On 28/06/2022 14:25, Derrick Stolee via GitGitGadget wrote:
Show 23 quoted lines
> (Update: This is now based on ds/branch-checked-out.) > > This is a feature I've wanted for quite a while. When working on the sparse > index topic, I created a long RFC that actually broke into three topics for > full review upstream. These topics were sequential, so any feedback on an > earlier one required updates to the later ones. I would work on the full > feature and use interactive rebase to update the full list of commits. > However, I would need to update the branches pointing to those sub-topics. > > This series adds a new --update-refs option to 'git rebase' (along with a > rebase.updateRefs config option) that adds 'update-ref' commands into the > TODO list. This is powered by the commit decoration machinery. > > As an example, here is my in-progress bundle URI RFC split into subtopics as > they appear during the TODO list of a git rebase -i --update-refs: > > pick 2d966282ff3 docs: document bundle URI standard > pick 31396e9171a remote-curl: add 'get' capability > pick 54c6ab70f67 bundle-uri: create basic file-copy logic > pick 96cb2e35af1 bundle-uri: add support for http(s):// and file:// > pick 6adaf842684 fetch: add --bundle-uri option > pick 6c5840ed77e fetch: add 'refs/bundle/' to log.excludeDecoration > update-ref refs/heads/bundle-redo/fetch
I like the direction this is moving, my only concern with this version is how we handle the user adding and removing update-ref commands when the user edits the todo list.
Best Wishes
Phillip
Show 762 quoted lines
> pick 1e3f6546632 clone: add --bundle-uri option
> pick 9e4a6fe9b68 clone: --bundle-uri cannot be combined with --depth
> update-ref refs/heads/bundle-redo/clone
>
> pick 5451cb6599c bundle-uri: create bundle_list struct and helpers
> pick 3029c3aca15 bundle-uri: create base key-value pair parsing
> pick a8b2de79ce8 bundle-uri: create "key=value" line parsing
> pick 92625a47673 bundle-uri: unit test "key=value" parsing
> pick a8616af4dc2 bundle-uri: limit recursion depth for bundle lists
> pick 9d6809a8d53 bundle-uri: parse bundle list in config format
> pick 287a732b54c bundle-uri: fetch a list of bundles
> update-ref refs/heads/bundle-redo/list
>
> pick b09f8226185 protocol v2: add server-side "bundle-uri" skeleton
> pick 520204dcd1c bundle-uri client: add minimal NOOP client
> pick 62e8b457b48 bundle-uri client: add "git ls-remote-bundle-uri"
> pick 00eae925043 bundle-uri: serve URI advertisement from bundle.* config
> pick 4277440a250 bundle-uri client: add boolean transfer.bundleURI setting
> pick caf4599a81d bundle-uri: allow relative URLs in bundle lists
> pick df255000b7e bundle-uri: download bundles from an advertised list
> pick d71beabf199 clone: unbundle the advertised bundles
> pick c9578391976 t5601: basic bundle URI tests
> # Ref refs/heads/bundle-redo/rfc-3 checked out at '/home/stolee/_git/git-bundles'
>
> update-ref refs/heads/bundle-redo/advertise
>
>
> Here is an outline of the series:
>
> * Patch 1 updates some tests for branch_checked_out() for the 'apply'
> backend.
> * Patch 2 updates branch_checked_out() to parse the
> rebase-merge/update-refs file to block concurrent ref updates and
> checkouts on branches selected by --update-refs.
> * Patch 3 updates the todo list documentation to remove some unnecessary
> dots in the 'merge' command. This makes it consistent with the 'fixup'
> command before we document the 'update-ref' command.
> * Patch 4 updates the definition of todo_command_info to use enum values as
> array indices.
> * Patches 5-7 implement the --update-refs logic itself.
> * Patch 8 adds the rebase.updateRefs config option similar to
> rebase.autoSquash.
>
>
> Updates in v3
> =============
>
> * The branch_checked_out() API was extracted to its own topic and is now
> the ds/branch-checked-out branch. This series is now based on that one.
> * The for_each_decoration() API was removed, since it became trivial once
> it did not take a commit directly.
> * The branch_checked_out() tests did not verify the rebase-apply data (for
> the apply backend), so that is fixed.
> * Instead of using the 'label' command and a final 'update-refs' command in
> the todo list, use a new 'update-ref ' command. This command updates the
> rebase-merge/update-refs file with the OID of HEAD at these steps. At the
> very end of the rebase sequence, those refs are updated to the stored OID
> values (assuming that they were not removed by the user, in which case we
> notice that the OID is the null OID and we do nothing).
> * New tests are added.
> * The todo-list comment documentation has some new formatting updates, but
> also includes a description of 'update-refs' in this version.
>
>
> Updates in v2
> =============
>
> As recommended by the excellent feedback, I have removed the 'exec' commands
> in favor of the 'label' commands and a new 'update-refs' command at the very
> end. This way, there is only one step that updates all of the refs at the
> end instead of updating refs during the rebase. If a user runs 'git rebase
> --abort' in the middle, then their refs are still where they need to be.
>
> Based on some of the discussion, it seemed like one way to do this would be
> to have an 'update-ref ' command that would take the place of these 'label'
> commands. However, this would require two things that make it a bit awkward:
>
> 1. We would need to replicate the storage of those positions during the
> rebase. 'label' already does this pretty well. I've added the
> "for-update-refs/" label to help here.
> 2. If we want to close out all of the refs as the rebase is finishing, then
> that "step" becomes invisible to the user (and a bit more complicated to
> insert). Thus, the 'update-refs' step performs this action. If the user
> wants to do things after that step, then they can do so by editing the
> TODO list.
>
> Other updates:
>
> * The 'keep_decorations' parameter was renamed to 'update_refs'.
> * I added tests for --rebase-merges=rebase-cousins to show how these labels
> interact with other labels and merge commands.
> * I changed the order of the insertion of these update-refs labels to be
> before the fixups are rearranged. This fixes a bug where the tip commit
> is a fixup! so its decorations are never inspected (and they would be in
> the wrong place even if they were). The fixup! commands are properly
> inserted between a pick and its following label command. Tests
> demonstrate this is correct.
> * Numerous style choices are updated based on feedback.
>
> Thank you for all of the detailed review and ideas in this space. I
> appreciate any more ideas that can make this feature as effective as it can
> be.
>
> Thanks, -Stolee
>
> Derrick Stolee (8):
> t2407: test branches currently using apply backend
> branch: consider refs under 'update-refs'
> rebase-interactive: update 'merge' description
> sequencer: define array with enum values
> sequencer: add update-ref command
> rebase: add --update-refs option
> rebase: update refs from 'update-ref' commands
> rebase: add rebase.updateRefs config option
>
> Documentation/config/rebase.txt | 3 +
> Documentation/git-rebase.txt | 11 ++
> branch.c | 13 ++
> builtin/rebase.c | 10 ++
> rebase-interactive.c | 9 +-
> sequencer.c | 303 ++++++++++++++++++++++++++++++--
> sequencer.h | 11 ++
> t/t2407-worktree-heads.sh | 52 +++++-
> t/t3404-rebase-interactive.sh | 107 +++++++++++
> 9 files changed, 496 insertions(+), 23 deletions(-)
>
>
> base-commit: 9bef0b1e6ec371e786c2fba3edcc06ad040a536c
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1247%2Fderrickstolee%2Frebase-keep-decorations-v3
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1247/derrickstolee/rebase-keep-decorations-v3
> Pull-Request: https://github.com/gitgitgadget/git/pull/1247
>
> Range-diff vs v2:
>
> 1: 4f9f3487641 < -: ----------- log-tree: create for_each_decoration()
> 2: 5f54766e103 < -: ----------- branch: add branch_checked_out() helper
> -: ----------- > 1: fbaedc7f1f0 t2407: test branches currently using apply backend
> -: ----------- > 2: 2bc647b6fcd branch: consider refs under 'update-refs'
> -: ----------- > 3: 669f4abd59e rebase-interactive: update 'merge' description
> 3: 9f261c7df2c = 4: 6528a50343f sequencer: define array with enum values
> 4: 842b2186d25 ! 5: e95ad41d355 sequencer: add update-refs command
> @@ Metadata
> Author: Derrick Stolee <derrickstolee@github.com>
>
> ## Commit message ##
> - sequencer: add update-refs command
> + sequencer: add update-ref command
>
> - Add the boilerplat for an "update-refs" command in the sequencer. This
> - connects to the current no-op do_update_refs() which will be filled in
> + Add the boilerplate for an "update-ref" command in the sequencer. This
> + connects to the current no-op do_update_ref() which will be filled in
> after more connections are created.
>
> + The syntax in the todo list will be "update-ref <ref-name>" to signal
> + that we should store the current commit as the value for updating
> + <ref-name> at the end of the rebase.
> +
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
>
> + ## rebase-interactive.c ##
> +@@ rebase-interactive.c: void append_todo_help(int command_count,
> + " create a merge commit using the original merge commit's\n"
> + " message (or the oneline, if no original merge commit was\n"
> + " specified); use -c <commit> to reword the commit message\n"
> ++"u, update-ref <ref> = track a placeholder for the <ref> to be updated\n"
> ++" to this position in the new commits. The <ref> is\n"
> ++" updated at the end of the rebase\n"
> + "\n"
> + "These lines can be re-ordered; they are executed from top to bottom.\n");
> + unsigned edit_todo = !(shortrevisions && shortonto);
> +
> ## sequencer.c ##
> @@ sequencer.c: static struct {
> [TODO_LABEL] = { 'l', "label" },
> [TODO_RESET] = { 't', "reset" },
> [TODO_MERGE] = { 'm', "merge" },
> -+ [TODO_UPDATE_REFS] = { 'u', "update-refs" },
> ++ [TODO_UPDATE_REF] = { 'u', "update-ref" },
> [TODO_NOOP] = { 0, "noop" },
> [TODO_DROP] = { 'd', "drop" },
> [TODO_COMMENT] = { 0, NULL },
> @@ sequencer.c: static int parse_insn_line(struct repository *r, struct todo_item *item,
> - padding = strspn(bol, " \t");
> - bol += padding;
> + command_to_string(item->command));
>
> -- if (item->command == TODO_NOOP || item->command == TODO_BREAK) {
> -+ if (item->command == TODO_NOOP ||
> -+ item->command == TODO_BREAK ||
> -+ item->command == TODO_UPDATE_REFS) {
> - if (bol != eol)
> - return error(_("%s does not accept arguments: '%s'"),
> - command_to_string(item->command), bol);
> + if (item->command == TODO_EXEC || item->command == TODO_LABEL ||
> +- item->command == TODO_RESET) {
> ++ item->command == TODO_RESET || item->command == TODO_UPDATE_REF) {
> + item->commit = NULL;
> + item->arg_offset = bol - buf;
> + item->arg_len = (int)(eol - bol);
> @@ sequencer.c: leave_merge:
> return ret;
> }
>
> -+static int do_update_refs(struct repository *r)
> ++static int do_update_ref(struct repository *r, const char *ref_name)
> +{
> + return 0;
> +}
> @@ sequencer.c: static int pick_commits(struct repository *r,
> return error_with_patch(r, item->commit,
> arg, item->arg_len,
> opts, res, 0);
> -+ } else if (item->command == TODO_UPDATE_REFS) {
> -+ if ((res = do_update_refs(r)))
> ++ } else if (item->command == TODO_UPDATE_REF) {
> ++ struct strbuf ref = STRBUF_INIT;
> ++ strbuf_add(&ref, arg, item->arg_len);
> ++ if ((res = do_update_ref(r, ref.buf)))
> + reschedule = 1;
> ++ strbuf_release(&ref);
> } else if (!is_noop(item->command))
> return error(_("unknown command %d"), item->command);
>
> @@ sequencer.h: enum todo_command {
> TODO_LABEL,
> TODO_RESET,
> TODO_MERGE,
> -+ TODO_UPDATE_REFS,
> ++ TODO_UPDATE_REF,
> /* commands that do nothing but are counted for reporting progress */
> TODO_NOOP,
> TODO_DROP,
> 5: 0a4c110127b ! 6: 918b398d6a2 rebase: add --update-refs option
> @@ Commit message
> 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 'label
> - for-update-refs/*' steps to the todo file whenever a commit that is
> - being rebased is decorated with that <ref>. At the very end, the
> - 'update-refs' step is added to update all of the branches referenced by
> - the 'label' steps. This allows the user to rebase a long list of commits
> - in a multi-part feature and keep all of their pointers to those parts.
> + 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.
>
> - NOTE: This change only introduce the --update-refs option and implements
> - the changes to the todo file. It does _not_ yet implement the action
> - taken by the 'update-refs' todo step, which will be implemented and
> - tested in a later change.
> -
> - Use the new for_each_decoration() while iterating over the existing todo
> - list. 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.
> + 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
> @@ Commit message
> of the rebased branch while the other sub branches come along for the
> ride without intervention.
>
> - Be careful to not attempt updating any branch that is checked out. The
> - most common example is the branch being rebased is checked out and
> - decorates the tip commit. If the user is rebasing commits reachable from
> - a different branch that is checked out in a different worktree, then
> - they may be surprised to not see that ref update. However, it's probably
> - best to not optimize for this scenario and do the safest thing that will
> - result in a successful rebase. A comment is left in the TODO list that
> - signals that these refs are currently checked out.
> + 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@github.com>
>
> @@ sequencer.c
> #include "rebase-interactive.h"
> #include "reset.h"
> +#include "branch.h"
> -+#include "log-tree.h"
>
> #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
>
> @@ sequencer.c: static int skip_unnecessary_picks(struct repository *r,
> + size_t items_alloc;
> + struct strbuf *buf;
> + struct commit *commit;
> ++ struct string_list refs_to_oids;
> +};
> +
> -+static int add_branch_for_decoration(const struct name_decoration *d, void *data)
> ++static int add_decorations_to_list(const struct commit *commit,
> ++ struct todo_add_branch_context *ctx)
> +{
> -+ struct todo_add_branch_context *ctx = data;
> -+ size_t base_offset = ctx->buf->len;
> -+ struct todo_item *item;
> -+ char *path;
> ++ const struct name_decoration *decoration = get_name_decoration(&commit->object);
> +
> -+ ALLOC_GROW(ctx->items,
> -+ ctx->items_nr + 1,
> -+ ctx->items_alloc);
> -+ item = &ctx->items[ctx->items_nr];
> -+ memset(item, 0, sizeof(*item));
> ++ while (decoration) {
> ++ struct todo_item *item;
> ++ const char *path;
> ++ size_t base_offset = ctx->buf->len;
> +
> -+ /* If the branch is checked out, then leave a comment instead. */
> -+ if (branch_checked_out(d->name, &path)) {
> -+ item->command = TODO_COMMENT;
> -+ strbuf_addf(ctx->buf, "# Ref %s checked out at '%s'\n",
> -+ d->name, path);
> -+ free(path);
> -+ } else {
> -+ item->command = TODO_LABEL;
> -+ strbuf_addf(ctx->buf, "for-update-refs/%s\n", d->name);
> -+ }
> ++ 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++;
> +
> -+ 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;
> +}
> @@ sequencer.c: static int skip_unnecessary_picks(struct repository *r,
> + 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
> ++ .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;
> @@ sequencer.c: static int skip_unnecessary_picks(struct repository *r,
> +
> + if (item->commit) {
> + ctx.commit = item->commit;
> -+ for_each_decoration(item->commit,
> -+ add_branch_for_decoration,
> -+ &ctx);
> ++ add_decorations_to_list(item->commit, &ctx);
> + }
> + }
> +
> -+ /* Add the "update-refs" step. */
> -+ ALLOC_GROW(ctx.items,
> -+ ctx.items_nr + 1,
> -+ ctx.items_alloc);
> -+ memset(&ctx.items[ctx.items_nr], 0, sizeof(struct todo_item));
> -+ ctx.items[ctx.items_nr].command = TODO_UPDATE_REFS;
> -+ ctx.items_nr++;
> -+
> ++ string_list_clear(&ctx.refs_to_oids, 1);
> + free(todo_list->items);
> + todo_list->items = ctx.items;
> + todo_list->nr = ctx.items_nr;
> @@ sequencer.h: int complete_action(struct repository *r, struct replay_opts *opts,
> int todo_list_rearrange_squash(struct todo_list *todo_list);
>
>
> + ## t/t2407-worktree-heads.sh ##
> +@@ t/t2407-worktree-heads.sh: 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
> +
> ## t/t3404-rebase-interactive.sh ##
> @@ t/t3404-rebase-interactive.sh: test_expect_success 'ORIG_HEAD is updated correctly' '
> test_cmp_rev ORIG_HEAD test-orig-head@{1}
> @@ t/t3404-rebase-interactive.sh: test_expect_success 'ORIG_HEAD is updated correct
> +
> + cat >expect <<-EOF &&
> + pick $(git log -1 --format=%h J) J
> -+ label for-update-refs/refs/heads/second
> -+ label for-update-refs/refs/heads/first
> ++ 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
> -+ label for-update-refs/refs/heads/third
> ++ update-ref refs/heads/third
> + pick $(git log -1 --format=%h M) M
> -+ label for-update-refs/refs/heads/no-conflict-branch
> -+ label for-update-refs/refs/heads/shared-tip
> -+ update-refs
> ++ 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 &&
> @@ t/t3404-rebase-interactive.sh: test_expect_success 'ORIG_HEAD is updated correct
> + reset onto
> + pick $(git log -1 --format=%h branch2~1) F
> + pick $(git log -1 --format=%h branch2) I
> -+ label for-update-refs/refs/heads/branch2
> ++ update-ref refs/heads/branch2
> + label merge
> + reset onto
> + pick $(git log -1 --format=%h refs/heads/second) J
> -+ label for-update-refs/refs/heads/second
> -+ label for-update-refs/refs/heads/first
> ++ 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
> -+ label for-update-refs/refs/heads/third
> ++ update-ref refs/heads/third
> + pick $(git log -1 --format=%h HEAD~2) M
> -+ label for-update-refs/refs/heads/no-conflict-branch
> ++ update-ref refs/heads/no-conflict-branch
> + merge -C $(git log -1 --format=%h HEAD~1) merge # merge
> -+ label for-update-refs/refs/heads/merge-branch
> -+ update-refs
> ++ update-ref refs/heads/merge-branch
> + EOF
> +
> + test_must_fail git rebase -i --autosquash \
> 6: 68f8e51b19c ! 7: 72e0481b643 sequencer: implement 'update-refs' command
> @@ Metadata
> Author: Derrick Stolee <derrickstolee@github.com>
>
> ## Commit message ##
> - sequencer: implement 'update-refs' command
> + rebase: update refs from 'update-ref' commands
>
> - The previous change allowed 'git rebase --update-refs' to create 'label'
> - commands for each branch among the commits being rewritten and add an
> - 'update-refs' command at the end of the todo list. Now, teach Git to
> - update the refs during that final 'update-refs' command.
> + The previous change introduced the 'git rebase --update-refs' option
> + which added 'update-ref <ref>' commands to the todo list of an
> + interactive rebase.
>
> - We need to create an array of new and old OIDs for each ref by iterating
> - over the refs/rewritten/for-update-refs/ namespace. We cannot update the
> - refs in-place since this will confuse the refs iterator.
> + Teach Git to record the HEAD position when reaching these 'update-ref'
> + commands. The ref/OID pair is stored in the
> + $GIT_DIR/rebase-merge/update-refs file. A previous change parsed this
> + file to avoid having other processes updating the refs in that file
> + while the rebase is in progress.
> +
> + Not only do we update the file when the sequencer reaches these
> + 'update-ref' commands, we then update the refs themselves at the end of
> + the rebase sequence. If the rebase is aborted before this final step,
> + then the refs are not updated.
>
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
>
> ## sequencer.c ##
> +@@
> + #include "rebase-interactive.h"
> + #include "reset.h"
> + #include "branch.h"
> ++#include "log-tree.h"
> +
> + #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
> +
> +@@ sequencer.c: static GIT_PATH_FUNC(rebase_path_squash_onto, "rebase-merge/squash-onto")
> + */
> + static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete")
> +
> ++/*
> ++ * The update-refs file stores a list of refs that will be updated at the end
> ++ * of the rebase sequence. The 'update-ref <ref>' commands in the todo file
> ++ * update the OIDs for the refs in this file, but the refs are not updated
> ++ * until the end of the rebase sequence.
> ++ */
> ++static GIT_PATH_FUNC(rebase_path_update_refs, "rebase-merge/update-refs")
> ++
> + /*
> + * The following files are written by git-rebase just after parsing the
> + * command-line.
> @@ sequencer.c: leave_merge:
> return ret;
> }
>
> --static int do_update_refs(struct repository *r)
> -+struct update_refs_context {
> -+ struct ref_store *refs;
> -+ char **ref_names;
> -+ struct object_id *old;
> -+ struct object_id *new;
> -+ size_t nr;
> -+ size_t alloc;
> -+};
> -+
> -+static int add_ref_to_context(const char *refname,
> -+ const struct object_id *oid,
> -+ int flags,
> -+ void *data)
> +-static int do_update_ref(struct repository *r, const char *ref_name)
> ++static int write_update_refs_state(struct string_list *refs_to_oids)
> ++{
> ++ int result = 0;
> ++ FILE *fp = NULL;
> ++ struct string_list_item *item;
> ++ char *path = xstrdup(rebase_path_update_refs());
> ++
> ++ if (safe_create_leading_directories(path)) {
> ++ result = error(_("unable to create leading directories of %s"),
> ++ path);
> ++ goto cleanup;
> ++ }
> ++
> ++ fp = fopen(path, "w");
> ++ if (!fp) {
> ++ result = error_errno(_("could not open '%s' for writing"), path);
> ++ goto cleanup;
> ++ }
> ++
> ++ for_each_string_list_item(item, refs_to_oids)
> ++ fprintf(fp, "%s\n%s\n", item->string, oid_to_hex(item->util));
> ++
> ++cleanup:
> ++ if (fp)
> ++ fclose(fp);
> ++ return result;
> ++}
> ++
> ++static int do_update_ref(struct repository *r, const char *refname)
> {
> -+ int f = 0;
> -+ const char *name;
> -+ struct update_refs_context *ctx = data;
> ++ struct string_list_item *item;
> ++ struct string_list list = STRING_LIST_INIT_DUP;
> ++ int found = 0;
> ++
> ++ sequencer_get_update_refs_state(r->gitdir, &list);
> +
> -+ ALLOC_GROW(ctx->ref_names, ctx->nr + 1, ctx->alloc);
> -+ ALLOC_GROW(ctx->old, ctx->nr + 1, ctx->alloc);
> -+ ALLOC_GROW(ctx->new, ctx->nr + 1, ctx->alloc);
> ++ for_each_string_list_item(item, &list) {
> ++ if (!strcmp(item->string, refname)) {
> ++ struct object_id oid;
> ++ free(item->util);
> ++ found = 1;
> +
> -+ if (!skip_prefix(refname, "refs/rewritten/for-update-refs/", &name))
> -+ return 1;
> ++ if (!read_ref("HEAD", &oid)) {
> ++ item->util = oiddup(&oid);
> ++ break;
> ++ }
> ++ }
> ++ }
> +
> -+ ctx->ref_names[ctx->nr] = xstrdup(name);
> -+ oidcpy(&ctx->new[ctx->nr], oid);
> -+ if (!refs_resolve_ref_unsafe(ctx->refs, name, 0,
> -+ &ctx->old[ctx->nr], &f))
> -+ return 1;
> ++ if (!found) {
> ++ struct object_id oid;
> ++ item = string_list_append(&list, refname);
> +
> -+ ctx->nr++;
> ++ if (!read_ref("HEAD", &oid))
> ++ item->util = oiddup(&oid);
> ++ else
> ++ item->util = oiddup(the_hash_algo->null_oid);
> ++ }
> ++
> ++ write_update_refs_state(&list);
> ++ string_list_clear(&list, 1);
> return 0;
> }
>
> +static int do_update_refs(struct repository *r)
> +{
> -+ int i, res;
> -+ struct update_refs_context ctx = {
> -+ .refs = get_main_ref_store(r),
> -+ .alloc = 16,
> -+ };
> -+ ALLOC_ARRAY(ctx.ref_names, ctx.alloc);
> -+ ALLOC_ARRAY(ctx.old, ctx.alloc);
> -+ ALLOC_ARRAY(ctx.new, ctx.alloc);
> -+
> -+ res = refs_for_each_fullref_in(ctx.refs,
> -+ "refs/rewritten/for-update-refs/",
> -+ add_ref_to_context,
> -+ &ctx);
> -+
> -+ for (i = 0; !res && i < ctx.nr; i++)
> -+ res = refs_update_ref(ctx.refs, "rewritten during rebase",
> -+ ctx.ref_names[i],
> -+ &ctx.new[i], &ctx.old[i],
> ++ int res = 0;
> ++ struct string_list_item *item;
> ++ struct string_list refs_to_oids = STRING_LIST_INIT_DUP;
> ++ struct ref_store *refs = get_main_ref_store(r);
> ++
> ++ sequencer_get_update_refs_state(r->gitdir, &refs_to_oids);
> ++
> ++ for_each_string_list_item(item, &refs_to_oids) {
> ++ struct object_id *oid_to = item->util;
> ++ struct object_id oid_from;
> ++
> ++ if (oideq(oid_to, the_hash_algo->null_oid)) {
> ++ /*
> ++ * Ref was not updated. User may have deleted the
> ++ * 'update-ref' step.
> ++ */
> ++ continue;
> ++ }
> ++
> ++ if (read_ref(item->string, &oid_from)) {
> ++ /*
> ++ * The ref does not exist. The user probably
> ++ * inserted a new 'update-ref' step with a new
> ++ * branch name.
> ++ */
> ++ oidcpy(&oid_from, the_hash_algo->null_oid);
> ++ }
> ++
> ++ res |= refs_update_ref(refs, "rewritten during rebase",
> ++ item->string,
> ++ oid_to, &oid_from,
> + 0, UPDATE_REFS_MSG_ON_ERR);
> ++ }
> +
> -+ for (i = 0; i < ctx.nr; i++)
> -+ free(ctx.ref_names[i]);
> -+ free(ctx.ref_names);
> -+ free(ctx.old);
> -+ free(ctx.new);
> ++ string_list_clear(&refs_to_oids, 1);
> + return res;
> +}
> +
> static int is_final_fixup(struct todo_list *todo_list)
> {
> int i = todo_list->current;
> +@@ sequencer.c: cleanup_head_ref:
> + strbuf_release(&head_ref);
> + }
> +
> ++ do_update_refs(r);
> ++
> + /*
> + * Sequence of picks finished successfully; cleanup by
> + * removing the .git/sequencer directory
> +@@ sequencer.c: static int todo_list_add_update_ref_commands(struct todo_list *todo_list)
> + }
> + }
> +
> ++ write_update_refs_state(&ctx.refs_to_oids);
> ++
> + string_list_clear(&ctx.refs_to_oids, 1);
> + free(todo_list->items);
> + todo_list->items = ctx.items;
>
> ## t/t3404-rebase-interactive.sh ##
> @@ t/t3404-rebase-interactive.sh: test_expect_success '--update-refs adds commands with --rebase-merges' '
> @@ t/t3404-rebase-interactive.sh: test_expect_success '--update-refs adds commands
> + git branch -f third HEAD~1 &&
> + test_commit extra2 fileX &&
> + git commit --amend --fixup=L &&
> -+ (
> -+ git rebase -i --autosquash --update-refs primary &&
> -+
> -+ compare_two_refs HEAD~3 refs/heads/first &&
> -+ compare_two_refs HEAD~3 refs/heads/second &&
> -+ compare_two_refs HEAD~1 refs/heads/third &&
> -+ compare_two_refs HEAD refs/heads/no-conflict-branch
> -+ )
> ++
> ++ git rebase -i --autosquash --update-refs primary &&
> ++
> ++ compare_two_refs HEAD~3 refs/heads/first &&
> ++ compare_two_refs HEAD~3 refs/heads/second &&
> ++ compare_two_refs HEAD~1 refs/heads/third &&
> ++ compare_two_refs HEAD refs/heads/no-conflict-branch
> +'
> +
> # This must be the last test in this file
> 7: 3d7d3f656b4 ! 8: d2cfdbfc431 rebase: add rebase.updateRefs config option
> @@ Documentation/git-rebase.txt: start would be overridden by the presence of
> or point to a `squash! ...` or `fixup! ...` commit are not updated
> in this way.
> ++
> -+If the `--update-refs` option is enabled by default using the
> -+configuration variable `rebase.updateRefs`, this option can be
> -+used to override and disable this setting.
> ++If the configuration variable `rebase.updateRefs` is set, then this option
> ++can be used to override and disable this setting.
>
> INCOMPATIBLE OPTIONS
> --------------------
>