patch: .diff interpreted incorrectly?

Bernhard Fischer rep.nop at aon.at
Mon Oct 10 18:21:52 UTC 2005


On Mon, Oct 10, 2005 at 12:38:16PM -0500, Rob Landley wrote:
>On Monday 10 October 2005 07:42, Bernhard Fischer wrote:
>> Hi,
>>
>> ./patch is the busybox one, it behaves differently as GNU patch,
>> i.e. it seems to patch the "wrong" file as compared to GNU patch
>> version 2.5.9? "wrong" because i'd expect _both_ patch commands to patch
>> the same file..
>>
>> Is this a feature or a bug in busybox' patch?
>
>Figuring out which file to patch is a perennial problem with the patch 
>command, because diff lists differences between two files, so patch should 
>apply those changes to which of those two?

SUSv sais that the one with *** is the src and the one with --- is the
dst. The only problem is that for _my_ usual invocations of (GNU) diff,
no *** is ever emitted.
>
>In real world use, only one of the two files mentioned ever actually exists 
>when you're applying a patch, so the problem doesn't come up much.  (The diff 
>is between old and new versions of a file in the tree you're applying to.  If 
>the diff is between two files that both exist in the tree you're applying to, 
>you created the diff wrong.)
>
>So this is a "corner case that's at best handled by a heuristic anyway".
>
>There was a nice discussion of this on linux-kernel years ago, but my 
>google-fu roll is failing.  Basically, this is why they diff two entire trees 
>and then use -p1 to chop off the differing names.  What's left is two 
>identical (and thus unambigous) paths to the file to modify.
>
>On the other hand, if you diff "editors/sed.bak" and "editors/sed.c" and the 
>person who applies it has a sed.bak lying around in their tree, there's a 50% 
>chance the wrong file will be patched.  (And it doesn't matter _which_ one 
>the heuristic picks because you could have made the patch the other way.  
>sed.c and sed.new, for example.)  And this problem actually bites high-volume 
>projects like linux-kernel that marshall large numbers of patches through 
>many different actively developed trees using automatic scripts.  A patch 

I'm not going into arguing about that one. My pet peeve -- as one of
those who loosely feel responsible for an out of tree console
implementation -- is upstream's console_printk, which should badly
renamed to console_printk_level as the implementation i'm concerned
about has the notion of console_printk being the printk which outputs to
a console. As said, not a good idea to get me started on that.. ;)

>that _fails_ to apply gives you a warning, but one that happily applies to 
>the wrong file means hunks get lost and not forwarded.
>
[]
>It looks like the gnu heuristic is checking the "old" first file first, and 
>we're checking the "new" file first.  We can do that, if you think it 
>matters. 

No. I personally think that looking at the target to which we're supposed to
apply it is a better strategy, fwiw.

>          But in the end, you're just testing that the heuristic is 
>consistent in a situation where there _is_ no right answer.  (Ours actually 
>makes more sense than gnu, as usual.  We're picking the "new" file.  The 
>result is that the pair of files look like what you diffed from after you 
>apply the patch.  Why is that wrong?)
>
>
>If you want to poke around with patch, there's plenty of to-do items.  In 

I don't want to. It was you who started to mention patch :)

>addition to fairly standard usage ("patch -p1 -i thingy.patch") not currently 

It should *currently* work right (except vodz mentioning to use bb_getlarg
instead of atoi, because the former would check/warn for -phaha,gotYa
but would add 6 bytes as compared to the dumb variant i (temporarily)
changed it to).

[skip]:x\ny
>working right due to yesterday's message about command option parsing going 
>"boing", we have two current problems with patch:
>
>1) No fuzz factor support.  (New feature, but useful new feature people 
>actually miss if it's not there.)
>
>2) We're not close to full susv3 feature set:
>
>http://www.opengroup.org/onlinepubs/009695399/utilities/patch.html
>
>Although our response to -c, -e, and -n should probably be "wontfix" because 
>those are obsolete formats we have no incentive to support.  (Only unified 
>diff format actually matters anymore.  Probably if full susv3 support is 
>enabled we should catch these and print some kind of brief "this lack is 
>intentional" message.)
>
>The other susv3 command line options would actually be nice to have, if they 
>can be configured out.  (I can see -b, -d, and -l actually coming in handy.)
>
>The essential minimal subset is really just -p and -i support, with no fuzz 
>factor, which is enough to consistently apply real-world patches via the 
>linux-kernel method (the ones that apply cleanly and aren't 
>whitespace-damaged, anyway).
>
>Rob
>_______________________________________________
>busybox mailing list
>busybox at busybox.net
>http://busybox.net/cgi-bin/mailman/listinfo/busybox
>



More information about the busybox mailing list