diff mbox series

[v4,07/12] rebase: add --update-refs option

Message ID 3ec2cc922f971af4e4a558188cf139cc0c0150d6.1657631226.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series rebase: update branches in multi-part topic | expand

Commit Message

Derrick Stolee July 12, 2022, 1:07 p.m. UTC
From: Derrick Stolee <derrickstolee@github.com>

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.

This change does _not_ include the actual behavior of tracking the
updated refs and writing the new ref values at the end of the rebase
process. That is deferred to a later change.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 Documentation/git-rebase.txt  |   8 +++
 builtin/rebase.c              |   5 ++
 sequencer.c                   | 107 ++++++++++++++++++++++++++++++++++
 sequencer.h                   |   1 +
 t/t2407-worktree-heads.sh     |  22 +++++++
 t/t3404-rebase-interactive.sh |  70 ++++++++++++++++++++++
 6 files changed, 213 insertions(+)

Comments

Elijah Newren July 16, 2022, 7:30 p.m. UTC | #1
On Tue, Jul 12, 2022 at 6:07 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Derrick Stolee <derrickstolee@github.com>
>
[...]
>
> +--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.

I think the second sentence here should be split.  In particular, I
don't think I understand the second half of the second sentence.  Do
you intend to say here that branches pointing to a `squash!` or
`fixup!` will instead update the first `pick` in the ancestry of such
a commit, rather than that such branches are entirely excluded from
any updates?  That's what I observed in my testing of your v3, at
least, and that's the behavior I'd expect this feature to implement,
but this documentation doesn't match.

[...]
> @@ -5660,6 +5764,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;
> +

As a tangent, I find it interesting that you add the update-ref
commands as a post-processing step rather than as a part of
sequencer_make_script().  I don't think you need to change anything,
but I am curious due to my git-replay work if you find it advantageous
to do it this way.
SZEDER Gábor July 18, 2022, 9:05 a.m. UTC | #2
On Tue, Jul 12, 2022 at 01:07:00PM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <derrickstolee@github.com>
> 
> 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.
> 
> This change does _not_ include the actual behavior of tracking the
> updated refs and writing the new ref values at the end of the rebase
> process. That is deferred to a later change.
> 
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  Documentation/git-rebase.txt  |   8 +++
>  builtin/rebase.c              |   5 ++
>  sequencer.c                   | 107 ++++++++++++++++++++++++++++++++++
>  sequencer.h                   |   1 +
>  t/t2407-worktree-heads.sh     |  22 +++++++
>  t/t3404-rebase-interactive.sh |  70 ++++++++++++++++++++++
>  6 files changed, 213 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::

So the option is called '--update-refs', but ...

> +--no-update-refs::
> +	Automatically force-update any branches that point to commits that

... its description talks about "branches".

> +	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
>  --------------------
>  

> @@ -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 "

And its short help talks about "local refs".

I think at least the documentation and short help should use
consistent terminology.

