Skip to content

fix: respect CARGO_HOME in install.sh #326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lawrence3699 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: respect CARGO_HOME in install.sh #326

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ci/install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Options:
--crate NAME Name of the crate to install (default <repository name>)
--tag TAG Tag (version) of the crate to install (default <latest release>)
--target TARGET Install the release compiled for $TARGET (default <`rustc` host>)
--to LOCATION Where to install the binary (default ~/.cargo/bin)
--to LOCATION Where to install the binary (default $CARGO_HOME/bin or ~/.cargo/bin)
EOF
}

Expand Down Expand Up @@ -130,7 +130,7 @@ fi
say_err "Target: $target"

if [ -z $dest ]; then
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

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

dest is optional and may be unset/empty when --to is omitted. In POSIX sh, [ -z $dest ] becomes [ -z ] and typically exits with a "missing argument"/"unary operator expected" error (and with set -e this aborts the script). Quote the variable in the test (and ideally keep it consistently quoted anywhere it’s tested).

Suggested change
if [ -z $dest ]; then
if [ -z "$dest" ]; then

Copilot uses AI. Check for mistakes.
dest="$HOME/.cargo/bin"
dest="${CARGO_HOME:-$HOME/.cargo}/bin"
fi

say_err "Installing to: $dest"
Expand Down
7 changes: 7 additions & 0 deletions tests/snapshots/failure-install-script-defaults-to-cargo-home
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
install.sh: GitHub repository: https://github.com/Byron/dua-cli
install.sh: Crate: dua
install.sh: Tag: v2.29.0
install.sh: Target: does-not-exist
install.sh: Installing to: /tmp/dua-cargo-home/bin
install.sh: Downloading: https://github.com/Byron/dua-cli/releases/download/v2.29.0/dua-v2.29.0-does-not-exist.tar.gz
stub tar failure
7 changes: 7 additions & 0 deletions tests/snapshots/failure-install-script-defaults-to-home-cargo-bin
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
install.sh: GitHub repository: https://github.com/Byron/dua-cli
install.sh: Crate: dua
install.sh: Tag: v2.29.0
install.sh: Target: does-not-exist
install.sh: Installing to: /tmp/dua-install-home/.cargo/bin
install.sh: Downloading: https://github.com/Byron/dua-cli/releases/download/v2.29.0/dua-v2.29.0-does-not-exist.tar.gz
stub tar failure
38 changes: 38 additions & 0 deletions tests/stateless-journey.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,41 @@ WITH_FAILURE=1
}
)
)

(with "the install script"
(sandbox
mkdir fakebin
cat > fakebin/curl <<'EOF'
#!/bin/sh
exit 0
EOF
cat > fakebin/tar <<'EOF'
#!/bin/sh
echo "stub tar failure" >&2
exit 1
EOF
chmod +x fakebin/curl fakebin/tar
rm -rf /tmp/dua-install-home /tmp/dua-cargo-home
it "uses CARGO_HOME for the default install location" && {
WITH_SNAPSHOT="$snapshot/failure-install-script-defaults-to-cargo-home" \
PATH="$PWD/fakebin:$PATH" \
HOME=/tmp/dua-install-home \
CARGO_HOME=/tmp/dua-cargo-home \
expect_run ${WITH_FAILURE} sh "$root/../ci/install.sh" \
Comment on lines +130 to +136
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

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

This test uses fixed global paths under /tmp and rm -rf on them. That can collide under parallel test runs and can delete unrelated data if those paths exist for some other purpose. Prefer using per-sandbox paths (e.g., $PWD/home, $PWD/cargo-home, or a subdir of the sandbox tempdir) and avoid removing global /tmp/... entries.

Copilot uses AI. Check for mistakes.
--git Byron/dua-cli \
--crate dua \
--tag v2.29.0 \
--target does-not-exist
}
it "falls back to HOME/.cargo/bin when CARGO_HOME is unset" && {
WITH_SNAPSHOT="$snapshot/failure-install-script-defaults-to-home-cargo-bin" \
PATH="$PWD/fakebin:$PATH" \
HOME=/tmp/dua-install-home \
Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

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

The “CARGO_HOME is unset” case isn’t forced here: if the runner’s environment already has CARGO_HOME set, it will be inherited by the sh ci/install.sh process and this snapshot will fail. Make the test deterministic by explicitly unsetting CARGO_HOME (or setting it to an empty value, which still exercises the fallback with ${CARGO_HOME:-...}).

Suggested change
HOME=/tmp/dua-install-home \
HOME=/tmp/dua-install-home \
CARGO_HOME= \

Copilot uses AI. Check for mistakes.
expect_run ${WITH_FAILURE} sh "$root/../ci/install.sh" \
--git Byron/dua-cli \
--crate dua \
--tag v2.29.0 \
--target does-not-exist
}
)
)
Loading