[Toybox] [PATCH] tar: fix warn_unused_result error.

[enh]

On Sat, Sep 4, 2021 at 5:33 AM Rob Landley <rob at landley.net> wrote:

> On 9/2/21 7:12 PM, enh wrote:
> > On Thu, Sep 2, 2021 at 4:37 PM Rob Landley <rob at landley.net
> >     > to me, if the open() succeeded but the write() failed --- we're in
> weird
> >     > uncharted "shouldn't happen" territory, and probably want to know?
> >
> >     Hmmm, I thought the open would succeed and the write fail, but if
> it's reliable
> >     that the open is what fails when selinux is disabled...
> >
> > (note that i haven't tested that!)
>
> And I can't because I don't have selinux test environment coverage, or the
> stomach to read through enough of the terrible selinux code in the kernel.
>
> This is not an area I have sufficient existing expertise to feel confident
> about, and unlike sed/bzip/bash/awk/bc I'm not planning on doing a
> multi-month
> deep dive to BUILD said expertise. (Because I still think NSA selinux was a
> terrible idea and cannot generate the necessary enthusiasm.)
>
> >     There's your problem: never -Werror.
> >
> > if you don't -Werror, you inevitably check in code with warnings,
> spamming the
> > build and making it easier for more warnings to creep in.
>
> And then you remove the warnings again? Toybox doesn't have -Werror...
>
> Way back when the linux kernel was full of warnings because they got lost
> in the
> noise of the always verbose output, and I made a blueberry.py script that
> would
> filter the output into something intelligible
> (https://static.lwn.net/2002/0117/bigpage.php3#:~:text=Blueberry), and the
> kernel guys did their usual response to a "don't ask questions post
> errors" and
> implemented the functionality in a much less terrible way inside (what
> became)
> kbuild, and once you could SEE the warnings they removed them from the
> build.
>
> I'm not sure "the linux kernel is so much smaller and simpler than our
> package,
> it just doesn't have the scalability problems we do" is a compelling
> argument
> for most packages?
>

Android tries to avoid falling into the trap of "oh, you didn't have $X
enabled in that corner? we just assumed because it's on everywhere else!".

https://www.phoronix.com/scan.php?page=news_item&px=Linux-5.15-Werror

>     Compiler version updates always hallucinate
> >     new weirdness. If nothing else you're making it harder to build old
> versions in
> >     new build environments for regression testing comparisons.
> >
> > with hermetic builds you're using the right compiler for that version
> anyway.
> > (forget features: when 100M lines of code you have, reliant on compiler
> bugs you
> > will be.)
>
> As with "reproducible builds", that's true if you accept it as a premise
> and a
> lot of tedious work to fix when you don't. :P
>
> This is why I wanted to split the build into layers:
>
> 1) Base OS layer with libc+toybox plus native compiler (and maybe kernel):
> minimum needed to rebuild itself under itself in a tiny VM/container
> accessed
> via serial console or similar. (Which makes it self-regression-testing to
> an
> extent.)
>
> 2) Natively building the generic non-gui infrastructure layer: git/repo if
> it
> isn't in the base, probably ninja and cmake go here, more languages (perl,
> python, java, lua..), more libraries (ssl, ncurses, libxml, libusb...),
> better
> access mechanism (ssh or adb), whatever daemons define your environment
> (httpd,
> bind, cups... I know android has its own suite of these starting with a
> custom
> init and a security  management thingy but don't have a relationship
> diagram in
> front of me...)
>
> 3) Building the GUI/desktop can of worms, UNDER stage 2 above.
>
> Back under aboriginal linux I had layer 1 working pretty well and had a
> proof of
> concept linux from scratch layer 2, and a friend built a gentoo layer 2
> but it
> was hack city. I did a couple years of research on building a red hat
> layer 2, a
> debian layer 2, and a suse layer 2, but all four of those distros were
> incestuous tangled crap. (Debian was the least bad, which is not an
> endorsement.)
>
> One of the advantages of a layered approach is you can stop and snapshot.
> Build
> stage 2 on top of a known stage 1, build stage 3 on top of a known stage 2.
>
> Shipping your own base OS layer (to build stage 2 on top of a known stage
> 1)
> _is_ a hermetic build. Rebuilding stage 3 on top of a known stage 2 should
> be
> similarly orthogonal. (Or at least versionable.)
>
> And I still think that at some point the NDK build should be dogfooded as
> the
> AOSP toolchain build. (Maybe with a config switch or extra build stage
> overlaying libraries NDK doesn't ship. But then "adding libraries after
> the fact
> to an existing filesystem" is a normal part of the stage 2 build above? :)
>
> I don't have my head properly wrapped around how selinux annotation works
> into
> this, but I do know there are tools to annotate a filesystem after the fact
> because I keep bumping into references to them. And I suspect there are
> more
> granular approaches but splitting off those 3 layers seems like the
> obvious start...
>
> >     > and any
> >     > given warning's false positive rate can be highly dependent on the
> specific
> >     > codebase, and i don't get the feeling compiler writers think about
> scaling to
> >     > "can i rebuild all of Debian?" or even just "can i rebuild all of
> Android?".)
> >
> >     Their userbase kinda does.
> >
> > it's too late by then :-(
>
> Alas, they're using the Microsoft approach: your users are unpaid beta
> testers,
> you ship perpetually broken crap and course correct on the fly as the bug
> reports come in, which never quite keeps up with new breakage.
>
> >     Those are all "cannot continue to parse the input file format", not
> "output had
> >     a problem"...
> >
> > ah, so you're talking about the perror_msg() versus perror_exit()
> distinction.
> > still a _failure_ but not an immediate one. (though as someone who
> always uses
> > 'v' with tar, i'd never notice the failure interactively unless it was
> in the
> > last 40 lines!)
>
> Hence the exit code so at least your script notices.


assuming you've told bash to exit on error, which most scripts don't. i
really wish that was the default...


> Also, my debian host tar says:
>
>   tar: Exiting with failure status due to previous errors
>
> Which is a sadly verbose message but we should say _something_. Hmmm...
>
> >     >     So I _think_ your patch changes "tar x --selinux" behavior to
> exit
> >     instead of
> >     >     spamming warnings when run on a system without sexlinux
> support.
> >     >
> >     > seems like a feature to me? don't say --selinux if you don't
> actually care?
> >
> >     Which would mean --selinux is special and behaves differently from
> the rest
> >     of tar.
> >
> > i was going to say "but if you got the wrong selinux labels, what you
> wrote out
> > probably won't work" ... but then realized that's true of chmod() too.
> i'm not
> > going to be able to run that binary if its 'x' bit isn't set, any more
> than if
> > it doesn't have the right selinux label.
>
> I just added a "tar: had errors" message at the end, ala "oy, check the
> backscroll".
>

yeah, that's a good idea. (and the debian message does sound vaguely
familiar now you mention it... though this conversation reminded me that
it's been a long time since i last ran out of space, and even longer since
i last had any other kind of error from tar!)


> Rob
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20210907/39bfa6e8/attachment-0001.htm>