> +			    "that are being rebased")),
>  		{ OPTION_STRING, 'S', "gpg-sign", &gpg_sign, N_("key-id"),
>  			N_("GPG-sign commits"),
>  			PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
Derrick Stolee July 18, 2022, 4:55 p.m. UTC | #3
On 7/18/2022 5:05 AM, SZEDER Gábor wrote:
> On Tue, Jul 12, 2022 at 01:07:00PM +0000, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <derrickstolee@github.com>

>> +--update-refs::
> 
> So the option is called '--update-refs', but ...
> 
>> +--no-update-refs::
>> +	Automatically force-update any branches that point to commits that
> 
> ... its description talks about "branches".

>> +		OPT_BOOL(0, "update-refs", &options.update_refs,
>> +			 N_("update local refs that point to commits "
> 
> And its short help talks about "local refs".
> 
> I think at least the documentation and short help should use
> consistent terminology.

Thanks for catching this. I think I should use "branches" here, but
keep the name "--update-refs". The biggest reason is that it provides
a nice parallel with the "update-ref" sequencer command. This command
allows updating _any_ ref, such as lightweight tags in refs/tags/*
or even refs in refs/my/namespace/*.

The --update-refs option doesn't create the commands to update tags
or refs in places other than refs/heads/*.

Thanks,
-Stolee
Junio C Hamano July 18, 2022, 7:35 p.m. UTC | #4
Derrick Stolee <derrickstolee@github.com> writes:

> ... I think I should use "branches" here, but
> keep the name "--update-refs". The biggest reason is that it provides
> a nice parallel with the "update-ref" sequencer command. This command
> allows updating _any_ ref, such as lightweight tags in refs/tags/*
> or even refs in refs/my/namespace/*.
>
> The --update-refs option doesn't create the commands to update tags
> or refs in places other than refs/heads/*.

I guess it would make the choice of "branch" the most appropriate.

I was hoping that we can repoint refs in private namespaces that are
not branches with the option.  But as long as the underlying
"update-ref" instruction can be used by advanced users, that is OK.
Derrick Stolee July 19, 2022, 3:50 p.m. UTC | #5
On 7/16/2022 3:30 PM, Elijah Newren wrote:
> On Tue, Jul 12, 2022 at 6:07 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> From: Derrick Stolee <derrickstolee@github.com>
>>
> [...]
>>
>> +--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.
> 
> I think the second sentence here should be split.  In particular, I
> don't think I understand the second half of the second sentence.  Do
> you intend to say here that branches pointing to a `squash!` or
> `fixup!` will instead update the first `pick` in the ancestry of such
> a commit, rather than that such branches are entirely excluded from
> any updates?  That's what I observed in my testing of your v3, at
> least, and that's the behavior I'd expect this feature to implement,
> but this documentation doesn't match.

Good find. You're right that these don't match, and its in fact that
I expected it to work this way, but it doesn't.

I've added a branch to my tests that points to a fixup! that is not
the tip commit and use it to demonstrate that it is not reordered, but
_does_ appear in the 'update-ref <ref>' list.

I'll update the documentation to match this behavior, too. This case
is unlikely to happen much in practice, but I now believe it's better
to include the ref than to ignore it.
 
> [...]
>> @@ -5660,6 +5764,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;
>> +
> 
> As a tangent, I find it interesting that you add the update-ref
> commands as a post-processing step rather than as a part of
> sequencer_make_script().  I don't think you need to change anything,
> but I am curious due to my git-replay work if you find it advantageous
> to do it this way.

I found it to be simple enough to do a scan of the steps directly
instead of injecting extra logic into the _make_script() method.
The simplest reason is that we would need to inject "update_refs"
into that method.

Thanks,
-Stolee
Derrick Stolee July 19, 2022, 3:53 p.m. UTC | #6
On 7/18/2022 3:35 PM, Junio C Hamano wrote:
> Derrick Stolee <derrickstolee@github.com> writes:
> 
>> ... I think I should use "branches" here, but
>> keep the name "--update-refs". The biggest reason is that it provides
>> a nice parallel with the "update-ref" sequencer command. This command
>> allows updating _any_ ref, such as lightweight tags in refs/tags/*
>> or even refs in refs/my/namespace/*.
>>
>> The --update-refs option doesn't create the commands to update tags
>> or refs in places other than refs/heads/*.
> 
> I guess it would make the choice of "branch" the most appropriate.
> 
> I was hoping that we can repoint refs in private namespaces that are
> not branches with the option.  But as long as the underlying
> "update-ref" instruction can be used by advanced users, that is OK.

I would like to keep the --update-refs name for a couple reasons:

1. 'update-ref' is the right name for the sequencer command. Having
   a parallel there is helpful for learning about the option.

2. We could extend the boolean '--update-refs' option into a more
   advanced multi-valued '--update-refs=<refspec>' option to allow
   advanced users to specify a ref namespace that they want included.

Thanks,
-Stolee
Junio C Hamano July 19, 2022, 4:44 p.m. UTC | #7
Derrick Stolee <derrickstolee@github.com> writes:

> On 7/18/2022 3:35 PM, Junio C Hamano wrote:
>> Derrick Stolee <derrickstolee@github.com> writes:
>> 
>>> ... I think I should use "branches" here, but
>>> keep the name "--update-refs". The biggest reason is that it provides
>>> a nice parallel with the "update-ref" sequencer command. This command
>>> allows updating _any_ ref, such as lightweight tags in refs/tags/*
>>> or even refs in refs/my/namespace/*.
>>>
>>> The --update-refs option doesn't create the commands to update tags
>>> or refs in places other than refs/heads/*.
>> 
>> I guess it would make the choice of "branch" the most appropriate.
>> 
>> I was hoping that we can repoint refs in private namespaces that are
>> not branches with the option.  But as long as the underlying
>> "update-ref" instruction can be used by advanced users, that is OK.
>
> I would like to keep the --update-refs name for a couple reasons:

I do not think anybody proposed to change the name of that option.

I was reacting to your "I should use branches here", with the
understanding that "here" is this place where you used "local refs".

>> +		OPT_BOOL(0, "update-refs", &options.update_refs,
>> +			 N_("update local refs that point to commits "

If "rebase --update-refs" uses "update-ref" insn (which is capable
of repointing non-branch refs) only for local branches, then the
help text for the "--update-refs" option can safely say "update
local branches" without being inaccurate.  That is where my "branch
is the most appropriate" comes from.
Derrick Stolee July 19, 2022, 4:47 p.m. UTC | #8
On 7/19/2022 12:44 PM, Junio C Hamano wrote:
> Derrick Stolee <derrickstolee@github.com> writes:
> 
>> On 7/18/2022 3:35 PM, Junio C Hamano wrote:
>>> Derrick Stolee <derrickstolee@github.com> writes:
>>>
>>>> ... I think I should use "branches" here, but
>>>> keep the name "--update-refs". The biggest reason is that it provides
>>>> a nice parallel with the "update-ref" sequencer command. This command
>>>> allows updating _any_ ref, such as lightweight tags in refs/tags/*
>>>> or even refs in refs/my/namespace/*.
>>>>
>>>> The --update-refs option doesn't create the commands to update tags
>>>> or refs in places other than refs/heads/*.
>>>
>>> I guess it would make the choice of "branch" the most appropriate.
>>>
>>> I was hoping that we can repoint refs in private namespaces that are
>>> not branches with the option.  But as long as the underlying
>>> "update-ref" instruction can be used by advanced users, that is OK.
>>
>> I would like to keep the --update-refs name for a couple reasons:
> 
> I do not think anybody proposed to change the name of that option.
> 
> I was reacting to your "I should use branches here", with the
> understanding that "here" is this place where you used "local refs".
> 
>>> +		OPT_BOOL(0, "update-refs", &options.update_refs,
>>> +			 N_("update local refs that point to commits "
> 
> If "rebase --update-refs" uses "update-ref" insn (which is capable
> of repointing non-branch refs) only for local branches, then the
> help text for the "--update-refs" option can safely say "update
> local branches" without being inaccurate.  That is where my "branch
> is the most appropriate" comes from.

Thanks for the clarification. Sorry for misunderstanding.

Thanks,
-Stolee
diff mbox series

Patch

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 1d6442c9639..e657862cda2 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"
 
@@ -5638,10 +5639,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_insert(&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
+ * 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];
@@ -5660,6 +5764,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 97f5c87f8c8..8a03f14df8d 100755
--- a/t/t2407-worktree-heads.sh
+++ b/t/t2407-worktree-heads.sh
@@ -164,4 +164,26 @@  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
+	)
+'
+
+# This must be the last test in this file
+test_expect_success '$EDITOR and friends are unchanged' '
+	test_editor_unchanged
+'
+
 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