All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jerry Zhang <jerry@skydio.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Elijah Newren <newren@gmail.com>, Ross Yeager <ross@skydio.com>,
	Abraham Bachrach <abe@skydio.com>,
	Brian Kubisiak <brian.kubisiak@skydio.com>,
	Jerry Zhang <jerryxzha@googlemail.com>
Subject: Re: [PATCH V2] git-apply: Allow simultaneous --cached and --3way options
Date: Mon, 05 Apr 2021 22:52:38 -0700	[thread overview]
Message-ID: <xmqqh7kk2c49.fsf@gitster.g> (raw)
In-Reply-To: <CAMKO5CuLpa9Sn_oXMpgP6oGE9NFA8aLeTfeyaW6TOTErE0KgEg@mail.gmail.com> (Jerry Zhang's message of "Mon, 5 Apr 2021 19:52:06 -0700")

Jerry Zhang <jerry@skydio.com> writes:

> Thanks for the comments! I've updated v3 with the changes. Let me know
> if you have any
> more thoughts on whether to block / warn the user before clobbering their cache.

Please do not top-post on this list.

I've already said that I think we should ensure the index is clean
by default, because, unlike the case where the application is done
on the working tree files, the use of "--cached" is a sign that the
next step is likely to write a tree out.  As I've already said so in
earlier reviews, there is nothing more from me to add on that issue.

>> Give an order to the codebase to "be like so".  Here is my attempt.
>>
>>     Teach "git apply" to accept "--cached" and "--3way" at the same
>>     time.  Only when all changes to all paths involved in the
>>     application auto-resolve cleanly, the result is placed in the
>>     index at stage #0 and the command exits with 0 status.  If there
>>     is any path whose conflict cannot be cleanly auto-resolved, the
>>     original contents from common ancestor (stage #1), our version
>>     (stage #2) and the contents from the patch (stage #3) for the
>>     conflicted paths are left at separate stages without any attempt
>>     to resolve the conflict at the content level, and the command
>>     exists with non-zero status, because there is no place (like the
>>     working tree files) to leave a half-resolved conflicted merge
>>     result to ask the end-user to resolve.

I wrote the above as an example to illustrate the tone and the level
of details expected in our proposed commit log message.  The
behaviour it describes may not necessarily match what you have
implemented in the patch.

For example, imagine that we are applying a patch for two paths,
where one auto-resolves cleanly and the other does not.  The above
description expects both paths will leave the higher stages (instead
of recording the auto-resolved path at stage #0, and leaving the
other path that cannot be auto-resolved at higher stages) and the
command exits with non-zero status, which may not be what you
implemented.  As an illustration, I didn't necessarily mean such an
all-or-none behaviour wrt resolving should be what we implement---I
do not want to choose, as this is your itch and I want _you_ with
the itch to think long and hard before deciding what the best design
for end-users would be, and present it as a proposed solution.  An
obvious alternative is to record auto-resolved paths at stage #0 and
leave only the paths for which auto-resolution failed in conflicted
state.

Thanks.

  reply	other threads:[~2021-04-06  5:52 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-03  1:34 [PATCH 0/1] git-apply: Allow simultaneous --cached and --3way options Jerry Zhang
2021-04-03  1:34 ` [PATCH 1/1] " Jerry Zhang
2021-04-03  3:46   ` Elijah Newren
2021-04-03  4:26     ` Junio C Hamano
2021-04-04  1:02       ` Junio C Hamano
2021-04-05 22:12         ` Jerry Zhang
2021-04-05 22:23           ` Junio C Hamano
2021-04-05 23:29             ` Jerry Zhang
2021-04-06  0:10               ` Junio C Hamano
2021-04-05 22:08     ` Jerry Zhang
2021-04-05 22:19   ` [PATCH V2] " Jerry Zhang
2021-04-05 22:46     ` Junio C Hamano
2021-04-06  2:52       ` Jerry Zhang
2021-04-06  5:52         ` Junio C Hamano [this message]
2021-04-06 21:56           ` Jerry Zhang
2021-04-07  2:25             ` Jerry Zhang
2021-04-06  2:49     ` [PATCH v3] git-apply: allow " Jerry Zhang
2021-04-07 18:03       ` [PATCH v4] " Jerry Zhang
2021-04-07 19:00         ` Junio C Hamano
2021-04-08  2:13         ` [PATCH v5] " Jerry Zhang
2021-04-08 13:33           ` Junio C Hamano
2021-04-12 15:45             ` Elijah Newren
2021-04-12 18:26               ` Junio C Hamano
2021-04-12 15:40           ` Elijah Newren
2021-04-12 18:27             ` Junio C Hamano
2021-04-03  3:04 ` [PATCH 0/1] git-apply: Allow " Elijah Newren
2021-04-05 22:05   ` Jerry Zhang
2021-04-03  5:24 ` Bagas Sanjaya
     [not found]   ` <CAMKO5CtiW84E4XjnPRf-yOPp+ua_u07LsAu=BB0YhmP3+3kYiw@mail.gmail.com>
2021-04-03  8:05     ` Bagas Sanjaya

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=xmqqh7kk2c49.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=abe@skydio.com \
    --cc=brian.kubisiak@skydio.com \
    --cc=git@vger.kernel.org \
    --cc=jerry@skydio.com \
    --cc=jerryxzha@googlemail.com \
    --cc=newren@gmail.com \
    --cc=ross@skydio.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.