[Toybox] Config.probed changed, run 'make oldconfig'

[enh]

On Fri, Jun 10, 2022 at 10:26 PM Rob Landley <rob at landley.net> wrote:

> On 6/10/22 14:47, enh via Toybox wrote:
> > i haven't updated in a week or two, but trying to update today, i hit
> this:
> >
> > enh-p920.mtv:/new-ssd/aosp-master-with-phones/external/toybox$
> ./post_update.sh .
> >
> > -------- device
> >
> > warning: using unfinished code from toys/pending
> > Library probe
> > generated/{Config.in
> > Config.probed changed, run 'make oldconfig'
>
> Sigh. I think I need to lobotomize it again for now... (Pushed a commit to
> turn
> it into a warning.)
>

thanks!


> Previously the dependencies weren't recalculating a lot of stuff unless
> you did
> a "make distclean" (or "make clean" and "make oldconfig"), but this caused
> build
> breaks when I switched between the host, musl, and NDK toolchains. So I
> tried to
> fix it.
>
> The problem I got to is, when it updates Config.probed I can easily-ish
> adjust
> the .config file for the changes, but there are a bunch of dependencies on
> those
> symbols that won't get adjusted unless I make a tool that can parse config
> syntax and resolve dependencies:
>
> https://landley.net/notes-2022.html#28-05-2022
>
> And THAT is half the "write a kconfig replacement" todo item. (The rest is
> writing a menuconfig replacement with all the ansi escapes.) Which is a
> can of
> worms I don't want to open right now.
>
> But I ALSO don't want anything in scripts/make.sh to call the old kconfig
> because the layering is part of the license isolation: .config is consumed
> read-only by scripts/make.sh and friends. You can "rm -rf kconfig", and
> should
> still be able to drop a .config file into the top level and call
> scripts/make.sh
> and scripts/install.sh and they should work.
>
> So my header dependency work stopped at an awkward place where it detects
> "this
> thing should be done" but refuses to do it for you.
>
> > if i do that though...
> >
> > rm -rf .config generated/ android/
> >
> > function generate() {
> >   which=$1
> >   echo -e "\n-------- $1\n"
> >
> >   # These are the only generated files we actually need.
> >   files="config.h flags.h globals.h help.h newtoys.h tags.h"
> >
> >   cp .config-$which .config
> >
> >   make oldconfig # added here
> >
> >   NOBUILD=1 scripts/make.sh
>
> You didn't set CROSS_COMPILE there, so it's querying the cc in your $PATH,
> which
> is probably glibc.
>

yeah, since none of that stuff is likely to work for me anyway i've always
just assumed those things need to be manually configured. (see later for a
specific example...)


> >   out=android/$which/generated/
> >   mkdir -p $out
> >   for f in $files; do cp generated/$f $out/$f ; done
> >   rm -rf .config generated/
> >
> >   make allnoconfig KCONFIG_ALLCONFIG=.config-$which
>
> Didn't set CROSS_COMPILE there either.
>
> The Config.probed symbols aren't selectable (the "bool" hasn't got a string
> after it so doesn't show up in menuconfig), which means they take their
> default
> value even in allyesconfig/allnoconfig unless acted upon by a dependency
> or a
> "selects" statement. (Allyesconfig and allnoconfig just affect the
> selectable
> symbols.)
>
> > }
> >
> > generate "device"
> > generate "linux"
> > generate "mac"
> >
> > ...i end up with incorrect configs. for example, despite
> >
> > enh-p920.mtv:/new-ssd/aosp-master-with-phones/external/toybox$ grep -r
> SHADOW
> > .config-*
> > .config-device:# CONFIG_TOYBOX_SHADOW is not set
> > .config-linux:# CONFIG_TOYBOX_SHADOW is not set
> > .config-mac:# CONFIG_TOYBOX_SHADOW is not set
> >
> > i end up with the wrong definition in my generated config.h files:
> >
> > android/device/generated/config.h:#define CFG_TOYBOX_SHADOW 1
> > android/device/generated/config.h:#define USE_TOYBOX_SHADOW(...)
> __VA_ARGS__
> > android/mac/generated/config.h:#define CFG_TOYBOX_SHADOW 1
> > android/mac/generated/config.h:#define USE_TOYBOX_SHADOW(...) __VA_ARGS__
> > android/linux/generated/config.h:#define CFG_TOYBOX_SHADOW 1
> > android/linux/generated/config.h:#define USE_TOYBOX_SHADOW(...)
> __VA_ARGS__
>
> Sigh, this is another reason I use miniconfigs. These symbols would drop
> out in
> a miniconfig because they're constant within a toolchain. :P
>

is this also why i need the "is not set" comment lines? i've never
understood why i can't just miss out the irrelevant stuff...


> (SO much backstory...)
>
> > on the other hand, if i disable the new check in scripts/make.sh
> >
> > #[ "$A" != "$B" ] &&
> > #  { echo -e "\nConfig.probed changed, run 'make oldconfig'" >&2; exit
> 1;}
> >
> > everything works again... what am i missing?
>
> Just because the dependencies say .config should have its dependencies
> recalculated (because the generated/Config.probed it includes changed)
> doesn't
> mean NOT doing it will necessarily cause a build break. If stuff that
> "depends
> on" a symbol that went away already wasn't selected, you're probably fine.
> That's "working by coincidence", but it's probably also your use case with
> a
> known .config that already works in your bionic toolchain. You've manually
> selected options that work on your toolchain, and are not running
> something like
> defconfig to figure out what's supported and enable it. Also, your
> toolchain is
> stable so features aren't appearing and disappearing like they do when you
> swap
> toolchains out from under an existing .config, which is what I do in
> testing a
> lot. (You have 3 potted configs already.)
>
> The Config.probed symbols mostly control what _can_ be selected, hiding
> commands/options that aren't available in this build environment.
>
> But only mostly: TOYBOX_SHADOW specifically controls whether #include
> <shadow.h>
> happens in portability.h, that's a build break in _either_ direction if
> you get
> it wrong.
>
> That said, gcc 5 introduced __has_include so in theory I could use that if
> clang
> also supports it these days. (gcc 5.1 was April 2015, 7 year time
> horizon...)
>

