[Toybox] [PATCH] patch: add -s to the synopsis line and fix typos.

[Rob Landley]

On 1/12/19 3:03 PM, enh wrote:
> On Sat, Jan 12, 2019 at 11:24 AM Rob Landley <rob at landley.net> wrote:
>>
>> On 1/12/19 11:05 AM, enh via Toybox wrote:
>>> ---
>>>  toys/posix/patch.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> I have a todo item to add "fuzz" support, would this be useful?
> 
> maybe. i don't know yet. my strategy for moving things over has been
> to just try a build locally, then to let the build bots tell me
> whether the main build targets/branches build okay, then "submit and
> see". in many cases that's worked fine (as far as we know), but in
> other cases (such as patch) something that isn't built by default for
> the main targets/branches breaks. then i revert the build change and
> send patches or bug reports to you.

Sigh. I had an exhausting $DAYJOB all last year and I'm flying back to do
another 6 months of it on sunday. I had 4 weeks off in between but basically
spent them limp and recovering.

One of the things I really _need_ to do is get mkroot to the point it's
replacing aboriginal linux and building linux from scratch. That way I can throw
lots of real-world data at stuff, which is the only way to catch the bugs
inspection and a test suite don't.

> right now 33 tools are switched over, five or six passed the build bot
> test and are waiting for "submit and see", sed needs a new prebuilt
> with the -z change, and there's a cp bug i haven't had time to look at
> yet:
> 
> --BEGIN QUOTE--
> toybox cp seems to handle the permission bits for symlinks differently:

Symlinks don't have permission bits? (man 2 fchmodat says AT_SYMLINK_NOFOLLOW is
not currently implemented. Did that change since 14.04? I'm trying to install
current devuan on my new laptop but the installer wants wireless firmware files
I haven't tracked down yet.)

> $ ls -al bootable/recovery/tests/testdata/testkey_v3.pk8
> lrwxrwxrwx 1 tbao primarygroup 14 Apr  2  2018
> bootable/recovery/tests/testdata/testkey_v3.pk8 -> testkey_v1.pk8
> $ ls -al bootable/recovery/tests/testdata/testkey_v1.pk8
> -rw-r--r-- 1 tbao primarygroup 1217 Apr  2  2018
> bootable/recovery/tests/testdata/testkey_v1.pk8
> $ rm testkey_v3.pk8 && cp
> bootable/recovery/tests/testdata/testkey_v3.pk8 . && ls -al
> testkey_v3.pk8
> -rw-r----- 1 tbao primarygroup 1217 Jan  9 21:42 testkey_v3.pk8

That's not right. Hmmm...

        fdout = openat(cfd, catch, O_RDWR|O_CREAT|O_TRUNC, try->st.st_mode);

Which is the "mode" of the symlink, except that mode says the filetype _is_ a
symlink and you can't O_CREAT one of them so it's gonna get _really_ confused...

Try now? (I added a test.)

> $ rm testkey_v3.pk8 && toybox cp
> bootable/recovery/tests/testdata/testkey_v3.pk8 . && ls -al
> testkey_v3.pk8
> -rwxr-x--- 1 tbao primarygroup 1217 Jan  9 21:42 testkey_v3.pk8
> --END QUOTE--
> 
> but getting back to the point, yeah, now i know a build that's reliant
> on patch(1) i'll examine that more thoroughly before i try it again.
> i'm not sure yet whether they're relying on fuzz because (as evidenced
> by the fact that i broke the build) i've never yet built that
> particular target...

Fuzz is generally a bad idea and the Linux devs rail against it all the time.
That said, conventional fuzz is _symmetrical_...

Ahem: backing up. Fuzz factor applies a patch when context lines don't match.
Ordinarily you need 3 leading and 3 trailing context lines, plus any
interstitial unchanged lines to match. (The offset into the file is essentially
cosmetic, although the hunks must apply in the order they occur in the file so
"this hunk would apply before that hunk" can't count as a match.)

So, what fuzz does is narrow the leading/trailing match context: fuzz 1 says the
first+last lines didn't match (so only 2 leading and 2 trailing lines matched),
and fuzz 2 says only ONE leading and ONE trailing line matched. Fuzzy 2 can
easily apply small hunks in the wrong place (especially if it's something like
adding/removing a curly bracket), and this doesn't always result in a build
break but _has_ resulted in some impressive rants on lkml.

So what _I_ was thinking of doing was treating _all_ context lines, leading,
trailing, and interstitial, the same. If you have a hunk with 3 leading, 3
trailing, and 2 interstitial context lines, that's 8 context lines. Fuzz 1 means
1 of them didn't match. Fuzz 2 means 2 of them didn't match. (I.E. My fuzz 1 is
THEIR fuzz 2.) And I'd allow interstitial context lines to be modified the same
way leading/trailing can (which the other patch didn't do last I checked).

> (so many of these things are really hard to search for too. "patch"
> crops up far more often as a false positive than as an actual
> reference to patch(1)...)

Indeed. "Try it and see what breaks" is often the main thing you can do.

Although what I did with the LFS builds is I tell it I'm doing a
single-processor build, put it in verbose mode, capture the entire output log,
and diff them. If autoconf spew changed, one of the tests was different and it
shouldn't be, so I'd track down why. (You're leveraging a lot of that stuff
already. I really should try to get that test environment _back_ but you know
about the aboriginal->mkroot stuff because of the toolchain versions, right?)

Rob