fix: rootless AWF install uses $HOME/.local and exports $GITHUB_PATH #41310
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,15 @@ for arg in "${@:2}"; do | |
| esac | ||
| done | ||
|
|
||
| # In rootless mode, install into the user's home directory instead of /usr/local | ||
| # so that standard GitHub-hosted runners (where /usr/local is root-owned) work | ||
| # without requiring any pre-chowning or sudo. | ||
| if [ "$ROOTLESS" = "true" ]; then | ||
| AWF_USER_PREFIX="${HOME}/.local" | ||
| AWF_INSTALL_DIR="${AWF_USER_PREFIX}/bin" | ||
| AWF_LIB_DIR="${AWF_USER_PREFIX}/lib/awf" | ||
| fi | ||
|
|
||
| # maybe_sudo runs a command with sudo unless --rootless was specified. | ||
| # In network-isolation mode, AWF runs rootless so sudo is not available or needed. | ||
| maybe_sudo() { | ||
|
|
@@ -64,21 +73,14 @@ ARCH="$(uname -m)" | |
|
|
||
| echo "Installing awf with checksum verification (version: ${AWF_VERSION}, os: ${OS}, arch: ${ARCH})" | ||
|
|
||
| # Rootless mode preflight: create and verify write access to install directories | ||
| if [ "$ROOTLESS" = "true" ]; then | ||
| if ! { mkdir -p "${AWF_INSTALL_DIR}" && [ -w "${AWF_INSTALL_DIR}" ]; }; then | ||
| echo "ERROR: --rootless could not create a writable install directory at ${AWF_INSTALL_DIR}" >&2 | ||
| exit 1 | ||
| fi | ||
| if ! { mkdir -p "${AWF_LIB_DIR}" && [ -w "${AWF_LIB_DIR}" ]; }; then | ||
| echo "ERROR: --rootless could not create a writable lib directory at ${AWF_LIB_DIR}" >&2 | ||
| exit 1 | ||
| fi | ||
| fi | ||
|
|
@@ -179,7 +181,7 @@ install_bundle() { | |
| # runtime argument forwarding. | ||
| maybe_sudo tee "${AWF_INSTALL_DIR}/${AWF_INSTALL_NAME}" > /dev/null <<WRAPPER | ||
| #!/bin/bash | ||
| exec "${node_bin}" "${AWF_LIB_DIR}/awf-bundle.js" "\$@" | ||
| WRAPPER | ||
|
pelikhan marked this conversation as resolved.
|
||
| maybe_sudo chmod +x "${AWF_INSTALL_DIR}/${AWF_INSTALL_NAME}" | ||
|
|
||
|
|
@@ -258,6 +260,16 @@ else | |
| install_platform_binary | ||
| fi | ||
|
|
||
| # In rootless mode, add the install dir to PATH for subsequent steps. | ||
| # $GITHUB_PATH is the mechanism for persisting PATH additions across steps in GitHub Actions. | ||
| if [ "$ROOTLESS" = "true" ]; then | ||
| if [ -n "${GITHUB_PATH:-}" ]; then | ||
|
Contributor
There was a problem hiding this comment. [/diagnose] When 💡 Consider a fallback hintEmitting a message when outside GHA costs nothing and removes a head-scratcher: if [ -n "${GITHUB_PATH:-}" ]; then
echo "${AWF_INSTALL_DIR}" >> "${GITHUB_PATH}"
else
echo "info: awf installed to ${AWF_INSTALL_DIR} — add it to PATH manually outside GitHub Actions" >&2
fi
Contributor
Author
There was a problem hiding this comment. Fixed in 60f4c4a — added an |
||
| echo "${AWF_INSTALL_DIR}" >> "${GITHUB_PATH}" | ||
| else | ||
| echo "WARNING: --rootless install complete but \$GITHUB_PATH is unset; add ${AWF_INSTALL_DIR} to PATH manually" >&2 | ||
| fi | ||
| fi | ||
|
Comment on lines
+266
to
+271
Contributor
There was a problem hiding this comment. Silent success-with-broken-install when 💡 Suggested fixAdd a warning (or error) when the guard condition is not met: if [ "$ROOTLESS" = "true" ]; then
if [ -n "${GITHUB_PATH:-}" ]; then
echo "${AWF_INSTALL_DIR}" >> "${GITHUB_PATH}"
else
echo "WARNING: --rootless install complete but \$GITHUB_PATH is unset; add ${AWF_INSTALL_DIR} to PATH manually" >&2
fi
fiIf rootless mode outside GitHub Actions is not a supported use case, promote the warning to
Contributor
Author
There was a problem hiding this comment. Fixed in 60f4c4a — the |
||
|
|
||
| # Verify installation by running --version. | ||
| # In legacy (non-rootless) mode, run with sudo (without -E, since we explicitly unset | ||
| # environment variables below). On GPU runners (e.g. aw-gpu-runner-T4), /usr/local/bin | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| #!/usr/bin/env bash | ||
| set +o histexpand | ||
|
|
||
| # Tests for install_awf_binary.sh flag-parsing and variable-override logic. | ||
| # Run: bash install_awf_binary_test.sh | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
|
|
||
| TESTS_PASSED=0 | ||
| TESTS_FAILED=0 | ||
|
Contributor
There was a problem hiding this comment. Nice helper functions — consider also printing a final summary count to make CI failures easier to scan. |
||
|
|
||
| pass() { echo "PASS: $1"; TESTS_PASSED=$((TESTS_PASSED + 1)); } | ||
| fail() { echo "FAIL: $1"; echo " $2"; TESTS_FAILED=$((TESTS_FAILED + 1)); } | ||
|
|
||
| # Inline the flag-parsing and directory-override block from install_awf_binary.sh | ||
| # in a subshell so we can vary the arguments without touching the real filesystem. | ||
| parse_and_override() { | ||
| local args=("$@") | ||
| bash -c ' | ||
| AWF_INSTALL_DIR="/usr/local/bin" | ||
| AWF_LIB_DIR="/usr/local/lib/awf" | ||
| ROOTLESS=false | ||
| for arg in "${@:1}"; do | ||
| case "$arg" in | ||
| --rootless) ROOTLESS=true ;; | ||
| esac | ||
| done | ||
| if [ "$ROOTLESS" = "true" ]; then | ||
| AWF_USER_PREFIX="${HOME}/.local" | ||
| AWF_INSTALL_DIR="${AWF_USER_PREFIX}/bin" | ||
| AWF_LIB_DIR="${AWF_USER_PREFIX}/lib/awf" | ||
| fi | ||
| echo "AWF_INSTALL_DIR=${AWF_INSTALL_DIR}" | ||
| echo "AWF_LIB_DIR=${AWF_LIB_DIR}" | ||
| ' -- "${args[@]}" | ||
| } | ||
|
|
||
| echo "Running install_awf_binary.sh tests..." | ||
| echo | ||
|
|
||
| # Test 1: rootless flag sets AWF_INSTALL_DIR to $HOME/.local/bin | ||
| echo "Test 1: --rootless sets AWF_INSTALL_DIR to \$HOME/.local/bin..." | ||
| result=$(parse_and_override --rootless) | ||
| expected_install_dir="${HOME}/.local/bin" | ||
| if echo "$result" | grep -q "AWF_INSTALL_DIR=${expected_install_dir}"; then | ||
| pass "AWF_INSTALL_DIR is ${expected_install_dir}" | ||
| else | ||
| fail "AWF_INSTALL_DIR was not ${expected_install_dir}" "$result" | ||
| fi | ||
|
|
||
| # Test 2: rootless flag sets AWF_LIB_DIR to $HOME/.local/lib/awf | ||
| echo "Test 2: --rootless sets AWF_LIB_DIR to \$HOME/.local/lib/awf..." | ||
| expected_lib_dir="${HOME}/.local/lib/awf" | ||
| if echo "$result" | grep -q "AWF_LIB_DIR=${expected_lib_dir}"; then | ||
| pass "AWF_LIB_DIR is ${expected_lib_dir}" | ||
| else | ||
| fail "AWF_LIB_DIR was not ${expected_lib_dir}" "$result" | ||
| fi | ||
|
|
||
| # Test 3: without --rootless, AWF_INSTALL_DIR stays /usr/local/bin | ||
| echo "Test 3: without --rootless, AWF_INSTALL_DIR stays /usr/local/bin..." | ||
| result=$(parse_and_override) | ||
| if echo "$result" | grep -q "AWF_INSTALL_DIR=/usr/local/bin"; then | ||
| pass "AWF_INSTALL_DIR is /usr/local/bin" | ||
| else | ||
| fail "AWF_INSTALL_DIR was not /usr/local/bin" "$result" | ||
| fi | ||
|
|
||
| # Test 4: without --rootless, AWF_LIB_DIR stays /usr/local/lib/awf | ||
| echo "Test 4: without --rootless, AWF_LIB_DIR stays /usr/local/lib/awf..." | ||
| if echo "$result" | grep -q "AWF_LIB_DIR=/usr/local/lib/awf"; then | ||
| pass "AWF_LIB_DIR is /usr/local/lib/awf" | ||
| else | ||
| fail "AWF_LIB_DIR was not /usr/local/lib/awf" "$result" | ||
| fi | ||
|
|
||
| # Test 5: GITHUB_PATH export — install dir is written when GITHUB_PATH is set | ||
| echo "Test 5: AWF_INSTALL_DIR is appended to GITHUB_PATH in rootless mode..." | ||
| FAKE_GITHUB_PATH=$(mktemp) | ||
| bash -c ' | ||
|
Contributor
There was a problem hiding this comment. Good coverage of the GITHUB_PATH export path. Using mktemp here is the right call for isolation. |
||
| AWF_INSTALL_DIR="${HOME}/.local/bin" | ||
| ROOTLESS=true | ||
| GITHUB_PATH="'"${FAKE_GITHUB_PATH}"'" | ||
| if [ "$ROOTLESS" = "true" ]; then | ||
| if [ -n "${GITHUB_PATH:-}" ]; then | ||
| echo "${AWF_INSTALL_DIR}" >> "${GITHUB_PATH}" | ||
| else | ||
| echo "WARNING: --rootless install complete but \$GITHUB_PATH is unset; add ${AWF_INSTALL_DIR} to PATH manually" >&2 | ||
| fi | ||
| fi | ||
| ' | ||
| expected_path_entry="${HOME}/.local/bin" | ||
| if grep -qF "${expected_path_entry}" "${FAKE_GITHUB_PATH}"; then | ||
| pass "${expected_path_entry} written to GITHUB_PATH" | ||
| else | ||
| fail "${expected_path_entry} not found in GITHUB_PATH" "$(cat "${FAKE_GITHUB_PATH}")" | ||
| fi | ||
| rm -f "${FAKE_GITHUB_PATH}" | ||
|
|
||
| # Test 6: warning emitted when GITHUB_PATH is unset in rootless mode | ||
| echo "Test 6: warning emitted when GITHUB_PATH is unset in rootless mode..." | ||
| warning_output=$(bash -c ' | ||
| AWF_INSTALL_DIR="${HOME}/.local/bin" | ||
| ROOTLESS=true | ||
| unset GITHUB_PATH | ||
| if [ "$ROOTLESS" = "true" ]; then | ||
| if [ -n "${GITHUB_PATH:-}" ]; then | ||
| echo "${AWF_INSTALL_DIR}" >> "${GITHUB_PATH}" | ||
| else | ||
| echo "WARNING: --rootless install complete but \$GITHUB_PATH is unset; add ${AWF_INSTALL_DIR} to PATH manually" >&2 | ||
| fi | ||
| fi | ||
| ' 2>&1) | ||
| if echo "$warning_output" | grep -q "WARNING"; then | ||
| pass "WARNING emitted when GITHUB_PATH is unset" | ||
| else | ||
| fail "No WARNING when GITHUB_PATH is unset" "$warning_output" | ||
| fi | ||
|
|
||
| echo | ||
| echo "Tests passed: $TESTS_PASSED" | ||
| echo "Tests failed: $TESTS_FAILED" | ||
|
|
||
| if [ "$TESTS_FAILED" -gt 0 ]; then | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "✓ All tests passed!" | ||
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[/tdd] This is the only script in
actions/setup/sh/that fixes a real regression but has no_test.shcounterpart — every neighbouring script (e.g.sanitize_path_test.sh,configure_gh_for_ghe_test.sh) has one.💡 What a minimal test file could cover
A
install_awf_binary_test.shusing the same pattern as siblings could include:AWF_INSTALL_DIRis$HOME/.local/binandAWF_LIB_DIRis$HOME/.local/lib/awf.AWF_INSTALL_DIRstays/usr/local/binwhen--rootlessis not passed.GITHUB_PATH, assert the install dir line is written after a rootless install.These tests would have caught the original regression and will guard against future drift.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
install_awf_binary_test.shin 60f4c4a — covers all three cases suggested: rootless dir override, non-rootless unchanged, GITHUB_PATH export, and the new GITHUB_PATH-unset warning.