git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Jerry Zhang <jerry@skydio.com>
To: Junio C Hamano <gitster@pobox.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, 5 Apr 2021 19:52:06 -0700	[thread overview]
Message-ID: <CAMKO5CuLpa9Sn_oXMpgP6oGE9NFA8aLeTfeyaW6TOTErE0KgEg@mail.gmail.com> (raw)
In-Reply-To: <xmqqr1jo4aex.fsf@gitster.g>

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.

On Mon, Apr 5, 2021 at 3:46 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jerry Zhang <jerry@skydio.com> writes:
>
> > Subject: Re: [PATCH V2] git-apply: Allow simultaneous --cached and --3way options
>
> s/Allow/allow/ (cf. "git shortlog --no-merged" output for recent examples)
>
> > Previously, --cached and --3way were not
> > allowed to be used together, since --3way
> > wrote conflict markers into the working tree.
>
> Hint that you are talking about the "git apply" command by
> mentioning the name somewhere.
>
> Drop "previously"; we talk about the status quo in the present tense
> in our proposed commit log messages to set the stage, and then describe
> what the patch author percieves as a problem, before describing the
> proposed solution to the problem.
>
> cf. Documentation/SubmittingPatches[[describe-changes]] (the whole section)
>
> > These changes change semantics so that if
> > these flags are given together and there is
> > a conflict, the conflicting objects are left
> > at a higher order in the cache, and the command
> > will return non-zero. If there is no conflict,
> > the patch is applied directly to cache as
> > expected and the command will return 0.
>
> 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.
>
> > The user can use `git diff` to view the contents
> > of the conflict, or `git checkout -m -- .` to
> > regenerate the conflict markers in the working
> > directory.
>
> Nice.
>
> > With the combined --3way and --cached flags,
> > The conflict markers won't be written to the
> > working directory, so there is no point in
> > attempting rerere.
>
> I am not sure what this paragraph is trying to convey here.
>
> I agree that when a *new* conflict is encountered in this new mode,
> writing out a rerere pre-image, in preparation for accepting the
> post-image the end-user gives us after the conflicts are resolved,
> does not make sense, because we are not giving the end-user the
> conflicted state and asking to help resolve it for us.
>
> But if a rerere database entry records a previous merge result in
> which conflicts were resolved by the end user, it would make sense
> to try reusing the resolution, I would think.  I offhand do not know
> how involved it would be to do so, so punting on that is fine, but
> that is "there is no point", but it is "we are not trying".
>
> Perhaps
>
>     When there are conflicts, theoretically, it would be nice to be
>     able to replay an existing entry in the rerere database that
>     records already resolved conflict that match the current one,
>     but that would be too much work, so let's not try it for now.
>
> would be a good explanation why we are not doing (i.e. we made a
> trade-off) and recording that is important, as it will allow others
> in the future to try building on the change we are proposing here
> (it is not like we decided that it is fundamentally wrong to try to
> use rerere in this situation).
>
> > Signed-off-by: Jerry Zhang <jerry@skydio.com>
> > Signed-off-by: Jerry Zhang <jerryxzha@googlemail.com>
>
> Unless we are interacting with two people with the same name, please
> sign-off with the same name/address as the name/address that will be
> recorded as the author of this change.  I am guessing that dropping
> the latter should be sufficient?
>
> Thanks.

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

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-03  1:34 [PATCH 0/1] " 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 [this message]
2021-04-06  5:52         ` Junio C Hamano
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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=CAMKO5CuLpa9Sn_oXMpgP6oGE9NFA8aLeTfeyaW6TOTErE0KgEg@mail.gmail.com \
    --to=jerry@skydio.com \
    --cc=abe@skydio.com \
    --cc=brian.kubisiak@skydio.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jerryxzha@googlemail.com \
    --cc=newren@gmail.com \
    --cc=ross@skydio.com \
    --subject='Re: [PATCH V2] git-apply: Allow simultaneous --cached and --3way options' \
    /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

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

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