[Toybox] [PATCH] Support diff --no-dereference

enh enh at google.com
Thu Sep 5 11:08:12 PDT 2024


On Thu, Sep 5, 2024 at 10:17 AM Rob Landley <rob at landley.net> wrote:
>
> FYI I hadn't sent this old reply because I instead just applied the patch as-is
> after _two_ timeout pokes, but if it _still_ doesn't quite work, for the record
> my initial objections were...
>
> On 8/26/24 16:30, enh wrote:
> > ...but ignoring the bikeshed of "should we do something about all the
> > _other_ readlink callers" --- ping on this specific patch :-)
>
> Sorry, fell a bit behind. Partly burnout, partly I started working with
> the j-core guys again and got a bit distracted (flew to Canada a couple
> weeks back, flew to japan wednesday). And I need to move my email to the
> new laptop (I'm typing this in vi so I can scp it to my website so I
> can wget it on the other machine to send it), but gmail is about to stop:
>
>
> https://www.osnews.com/story/138392/google-to-restricts-access-to-imap-smtp-pop-to-oauth-this-year/
>
> So I need to point my domain's mx record to dreamhost and have the new
> machine fetch mail from there instead, and I've been a bit reluctant to
> pull the trigger on that. (I don't THINK fiddling with my domain's mx record
> is going to break the mailing list? It's their web GUI doing it. They have
> support people... It's just, big red button.)
>
> I saw this patch when it came in (web archive, lovely thing) but didn't
> apply it immediately because:
>
> 1) Adding a --longopt without a corresponding short option sets my
> teeth on edge. (It's not unix.)

(insert "gnu's not unix" joke here...)

> In this case, cp -P and chgrp -P says
> not to follow symlinks, and diff isn't currently using -P, so I might do
> that.
>
> 2) It added a new function to lib/ that only has a single user. I tend
> to add it to just the command and move it into lib/ when there's a second
> user. (It's not re-use when it's just use, even this is only calling
> the function once so having a wrapper function even inline in diff.c
> isn't a win yet.)

yeah, it only has one user in diff.c too. i think the point was to be
able to use the `(FLAG(..) ? lstat : stat)(...)` trick that's used
elsewhere in toybox, so i've sent a patch to do that and address the
warnings in diff.c.

> 3) Then there was the readlink0() vs xreadlink() thread in the list giving me an
> excuse to delay...
>
> 4) But the real blocker is I did most of a rewrite of diff but shelved it amidst
> a wave of indignation and was pretty much ignoring the one in pending as
> "something I really want to throw out but people get mad". If it wasn't for this
> patch touching lib and setting API precedent I'd happily apply arbitrary crap to
> that one because I don't care. (Otherwise I'd have various style cleanups and
> think 96 lines was a bit much for two readlinks a strcmp and a print.)
>
> The tests are nice though. :)

that was my favorite part too. usually is in any change, but
_especially_ for a toybox change in pending...

> Sigh, I'll try to get it in this weekend...
>
> Rob


More information about the Toybox mailing list