search

landley / toybox

toybox
http://landley.net/toybox
BSD Zero Clause License
2.39k stars 335 forks source link

sh: failure in cmake OS detection script #426

Open enh-google opened 1 year ago

enh-google commented 1 year ago

example from https://android-review.googlesource.com/c/platform/external/toybox/+/2555514?tab=comments ---

if (echo '#ifdef __amd64'; echo IS_64BIT_ARCH; echo '#endif') | \
    # ERROR ON LINE BELOW
    (CCOPTS= $CC_FOR_BUILD -E - 2>/dev/null) | \
    grep IS_64BIT_ARCH >/dev/null
then
    SUN_ARCH="x86_64"
fi

(@chriswailes didn't say what the error was)

chriswailes commented 1 year ago

Invalid syntax: (CCOPTS=

landley commented 1 year ago

Reproduced here.

landley commented 1 year ago

Minimal reproduction sequence is somewhere around $ ./sh -c $'if true | \\\n(true);then echo true;fi' (removing the parentheses or the line continuation works, both together fail)...

Ah, line continuations were leaving the \ when it glued the lines together, which coincidentally works most of the time because escaping most characters is a NOP, but (true) and \(true) are not the same.

Try commit 60afef10dee1

landley commented 1 year ago

Did that fix it for you?

chriswailes commented 1 year ago

It certainly did, but I unfortunately ran into another error further down the build process:

cd "/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/rustc/build/tmp/empty_dir" && "sh" "/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/rustc/build/tmp/tarball/rust-std/x86_64-unknown-linux-gnu/rust-std-1.69.0-dev-x86_64-unknown-linux-gnu/install.sh" "--prefix=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package" "--sysconfdir=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package/etc" "--datadir=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package/share" "--docdir=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package/share/doc/rust" "--bindir=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package/bin" "--libdir=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package/lib" "--mandir=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package/share/man" "--disable-ldconfig"
sh: Unknown option 'prefix=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package' (see "sh --help")

This is being executed through a Rust std::process::Command and the sh symlink to toybox is the only shell on the PATH.

landley commented 1 year ago

Blah, trivial fix. Try commit a07853cd9387

Happy to take bug reports and fix 'em.

I was a going down this whack-a-mole testing route with the toybox build, the toybox test suite, and glibc's /usr/bin/ldd, each of which currently has a different subtle error I need to fix in my todo heap, but got distracted. Also passing the whole current test suite, and I think there's still some things left from Eric Roshan-Eisner's fuzzing list, and there's a pull request here about popping the wrong entry for function lifetime cleanup which conflicts with a fork I have redoing function lifetimes so among other things "set" output can show functions...

So many plates to keep spinning...

chriswailes commented 1 year ago

Thanks for all the help. I'll keep posting the errors as long as you don't mind :-)

cd "/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/rustc/build/tmp/empty_dir" && "sh" "/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/rustc/build/tmp/tarball/rust-std/x86_64-unknown-linux-gnu/rust-std-1.69.0-dev-x86_64-unknown-linux-gnu/install.sh" "--prefix=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package" "--sysconfdir=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package/etc" "--datadir=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package/share" "--docdir=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package/share/doc/rust" "--bindir=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package/bin" "--libdir=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package/lib" "--mandir=/usr/local/google/home/chriswailes/projects/android/aosp-plus-rust/out/package/share/man" "--disable-ldconfig"
set: bad -u
sh: local: No such file or directory
sh: local: No such file or directory
sh: local: No such file or directory

sh: local: No such file or directory
enh-google commented 1 year ago

don't you need to include install.sh?

chriswailes commented 1 year ago

Sorry about that.

install.sh.gz

landley commented 1 year ago

Hadn't implemented "set -u" yet. (Not hard, just various corner cases I wanted to confirm like "$@" isn't considered an error when there are no arguments and so on. Unset vs set to empty...)

First attempt at it in commit 216e4d139826

Looks like I haven't implemented the "local" built-in function yet either. Mostly the same logic as "export" I expect...

landley commented 1 year ago

Sorry for the delay, traveling again. should be back in the states Thursday.

chriswailes commented 1 year ago

The next error (from the same install.sh file above) is:

sh: CFG_VERBOSE: bad substitution
sh: local: No such file or directory
sh: LOGFILE: bad substitution
sh: CFG_VERBOSE: bad substitution
sh: 1: bad substitution
chriswailes commented 1 year ago

Ping on the latest iteration of this issue.

P.S. Thanks for all your help so far.

landley commented 1 year ago

I'm hours away from cutting a toybox release and then I have to do a (remote) talk at https://pretalx.coscup.org/coscup-2023/talk/APY3ST/ but I have this 2/3 done in a branch. I'll try to cycle back to it this weekend. Thanks for the reminder.

landley commented 1 year ago

I've cycled back to this one (although I'm fixing a regression I introduced in the recent recursion change first), but FYI my normal "poke me if I forget" cycle is one week. (As in if I didn't respond in one week from last "oh yeah, I should do that but...", feel free to poke me again.)

landley commented 1 year ago

Um, "${CFG_VERBOSE-}" means use a default value of nothing?

...why? I mean yes, it's a NOP that shouldn't error, but... what were they TRYING to do?

E5ten commented 1 year ago

Um, "${CFG_VERBOSE-}" means use a default value of nothing?

...why? I mean yes, it's a NOP that shouldn't error, but... what were they TRYING to do?

I don't know if the script in question uses set -u, but if it does, then doing ${var-} for a var that is allowed to be unset is often used as a way to avoid erroring in a script that otherwise wants all variables to be set. If it isn't using set -u then yeah I can't explain that decision.

landley commented 1 year ago

Ok, there's a test I should have in the test suite...

landley commented 1 year ago

Cycled back to look at this and ./sh -c 'if [ -n "${CFG_VERBOSE-}" ]; then echo hello; fi' didn't error. (and CFG_VERBOSE=walrus ./sh -c 'if [ -n "${CFG_VERBOSE-}" ]; then echo hello; fi' printed hello), and also I thought I wired up "local" back in commit 3074e65693cf?

What environment does this install.sh expect to run in? Is there an invocation where I can see it error myself?