git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jerry Zhang <jerry@skydio.com>
Cc: git@vger.kernel.org, newren@gmail.com, ross@skydio.com,
	abe@skydio.com, 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 15:46:30 -0700	[thread overview]
Message-ID: <xmqqr1jo4aex.fsf@gitster.g> (raw)
In-Reply-To: <20210405221902.27998-1-jerry@skydio.com> (Jerry Zhang's message of "Mon, 5 Apr 2021 15:19:02 -0700")

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-05 22:46 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 [this message]
2021-04-06  2:52       ` Jerry Zhang
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=xmqqr1jo4aex.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 \
    --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).