git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: gitster@pobox.com
Cc: jonathantanmy@google.com, git@vger.kernel.org
Subject: Re: [PATCH] apply: when -R, also reverse list of sections
Date: Tue, 20 Oct 2020 12:12:06 -0700	[thread overview]
Message-ID: <20201020191206.104924-1-jonathantanmy@google.com> (raw)
In-Reply-To: <xmqqy2ktk11t.fsf@gitster.c.googlers.com>

> Jonathan Tan <jonathantanmy@google.com> writes:
> 
> > A patch changing a symlink into a file is written with 2 sections (in
> > the code, represented as "struct patch"): firstly, the deletion of the
> > symlink, and secondly, the creation of the file. When applying that
> > patch with -R, the sections are reversed, so we get:
> >
> >  (1) creation of a symlink, then
> >  (2) deletion of a file.
> 
> Good observation.
> 
> 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).

> (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 - the patches you see
in the input are all applied in order (or reverse order, in the case of
"-R"). You also wouldn't end up in the situation where you think that
your input works because it works with your version of Git, but it
actually doesn't work with an older version of Git or another
implementation. (I haven't researched how other implementation do it,
though.)

  reply	other threads:[~2020-10-20 19:12 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 [this message]
2020-10-20 20:06     ` Junio C Hamano
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=20201020191206.104924-1-jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).