Re: [PATCH] sequencer: refuse to revert shallow boundary commits by default

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Nakul

On 21/10/2026 11:09, Nakul V S via GitGitGadget wrote:
> 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?

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

> 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

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