patchsequencer: refuse to revert shallow boundary commits by default

From: Nakul V S <vsnakul@gmail.com>

When a repository is cloned with --depth=1, Git records the boundary commit in .git/shallow and rewrites its parent links via the graft mechanism, making it appear as a root commit. Reverting such a commit will therefore delete all tracked files, which is almost always unintended by the user.

Add a check in do_pick_commit() that detects when the target commit is a shallow boundary (nr_parent < 0 in the commit graft) and refuses the operation by default. A new --allow-shallow-boundary flag is provided for the rare cases where the user genuinely intends this behavior.

Signed-off-by: Nakul V S <vsnakul@gmail.com>
---
    sequencer: refuse to revert shallow boundary commits by default

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2273%2Fvsnakul%2Fsequencer-shallow-boundary-check-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2273/vsnakul/sequencer-shallow-boundary-check-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2273

 Documentation/git-revert.adoc | 12 +++++++++++-
 builtin/revert.c              |  4 +++-
 sequencer.c                   | 14 ++++++++++++++
 sequencer.h                   |  1 +
 t/t3501-revert-cherry-pick.sh | 25 +++++++++++++++++++++++++
 5 files changed, 54 insertions(+), 2 deletions(-)

Hi Nakul

On 21/10/2026 11:09, Nakul V S via GitGitGadget wrote:

Thanks for working on this, it is a useful and timely improvement as there was some discussion about improving the behavior of git with regard to shallow boundaries at the recent git merge conference. I wonder if it would be better to refuse to cherry-pick and rebase a shallow boundary commit as well. Do you know of a sensible use for reverting a shallow boundary or is the new option added out of caution in case someone wants it?

For such a rarely used option I don't think we need to add it to the synopsis (we already omit some other options like --rerere-autoupdate)

It would be good to extract this into a helper function so that we don't have to copy the code to detect a shallow boundary each time we need to detect one.

Thanks

Phillip

Phillip Wood <phillip.wood123@gmail.com> writes:

Very good point.

The next natural question is where the modification should go if we want to make sure we protect these shallow boundaries not just from revert but also from "cherry-pick", "rebase", and possibly "commit --amend". Is sequencer.c:do_pick_commit() the right place to cover all of them? If not, would it make sense to have a helper function that different code paths to protect shallow boundaries from these operations can call, and add the first call to that helper funcion to the location this patch adds new code to?

Thanks.

On 21/10/2026 20:11, Junio C Hamano wrote:

From the user's point of view, I think do_pick_commit() is a bit late to error out. It would be nicer to refuse to add a shallow commit to the todo list in the first place which means we'd need a helper function that's called from sequencer_make_script(), make_script_with_merges(), single_pick() and sequencer_pick_revisions(). That is a bit of a pain but would make for a better UI.

Thanks

Phillip

Hi Phillip,

You're right, and I hadn't thought about it from the user's side. I tried "git revert <tip> <boundary>" with v1. It reverted the tip, made a commit, then failed on the boundary and left .git/sequencer behind. That's a messy state to be left in.

So v2 does what you describe. The check is made where each command builds its list of commits, and they all call the one helper. If any of the commits is a boundary, the command refuses up front: no commit is created, HEAD doesn't move, and there is nothing to abort.

"rebase --apply" doesn't go through the sequencer, so it gets a small check of its own before it rewinds HEAD. That check does nothing in a repository that isn't shallow.

One thing to be aware of: if someone types a "pick" or "revert" line for a boundary commit into the rebase todo list by hand, it won't be caught, because the check runs when the list is generated and not after the editor. Git doesn't catch that today either, and the person would have to type the hash on purpose, so I think it's an acceptable price for failing early. If you'd prefer, I can add a second check in do_pick_commit() as well.

Thanks, Nakul

Hi Junio,

Thanks for the review, and for the question.

No, it isn't. When I wrote v1 I was only thinking about "revert", since that is what the original thread was about. do_pick_commit() is where every pick and revert ends up, so it looked like the easiest place for a single check. I honestly didn't think about cherry-pick or rebase at that point, or about how late that is for the user.

v2 moves it. The check now happens where each command builds its list of commits, and they all share one helper. "rebase --apply" doesn't go through the sequencer, so it gets a small check of its own before HEAD is touched.

I've left "git commit --amend" alone for now. It is a different code path, so I'd rather do it as a separate patch than make this one bigger. Let me know if you'd prefer it in the same series.

v2 is here: https://lore.kernel.org/git/pull.2273.v2.git.1793455522168.gitgitgadget@gmail.com

Thanks, Nakul

Hi Phillip,

Thanks for the review. I've sent a v2 that addresses it: https://lore.kernel.org/git/pull.2273.v2.git.1793455522168.gitgitgadget@gmail.com

Yes, I think so. v2 covers revert, cherry-pick and rebase, including the "apply" backend of rebase.

Honestly it was out of caution.I can't think of a real use for it. v2 keeps --allow-shallow-boundary for revert and cherry-pick only, and rebase always refuses. If you'd rather not have an escape hatch at all, I'm happy to drop it.

Fair enough, it's out of the synopsis now.

Done. There is one helper now, so the error message lives in a single place.

Thanks, Nakul

From: Nakul V S <vsnakul@gmail.com>

In a shallow repository, the commits at the edge of the downloaded history are recorded in $GIT_DIR/shallow and have their parents hidden from the revision machinery. Such a "shallow boundary" commit looks like a root commit that added every file in its tree at once.

Reverting it therefore removes every tracked file, and cherry-picking or rebasing it adds all of them again. In a "git clone --depth=1" this is the only commit there is, so "git revert HEAD" empties the working tree, which is almost never what the user meant.

Refuse to do so by default for "revert", "cherry-pick" and "rebase". For "revert" and "cherry-pick" only, add --allow-shallow-boundary to override the check; it is remembered across --continue. "rebase" always refuses, and the error message suggests "git fetch --unshallow" instead.

The check is made when the list of commits to replay is built, not when each commit is picked, so a command that is going to be refused does so before it has done anything: no commit is created, HEAD and the working tree are untouched, and no sequencer or rebase state is left behind for the user to clean up.

The "apply" backend of "rebase" does not go through the sequencer, so it checks the commits "git format-patch" is about to turn into patches before it rewinds HEAD. This is skipped altogether in a repository that is not shallow.

A "pick" or "revert" line for a shallow boundary commit that the user types into the todo list by hand is not checked, as the check is not made again after the todo list has been edited. "git commit --amend" is also left alone for now.

Signed-off-by: Nakul V S <vsnakul@gmail.com>
---
    sequencer: refuse to pick or revert shallow boundary commits by default
    
    Changes since v1:
    
     * Also refuse cherry-pick and rebase (merge, interactive and --apply
       backends), as suggested by Phillip.
     * Check when the list of commits is built rather than in
       do_pick_commit(), so a refused command leaves no state behind. Uses a
       single shared helper.
     * --allow-shallow-boundary is now accepted by cherry-pick as well, and
       is saved across --continue.
     * Dropped the option from the revert synopsis.
     * rebase --autostash restores the user's changes if the rebase is
       refused.
     * Rebased onto current master.
    
    Not covered here: git commit --amend, and pick/revert lines typed into
    the rebase todo by hand.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2273%2Fvsnakul%2Fsequencer-shallow-boundary-check-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2273/vsnakul/sequencer-shallow-boundary-check-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2273

Range-diff vs v1:

 1:  526b96f6ba ! 1:  fef8178d4d sequencer: refuse to revert shallow boundary commits by default
     @@ Metadata
      Author: Nakul V S <vsnakul@gmail.com>
      
       ## Commit message ##
     -    sequencer: refuse to revert shallow boundary commits by default
     +    sequencer: refuse to pick or revert shallow boundary commits
      
     -    When a repository is cloned with --depth=1, Git records the boundary
     -    commit in .git/shallow and rewrites its parent links via the graft
     -    mechanism, making it appear as a root commit. Reverting such a commit
     -    will therefore delete all tracked files, which is almost always
     -    unintended by the user.
     +    In a shallow repository, the commits at the edge of the downloaded
     +    history are recorded in $GIT_DIR/shallow and have their parents hidden
     +    from the revision machinery.  Such a "shallow boundary" commit looks
     +    like a root commit that added every file in its tree at once.
      
     -    Add a check in do_pick_commit() that detects when the target commit is
     -    a shallow boundary (nr_parent < 0 in the commit graft) and refuses the
     -    operation by default. A new --allow-shallow-boundary flag is provided
     -    for the rare cases where the user genuinely intends this behavior.
     +    Reverting it therefore removes every tracked file, and cherry-picking
     +    or rebasing it adds all of them again.  In a "git clone --depth=1" this
     +    is the only commit there is, so "git revert HEAD" empties the working
     +    tree, which is almost never what the user meant.
     +
     +    Refuse to do so by default for "revert", "cherry-pick" and "rebase".
     +    For "revert" and "cherry-pick" only, add --allow-shallow-boundary to
     +    override the check; it is remembered across --continue.  "rebase"
     +    always refuses, and the error message suggests "git fetch --unshallow"
     +    instead.
     +
     +    The check is made when the list of commits to replay is built, not when
     +    each commit is picked, so a command that is going to be refused does so
     +    before it has done anything: no commit is created, HEAD and the working
     +    tree are untouched, and no sequencer or rebase state is left behind for
     +    the user to clean up.
     +
     +    The "apply" backend of "rebase" does not go through the sequencer, so
     +    it checks the commits "git format-patch" is about to turn into patches
     +    before it rewinds HEAD.  This is skipped altogether in a repository
     +    that is not shallow.
     +
     +    A "pick" or "revert" line for a shallow boundary commit that the user
     +    types into the todo list by hand is not checked, as the check is not
     +    made again after the todo list has been edited.  "git commit --amend"
     +    is also left alone for now.
      
          Signed-off-by: Nakul V S <vsnakul@gmail.com>
      
     + ## Documentation/git-cherry-pick.adoc ##
     +@@ Documentation/git-cherry-pick.adoc: fail unless one of `--empty=keep` or `--allow-empty` are specified.
     + 	Pass the merge strategy-specific option through to the
     + 	merge strategy.  See linkgit:git-merge[1] for details.
     + 
     ++--allow-shallow-boundary::
     ++	By default, 'git cherry-pick' refuses to pick a commit that is a
     ++	shallow boundary (i.e. a commit listed in `.git/shallow`).
     ++	Its parents are missing from this repository, so it looks like
     ++	a root commit that added every file in its tree, and picking it
     ++	would add all of them.  This option overrides that safeguard.
     +++
     ++To get the missing history instead, use `git fetch --unshallow`.
     ++
     + include::rerere-options.adoc[]
     + 
     + SEQUENCER SUBCOMMANDS
     +
     + ## Documentation/git-rebase.adoc ##
     +@@ Documentation/git-rebase.adoc: repository that you share.
     + See also <<RECOVERING_FROM_UPSTREAM_REBASE,RECOVERING FROM UPSTREAM REBASE>>
     + below.
     + 
     ++If the commits to be rebased include a shallow boundary commit (a
     ++commit listed in `.git/shallow`), the rebase refuses to start, because
     ++the missing parents make that commit look like a root commit that added
     ++every file in its tree.  This applies to both the `merge` and `apply`
     ++backends, and nothing is changed when the rebase is refused.  Run
     ++`git fetch --unshallow` to get the missing history first.
     ++
     + When the rebase is run, it will first execute a `pre-rebase` hook if one
     + exists.  You can use this hook to do sanity checks and reject the rebase
     + if it isn't appropriate.  Please see the template `pre-rebase` hook script
     +
       ## Documentation/git-revert.adoc ##
     -@@ Documentation/git-revert.adoc: git-revert - Revert some existing commits
     - SYNOPSIS
     - --------
     - [verse]
     --'git revert' [--[no-]edit] [-n] [-m <parent-number>] [-s] [-S[<keyid>]] <commit>...
     -+'git revert' [--[no-]edit] [-n] [-m <parent-number>] [-s] [-S[<keyid>]] [--allow-shallow-boundary] <commit>...
     - 'git revert' (--continue | --skip | --abort | --quit)
     - 
     - DESCRIPTION
      @@ Documentation/git-revert.adoc: include::rerere-options.adoc[]
       	configuration variable can be used to enable this option by
       	default.
       
      +--allow-shallow-boundary::
      +	By default, 'git revert' refuses to revert a commit that is a
     -+	shallow boundary (i.e. a commit listed in `.git/shallow`),
     -+	because its true parent history is absent from the clone and
     -+	reverting it would remove all tracked files. This option
     -+	overrides that safeguard and allows the revert to proceed.
     ++	shallow boundary (i.e. a commit listed in `.git/shallow`).
     ++	Its parents are missing from this repository, so it looks like
     ++	a root commit that added every file in its tree, and reverting
     ++	it would remove all of them.  This option overrides that
     ++	safeguard.
      ++
     -+To restore full history before reverting, use
     -+`git fetch --unshallow` instead.
     ++To get the missing history instead, use `git fetch --unshallow`.
      +
       
       SEQUENCER SUBCOMMANDS
       ---------------------
      
     - ## builtin/revert.c ##
     + ## builtin/rebase.c ##
      @@
     -  */
     - 
     - static const char * const revert_usage[] = {
     --	N_("git revert [--[no-]edit] [-n] [-m <parent-number>] [-s] [-S[<keyid>]] <commit>..."),
     -+	N_("git revert [--[no-]edit] [-n] [-m <parent-number>] [-s] [-S[<keyid>]] [--allow-shallow-boundary] <commit>..."),
     - 	N_("git revert (--continue | --skip | --abort | --quit)"),
     - 	NULL
     - };
     + #include "sequencer.h"
     + #include "rebase-interactive.h"
     + #include "reset.h"
     ++#include "shallow.h"
     + #include "trace2.h"
     + #include "hook.h"
     + #include "trailer.h"
     +@@ builtin/rebase.c: static int init_basic_state(struct replay_opts *opts, const char *head_name,
     + 	return write_basic_state(opts, head_name, onto, orig_head);
     + }
     + 
     ++static int cleanup_autostash(struct rebase_options *opts);
     ++
     + static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
     + {
     + 	int ret = -1;
     +@@ builtin/rebase.c: static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
     + 				    &make_script_args, flags);
     + 	if (ret) {
     + 		error(_("could not generate todo list"));
     ++		/* Nothing has been started yet; put back what we stashed */
     ++		cleanup_autostash(opts);
     + 		goto cleanup;
     + 	}
     + 
     +@@ builtin/rebase.c: static int move_to_original_branch(struct rebase_options *opts)
     + 	return ret;
     + }
     + 
     ++/*
     ++ * The "apply" backend does not go through the sequencer, so check here that
     ++ * none of the commits "git format-patch" is about to turn into patches is a
     ++ * shallow boundary commit.  This walks the same set of commits as run_am().
     ++ */
     ++static int check_no_shallow_boundary_to_apply(struct rebase_options *opts)
     ++{
     ++	struct rev_info revs;
     ++	struct strvec args = STRVEC_INIT;
     ++	struct commit *commit;
     ++	int ret = 0;
     ++
     ++	if (!is_repository_shallow(the_repository))
     ++		return 0;
     ++
     ++	repo_init_revisions(the_repository, &revs, NULL);
     ++	revs.max_parents = 1;
     ++	revs.cherry_pick = 1;
     ++	revs.right_only = 1;
     ++	revs.limited = 1;
     ++
     ++	strvec_push(&args, "");
     ++	strvec_pushf(&args, "%s...%s",
     ++		     oid_to_hex(opts->root ?
     ++				&opts->onto->object.oid :
     ++				&opts->upstream->object.oid),
     ++		     oid_to_hex(&opts->orig_head->object.oid));
     ++	if (opts->restrict_revision)
     ++		strvec_pushf(&args, "^%s",
     ++			     oid_to_hex(&opts->restrict_revision->object.oid));
     ++
     ++	setup_revisions_from_strvec(&args, &revs, NULL);
     ++	if (prepare_revision_walk(&revs)) {
     ++		ret = error(_("revision walk setup failed"));
     ++		goto out;
     ++	}
     ++	while ((commit = get_revision(&revs))) {
     ++		if (sequencer_refuse_shallow_boundary(the_repository, commit, 0)) {
     ++			ret = -1;
     ++			break;
     ++		}
     ++	}
     ++
     ++out:
     ++	release_revisions(&revs);
     ++	strvec_clear(&args);
     ++	return ret;
     ++}
     ++
     + static int run_am(struct rebase_options *opts)
     + {
     + 	struct child_process am = CHILD_PROCESS_INIT;
     +@@ builtin/rebase.c: int cmd_rebase(int argc,
     + 	if (is_merge(&options))
     + 		goto run_rebase;
     + 
     ++	/* Fail before HEAD or the working tree has been touched */
     ++	if (check_no_shallow_boundary_to_apply(&options)) {
     ++		ret = -1;
     ++		goto cleanup_autostash;
     ++	}
     ++
     + 	/* Detach HEAD and reset the tree */
     + 	if (options.flags & REBASE_NO_QUIET)
     + 		printf(_("First, rewinding head to replay your work on top of "
     +
     + ## builtin/revert.c ##
      @@ builtin/revert.c: static int run_sequencer(int argc, const char **argv, const char *prefix,
       		OPT_BOOL('e', "edit", &opts->edit, N_("edit the commit message")),
       		OPT_NOOP_NOARG('r', NULL),
       		OPT_BOOL('s', "signoff", &opts->signoff, N_("add a Signed-off-by trailer")),
      +		OPT_BOOL(0, "allow-shallow-boundary", &opts->allow_shallow_boundary,
     -+			 N_("allow reverting a shallow boundary commit")),
     ++			 N_("allow picking or reverting a shallow boundary commit")),
       		OPT_CALLBACK('m', "mainline", opts, N_("parent-number"),
       			     N_("select mainline parent"), option_parse_m),
       		OPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),
     +@@ builtin/revert.c: static int run_sequencer(int argc, const char **argv, const char *prefix,
     + 				"--strategy-option", opts->xopts.nr ? 1 : 0,
     + 				"-x", opts->record_origin,
     + 				"--ff", opts->allow_ff,
     ++				"--allow-shallow-boundary", opts->allow_shallow_boundary,
     + 				"--rerere-autoupdate", opts->allow_rerere_auto == RERERE_AUTOUPDATE,
     + 				"--no-rerere-autoupdate", opts->allow_rerere_auto == RERERE_NOAUTOUPDATE,
     + 				"--keep-redundant-commits", opts->keep_redundant_commits,
      
       ## sequencer.c ##
      @@
     @@ sequencer.c
       
       #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
       
     -@@ sequencer.c: static enum pick_result do_pick_commit(struct repository *r,
     +@@ sequencer.c: static int populate_opts_cb(const char *key, const char *value,
     + 		opts->record_origin = git_config_bool_or_int(key, value, ctx->kvi, &error_flag);
     + 	else if (!strcmp(key, "options.allow-ff"))
     + 		opts->allow_ff = git_config_bool_or_int(key, value, ctx->kvi, &error_flag);
     ++	else if (!strcmp(key, "options.allow-shallow-boundary"))
     ++		opts->allow_shallow_boundary =
     ++			git_config_bool_or_int(key, value, ctx->kvi, &error_flag);
     + 	else if (!strcmp(key, "options.mainline"))
     + 		opts->mainline = git_config_int(key, value, ctx->kvi);
     + 	else if (!strcmp(key, "options.strategy"))
     +@@ sequencer.c: int write_basic_state(struct replay_opts *opts, const char *head_name,
     + 	return 0;
     + }
     + 
     ++/*
     ++ * A shallow boundary commit has had its parents hidden, so it looks like
     ++ * a root commit that added every file at once.  Picking or reverting it
     ++ * would not do what the user expects, so refuse before anything is
     ++ * started.
     ++ */
     ++int sequencer_refuse_shallow_boundary(struct repository *r,
     ++				      const struct commit *commit, int allow)
     ++{
     ++	if (allow || !is_shallow_boundary_commit(r, &commit->object.oid))
     ++		return 0;
     ++	return error(_("commit %s is a shallow boundary commit; its parents "
     ++		       "are missing from this clone (use 'git fetch --unshallow' "
     ++		       "to get them)"),
     ++		     oid_to_hex(&commit->object.oid));
     ++}
     ++
     + static int walk_revs_populate_todo(struct todo_list *todo_list,
     + 				struct replay_opts *opts)
     + {
     +@@ sequencer.c: static int walk_revs_populate_todo(struct todo_list *todo_list,
     + 	encoding = get_log_output_encoding();
     + 
     + 	while ((commit = get_revision(opts->revs))) {
     +-		struct todo_item *item = append_new_todo(todo_list);
     +-		const char *commit_buffer = repo_logmsg_reencode(the_repository,
     +-								 commit, NULL,
     +-								 encoding);
     ++		struct todo_item *item;
     ++		const char *commit_buffer;
     + 		const char *subject;
     + 		int subject_len;
     + 
     ++		if (sequencer_refuse_shallow_boundary(the_repository, commit,
     ++					    opts->allow_shallow_boundary))
     ++			return -1;
     ++
     ++		item = append_new_todo(todo_list);
     ++		commit_buffer = repo_logmsg_reencode(the_repository, commit,
     ++						     NULL, encoding);
     + 		item->command = command;
     + 		item->commit = commit;
     + 		item->arg_offset = 0;
     +@@ sequencer.c: static int save_opts(struct replay_opts *opts)
     + 	if (opts->allow_ff)
     + 		res |= repo_config_set_in_file_gently(the_repository, opts_file,
     + 					"options.allow-ff", NULL, "true");
     ++	if (opts->allow_shallow_boundary)
     ++		res |= repo_config_set_in_file_gently(the_repository, opts_file,
     ++				"options.allow-shallow-boundary", NULL, "true");
     + 	if (opts->mainline) {
     + 		struct strbuf buf = STRBUF_INIT;
     + 		strbuf_addf(&buf, "%d", opts->mainline);
     +@@ sequencer.c: static int single_pick(struct repository *r,
     + 	int check_todo;
     + 	struct todo_item item;
     + 
     ++	if (sequencer_refuse_shallow_boundary(r, cmit, opts->allow_shallow_boundary))
     ++		return -1;
     ++
     + 	item.command = opts->action == REPLAY_PICK ?
     + 			TODO_PICK : TODO_REVERT;
     + 	item.commit = cmit;
     +@@ sequencer.c: static int make_script_with_merges(struct pretty_print_context *pp,
     + 	int rebase_cousins = flags & TODO_LIST_REBASE_COUSINS;
     + 	int root_with_onto = flags & TODO_LIST_ROOT_WITH_ONTO;
     + 	int skipped_commit = 0;
     ++	int ret = 0;
     + 	struct strbuf buf = STRBUF_INIT, oneline = STRBUF_INIT;
     + 	struct strbuf label_from_message = STRBUF_INIT;
     + 	struct commit_list *commits = NULL, **tail = &commits, *iter;
     +@@ sequencer.c: static int make_script_with_merges(struct pretty_print_context *pp,
     + 		}
     + 		if (is_empty && !keep_empty)
     + 			continue;
     ++		if (sequencer_refuse_shallow_boundary(the_repository, commit, 0)) {
     ++			ret = -1;
     ++			goto cleanup;
     ++		}
     + 
     + 		strbuf_reset(&oneline);
     + 		pretty_print_commit(pp, commit, &oneline);
     +@@ sequencer.c: static int make_script_with_merges(struct pretty_print_context *pp,
     + 		commit_list_free(list);
       	}
     - 	discard_index(r->index);
     - 
     -+	if (is_repository_shallow(r)) {
     -+		struct commit_graft *graft = lookup_commit_graft(r, &commit->object.oid);
     -+		if (graft && graft->nr_parent < 0 && !opts->allow_shallow_boundary) {
     -+			return error(_("%s: commit %s is a shallow boundary commit.\n"
     -+				       "Reverting it will remove all tracked files because its\n"
     -+				       "true parent history is not present in this clone.\n"
     -+				       "Use --allow-shallow-boundary to override, or\n"
     -+				       "'git fetch --unshallow' to restore full history first."),
     -+				     command_to_string(command),
     -+				     oid_to_hex(&commit->object.oid));
     + 
     ++cleanup:
     + 	commit_list_free(commits);
     + 	commit_list_free(tips);
     + 
     +@@ sequencer.c: static int make_script_with_merges(struct pretty_print_context *pp,
     + 	hashmap_clear_and_free(&state.labels, struct labels_entry, entry);
     + 	strbuf_release(&state.buf);
     + 
     +-	return 0;
     ++	return ret;
     + }
     + 
     + int sequencer_make_script(struct repository *r, struct strbuf *out,
     +@@ sequencer.c: int sequencer_make_script(struct repository *r, struct strbuf *out,
     + 		}
     + 		if (is_empty && !keep_empty)
     + 			continue;
     ++		if (sequencer_refuse_shallow_boundary(r, commit, 0)) {
     ++			ret = -1;
     ++			goto cleanup;
      +		}
     -+	}
     -+
     - 	if (!commit->parents)
     - 		parent = NULL;
     - 	else if (commit->parents->next) {
     + 		strbuf_addf(out, "%s %s ", insn,
     + 			    oid_to_hex(&commit->object.oid));
     + 		pretty_print_commit(&pp, commit, out);
      
       ## sequencer.h ##
      @@ sequencer.h: struct replay_opts {
     @@ sequencer.h: struct replay_opts {
       
       	struct strvec trailer_args;
       
     +@@ sequencer.h: int sequencer_remove_state(struct replay_opts *opts);
     + #define TODO_LIST_REAPPLY_CHERRY_PICKS (1U << 7)
     + #define TODO_LIST_WARN_SKIPPED_CHERRY_PICKS (1U << 8)
     + 
     ++/*
     ++ * Return -1 after reporting an error if "commit" is a shallow boundary
     ++ * commit and "allow" is zero, 0 otherwise.
     ++ */
     ++int sequencer_refuse_shallow_boundary(struct repository *r,
     ++				      const struct commit *commit, int allow);
     + int sequencer_make_script(struct repository *r, struct strbuf *out,
     + 			  struct strvec *argv, unsigned flags);
     + 
     +
     + ## shallow.c ##
     +@@ shallow.c: int is_repository_shallow(struct repository *r)
     + 	return r->parsed_objects->is_shallow;
     + }
     + 
     ++int is_shallow_boundary_commit(struct repository *r, const struct object_id *oid)
     ++{
     ++	struct commit_graft *graft;
     ++	if (!is_repository_shallow(r))
     ++		return 0;
     ++	graft = lookup_commit_graft(r, oid);
     ++	return graft && graft->nr_parent < 0;
     ++}
     ++
     + static void reset_repository_shallow(struct repository *r)
     + {
     + 	r->parsed_objects->is_shallow = -1;
     +
     + ## shallow.h ##
     +@@ shallow.h: void set_alternate_shallow_file(struct repository *r, const char *path);
     + int register_shallow(struct repository *r, const struct object_id *oid);
     + int unregister_shallow(const struct object_id *oid);
     + int is_repository_shallow(struct repository *r);
     ++int is_shallow_boundary_commit(struct repository *r, const struct object_id *oid);
     + 
     + /*
     +  * Lock for updating the $GIT_DIR/shallow file.
      
       ## t/t3501-revert-cherry-pick.sh ##
      @@ t/t3501-revert-cherry-pick.sh: test_expect_success 'cherry-pick is unaware of --reference (for now)' '
     @@ t/t3501-revert-cherry-pick.sh: test_expect_success 'cherry-pick is unaware of --
      +# Tests for shallow boundary safeguard
      +
      +test_expect_success 'setup shallow boundary tests' '
     -+	git init src &&
     -+	test_commit -C src A &&
     -+	test_commit -C src B &&
     -+	git clone --no-local --depth=1 src shallow-repo
     ++	git init shallow-src &&
     ++	test_commit -C shallow-src A &&
     ++	test_commit -C shallow-src B &&
     ++	test_commit -C shallow-src C &&
     ++	git clone --no-local --depth=2 "file://$(pwd)/shallow-src" shallow-repo &&
     ++	git -C shallow-repo rev-parse --verify B >boundary &&
     ++	test_cmp shallow-repo/.git/shallow boundary
     ++'
     ++
     ++test_expect_success 'revert of shallow boundary commit is refused' '
     ++	git -C shallow-repo reset --hard C &&
     ++	test_must_fail git -C shallow-repo revert --no-edit B 2>err &&
     ++	test_grep "shallow boundary commit" err &&
     ++	test_path_is_missing shallow-repo/.git/sequencer &&
     ++	test_path_is_missing shallow-repo/.git/REVERT_HEAD &&
     ++	git -C shallow-repo rev-parse C >expect &&
     ++	git -C shallow-repo rev-parse HEAD >actual &&
     ++	test_cmp expect actual
     ++'
     ++
     ++test_expect_success 'cherry-pick of shallow boundary commit is refused' '
     ++	git -C shallow-repo checkout -q --detach C &&
     ++	git -C shallow-repo rm -rq . &&
     ++	git -C shallow-repo commit -q -m empty &&
     ++	git -C shallow-repo rev-parse HEAD >expect &&
     ++	test_must_fail git -C shallow-repo cherry-pick B 2>err &&
     ++	test_grep "shallow boundary commit" err &&
     ++	git -C shallow-repo rev-parse HEAD >actual &&
     ++	test_cmp expect actual &&
     ++	git -C shallow-repo reset -q --hard C
     ++'
     ++
     ++test_expect_success 'revert of non-boundary commit in shallow repo works' '
     ++	git -C shallow-repo revert --no-edit C &&
     ++	git -C shallow-repo reset -q --hard C
     ++'
     ++
     ++test_expect_success 'multi-commit revert is refused before anything is reverted' '
     ++	test_must_fail git -C shallow-repo revert --no-edit C B 2>err &&
     ++	test_grep "shallow boundary commit" err &&
     ++	test_path_is_missing shallow-repo/.git/sequencer &&
     ++	git -C shallow-repo rev-parse C >expect &&
     ++	git -C shallow-repo rev-parse HEAD >actual &&
     ++	test_cmp expect actual
     ++'
     ++
     ++test_expect_success '--allow-shallow-boundary overrides the revert check' '
     ++	git -C shallow-repo revert --no-edit --allow-shallow-boundary B &&
     ++	test_path_is_missing shallow-repo/B.t &&
     ++	git -C shallow-repo reset -q --hard C
     ++'
     ++
     ++test_expect_success '--allow-shallow-boundary overrides the cherry-pick check' '
     ++	git -C shallow-repo checkout -q --detach C &&
     ++	git -C shallow-repo rm -rq . &&
     ++	git -C shallow-repo commit -q -m empty &&
     ++	git -C shallow-repo cherry-pick --allow-shallow-boundary B &&
     ++	test_path_is_file shallow-repo/B.t &&
     ++	git -C shallow-repo reset -q --hard C
     ++'
     ++
     ++test_expect_success '--allow-shallow-boundary survives --continue' '
     ++	test_commit -C shallow-repo conflict-1 file one &&
     ++	test_commit -C shallow-repo conflict-2 file two &&
     ++	test_must_fail git -C shallow-repo revert --no-edit \
     ++		--allow-shallow-boundary conflict-1 B &&
     ++	echo resolved >shallow-repo/file &&
     ++	git -C shallow-repo add file &&
     ++	git -C shallow-repo revert --continue &&
     ++	test_path_is_missing shallow-repo/B.t &&
     ++	git -C shallow-repo reset -q --hard C
     ++'
     ++
     ++test_expect_success 'revert and cherry-pick in a non-shallow repo are unaffected' '
     ++	git init full-repo &&
     ++	test_commit -C full-repo A &&
     ++	test_commit -C full-repo B &&
     ++	git -C full-repo revert --no-edit B &&
     ++	git -C full-repo cherry-pick B &&
     ++	test_path_is_file full-repo/B.t
     ++'
     ++
     ++test_expect_success 'rebase of a range containing a shallow boundary is refused' '
     ++	git -C shallow-repo checkout -q --detach C &&
     ++	git -C shallow-repo rev-parse C >expect &&
     ++	test_must_fail git -C shallow-repo rebase --root 2>err &&
     ++	test_grep "shallow boundary commit" err &&
     ++	test_path_is_missing shallow-repo/.git/rebase-merge &&
     ++	git -C shallow-repo rev-parse HEAD >actual &&
     ++	test_cmp expect actual
     ++'
     ++
     ++test_expect_success 'rebase --rebase-merges is refused as well' '
     ++	test_must_fail git -C shallow-repo rebase --rebase-merges --root 2>err &&
     ++	test_grep "shallow boundary commit" err &&
     ++	test_path_is_missing shallow-repo/.git/rebase-merge
     ++'
     ++
     ++test_expect_success 'rebase --apply of a range containing a shallow boundary is refused' '
     ++	git -C shallow-repo checkout -q --detach C &&
     ++	git -C shallow-repo checkout -q --orphan unrelated &&
     ++	git -C shallow-repo rm -rfq . &&
     ++	test_commit -C shallow-repo unrelated &&
     ++	git -C shallow-repo checkout -q --detach C &&
     ++	git -C shallow-repo rev-parse C >expect &&
     ++	test_must_fail git -C shallow-repo rebase --apply \
     ++		--onto unrelated unrelated 2>err &&
     ++	test_grep "shallow boundary commit" err &&
     ++	test_path_is_missing shallow-repo/.git/rebase-apply &&
     ++	git -C shallow-repo rev-parse HEAD >actual &&
     ++	test_cmp expect actual
     ++'
     ++
     ++test_expect_success 'rebase --apply restores an autostash when refusing' '
     ++	echo dirty >shallow-repo/C.t &&
     ++	test_must_fail git -C shallow-repo rebase --apply --autostash \
     ++		--onto unrelated unrelated 2>err &&
     ++	test_grep "shallow boundary commit" err &&
     ++	echo dirty >expect &&
     ++	test_cmp expect shallow-repo/C.t &&
     ++	git -C shallow-repo stash list >stashes &&
     ++	test_must_be_empty stashes &&
     ++	git -C shallow-repo checkout -q -- C.t
     ++'
     ++
     ++test_expect_success 'rebase --apply that does not replay the boundary is unaffected' '
     ++	git -C shallow-repo checkout -q -B apply-topic C &&
     ++	test_commit -C shallow-repo E &&
     ++	git -C shallow-repo rebase --apply --onto unrelated C &&
     ++	test_path_is_file shallow-repo/E.t
     ++'
     ++
     ++test_expect_success 'rebase that does not replay the boundary is unaffected' '
     ++	git -C shallow-repo checkout -q -B topic C &&
     ++	test_commit -C shallow-repo D &&
     ++	git -C shallow-repo rebase C
     ++'
     ++
     ++test_expect_success 'rebase restores an autostash when refusing (merge backend)' '
     ++	git -C shallow-repo checkout -q --detach C &&
     ++	echo dirty >shallow-repo/C.t &&
     ++	test_must_fail git -C shallow-repo rebase --autostash --root 2>err &&
     ++	test_grep "shallow boundary commit" err &&
     ++	test_path_is_missing shallow-repo/.git/rebase-merge &&
     ++	echo dirty >expect &&
     ++	test_cmp expect shallow-repo/C.t &&
     ++	git -C shallow-repo stash list >stashes &&
     ++	test_must_be_empty stashes &&
     ++	git -C shallow-repo checkout -q -- C.t
     ++'
     ++
     ++test_expect_success 'interactive rebase restores an autostash when refusing' '
     ++	echo dirty >shallow-repo/C.t &&
     ++	test_must_fail env GIT_SEQUENCE_EDITOR=: \
     ++		git -C shallow-repo rebase -i --autostash --root 2>err &&
     ++	test_grep "shallow boundary commit" err &&
     ++	test_path_is_missing shallow-repo/.git/rebase-merge &&
     ++	echo dirty >expect &&
     ++	test_cmp expect shallow-repo/C.t &&
     ++	git -C shallow-repo stash list >stashes &&
     ++	test_must_be_empty stashes &&
     ++	git -C shallow-repo checkout -q -- C.t
     ++'
     ++
     ++test_expect_success 'rebase onto new upstream work in a shallow clone still works' '
     ++	git clone --no-local --depth=1 "file://$(pwd)/shallow-src" shallow-local &&
     ++	test_commit -C shallow-local local &&
     ++	test_commit -C shallow-src upstream &&
     ++	git -C shallow-local fetch origin &&
     ++	git -C shallow-local rebase origin/HEAD &&
     ++	test_path_is_file shallow-local/upstream.t &&
     ++	test_path_is_file shallow-local/local.t
      +'
      +
     -+test_expect_success 'revert of shallow boundary commit is refused by default' '
     -+	test_must_fail git -C shallow-repo revert --no-edit HEAD 2>err &&
     -+	test_grep "shallow boundary commit" err
     ++test_expect_success 'single-commit shallow clone: revert and cherry-pick are refused' '
     ++	git init one-src &&
     ++	test_commit -C one-src only &&
     ++	git clone --no-local --depth=1 "file://$(pwd)/one-src" one-shallow &&
     ++	git -C one-shallow rev-parse HEAD >expect &&
     ++	test_must_fail git -C one-shallow revert --no-edit HEAD 2>err &&
     ++	test_grep "shallow boundary commit" err &&
     ++	test_must_fail git -C one-shallow cherry-pick -n HEAD 2>err &&
     ++	test_grep "shallow boundary commit" err &&
     ++	test_path_is_file one-shallow/only.t &&
     ++	git -C one-shallow rev-parse HEAD >actual &&
     ++	test_cmp expect actual
      +'
      +
     -+test_expect_success 'revert of shallow boundary succeeds with --allow-shallow-boundary' '
     -+	git -C shallow-repo revert --no-edit --allow-shallow-boundary HEAD
     ++test_expect_success 'revert and cherry-pick work once the clone is unshallowed' '
     ++	git clone --no-local --depth=2 "file://$(pwd)/shallow-src" unshallowed &&
     ++	test_must_fail git -C unshallowed revert --no-edit HEAD~1 2>err &&
     ++	test_grep "shallow boundary commit" err &&
     ++	git -C unshallowed fetch --unshallow &&
     ++	git -C unshallowed revert --no-edit C &&
     ++	test_path_is_missing unshallowed/C.t &&
     ++	git -C unshallowed cherry-pick C &&
     ++	test_path_is_file unshallowed/C.t
      +'
      +
     -+test_expect_success 'revert in non-shallow repo is unaffected' '
     -+	git init non-shallow &&
     -+	test_commit -C non-shallow A &&
     -+	test_commit -C non-shallow B &&
     -+	git -C non-shallow revert --no-edit HEAD
     ++test_expect_success '--allow-shallow-boundary cannot be combined with --abort' '
     ++	test_must_fail git -C shallow-repo revert --abort \
     ++		--allow-shallow-boundary 2>err &&
     ++	test_grep "cannot be used with" err
      +'
      +
       test_done

 Documentation/git-cherry-pick.adoc |   9 ++
 Documentation/git-rebase.adoc      |   7 +
 Documentation/git-revert.adoc      |  10 ++
 builtin/rebase.c                   |  60 +++++++++
 builtin/revert.c                   |   3 +
 sequencer.c                        |  52 +++++++-
 sequencer.h                        |   7 +
 shallow.c                          |   9 ++
 shallow.h                          |   1 +
 t/t3501-revert-cherry-pick.sh      | 208 +++++++++++++++++++++++++++++
 10 files changed, 361 insertions(+), 5 deletions(-)

"Nakul V S via GitGitGadget" <gitgitgadget@gmail.com> writes:

All of these sounds sensible.

Was it necessary? On which commit was the previous round based? Has there been any changes that made it necessary to rebase since then?

It is OK to start small and extend coverage step by step, as we are making strict improvements. Do we however have some vision on how these would later be supported?

Hmph, why not git_config_bool()?

Unlike commit.verbose or status.submodulesummary, I didn't expect this to have anything more than a simple "yes we allow it" or "no don't allow it" answer. "all the other boolean neighbors somehow use it" is an understandable answer, but I expect people to do better than that.

Overall things seem to be in a good shape. Except for tiny details like this one.

Thanks.

"Nakul V S via GitGitGadget" <gitgitgadget@gmail.com> writes:

For a single-patch series a large range-diff like this before the patch makes it very distracting. Isn't there a way to tell GitGitGadget not to do this (or perhaps add it after the patch)?

Please check how badly your change conflicts with at least 'next' and 'seen' before sending any non-trivial patch. Doing so would have shown you that you do not need to write this function, and should instead discuss the best course to resolve the conflict with Harald, who wrote f8bc48ea42 (object-name: explain why <rev>~N fails in a shallow clone, 2026-09-23).

That other topic is currently stagnant. You can see how topics are doing by visiting the mailing list archive. For example, a trial merge of your patch into 'seen' would have shown that Harald's f8bc48ea42 is highly relevant. Using its Message-ID, you can find it on the lore archive:

  https://lore.kernel.org/git/pull.2413.v3.git.git.1790189759802.gitgitgadget@gmail.com/

The archive shows that v3 was posted without replying to reviews on v2, and no new reviewers have shown interest since then, leaving the topic stuck.

The best path forward is for someone with a stake in getting this topic unstuck, such as yourself, to provide a fresh review. This can help push it to the finish line, or help us realize that it is not viable so the topic can be dropped. In the former case, you can reuse the function introduced by that topic in your patch. In the latter case, we no longer need to worry about the two topics conflicting.

Thanks.

On 31/10/2026 14:05, Nakul V S via GitGitGadget wrote:

Sounds good

As Junio said, we can add those later.

Unless someone can come up with a realistic use for this option I'd be inclined to drop it.

I think
	because it looks like a root commit. Run `git fetch --unshallow`
	to fetch the missing history first.

would be sufficient

As above, do we really need this option?

Lets move cleaunup_autostash() in a separate, preparatory, commit, rather than littering the code with unnecessary forward declarations.

Is this related to the other changes in this patch?

It's good that this returns early in non-shallow repositories, but cherry-pick detection is expensive as we have to calculate the diff of each commit on both sides of the merge-base. Ideally we'd only walk the commits once and feed the commit object ids into

	git format-patch --stdin --no-walk=unsorted ...

to generate the patches. The way the code is currently structured makes that slightly tricky as it checks out "onto" before generating the patches. I think we could move the checkout of "onto" into run_am() so that it is separate to the fast-forward case in cmd_rebase(). Then we could generate the patches and check for shallow grafts with a single revision walk before checking out "onto".

I wonder if it would be helpful to add a hint explaining why we refuse to cherry-pick, revert or rebase shallow boundary commits. There is nothing specific to the sequencer here, so if we want to add checks to other commands like "git commit --amend" in the future, maybe this should live in shallow.c? Alternatively we may want to use separate, more specific messages in each case.

This covers cherry-picking or reverting multiple commits

This covers cherry-picking or reverting a single commit.

This covers "git rebase -r", and on error we cleanup properly - good

This covers a regular rebase.

There are an awful lot of new tests here, many more than there are places where we check for a shallow boundary. It would be a good idea to check whether each test actually adds any new coverage. Also the rebase tests should live in one of the rebase test files (maybe t3400?)

Thanks for re-rolling, this is definitely moving in the right direction.

Thanks

Phillip