Thread (39 messages) flat view 39 messages, 6 authors, 5h ago

Re: [PATCH v2 2/3] sequencer: run auto maintenance once a sequence is done

From: Patrick Steinhardt <hidden>
Date: 2026-09-07 08:14:17

On Fri, Sep 04, 2026 at 03:51:25PM +0000, Thomas Bachem via GitGitGadget wrote:
From: Thomas Bachem <redacted>

The apply backend of "git rebase" runs "git maintenance run --auto"
from finish_rebase() once it has applied its patches. The merge
backend, "git cherry-pick" and "git revert" do not run it when they
finish. They create their commits in process, and only the "git
commit" they spawn for an edited message or a resolved conflict, the
"git merge" a "rebase -r" spawns and an exec command start it, in the
middle of the sequence.
This paragraph just doesn't parse for me, it's really hard to tell what
it even wants to say.
Run it where the sequencer finishes, so that every sequence ends the
way the apply backend does, and so that the next commit can keep it
out of the commands a sequence spawns.
Besides moving stuff around to prep for the next commit, what does this
change? Like, do we now run the command in cases where we didn't before?
And if so, what are the consequences of doing so?
quoted hunk ↗ jump to hunk
diff --git a/sequencer.c b/sequencer.c
index 65afd100d9..67e1c38762 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -5313,6 +5313,12 @@ cleanup_head_ref:
 			return -1;
 	}
 
+	/*
+	 * We ignore errors in 'git maintenance run --auto', since the
+	 * user should see them.
+	 */
+	run_auto_maintenance(r, opts->quiet);
+
 	/*
 	 * Sequence of picks finished successfully; cleanup by
 	 * removing the .git/sequencer directory
@@ -5577,10 +5583,14 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts)
 			res = -1;
 			goto release_todo_list;
 		}
-	} else if (!file_exists(get_todo_path(opts)))
-		return continue_single_pick(r, opts);
-	else if ((res = read_populate_todo(r, &todo_list, opts)))
+	} else if (!file_exists(get_todo_path(opts))) {
+		res = continue_single_pick(r, opts);
+		if (!res)
+			run_auto_maintenance(r, opts->quiet);
+		return res;
+	} else if ((res = read_populate_todo(r, &todo_list, opts))) {
 		goto release_todo_list;
+	}
 
 	if (!is_rebase_i(opts)) {
 		/* Verify that the conflict has been resolved */
@@ -5698,6 +5708,8 @@ int sequencer_pick_revisions(struct repository *r,
 			BUG("unexpected extra commit from walk");
 
 		res = single_pick(r, cmit, opts);
+		if (!res)
+			run_auto_maintenance(r, opts->quiet);
 		goto out;
 	}
 
It's surprisingly many sites where you add the call to
`run_auto_maintenance()`. My hope was that there is a single exit path
somewhere that is used by both the "apply" and "merge" strategy that we
could adapt to unify when exactly we run auto-maintenance across both
backends.

Patrick
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help