Skip to content

Fix install script failing when piped in a shell#9178

Merged
begelundmuller merged 2 commits intomainfrom
begelundmuller/fix-install-script-login-shell
Apr 6, 2026
Merged

Fix install script failing when piped in a shell#9178
begelundmuller merged 2 commits intomainfrom
begelundmuller/fix-install-script-login-shell

Conversation

@begelundmuller
Copy link
Copy Markdown
Contributor

@begelundmuller begelundmuller commented Apr 3, 2026

This PR makes one fix and two improvements to the install script:

  1. Fixes installation when the script is piped into sh
    • The bug was basename failing with an unknown flag error for the PARENT_NAME checks; this happened because the current shell process name has a dash prefix like -zsh when executing a pipeline
    • I fixed it by changing to a /dev/tty check instead of checking the parent process name
  2. Moves the dependency checks to a single function such that all missing dependencies are printed before exiting
    • This ensures all missing dependencies are printed immediately, avoiding multiple rounds of install failures
  3. Adds quotes around variables containing paths (was missing in some cases)

I have verified the script works with cat ./scripts/install.sh | sh in the following settings:

  1. Local shell
  2. Docker container
  3. Claude shell
  4. Claude tool call

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@begelundmuller begelundmuller force-pushed the begelundmuller/fix-install-script-login-shell branch 2 times, most recently from 2c7dcb2 to 2ef16a0 Compare April 3, 2026 10:24
@begelundmuller begelundmuller force-pushed the begelundmuller/fix-install-script-login-shell branch from 2ef16a0 to e639431 Compare April 3, 2026 11:06
@begelundmuller begelundmuller self-assigned this Apr 3, 2026
@begelundmuller begelundmuller requested a review from kaspersjo April 3, 2026 11:18
@begelundmuller begelundmuller changed the title Fix install script failing when piped in a login shell Fix install script failing when piped in a shell Apr 3, 2026
Comment on lines -49 to -51
sha256_verify="shasum --algorithm 256 --ignore-missing --check"
elif [ -x "$(command -v sha256sum)" ]; then
sha256_verify="sha256sum --ignore-missing --check"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Turned this into explicit calls to avoid relying on unquoted expansion

Comment on lines -70 to -72
set -- "--silent" "--show-error"
curl --location --silent --show-error "${BINARY_URL}" --output "rill_${PLATFORM}.zip"
else
set -- "--progress-bar"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: changed to explicit case handling instead of overriding args (which is fragile)

SYFT_URL=https://event.syftdata.com/log
SYFT_ID=clp76quhs0006l908bux79l4v
if [ -z "$RILL_INSTALL_DISABLE_TELEMETRY" ]; then
curl --silent --show-error --header "Authorization: ${SYFT_ID}" --header "Content-Type: application/json" --data "{\"event_name\":\"$1\"}" $SYFT_URL > /dev/null || true >&2
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: the pipe applied to true, but should apply to curl

@kaspersjo
Copy link
Copy Markdown
Contributor

LGTM!

@begelundmuller begelundmuller merged commit ba57736 into main Apr 6, 2026
10 checks passed
@begelundmuller begelundmuller deleted the begelundmuller/fix-install-script-login-shell branch April 6, 2026 08:35
begelundmuller added a commit that referenced this pull request Apr 6, 2026
* Various install script fixes

* Formatting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants