Re: [PATCH] sequencer: refuse to revert shallow boundary commits by default
Hi Nakul
On 21/10/2026 11:09, Nakul V S via GitGitGadget wrote:
Show 10 quoted lines
> 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.
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?
Show 9 quoted lines
> diff --git a/builtin/revert.c b/builtin/revert.c
> index 52100a20cb..f27397fdec 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -24,7 +24,7 @@
> */
> > 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>..."),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)
Show 10 quoted lines
> diff --git a/sequencer.c b/sequencer.c
> index efbfd1315b..6de59bf6aa 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2366,6 +2367,19 @@ static enum pick_result do_pick_commit(struct repository *r,
> }
> 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) {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
Show 57 quoted lines
> + 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));
> + }
> + }
> +
> if (!commit->parents)
> parent = NULL;
> else if (commit->parents->next) {
> diff --git a/sequencer.h b/sequencer.h
> index a5d12a1070..f334bc5357 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -57,6 +57,7 @@ struct replay_opts {
> int committer_date_is_author_date;
> int ignore_date;
> int commit_use_reference;
> + int allow_shallow_boundary;
> > struct strvec trailer_args;
> > diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
> index 939e7a16a6..508464004f 100755
> --- a/t/t3501-revert-cherry-pick.sh
> +++ b/t/t3501-revert-cherry-pick.sh
> @@ -256,4 +256,29 @@ test_expect_success 'cherry-pick is unaware of --reference (for now)' '
> test_grep "^usage: git cherry-pick" actual
> '
> > +# 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
> +'
> +
> +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 'revert of shallow boundary succeeds with --allow-shallow-boundary' '
> + git -C shallow-repo revert --no-edit --allow-shallow-boundary HEAD
> +'
> +
> +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_done
> > base-commit: 47fdf2208c5b94c8cc01c106b8f7c85efb91c926