fix(tuxcloud): run install step via script, not inline POSIX shell - #2
Merged
Conversation
The "Enabling rclone tuxcloud mount" step failed on ./install with a bare
"Command [...] failed", despite the mount itself being healthy.
Cause: dotbot's shell plugin runs commands with
`executable = os.environ.get("SHELL")` (dotbot/src/dotbot/util/common.py).
.tmux.conf sets tmux default-shell to /bin/fish, so a pane exports
SHELL=/bin/fish, and the inline `if ... then ... fi` was handed to fish,
which does not accept POSIX if-syntax. Exit 127.
It passed every check before merge because those ran from a bash context
where SHELL=/bin/bash. Reproduced directly:
SHELL=/bin/bash exit = 0
SHELL=/bin/fish exit = 127
Moving the logic into a script with a `#!/bin/bash` shebang pins the
interpreter regardless of the caller's shell. The Nautilus bookmark step is
folded in as well - it happened to be fish-compatible, but only by luck.
Also stops swallowing the enable failure with `|| true`. The server is
LAN-only, so `enable --now` genuinely fails off-LAN; that now prints a
warning and leaves the unit enabled for the next boot, rather than
reporting success.
Verified dotbot-style under SHELL=bash, fish, and sh, twice each: exit 0,
no duplicate bookmark, mount still serving 43 entries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the
Command [...] failedon the tuxcloud step in #1.Cause
Dotbot runs shell commands through whatever
$SHELLsays:.tmux.conf:2setsdefault-shell /bin/fish, so a tmux pane exportsSHELL=/bin/fish. The inlineif ... then ... fiwas handed to fish, which does not accept POSIX if-syntax:Reproduced with the exact string parsed from
install.conf.yaml, through dotbot's ownshell_command:This passed pre-merge verification because those checks ran from a bash context. My mistake: I validated the step by hand instead of through the shell
./installactually uses.Fix
Logic moves to
scripts/install-tuxcloud-mount.shwith a#!/bin/bashshebang, which pins the interpreter no matter what the caller's shell is.install.conf.yamljust invokes it, with a comment recording why.The Nautilus bookmark step is folded into the same script. It happened to be fish-compatible, but only by luck.
Also drops the
|| truethat was swallowing theenable --nowresult. The server is LAN-only, so that genuinely fails off-LAN; it now prints a warning and leaves the unit enabled for next boot instead of reporting success.Verification
Run dotbot-style under each shell, twice, for idempotency:
Related, not fixed here
The other
./installfailure (voyager-cmdline-tweaks.sh) is a separate, pre-existing bug: dotbot's shell plugin ignoresif:. Only the link plugin implements it (link.py:51,56);shell.pyreads justcommand/description/stdin/stdout/stderr/quiet. So every host-gated shell command runs on every host. All eight voyager system files are currently installed on nighthawk, andvoyager-power-state.serviceis enabled on the desktop. Worth its own PR - needs a decision on approach.