yes, clang's supported __has_include() for as long as we've been using
clang. (annoyingly, their documentation really sucks -- far worse than gcc
-- so i couldn't find a reference to what version it was introduced in.)


> Any compile time probes I can turn into build-time probes, I'm all for it.
>

certainly __has_include(), yes, though see later for why syscalls are
hairy...


> Figuring out how that impacts the dependency resolution is a design issue
> though. What's the right thing to do when this command depends on a feature
> that's not available in this build environment? Before menuconfig hid
> them, but
> now it wouldn't test it until compile time. I can probably figure out how
> to
> make commands drop out of the list (define HAS_BLAH() macros inside the
> __has_include stanzas maybe) and get --gc-sections to dead-code-eliminate
> them... but is that the right thing to do? Avoids a build break, at the
> cost of
> "I built this command and it's not in the resulting toybox binary". Is a
> build
> break better? A #warning output?
>

(+1 to build break. your practice of "you don't have this constant from the
future? here you go then..." [and then a possible runtime failure because
you really are on too old a kernel] is also fine. but see later for my
problem...)


> It's always the aesthetic issues that are particularly terrible, because
> they
> don't have a definitive correct answer.
>
> > (i also don't understand why `make oldconfig` hassled me about
> > TOYBOX_FORCE_NOMMU until i added it to the .config files --- it doesn't
> look
> > like that actually does anything in practice?)
>
> scripts/genconfig.sh:
>
>   # nommu support
>   probesymbol TOYBOX_FORK << EOF
>     #include <unistd.h>
>     int main(int argc, char *argv[]) { return fork(); }
> EOF
>   echo -e '\tdepends on !TOYBOX_FORCE_NOMMU'
>
> I.E. in generated/Config.probed:
>
> config TOYBOX_FORK
>         bool
>         default y
>
>         depends on !TOYBOX_FORCE_NOMMU
>
> It disables TOYBOX_FORK even if the compile-time probe succeeded.
>
> (You can't fork() on nommu because there's no address translation, so if
> you
> make a copy all the pointers in your new copy of the memory would point
> into the
> old process's memory. Adjusting them is an AI-complete problem, equivalent
> to
> doing reliable garbage collection in C. You _must_ vfork() and then exec a
> new
> process setting up new memory segments and populating them with your own
> pointers. In uclibc fork() wasn't there on nommu targets, but musl
> provides a
> broken fork() that always returns -ENOSYS,


this doesn't apply to fork(), but the trouble with ENOSYS on Android is
that you have to ask yourself "was there a hole punched in the seccomp()
filter for this syscall?". this is my concern with copy_file_range(), for
example. although that's in really old kernels at this point, you can't
call it on Android without getting SIGSYS courtesy of seccomp until <checks
notes> next year in Android U. which is "fine" in a sense --- AOSP is "U"
at this point, so this version of toybox is being built with a version of
the OS that has a seccomp filter that will allow you to probe for
copy_file_range(). but it's "bad" in that it means that you can't run even
a static toybox binary from U on pre-U (at least not for a codepath that
actually uses copy_file_range()).

my initial plan (and the reason i don't think i even mentioned this when
the recent copy_file_range() changes went in) was to just keep it disabled
for the next 7 years... but that's trickier if it's `#ifdef __NR_foo`ed
like copy_file_range() now is, rather than a regular "config" item.
obviously we can just add ` && !defined(__ANDROID__)` and a comment
explaining why, but i'm tempted to wait until someone actually notices and
complains? i'm _trying_ to persuade people to use a static toybox for
hermetic testing on old android releases, but i'm not sure i've actually
succeeded yet... ugh, no, i don't want to leave a known roadblock for those
folks (_especially_ because i keep telling people to drive down that
road!), so i'll send you a patch.

(before you complain, no, i wasn't a huge fan of the whole seccomp thing
either, but if nothing else it's prevented kernels from shipping with
"random" syscalls occupying the next few syscall numbers. which was a thing
that happened: given a sufficiently large ecosystem, for anything you can
imagine, someone's done it.)

anyway, looking at the full list:

* TOYBOX_CONTAINER seems years out of date, at least as a "does this
compile?" probe? everyone should have setns()/unshare() by now?

* TOYBOX_FIFREEZE seems years out of date? everyone should have that
constant by now, and your usual way of doing that today would be to
manually #define it anyway.

* TOYBOX_UTMPX can be __has_include(<utmpx.h>)?

* TOYBOX_SHADOW can be __has_include(<shadow.h>)?

* TOYBOX_ANDROID_SCHEDPOLICY isn't used?

* TOYBOX_PRLIMIT is uclibc only? does uclibc have a __UCLIBC__ or something?

* TOYBOX_GETRANDOM can be __has_include(<sys/random.h>)?

* TOYBOX_HASTIMERS is actually a workaround for a gcc issue, and should
probably test the gcc version macros instead?

i won't send a patch/patches in case you're already doing stuff in there,
but let me know if you'd like any/all of those...


> which makes it hard to detect "I'm
> running on nommu and need to do nommu things" at compile time. As with
> refusing
> to have an #ifdef __MUSL__ this is basically a religious conviction on
> Rich's
> part and we've all given up trying to convince him he's wrong after YEARS
> of
> trying. I run "sed" against the musl source in the toolchains I build to
> fix it,
> but if you're using somebody else's musl toolchain on a nommu system you
> need an
> manual indicator....)
>
> Rob
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20220613/17616d81/attachment-0001.htm>