All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Phillip Wood <phillip.wood123@gmail.com>
Cc: Nakul V S via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org,  Patrick Steinhardt <ps@pks.im>,
	 Nakul V S <vsnakul@gmail.com>
Subject: Re: [PATCH] sequencer: refuse to revert shallow boundary commits by default
Date: Wed, 21 Oct 2026 12:11:13 -0700	[thread overview]
Message-ID: <xmqqqzhina26.fsf@gitster.g> (raw)
In-Reply-To: <7179f371-a709-476f-8e94-807d7b62ce6e@gmail.com> (Phillip Wood's message of "Wed, 21 Oct 2026 16:20:42 +0100")

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

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

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.

  reply	other threads:[~2026-10-21 19:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-21 10:09 [PATCH] sequencer: refuse to revert shallow boundary commits by default Nakul V S via GitGitGadget
2026-10-21 15:20 ` Phillip Wood
2026-10-21 19:11   ` Junio C Hamano [this message]
2026-10-26 10:58     ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqqzhina26.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=phillip.wood123@gmail.com \
    --cc=ps@pks.im \
    --cc=vsnakul@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.