Обсуждение: git apply vs patch -p1
Folks, Lately I've been running into a lot of reports of false conflicts reported by "git apply". The most recent one was the "points" patch, which git apply rejected for completely ficticious reasons (it claimed that the patch was trying to create a new file where a file already existed, which it wasn't). I think we should modify the patch review and developer instructions to recommend always using patch -p1 (or -p0, depending), even if the patch was produced with "git diff". Thoughts? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
On 09/14/2013 02:37 PM, Josh Berkus wrote: > Folks, > > Lately I've been running into a lot of reports of false conflicts > reported by "git apply". The most recent one was the "points" patch, > which git apply rejected for completely ficticious reasons (it claimed > that the patch was trying to create a new file where a file already > existed, which it wasn't). > > I think we should modify the patch review and developer instructions to > recommend always using patch -p1 (or -p0, depending), even if the patch > was produced with "git diff". > > Thoughts? > FWIW that's what I invariably use. You do have to be careful to git-add/git-rm any added/deleted files, which git-apply does for you (as well as renames) - I've been caught by that a couple of times. cheers andrew
On 2013-09-14 15:03:52 -0400, Andrew Dunstan wrote: > > On 09/14/2013 02:37 PM, Josh Berkus wrote: > >Folks, > > > >Lately I've been running into a lot of reports of false conflicts > >reported by "git apply". The most recent one was the "points" patch, > >which git apply rejected for completely ficticious reasons (it claimed > >that the patch was trying to create a new file where a file already > >existed, which it wasn't). > > > >I think we should modify the patch review and developer instructions to > >recommend always using patch -p1 (or -p0, depending), even if the patch > >was produced with "git diff". > > > >Thoughts? > > > > > FWIW that's what I invariably use. > > You do have to be careful to git-add/git-rm any added/deleted files, which > git-apply does for you (as well as renames) - I've been caught by that a > couple of times. git reset? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 09/14/2013 03:08 PM, Andres Freund wrote: > On 2013-09-14 15:03:52 -0400, Andrew Dunstan wrote: >> On 09/14/2013 02:37 PM, Josh Berkus wrote: >>> Folks, >>> >>> Lately I've been running into a lot of reports of false conflicts >>> reported by "git apply". The most recent one was the "points" patch, >>> which git apply rejected for completely ficticious reasons (it claimed >>> that the patch was trying to create a new file where a file already >>> existed, which it wasn't). >>> >>> I think we should modify the patch review and developer instructions to >>> recommend always using patch -p1 (or -p0, depending), even if the patch >>> was produced with "git diff". >>> >>> Thoughts? >>> >> >> FWIW that's what I invariably use. >> >> You do have to be careful to git-add/git-rm any added/deleted files, which >> git-apply does for you (as well as renames) - I've been caught by that a >> couple of times. > git reset? > > Yes, of course you can roll back as long as you haven't published your commits. But it's a nuisance. cheers andrew
On Sat, 2013-09-14 at 11:37 -0700, Josh Berkus wrote: > Lately I've been running into a lot of reports of false conflicts > reported by "git apply". The most recent one was the "points" patch, > which git apply rejected for completely ficticious reasons (it claimed > that the patch was trying to create a new file where a file already > existed, which it wasn't) Every file in that patch contains new file mode 100644 which means it is creating a new file. I would review how that patch was created.
On Sun, Sep 15, 2013 at 12:38 AM, Andres Freund <andres@2ndquadrant.com> wrote:
On 2013-09-14 15:03:52 -0400, Andrew Dunstan wrote:git reset?
>
> On 09/14/2013 02:37 PM, Josh Berkus wrote:
> >Folks,
> >
> >Lately I've been running into a lot of reports of false conflicts
> >reported by "git apply". The most recent one was the "points" patch,
> >which git apply rejected for completely ficticious reasons (it claimed
> >that the patch was trying to create a new file where a file already
> >existed, which it wasn't).
> >
> >I think we should modify the patch review and developer instructions to
> >recommend always using patch -p1 (or -p0, depending), even if the patch
> >was produced with "git diff".
> >
> >Thoughts?
> >
>
>
> FWIW that's what I invariably use.
>
> You do have to be careful to git-add/git-rm any added/deleted files, which
> git-apply does for you (as well as renames) - I've been caught by that a
> couple of times.
git reset wouldn't remove the files that were newly added by the patch, would it?
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
On 2013-09-16 10:16:37 +0530, Ashutosh Bapat wrote: > On Sun, Sep 15, 2013 at 12:38 AM, Andres Freund <andres@2ndquadrant.com>wrote: > > > On 2013-09-14 15:03:52 -0400, Andrew Dunstan wrote: > > > > > > On 09/14/2013 02:37 PM, Josh Berkus wrote: > > > >Folks, > > > > > > > >Lately I've been running into a lot of reports of false conflicts > > > >reported by "git apply". The most recent one was the "points" patch, > > > >which git apply rejected for completely ficticious reasons (it claimed > > > >that the patch was trying to create a new file where a file already > > > >existed, which it wasn't). > > > > > > > >I think we should modify the patch review and developer instructions to > > > >recommend always using patch -p1 (or -p0, depending), even if the patch > > > >was produced with "git diff". > > > > > > > >Thoughts? > > > > > > > > > > > > > FWIW that's what I invariably use. > > > > > > You do have to be careful to git-add/git-rm any added/deleted files, > > which > > > git-apply does for you (as well as renames) - I've been caught by that a > > > couple of times. > > > > git reset? > > > > > git reset wouldn't remove the files that were newly added by the patch, > would it? Depends on how you do it. I simply commit patches I look at - then they can easily be removed using git reset --hard HEAD^. And it allows to make further changes/annotations during review that are clearly separated from the patch. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 09/15/2013 11:46 PM, Ashutosh Bapat wrote: > On Sun, Sep 15, 2013 at 12:38 AM, Andres Freund <andres@2ndquadrant.com>wrote: >> git reset? >> >> > git reset wouldn't remove the files that were newly added by the patch, > would it? The issue isn't that, it's that git apply is just buggy and can't tell the difference between a new file and a modified one. The "points" patch contained no new files, just modifications. But for some reason, git apply read it as being all new files, which failed. "patch -p1" worked fine. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
>>>>> "Josh" == Josh Berkus <josh@agliodbs.com> writes: Josh> The issue isn't that, it's that git apply is just buggy andJosh> can't tell the difference between a new file and amodifiedJosh> one. It's not the fault of git apply; the patch contained explicit annotations on all the files claiming that they were new. Both the patches I've looked at so far (picksplit NaNs and enable_material) had the same defect. The question is, how are these submitters preparing their patches? -- Andrew (irc:RhodiumToad)
On Mon, Sep 16, 2013 at 12:11 PM, Andrew Gierth <andrew@tao11.riddles.org.uk> wrote:
>>>>> "Josh" == Josh Berkus <josh@agliodbs.com> writes:
Josh> The issue isn't that, it's that git apply is just buggy and
Josh> can't tell the difference between a new file and a modified
Josh> one.
It's not the fault of git apply; the patch contained explicit
annotations on all the files claiming that they were new. Both the
patches I've looked at so far (picksplit NaNs and enable_material)
had the same defect.
The question is, how are these submitters preparing their patches?
I used "git diff" configured to use src/tools/git-external-diff, as described here:
The resulting patch applies fine with patch, but not with git apply.
If I instead generate a patch with git diff --no-ext-diff, then it applies with git apply.
Cheers,
Jeff
>>>>> "Jeff" == Jeff Janes <jeff.janes@gmail.com> writes: Jeff> I used "git diff" configured to useJeff> src/tools/git-external-diff, as described here: hmm... so that git-external-diff script is trying to fake git diff output, including using 'diff -git' and index lines, but the context-diff output wouldn't work with git apply even if the header lines were correct (as far as I can see git apply accepts only git's unified-diff format - the git people clearly have no truck with context diffs). -- Andrew (irc:RhodiumToad)