From: Junio C Hamano <gitster@pobox.com>
To: Tao Klerks <tao@klerks.biz>
Cc: Tao Klerks via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org
Subject: Re: [PATCH] cherry-pick: refuse cherry-pick sequence if index is dirty
Date: Wed, 24 May 2023 09:06:26 +0900 [thread overview]
Message-ID: <xmqqjzwyh9tp.fsf@gitster.g> (raw)
In-Reply-To: <CAPMMpojV1Ts=OKM0FbBHU6=EB5RKNxHucX-8VQmYoQBNefKpqQ@mail.gmail.com> (Tao Klerks's message of "Tue, 23 May 2023 18:01:25 +0200")
Tao Klerks <tao@klerks.biz> writes:
> The current implementation of this patch is far too restrictive. It
> doesn't break any tests (and maybe I should add one now that I know),
> but it's doing the wrong thing.
I am ambivalent. What do we want to see in a multi-pick sequence
that is different from rebase? A single-step cherry-pick can fail
safely before it touches the index or the working tree files, but if
two-step cherry-pick, whose first step succeeds, finds that it
cannot safely carry out its second step without clobbering the local
changes made to the working tree files, what should happen? Are we
OK if we stopped in the state just after the first step has already
been done?
My (tentative) answer to that question is "yes", but the recovery
options of "cherry-pick" may want to work differently from what we
have seen them traditionally do. Namely, the user accepts that the
first step is already done, and stopping "cherry-pick", be it called
"--abort" or something else, should just remove the sequencer state
and behave as if the single-pick cherry-pick on the first step only
has just finished and leave such a state in the index and the
working tree. If that is what we are going to do, then it would
make sense to adopt the same safety semantics we use for "git merge"
and "git checkout" to ensure only that the index is clean, relying
on the unpack-trees machinery that stops before clobbering a locally
modified working tree files. But if we are to aim for "all-or-none"
semantics people expect from aborting "git rebase", I suspect that
it would be way too complicated to allow random changes in the
working tree files that we may only discover to be problems after
starting the sequence of replaying commits one-by-one, and "too
restrictive" check may be justified. To put it differently, if it
is too restrictive for multi-pick, then we would want to loosen it
for "git rebase" as well, as the issues are likely to be the same.
Thanks.
next prev parent reply other threads:[~2023-05-24 0:06 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-23 8:32 [PATCH] cherry-pick: refuse cherry-pick sequence if index is dirty Tao Klerks via GitGitGadget
2023-05-23 16:01 ` Tao Klerks
2023-05-24 0:06 ` Junio C Hamano [this message]
2023-05-24 9:33 ` Tao Klerks
2023-05-30 13:01 ` Phillip Wood
2023-05-28 9:08 ` [PATCH v2] " Tao Klerks via GitGitGadget
2023-05-30 14:16 ` Phillip Wood
2023-09-06 5:02 ` Tao Klerks
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=xmqqjzwyh9tp.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=tao@klerks.biz \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).