From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] apply: when -R, also reverse list of sections
Date: Tue, 20 Oct 2020 13:06:19 -0700 [thread overview]
Message-ID: <xmqqeelsbr2c.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20201020191206.104924-1-jonathantanmy@google.com> (Jonathan Tan's message of "Tue, 20 Oct 2020 12:12:06 -0700")
Jonathan Tan <jonathantanmy@google.com> writes:
>> But I have to wonder if it breaks the support for (arguably outside
>> the Git usecase) input that has more than one patch that touches the
>> same path to blindly reverse the order of all patches
>
> Sorry for getting back to this so late.
>
> The only other case I can think of (besides symlink<->file) is
> directory<->file, and even in that case, I think blindly reversing the
> order still works.
>
> If a more sophisticated rearrangement was needed, I would think that
> even applying the patches in the forward direction (that is, without
> "-R") wouldn't work, since Git is sensitive to the order of the patches.
> So I don't think we need to support such input (since they wouldn't work
> in the forward direction anyway).
I wish you told that to those who added fn_table kludge to apply.c
back when they did so. They apparently wanted to have a patch that
has more than one "diff --git a/hello.c b/hello.c" that talks about
the same file applied with a single invocation of "git apply".
Perhaps what they did is already broken with "apply -R", and blind
reversal of everything magically makes it work? Or what they did
already works with "apply -R" and your blind reversal would break,
unless you undo what they did?
>> (instead of
>> the obvious implementation of the fix for the above stated problem
>> --- i.e. make sure the first patch is a deletion of a symlink and
>> what immediately follows is a creation of a regular file, and swap
>> them only in such a case).
>
> This would make patch application more robust, but I still appreciate
> the relative simplicity of the existing approach
I'd rather want to see that we keep the normal cases simple,
i.e. majority parts of a patch with "apply -R" that did *not* have
to futz with the application order will keep what we do, and if
there are tricky cases like typechange diff, only special case them.
Thanks.
next prev parent reply other threads:[~2020-10-20 20:06 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-28 21:20 [PATCH] apply: when -R, also reverse list of sections Jonathan Tan
2020-09-28 22:07 ` Junio C Hamano
2020-10-20 19:12 ` Jonathan Tan
2020-10-20 20:06 ` Junio C Hamano [this message]
2020-10-20 20:50 ` Junio C Hamano
2020-10-20 21:36 ` Jonathan Tan
2020-10-20 21:48 ` Junio C Hamano
2020-10-20 22:04 ` [PATCH v2] " Jonathan Tan
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=xmqqeelsbr2c.fsf@gitster.c.googlers.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.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.
Code repositories for project(s) associated with this public 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).