git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Elijah Newren <newren@gmail.com>
Cc: Jerry Zhang <jerry@skydio.com>,
	Git Mailing List <git@vger.kernel.org>,
	ross@skydio.com, abe@skydio.com, brian.kubisiask@skydio.com,
	Jerry Zhang <jerryxzha@googlemail.com>
Subject: Re: [PATCH 1/1] git-apply: Allow simultaneous --cached and --3way options
Date: Sat, 03 Apr 2021 18:02:31 -0700	[thread overview]
Message-ID: <xmqq1rbq276g.fsf@gitster.g> (raw)
In-Reply-To: <xmqqy2e00zaf.fsf@gitster.g> (Junio C. Hamano's message of "Fri, 02 Apr 2021 21:26:00 -0700")

Junio C Hamano <gitster@pobox.com> writes:

> It might be OK to only allow the combination when everything auto
> resolves cleanly and fail the operation without touching either the
> index or the working tree.  Pretending there was no delete/modify
> conflicts or adding contents with unresolved conflicts as if nothing
> bad happened as stage 0 entries would never be acceptable.
>
> Perhaps
>
>  * Error out if the index does not match HEAD.
>
>  * Try applying to the contents in the index.  If there are any
>    structural conflicts, leave these paths at higher stage and do
>    not touch their contents.
>
>  * For paths without structural conflict but need content merges,
>    attempt ll-merge of the contents.  If autoresolves cleanly,
>    register the result at stage 0.  Otherwise, discard the failed
>    conflicted merge, and leave stages 1, 2 and 3 as they are.
>
>  * Exit with status 0 if and only if everything has resolved
>    cleanly.  Otherwise, exit with non-zero status.
>
> would be the minimally-acceptably-safe behaviour.

Note that, while a lot unsatisfactory than the above, the following
would also be acceptable.

  * Error out if the index does not match HEAD.

  * Try applying to the contents in the index.  If there are any
    structural conflicts, abort without touching the index (or the
    working tree --- but that is best left unsaid as we all know we
    are talking about '--cached').

  * For paths without structural conflict but need content merges,
    attempt ll-merge of the contents.  If ALL SUCh PATHS autoresolve
    cleanly, register their result at stage 0.  Otherwise, abort
    without touching the index (or the working tree).

  * Exit with status 0 if and only if everything has resolved
    cleanly.  Otherwise, exit with non-zero status (and never touch
    the index or the working tree).

The version I earlier gave would give a good starting point to
manually resolve the conflicts in the index and when resolved fully,
it is safely recorded as the result of applying the patch on top of
HEAD, because the non-final results are all in higher stages, and
all the paths at stage 0 are either from the HEAD and unaffected by
the merge, or the ones that cleanly resolved.  The "the index must
match HEAD" upfront is to ensure that.  Otherwise it would make it
very tempting, after spending all that time to resolve the conflicts
only in the higher stages of the index, to commit the index as-is to
make a child commit of HEAD and record that it is the result of
applying the patch.  But if the starting condition had a change
unrelated to the change the patch brings in already in the index,
the resulting commit would be _more_ than what the patch did to the
codebase.

The simplified version would let the user proceed only when the
conflicts can mechanically resolved, but it still has the "make sure
what is recorded is only from the incoming patch" safety.

Of course, if the user is trying to cherry-pick parts of multiple
patches and combine them to create a new single commit, the second
and subsequent applycation of the patches would be thwarted by the
"the index must match HEAD" rule, but it is far safer to make each
step into its own snapshot commit during such a workflow to combine
multiple patch pieces and then squash them together after finishing,
than carrying an intermediate result only in the index and risk
losing work you did in the previous step(s) to incorrect resolution
in later step(s).

  reply	other threads:[~2021-04-04  1:06 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 [this message]
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
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=xmqq1rbq276g.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=abe@skydio.com \
    --cc=brian.kubisiask@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 1/1] 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

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

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

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

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git