Repository navigation
fix(install): reject unsupported Android portable installs - #2245
lorenzozanee wants to merge 1 commit into
Conversation
Signed-off-by: lorenzozanee <wyz0707@proton.me>
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
DeusData
left a comment
There was a problem hiding this comment.
Thank you — this is the right fix and the right size. On Termux the installer currently downloads the portable asset, which then dies with "Bad system call" — it is statically linked glibc, and it makes system calls Android's seccomp filter rejects; stopping before the download, with a message that says why, is exactly what should happen. Detecting on three independent signals (uname -o, TERMUX_VERSION, the $PREFIX path) is thorough, guarding it behind OS = linux keeps every other platform untouched, and the test is nicely built — stubbing curl to exit 99 proves "rejected before download" rather than merely "rejected".
Three small things, and then I would like to merge it:
1. The test will never run as it stands. scripts/test.sh does not glob the tests/ directory — it lists each shell contract test explicitly (bash "$ROOT/tests/test_build_dir_safety.sh" and so on, around lines 200–270), and every one of them is named test_*.sh with underscores. tests/test-install-android-guard.sh is neither listed nor named to the convention, so no lane would ever execute it, and a guard nobody runs protects nothing. Please rename it to tests/test_install_android_guard.sh and add one line for it next to the other contract tests in scripts/test.sh. (It would be the first installer test in that list — a good precedent.)
2. Fixes #1202 → Refs #1202, please. #1202 is much wider than this: it also covers the gaps in building from source under Termux and an ENOENT on the temporary database that is still unexplained. Merging with Fixes would auto-close all of that. This PR resolves the first failure mode in that issue — cleanly — and should say so without closing the rest.
3. One sentence in the error message promises slightly more than we can. "Build from source with scripts/build.sh in Termux" reads as a supported path; the reporter on #1202 got it mostly working, but with open problems, and Termux support is an undecided question here. Something like
error: Android/Termux cannot run the Linux portable binary (static glibc; Android kills it with "Bad system call").
Building from source in Termux may work but is not a supported platform yet - see issue #1202.
tells the user the truth and gives them the thread where the workarounds are collected. Your test greps for Android/Termux, so it keeps passing.
Two things that are ours, not yours:
- The red
test-msanyou saw earlier wasdaemon_bootstrap_fails_fast_when_daemon_dies_at_publication— a daemon test that has nothing to do withinstall.sh. I re-ran it on the identical commit and it passed, which makes it a nondeterministic test on our side; it is recorded as such and will not be held against this PR. lint / lintis currently red onmainitself for an unrelated reason, and a fix is in flight (#2257). If you rebase after that lands you will see a clean run.
A note on process, since you are new here: force-pushing your own PR branch is completely fine, and keep the Signed-off-by on any amended commit. Thanks again — small, careful fixes to the installer are some of the most valuable contributions this project gets, because that script is the first thing every new user runs.
… its verdict daemon_bootstrap_fails_fast_when_daemon_dies_at_publication went red on the MSan lane of three PRs that touch no daemon code at all (DeusData#2245, DeusData#2158, DeusData#2140) with tests/test_daemon_bootstrap.c:1116: ASSERT(daemon_named_cause) and green on a rerun of the identical commit. Cause, attributed 2026-09-20. The test forks a real daemon host whose record publication fails with ENOSPC, and asserts that the client names that cause instead of waiting out its 30 s startup deadline. But before it listens, the host SHA-256s its own executable image, and the test runner is ~450 MB: ~3 s natively, 20-32 s under MSan (48 CI samples, median 25.8 s). The spawn override returned as soon as it had forked, so the client's 30 s clock raced the host's hash. On a slow run the deadline fired first, the reaper SIGKILLed a host that had not failed yet, and the log had no listen_failed line to find. Nothing was wrong with the product: with a 28 s artificial delay the client does learn the recorded ENOSPC cause. The spawn override now reaps the forked host before it returns. The host writes its start-failure record and only then releases its lifetime reservation and exits, so "record on disk, reservation released" is a stable state pinned by construction when the client takes its first look -- and that first look is guaranteed: the client's post-spawn wait loop runs at least once even past the deadline, and its first failure check after a spawn is unthrottled. How long the host takes no longer matters. No production code changes; the assertions are unchanged. A regression to the pre-DeusData#1828 behaviour (a client that ignores the record) still fails the unchanged assertions: its message no longer names the failure, the errno and the path. Proof, one build with temporary env-switched instrumentation that is not part of this commit (a sleep in the forked host before it starts, and a switch back to the old fire-and-forget spawn): spawn forked host delayed daemon_bootstrap suite synchronous no 28 passed old (async) 31 s 27 passed, 1 failed: ASSERT(log_read) synchronous 31 s 28 passed The old spawn fails under the delay one assertion earlier than CI did (log_read, not daemon_named_cause): the injected sleep sits before the host opens its log, whereas in CI the host was killed mid-hash with the log already open. Same mechanism -- the client's deadline beating the host -- and the 2026-09-20 attribution reproduced the CI message itself with a delay placed after the log is opened. The final tree, without the instrumentation: daemon_bootstrap 28 passed, three runs in a row. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
… its verdict daemon_bootstrap_fails_fast_when_daemon_dies_at_publication went red on the MSan lane of three PRs that touch no daemon code at all (DeusData#2245, DeusData#2158, DeusData#2140) with tests/test_daemon_bootstrap.c:1116: ASSERT(daemon_named_cause) and green on a rerun of the identical commit. Cause, attributed 2026-09-20. The test forks a real daemon host whose record publication fails with ENOSPC, and asserts that the client names that cause instead of waiting out its 30 s startup deadline. But before it listens, the host SHA-256s its own executable image, and the test runner is ~450 MB: ~3 s natively, 20-32 s under MSan (48 CI samples, median 25.8 s). The spawn override returned as soon as it had forked, so the client's 30 s clock raced the host's hash. On a slow run the deadline fired first, the reaper SIGKILLed a host that had not failed yet, and the log had no listen_failed line to find. Nothing was wrong with the product: with a 28 s artificial delay the client does learn the recorded ENOSPC cause. The spawn override now reaps the forked host before it returns. The host writes its start-failure record and only then releases its lifetime reservation and exits, so "record on disk, reservation released" is a stable state pinned by construction when the client takes its first look -- and that first look is guaranteed: the client's post-spawn wait loop runs at least once even past the deadline, and its first failure check after a spawn is unthrottled. How long the host takes no longer matters. No production code changes; the assertions are unchanged. A regression to the pre-DeusData#1828 behaviour (a client that ignores the record) still fails the unchanged assertions: its message no longer names the failure, the errno and the path. Proof, one build with temporary env-switched instrumentation that is not part of this commit (a sleep in the forked host before it starts, and a switch back to the old fire-and-forget spawn): spawn forked host delayed daemon_bootstrap suite synchronous no 28 passed old (async) 31 s 27 passed, 1 failed: ASSERT(log_read) synchronous 31 s 28 passed The old spawn fails under the delay one assertion earlier than CI did (log_read, not daemon_named_cause): the injected sleep sits before the host opens its log, whereas in CI the host was killed mid-hash with the log already open. Same mechanism -- the client's deadline beating the host -- and the 2026-09-20 attribution reproduced the CI message itself with a delay placed after the log is opened. The final tree, without the instrumentation: daemon_bootstrap 28 passed, three runs in a row. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
… its verdict daemon_bootstrap_fails_fast_when_daemon_dies_at_publication went red on the MSan lane of three PRs that touch no daemon code at all (DeusData#2245, DeusData#2158, DeusData#2140) with tests/test_daemon_bootstrap.c:1116: ASSERT(daemon_named_cause) and green on a rerun of the identical commit. Cause, attributed 2026-09-20. The test forks a real daemon host whose record publication fails with ENOSPC, and asserts that the client names that cause instead of waiting out its 30 s startup deadline. But before it listens, the host SHA-256s its own executable image, and the test runner is ~450 MB: ~3 s natively, 20-32 s under MSan (48 CI samples, median 25.8 s). The spawn override returned as soon as it had forked, so the client's 30 s clock raced the host's hash. On a slow run the deadline fired first, the reaper SIGKILLed a host that had not failed yet, and the log had no listen_failed line to find. Nothing was wrong with the product: with a 28 s artificial delay the client does learn the recorded ENOSPC cause. The spawn override now reaps the forked host before it returns. The host writes its start-failure record and only then releases its lifetime reservation and exits, so "record on disk, reservation released" is a stable state pinned by construction when the client takes its first look -- and that first look is guaranteed: the client's post-spawn wait loop runs at least once even past the deadline, and its first failure check after a spawn is unthrottled. How long the host takes no longer matters. No production code changes; the assertions are unchanged. A regression to the pre-DeusData#1828 behaviour (a client that ignores the record) still fails the unchanged assertions: its message no longer names the failure, the errno and the path. Proof, one build with temporary env-switched instrumentation that is not part of this commit (a sleep in the forked host before it starts, and a switch back to the old fire-and-forget spawn): spawn forked host delayed daemon_bootstrap suite synchronous no 28 passed old (async) 31 s 27 passed, 1 failed: ASSERT(log_read) synchronous 31 s 28 passed The old spawn fails under the delay one assertion earlier than CI did (log_read, not daemon_named_cause): the injected sleep sits before the host opens its log, whereas in CI the host was killed mid-hash with the log already open. Same mechanism -- the client's deadline beating the host -- and the 2026-09-20 attribution reproduced the CI message itself with a delay placed after the log is opened. The final tree, without the instrumentation: daemon_bootstrap 28 passed, three runs in a row. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
What does PR do?
On Android/Termux,
install.shpreviously selected and downloaded an incompatible Linux portable binary. Detect Android/Termux, report the supported alternatives, and stop before download. Add a regression test that verifies the downloader is not invoked.Fixes #1202
Checklist
git commit -s) — required, CI rejects unsigned commits (DCO, see CONTRIBUTING.md)make -f Makefile.cbm test)make -f Makefile.cbm lint-ci)