Re: [HACKERS] git apply vs patch -p1

2013-09-16 Thread Andres Freund
On 2013-09-16 10:16:37 +0530, Ashutosh Bapat wrote:
 On Sun, Sep 15, 2013 at 12:38 AM, Andres Freund and...@2ndquadrant.comwrote:
 
  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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] git apply vs patch -p1

2013-09-16 Thread Andrew Gierth
 Josh == Josh Berkus j...@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?

-- 
Andrew (irc:RhodiumToad)


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] git apply vs patch -p1

2013-09-16 Thread Josh Berkus
On 09/15/2013 11:46 PM, Ashutosh Bapat wrote:
 On Sun, Sep 15, 2013 at 12:38 AM, Andres Freund and...@2ndquadrant.comwrote:
 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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] git apply vs patch -p1

2013-09-16 Thread Jeff Janes
On Mon, Sep 16, 2013 at 12:11 PM, Andrew Gierth and...@tao11.riddles.org.uk
 wrote:

  Josh == Josh Berkus j...@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:

http://wiki.postgresql.org/wiki/Working_with_Git

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


Re: [HACKERS] git apply vs patch -p1

2013-09-16 Thread Andrew Gierth
 Jeff == Jeff Janes jeff.ja...@gmail.com writes:

 Jeff I used git diff configured to use
 Jeff 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)


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] git apply vs patch -p1

2013-09-15 Thread Ashutosh Bapat
On Sun, Sep 15, 2013 at 12:38 AM, Andres Freund and...@2ndquadrant.comwrote:

 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?


 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


Re: [HACKERS] git apply vs patch -p1

2013-09-14 Thread Andrew Dunstan


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




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] git apply vs patch -p1

2013-09-14 Thread Andres Freund
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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] git apply vs patch -p1

2013-09-14 Thread Andrew Dunstan


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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] git apply vs patch -p1

2013-09-14 Thread Peter Eisentraut
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.




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers