From d42ea0d8ddbc5afccfe8844eed7df6a0f720ad56 Mon Sep 17 00:00:00 2001 From: Le He Date: Fri, 2 Oct 2026 09:39:43 +0000 Subject: [PATCH 1/5] fix(install): preserve working hooks when downloads fail --- .gitignore | 3 ++ README.md | 6 ++++ install.bash | 43 ++++++++++++++++---------- tests/test_install.py | 70 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 16 deletions(-) create mode 100644 tests/test_install.py diff --git a/.gitignore b/.gitignore index c6bba59..7fa11fd 100644 --- a/.gitignore +++ b/.gitignore @@ -128,3 +128,6 @@ dist .yarn/build-state.yml .yarn/install-state.gz .pnp.* + +# Python installer regression tests +__pycache__/ diff --git a/README.md b/README.md index 3c1f8eb..9a99a28 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,8 @@ Welcome to shelltime! This guide will help you install the necessary tools using ## Quick Install +The installer supports macOS and Linux (including WSL). Failed downloads stop installation and preserve existing shell hooks. + You can install shelltime tools by running the following command in your terminal: ```bash @@ -16,6 +18,10 @@ Once the installation script finishes successfully, you should be able to track Visit [shelltime.xyz](https://shelltime.xyz) for guides and usage documentation. +## Testing + +Run `python3 -m unittest discover -s tests` to check download failures and hook preservation without network requests or changes to your shell configuration. + ## Having Issues? If you encounter any problems during the installation or have any questions, please: diff --git a/install.bash b/install.bash index 5021b7e..5d59936 100644 --- a/install.bash +++ b/install.bash @@ -1,9 +1,15 @@ #!/bin/bash +set -euo pipefail # Determine the OS and architecture OS=$(uname -s) ARCH=$(uname -m) +case "$OS" in + Darwin|Linux) ;; + *) echo "Unsupported OS: $OS. Use macOS, Linux, or WSL." >&2; exit 1 ;; +esac + # Function to check if a command is available command_exists() { command -v "$1" >/dev/null 2>&1 @@ -43,9 +49,8 @@ if [ "$BREW_INSTALLED" = false ]; then CLI_FILE_NAME="https://github.com/shelltime/cli/releases/latest/download/cli_" DAEMON_FILE_NAME="${CLI_FILE_NAME}daemon_" -cd /tmp -curr_time_dir="shelltime_install_$(date +"%Y%m%d_%H%M%S")" -mkdir -p "$curr_time_dir" +curr_time_dir=$(mktemp -d "${TMPDIR:-/tmp}/shelltime-install.XXXXXX") +trap 'rm -rf -- "$curr_time_dir"' EXIT cd "$curr_time_dir" get_download_url() { @@ -70,7 +75,7 @@ get_download_url() { baseUrl="${baseUrl}${OS}" if [[ "$ARCH" == "x86_64" ]]; then downloadUrl="${baseUrl}_x86_64.tar.gz" - elif [[ "$ARCH" == "aarch64" ]]; then + elif [[ "$ARCH" == "aarch64" || "$ARCH" == "arm64" ]]; then downloadUrl="${baseUrl}_arm64.tar.gz" else echo "Unsupported architecture: $ARCH on Linux" @@ -84,7 +89,7 @@ get_download_url() { baseUrl="${baseUrl}Windows" if [[ "$ARCH" == "x86_64" ]]; then downloadUrl="${baseUrl}_x86_64.zip" - elif [[ "$ARCH" == "aarch64" ]]; then + elif [[ "$ARCH" == "aarch64" || "$ARCH" == "arm64" ]]; then downloadUrl="${baseUrl}_arm64.zip" else echo "Unsupported architecture: $ARCH on Windows" @@ -106,7 +111,7 @@ URL=$(get_download_url "$CLI_FILE_NAME") # Download the file FILENAME=$(basename "$URL") -curl -sSLO "$URL" +curl -fSLO --connect-timeout 15 --max-time 300 "$URL" # Check if the download was successful if [ ! -f "$FILENAME" ]; then @@ -141,8 +146,10 @@ fi # Move the binary to the appropriate location if [[ "$OS" == "Darwin" ]] || [[ "$OS" == "Linux" ]]; then + chmod 755 shelltime mv shelltime "$HOME/.shelltime/bin/" if [ -f "shelltime-daemon" ]; then + chmod 755 shelltime-daemon mv shelltime-daemon "$HOME/.shelltime/bin/" else echo "" >&2 @@ -190,10 +197,7 @@ fi # Clean up -cd /tmp -if [ -d "/tmp/$curr_time_dir" ]; then - rm -rf "/tmp/$curr_time_dir" -fi +cd "${TMPDIR:-/tmp}" # HELP WANTED @@ -243,20 +247,25 @@ process_file() { local file="$1" local url="$2" - # Check if the file exists and rename it + # Download successfully before replacing a working hook. + local pending_file + pending_file=$(mktemp "${hooks_path}/${file}.XXXXXX") + if ! curl -fsSL --connect-timeout 15 --max-time 60 "$url" -o "$pending_file"; then + rm -f -- "$pending_file" + echo "Error: Failed to download $file. Existing hook preserved." >&2 + return 1 + fi if [ -f "${hooks_path}/${file}" ]; then mv "${hooks_path}/${file}" "${hooks_path}/${file}.bak" fi - - # Download the new file - curl -sSL "${url}" -o "${hooks_path}/${file}" + mv "$pending_file" "${hooks_path}/${file}" } # Function to add source line to config file if not already present add_source_to_config() { local config_file="$1" local source_file="$2" - local source_line="source ${source_file}" + local source_line="source \"${source_file}\"" if ! grep -qF "${source_line}" "${config_file}"; then echo "${source_line}" >> "${config_file}" @@ -293,7 +302,9 @@ fi # Reinstall daemon if shelltime is available if command_exists shelltime; then - shelltime daemon reinstall > /dev/null 2>&1 + if ! shelltime daemon reinstall > /dev/null 2>&1; then + echo "Warning: Daemon setup failed. Run shelltime doctor after authentication." >&2 + fi fi echo "" diff --git a/tests/test_install.py b/tests/test_install.py new file mode 100644 index 0000000..f94c000 --- /dev/null +++ b/tests/test_install.py @@ -0,0 +1,70 @@ +"""Installer failure paths, using stubs instead of network or home changes.""" + +import os +from pathlib import Path +import subprocess +import tempfile +import unittest + + +SCRIPT = Path(__file__).resolve().parents[1] / "install.bash" + + +class InstallerFailureTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory(prefix="shelltime-installer-test-") + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + binary_dir = self.root / "bin" + binary_dir.mkdir() + stubs = { + "uname": '#!/bin/bash\nif [[ "$1" == -s ]]; then echo Linux; else echo x86_64; fi\n', + "curl": "#!/bin/bash\nexit 22\n", + } + for name, body in stubs.items(): + binary = binary_dir / name + binary.write_text(body) + binary.chmod(0o755) + self.env = os.environ.copy() + self.env["PATH"] = str(binary_dir) + os.pathsep + self.env["PATH"] + self.env["TMPDIR"] = str(self.root) + + def test_archive_failure_exits_and_cleans_temporary_directory(self): + result = subprocess.run( + ["bash", str(SCRIPT)], + env=self.env, + capture_output=True, + text=True, + timeout=10, + ) + self.assertNotEqual(result.returncode, 0) + self.assertEqual(list(self.root.glob("shelltime-install.*")), []) + self.assertNotIn("Installation complete!", result.stdout) + + def test_hook_failure_preserves_working_file(self): + hooks = self.root / "hooks" + hooks.mkdir() + original = hooks / "bash.bash" + original.write_text("working hook") + source = SCRIPT.read_text() + start = source.index("process_file() {") + end = source.index("\n# Function to add source", start) + command = ( + 'set -euo pipefail\nhooks_path="$1"\n' + + source[start:end] + + "\nprocess_file bash.bash https://example.invalid/hook" + ) + result = subprocess.run( + ["bash", "-c", command, "installer-test", str(hooks)], + env=self.env, + capture_output=True, + text=True, + timeout=10, + ) + self.assertNotEqual(result.returncode, 0) + self.assertEqual(original.read_text(), "working hook") + self.assertEqual(list(hooks.glob("bash.bash.*")), []) + + +if __name__ == "__main__": + unittest.main() From 8aea392e167bcbcb3c25f2cec8ebb7fa536d6841 Mon Sep 17 00:00:00 2001 From: Le He Date: Fri, 2 Oct 2026 10:14:00 +0000 Subject: [PATCH 2/5] fix(install): migrate hook sources and recover from partial failures --- .github/workflows/test.yaml | 7 + README.md | 10 +- install.bash | 277 +++++++++++++++++++----------------- tests/test_install.py | 219 +++++++++++++++++++++++----- 4 files changed, 339 insertions(+), 174 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index e690409..707d846 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -21,6 +21,9 @@ jobs: steps: - uses: actions/checkout@v4 + - name: Test installer regressions without network access + run: python3 -m unittest discover -s tests -v + - name: Install Zsh and Fish on Ubuntu if: matrix.os == 'ubuntu-latest' run: | @@ -49,6 +52,10 @@ jobs: bash ./install.bash shell: fish {0} + - name: Test install script in Bash + if: matrix.shell == 'bash' + run: bash ./install.bash + - name: Verify installation run: | if [ "${{ matrix.shell }}" = "zsh" ]; then diff --git a/README.md b/README.md index 9a99a28..4d398fd 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,11 @@ Welcome to shelltime! This guide will help you install the necessary tools using ## Quick Install -The installer supports macOS and Linux (including WSL). Failed downloads stop installation and preserve existing shell hooks. +The installer supports macOS and Linux (including WSL). A failed binary download +stops installation. If a hook download fails, its existing hook and backup are +preserved while the remaining hooks and daemon setup continue. The installer +then exits with an error so you can retry. Upgrades migrate existing source lines +without loading hooks twice. You can install shelltime tools by running the following command in your terminal: @@ -20,7 +24,9 @@ Visit [shelltime.xyz](https://shelltime.xyz) for guides and usage documentation. ## Testing -Run `python3 -m unittest discover -s tests` to check download failures and hook preservation without network requests or changes to your shell configuration. +Run `python3 -m unittest discover -s tests` to check upgrades, download failures, +paths containing spaces, and platform detection without network requests or +changes to your shell configuration. CI runs these tests on Linux and macOS. ## Having Issues? diff --git a/install.bash b/install.bash index 5d59936..d8541d0 100644 --- a/install.bash +++ b/install.bash @@ -1,6 +1,116 @@ #!/bin/bash set -euo pipefail +command_exists() { + command -v "$1" >/dev/null 2>&1 +} + +get_download_url() { + local baseUrl="$1" + local downloadUrl="" + + if [[ "$OS" == "Darwin" ]]; then + baseUrl="${baseUrl}${OS}" + if [[ "$ARCH" == "x86_64" ]]; then + downloadUrl="${baseUrl}_x86_64.zip" + elif [[ "$ARCH" == "arm64" ]]; then + downloadUrl="${baseUrl}_arm64.zip" + else + echo "Unsupported architecture: $ARCH on macOS" + exit 1 + fi + if ! command_exists unzip; then + echo "Error: unzip is not installed." + exit 1 + fi + elif [[ "$OS" == "Linux" ]]; then + baseUrl="${baseUrl}${OS}" + if [[ "$ARCH" == "x86_64" ]]; then + downloadUrl="${baseUrl}_x86_64.tar.gz" + elif [[ "$ARCH" == "aarch64" || "$ARCH" == "arm64" ]]; then + downloadUrl="${baseUrl}_arm64.tar.gz" + else + echo "Unsupported architecture: $ARCH on Linux" + exit 1 + fi + if ! command_exists tar; then + echo "Error: tar is not installed." + exit 1 + fi + else + echo "Unsupported OS: $OS" + exit 1 + fi + + echo "$downloadUrl" +} + +process_file() { + local file="$1" + local url="$2" + local pending_file + pending_file=$(mktemp "${hooks_path}/${file}.XXXXXX") || return 1 + if ! curl -fsSL --connect-timeout 15 --max-time 60 "$url" -o "$pending_file"; then + rm -f -- "$pending_file" + echo "Error: Failed to download $file. Existing hook and backup preserved." >&2 + return 1 + fi + if ! chmod 644 "$pending_file"; then + rm -f -- "$pending_file" + return 1 + fi + if [ -f "${hooks_path}/${file}" ]; then + if ! mv -f -- "${hooks_path}/${file}" "${hooks_path}/${file}.bak"; then + rm -f -- "$pending_file" + return 1 + fi + fi + if ! mv -- "$pending_file" "${hooks_path}/${file}"; then + if [ -f "${hooks_path}/${file}.bak" ]; then + mv -- "${hooks_path}/${file}.bak" "${hooks_path}/${file}" || true + fi + rm -f -- "$pending_file" + return 1 + fi +} + +add_source_to_config() { + local config_file="$1" + local source_file="$2" + local pending_config + pending_config=$(mktemp "${config_file}.XXXXXX") || return 1 + + # Migrate the legacy unquoted spelling and remove previously emitted duplicates. + if ! SHELLTIME_SOURCE_FILE="$source_file" awk ' + BEGIN { + legacy = "source " ENVIRON["SHELLTIME_SOURCE_FILE"] + quoted = "source \"" ENVIRON["SHELLTIME_SOURCE_FILE"] "\"" + } + { + line = $0 + sub(/^[[:space:]]+/, "", line) + sub(/[[:space:]]+$/, "", line) + if (line == legacy || line == quoted) { + if (!found) print quoted + found = 1 + } else { + print + } + } + END { if (!found) print quoted } + ' "$config_file" > "$pending_config"; then + rm -f -- "$pending_config" + return 1 + fi + # Preserve permissions and symlinks on the user's shell configuration. + if ! cat "$pending_config" > "$config_file"; then + rm -f -- "$pending_config" + return 1 + fi + rm -f -- "$pending_config" +} + +install_shelltime() { # Determine the OS and architecture OS=$(uname -s) ARCH=$(uname -m) @@ -10,11 +120,6 @@ case "$OS" in *) echo "Unsupported OS: $OS. Use macOS, Linux, or WSL." >&2; exit 1 ;; esac -# Function to check if a command is available -command_exists() { - command -v "$1" >/dev/null 2>&1 -} - # Flag to track whether Homebrew installation was used BREW_INSTALLED=false @@ -47,71 +152,16 @@ fi if [ "$BREW_INSTALLED" = false ]; then CLI_FILE_NAME="https://github.com/shelltime/cli/releases/latest/download/cli_" -DAEMON_FILE_NAME="${CLI_FILE_NAME}daemon_" curr_time_dir=$(mktemp -d "${TMPDIR:-/tmp}/shelltime-install.XXXXXX") trap 'rm -rf -- "$curr_time_dir"' EXIT cd "$curr_time_dir" -get_download_url() { - local baseUrl="$1" - local downloadUrl="" - - if [[ "$OS" == "Darwin" ]]; then - baseUrl="${baseUrl}${OS}" - if [[ "$ARCH" == "x86_64" ]]; then - downloadUrl="${baseUrl}_x86_64.zip" - elif [[ "$ARCH" == "arm64" ]]; then - downloadUrl="${baseUrl}_arm64.zip" - else - echo "Unsupported architecture: $ARCH on macOS" - exit 1 - fi - if ! command_exists unzip; then - echo "Error: unzip is not installed." - exit 1 - fi - elif [[ "$OS" == "Linux" ]]; then - baseUrl="${baseUrl}${OS}" - if [[ "$ARCH" == "x86_64" ]]; then - downloadUrl="${baseUrl}_x86_64.tar.gz" - elif [[ "$ARCH" == "aarch64" || "$ARCH" == "arm64" ]]; then - downloadUrl="${baseUrl}_arm64.tar.gz" - else - echo "Unsupported architecture: $ARCH on Linux" - exit 1 - fi - if ! command_exists tar; then - echo "Error: tar is not installed." - exit 1 - fi - elif [[ "$OS" == "MINGW64_NT" ]] || [[ "$OS" == "MSYS_NT" ]] || [[ "$OS" == "CYGWIN_NT" ]]; then - baseUrl="${baseUrl}Windows" - if [[ "$ARCH" == "x86_64" ]]; then - downloadUrl="${baseUrl}_x86_64.zip" - elif [[ "$ARCH" == "aarch64" || "$ARCH" == "arm64" ]]; then - downloadUrl="${baseUrl}_arm64.zip" - else - echo "Unsupported architecture: $ARCH on Windows" - exit 1 - fi - if ! command_exists unzip; then - echo "Error: unzip is not installed." - exit 1 - fi - else - echo "Unsupported OS: $OS" - exit 1 - fi - - echo "$downloadUrl" -} - URL=$(get_download_url "$CLI_FILE_NAME") # Download the file FILENAME=$(basename "$URL") -curl -fSLO --connect-timeout 15 --max-time 300 "$URL" +curl -fsSLO --connect-timeout 15 --max-time 300 "$URL" # Check if the download was successful if [ ! -f "$FILENAME" ]; then @@ -137,8 +187,7 @@ fi # Check if $HOME/.shelltime/bin exists, create if not if [ ! -d "$HOME/.shelltime/bin" ]; then - mkdir -p "$HOME/.shelltime/bin" - if [ $? -ne 0 ]; then + if ! mkdir -p "$HOME/.shelltime/bin"; then echo "Error: Failed to create $HOME/.shelltime/bin directory." exit 1 fi @@ -158,8 +207,6 @@ if [[ "$OS" == "Darwin" ]] || [[ "$OS" == "Linux" ]]; then echo " 'shelltime daemon install/reinstall'." >&2 echo "" >&2 fi -# elif [[ "$OS" == "MINGW64_NT" ]] || [[ "$OS" == "MSYS_NT" ]] || [[ "$OS" == "CYGWIN_NT" ]]; then - # mv shelltime /c/Windows/System32/ fi # Add $HOME/.shelltime/bin to user path @@ -199,23 +246,13 @@ fi cd "${TMPDIR:-/tmp}" - -# HELP WANTED -# I don't know where the `/bin` folder in windows. so i don't know where should the binaries be installed. -# if you know, please let me know. - -if [[ "$OS" == "MINGW64_NT" ]] || [[ "$OS" == "MSYS_NT" ]] || [[ "$OS" == "CYGWIN_NT" ]]; then - echo "Note: Please move /tmp/shelltime to your bin folder manually." - echo "If you know where binaries should be installed on Windows, please open an issue: https://github.com/shelltime/cli" -fi - fi # end of manual installation block # Check if $HOME/.shelltime/daemon exists, create if not if [ ! -d "$HOME/.shelltime/daemon" ]; then - mkdir -p "$HOME/.shelltime/daemon" - if [ $? -ne 0 ]; then - echo "Warning: Failed to create $HOME/.shelltime/daemon directory. Daemon functionality may be unavailable." + if ! mkdir -p "$HOME/.shelltime/daemon"; then + echo "Warning: Failed to create $HOME/.shelltime/daemon directory. Daemon functionality may be unavailable." >&2 + return 1 fi fi @@ -227,76 +264,40 @@ hooks_path="$HOME/.shelltime/hooks" # Check if the directory exists if [ ! -d "$hooks_path" ]; then - mkdir -p "$hooks_path" - if [ $? -ne 0 ]; then - echo "Warning: Failed to create $hooks_path directory. Shell hooks may be unavailable." - fi -fi - - -# Function to check and delete .bak files -check_and_delete_bak() { - local file="$1" - if [ -f "${hooks_path}/${file}.bak" ]; then - rm "${hooks_path}/${file}.bak" - fi -} - -# Function to check, rename, and download files -process_file() { - local file="$1" - local url="$2" - - # Download successfully before replacing a working hook. - local pending_file - pending_file=$(mktemp "${hooks_path}/${file}.XXXXXX") - if ! curl -fsSL --connect-timeout 15 --max-time 60 "$url" -o "$pending_file"; then - rm -f -- "$pending_file" - echo "Error: Failed to download $file. Existing hook preserved." >&2 + if ! mkdir -p "$hooks_path"; then + echo "Warning: Failed to create $hooks_path directory. Shell hooks may be unavailable." >&2 return 1 fi - if [ -f "${hooks_path}/${file}" ]; then - mv "${hooks_path}/${file}" "${hooks_path}/${file}.bak" - fi - mv "$pending_file" "${hooks_path}/${file}" -} - -# Function to add source line to config file if not already present -add_source_to_config() { - local config_file="$1" - local source_file="$2" - local source_line="source \"${source_file}\"" - - if ! grep -qF "${source_line}" "${config_file}"; then - echo "${source_line}" >> "${config_file}" - fi -} - -# Ensure hooks_path exists -mkdir -p "$hooks_path" +fi -# Check and delete .bak files -check_and_delete_bak "zsh.zsh" -check_and_delete_bak "fish.fish" +installation_failed=false # Process zsh.zsh -process_file "zsh.zsh" "https://raw.githubusercontent.com/shelltime/installation/master/hooks/zsh.zsh" +if ! process_file "zsh.zsh" "https://raw.githubusercontent.com/shelltime/installation/master/hooks/zsh.zsh"; then + installation_failed=true +fi # Process fish.fish -process_file "fish.fish" "https://raw.githubusercontent.com/shelltime/installation/master/hooks/fish.fish" +if ! process_file "fish.fish" "https://raw.githubusercontent.com/shelltime/installation/master/hooks/fish.fish"; then + installation_failed=true +fi # Process bash.bash -process_file "bash-preexec.sh" "https://raw.githubusercontent.com/rcaloras/bash-preexec/master/bash-preexec.sh" -process_file "bash.bash" "https://raw.githubusercontent.com/shelltime/installation/master/hooks/bash.bash" +if ! process_file "bash-preexec.sh" "https://raw.githubusercontent.com/rcaloras/bash-preexec/master/bash-preexec.sh"; then + installation_failed=true +fi +if ! process_file "bash.bash" "https://raw.githubusercontent.com/shelltime/installation/master/hooks/bash.bash"; then + installation_failed=true +fi # Add source lines to config files -if [ -f "$HOME/.zshrc" ]; then +if [ -f "$HOME/.zshrc" ] && [ -f "${hooks_path}/zsh.zsh" ]; then add_source_to_config "$HOME/.zshrc" "${hooks_path}/zsh.zsh" fi -if [ -f "$HOME/.config/fish/config.fish" ]; then +if [ -f "$HOME/.config/fish/config.fish" ] && [ -f "${hooks_path}/fish.fish" ]; then add_source_to_config "$HOME/.config/fish/config.fish" "${hooks_path}/fish.fish" fi -if [ -f "$HOME/.bashrc" ]; then +if [ -f "$HOME/.bashrc" ] && [ -f "${hooks_path}/bash.bash" ] && [ -f "${hooks_path}/bash-preexec.sh" ]; then add_source_to_config "$HOME/.bashrc" "${hooks_path}/bash.bash" fi @@ -308,9 +309,19 @@ if command_exists shelltime; then fi echo "" +if [ "$installation_failed" = true ]; then + echo "Installation incomplete: some hooks could not be updated. Rerun the installer to retry." >&2 + return 1 +fi echo "Installation complete!" echo "" echo "Next steps:" echo " 1. Reload your shell: source ~/.zshrc (or ~/.bashrc / ~/.config/fish/config.fish)" echo " 2. Run: shelltime init" echo "" +} + +# Also run when piped to Bash, where BASH_SOURCE is empty. +if [[ -z "${BASH_SOURCE[0]:-}" || "${BASH_SOURCE[0]}" == "$0" ]]; then + install_shelltime +fi diff --git a/tests/test_install.py b/tests/test_install.py index f94c000..d112843 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -1,8 +1,10 @@ -"""Installer failure paths, using stubs instead of network or home changes.""" +"""Installer regressions using isolated homes, local archives, and network stubs.""" +import io import os from pathlib import Path import subprocess +import tarfile import tempfile import unittest @@ -10,60 +12,199 @@ SCRIPT = Path(__file__).resolve().parents[1] / "install.bash" -class InstallerFailureTests(unittest.TestCase): +class InstallerTests(unittest.TestCase): def setUp(self): self.temp = tempfile.TemporaryDirectory(prefix="shelltime-installer-test-") self.addCleanup(self.temp.cleanup) self.root = Path(self.temp.name) - binary_dir = self.root / "bin" - binary_dir.mkdir() - stubs = { - "uname": '#!/bin/bash\nif [[ "$1" == -s ]]; then echo Linux; else echo x86_64; fi\n', - "curl": "#!/bin/bash\nexit 22\n", - } - for name, body in stubs.items(): - binary = binary_dir / name - binary.write_text(body) - binary.chmod(0o755) + self.binary_dir = self.root / "bin" + self.binary_dir.mkdir() + self.home = self.root / "home with spaces" + self.home.mkdir() + self.hooks = self.home / ".shelltime/hooks" + self.hooks.mkdir(parents=True) self.env = os.environ.copy() - self.env["PATH"] = str(binary_dir) + os.pathsep + self.env["PATH"] - self.env["TMPDIR"] = str(self.root) + self.env.update({ + "HOME": str(self.home), + "PATH": str(self.binary_dir) + os.pathsep + self.env["PATH"], + "TMPDIR": str(self.root), + "STUB_OS": "Linux", + "STUB_ARCH": "x86_64", + "CURL_LOG": str(self.root / "curl.log"), + "DAEMON_LOG": str(self.root / "daemon.log"), + }) + self.stub("uname", 'if [[ "$1" == -s ]]; then echo "$STUB_OS"; else echo "$STUB_ARCH"; fi') + self.stub("curl", "exit 22") + self.stub("fish", "exit 0") + self.stub("shelltime", 'echo "$*" >> "$DAEMON_LOG"') + + def stub(self, name, body): + binary = self.binary_dir / name + binary.write_text("#!/bin/bash\nset -euo pipefail\n" + body + "\n") + binary.chmod(0o755) + + def run_script(self, command=None, *args): + argv = ["bash", str(SCRIPT)] if command is None else [ + "bash", "-c", 'source "$1"; shift; ' + command, + "installer-test", str(SCRIPT), *map(str, args), + ] + return subprocess.run(argv, env=self.env, capture_output=True, text=True, timeout=10) + + def enable_downloads(self, failed_hook=""): + archive = self.root / "release.tar.gz" + with tarfile.open(archive, "w:gz") as bundle: + for name in ("shelltime", "shelltime-daemon"): + body = b"#!/bin/bash\nexit 0\n" + entry = tarfile.TarInfo(name) + entry.size = len(body) + entry.mode = 0o755 + bundle.addfile(entry, io.BytesIO(body)) + self.env["STUB_ARCHIVE"] = str(archive) + self.env["FAILED_HOOK"] = failed_hook + self.stub("curl", ''' +url="${@: -1}" +output="" +while [[ $# -gt 0 ]]; do + case "$1" in + -o) output="$2"; shift 2 ;; + https://*) url="$1"; shift ;; + *) shift ;; + esac +done +echo "$url" >> "$CURL_LOG" +if [[ -z "$output" ]]; then + cp "$STUB_ARCHIVE" "$(basename "$url")" +elif [[ -n "$FAILED_HOOK" && "$url" == */"$FAILED_HOOK" ]]; then + exit 22 +else + echo "# updated hook" > "$output" +fi''') def test_archive_failure_exits_and_cleans_temporary_directory(self): - result = subprocess.run( - ["bash", str(SCRIPT)], - env=self.env, - capture_output=True, - text=True, - timeout=10, - ) + result = self.run_script() self.assertNotEqual(result.returncode, 0) self.assertEqual(list(self.root.glob("shelltime-install.*")), []) self.assertNotIn("Installation complete!", result.stdout) + self.assertEqual(list(self.home.glob(".shelltime/bin/*")), []) + + def test_hook_failure_preserves_working_file_and_backup(self): + original = self.hooks / "bash.bash" + backup = self.hooks / "bash.bash.bak" + original.write_text("working hook") + backup.write_text("previous backup") + result = self.run_script( + 'hooks_path="$1"; process_file bash.bash https://example.invalid/hook', self.hooks, + ) + self.assertNotEqual(result.returncode, 0) + self.assertEqual(original.read_text(), "working hook") + self.assertEqual(backup.read_text(), "previous backup") + self.assertEqual(sorted(p.name for p in self.hooks.iterdir()), ["bash.bash", "bash.bash.bak"]) - def test_hook_failure_preserves_working_file(self): - hooks = self.root / "hooks" - hooks.mkdir() - original = hooks / "bash.bash" + def test_hook_success_updates_backup_and_sets_permissions(self): + self.enable_downloads() + original = self.hooks / "bash.bash" + backup = self.hooks / "bash.bash.bak" original.write_text("working hook") - source = SCRIPT.read_text() - start = source.index("process_file() {") - end = source.index("\n# Function to add source", start) - command = ( - 'set -euo pipefail\nhooks_path="$1"\n' - + source[start:end] - + "\nprocess_file bash.bash https://example.invalid/hook" + backup.write_text("previous backup") + result = self.run_script( + 'hooks_path="$1"; process_file bash.bash https://example.invalid/hook', self.hooks, + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(original.read_text(), "# updated hook\n") + self.assertEqual(backup.read_text(), "working hook") + self.assertEqual(original.stat().st_mode & 0o777, 0o644) + + def test_legacy_and_duplicate_source_lines_are_migrated_once(self): + config = self.home / ".bashrc" + hook = self.hooks / "bash.bash" + hook.write_text('loads=$(( ${loads:-0} + 1 ))\n') + config.write_text(f'# user config\nsource {hook}\nsource "{hook}"\n') + result = self.run_script( + 'add_source_to_config "$1" "$2"; add_source_to_config "$1" "$2"; ' + 'source "$1"; echo "$loads"', config, hook, + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), "1") + self.assertEqual(config.read_text(), f'# user config\nsource "{hook}"\n') + + def test_new_source_line_with_spaces_is_idempotent(self): + config = self.home / ".bashrc" + hook = self.hooks / "bash.bash" + config.write_text("# keep me\n") + hook.write_text('echo "hook loaded"\n') + result = self.run_script( + 'add_source_to_config "$1" "$2"; add_source_to_config "$1" "$2"; source "$1"', + config, hook, ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), "hook loaded") + self.assertEqual(config.read_text(), f'# keep me\nsource "{hook}"\n') + + def test_partial_hook_failure_continues_setup_and_exits_nonzero(self): + self.enable_downloads(failed_hook="fish.fish") + fish = self.hooks / "fish.fish" + fish.write_text("working fish hook") + fish.with_suffix(".fish.bak").write_text("previous fish backup") + (self.home / ".bashrc").touch() + (self.home / ".zshrc").touch() + result = self.run_script() + self.assertNotEqual(result.returncode, 0) + self.assertIn("Installation incomplete", result.stderr) + self.assertNotIn("Installation complete!", result.stdout) + self.assertEqual(fish.read_text(), "working fish hook") + self.assertEqual(fish.with_suffix(".fish.bak").read_text(), "previous fish backup") + for name in ("zsh.zsh", "bash.bash", "bash-preexec.sh"): + self.assertEqual((self.hooks / name).read_text(), "# updated hook\n") + self.assertIn(f'source "{self.hooks}/bash.bash"', (self.home / ".bashrc").read_text()) + self.assertIn(f'source "{self.hooks}/zsh.zsh"', (self.home / ".zshrc").read_text()) + self.assertEqual((self.root / "daemon.log").read_text(), "daemon reinstall\n") + self.assertEqual(list(self.root.glob("shelltime-install.*")), []) + + def test_successful_install_is_repeatable_with_a_spaced_home(self): + self.enable_downloads() + (self.home / ".bashrc").touch() + for _ in range(2): + result = self.run_script() + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("Installation complete!", result.stdout) + config = (self.home / ".bashrc").read_text() + self.assertEqual(config.count(f'source "{self.hooks}/bash.bash"'), 1) + self.assertEqual((self.home / ".shelltime/bin/shelltime").stat().st_mode & 0o777, 0o755) + self.assertEqual(list(self.root.glob("shelltime-install.*")), []) + + def test_linux_arm_architectures_use_arm64_archives(self): + for arch in ("arm64", "aarch64"): + with self.subTest(arch=arch): + result = self.run_script( + 'OS=Linux; ARCH="$1"; get_download_url https://example.invalid/cli_', arch, + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), "https://example.invalid/cli_Linux_arm64.tar.gz") + + def test_unsupported_os_fails_before_downloads(self): + self.env["STUB_OS"] = "FreeBSD" + result = self.run_script() + self.assertNotEqual(result.returncode, 0) + self.assertIn("Unsupported OS: FreeBSD", result.stderr) + self.assertFalse((self.root / "curl.log").exists()) + + def test_piped_install_runs_platform_checks(self): + self.env["STUB_OS"] = "FreeBSD" result = subprocess.run( - ["bash", "-c", command, "installer-test", str(hooks)], - env=self.env, - capture_output=True, - text=True, - timeout=10, + ["bash"], input=SCRIPT.read_text(), env=self.env, + capture_output=True, text=True, timeout=10, ) self.assertNotEqual(result.returncode, 0) - self.assertEqual(original.read_text(), "working hook") - self.assertEqual(list(hooks.glob("bash.bash.*")), []) + self.assertIn("Unsupported OS: FreeBSD", result.stderr) + + def test_failed_new_hook_is_not_sourced(self): + self.enable_downloads(failed_hook="fish.fish") + result = self.run_script() + self.assertNotEqual(result.returncode, 0) + fish_config = self.home / ".config/fish/config.fish" + self.assertNotIn("source ", fish_config.read_text()) + self.assertFalse((self.hooks / "fish.fish").exists()) + self.assertTrue((self.hooks / "bash.bash").exists()) if __name__ == "__main__": From 92081bd42297301224c767e3abb8ff4d1d08f6c7 Mon Sep 17 00:00:00 2001 From: Le He Date: Fri, 2 Oct 2026 10:29:38 +0000 Subject: [PATCH 3/5] fix(install): replace hooks and shell configs atomically and clean interruptions --- CLAUDE.md | 7 +- install.bash | 375 ++++++++++++++++++++++-------------------- tests/test_install.py | 43 ++++- 3 files changed, 248 insertions(+), 177 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 90c88f0..58f7c26 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,7 +16,12 @@ This is the **shelltime installation** repository - a collection of shell script ## Testing -Run the CI workflow locally or on GitHub Actions: +Run the isolated regression suite with Python 3 (also run by CI): +```bash +python3 -m unittest discover -s tests -v +``` + +Run the shell installation workflow locally or on GitHub Actions: ```bash # The test workflow runs on ubuntu-latest and macos-latest # Testing shells: zsh, fish, bash diff --git a/install.bash b/install.bash index d8541d0..f4cdae8 100644 --- a/install.bash +++ b/install.bash @@ -50,6 +50,7 @@ process_file() { local url="$2" local pending_file pending_file=$(mktemp "${hooks_path}/${file}.XXXXXX") || return 1 + installer_temp_files+=("$pending_file") if ! curl -fsSL --connect-timeout 15 --max-time 60 "$url" -o "$pending_file"; then rm -f -- "$pending_file" echo "Error: Failed to download $file. Existing hook and backup preserved." >&2 @@ -60,15 +61,13 @@ process_file() { return 1 fi if [ -f "${hooks_path}/${file}" ]; then - if ! mv -f -- "${hooks_path}/${file}" "${hooks_path}/${file}.bak"; then + # Keep the active hook in place until the final atomic replacement. + if ! cp -p -- "${hooks_path}/${file}" "${hooks_path}/${file}.bak"; then rm -f -- "$pending_file" return 1 fi fi if ! mv -- "$pending_file" "${hooks_path}/${file}"; then - if [ -f "${hooks_path}/${file}.bak" ]; then - mv -- "${hooks_path}/${file}.bak" "${hooks_path}/${file}" || true - fi rm -f -- "$pending_file" return 1 fi @@ -77,8 +76,22 @@ process_file() { add_source_to_config() { local config_file="$1" local source_file="$2" + local target_config="$config_file" + local link_target + while [ -L "$target_config" ]; do + link_target=$(readlink "$target_config") || return 1 + case "$link_target" in + /*) target_config="$link_target" ;; + *) target_config="$(dirname "$target_config")/$link_target" ;; + esac + done local pending_config - pending_config=$(mktemp "${config_file}.XXXXXX") || return 1 + pending_config=$(mktemp "${target_config}.XXXXXX") || return 1 + installer_temp_files+=("$pending_config") + if ! cp -p -- "$target_config" "$pending_config"; then + rm -f -- "$pending_config" + return 1 + fi # Migrate the legacy unquoted spelling and remove previously emitted duplicates. if ! SHELLTIME_SOURCE_FILE="$source_file" awk ' @@ -98,227 +111,239 @@ add_source_to_config() { } } END { if (!found) print quoted } - ' "$config_file" > "$pending_config"; then + ' "$target_config" > "$pending_config"; then rm -f -- "$pending_config" return 1 fi - # Preserve permissions and symlinks on the user's shell configuration. - if ! cat "$pending_config" > "$config_file"; then + # Replace the resolved target atomically, preserving permissions and symlinks. + if ! mv -- "$pending_config" "$target_config"; then rm -f -- "$pending_config" return 1 fi rm -f -- "$pending_config" } +installer_temp_files=() + +cleanup_installer_files() { + local file + for file in "${installer_temp_files[@]:-}"; do + if [ -n "$file" ]; then rm -f -- "$file"; fi + done + if [ -n "${curr_time_dir:-}" ]; then rm -rf -- "$curr_time_dir"; fi +} + install_shelltime() { -# Determine the OS and architecture -OS=$(uname -s) -ARCH=$(uname -m) - -case "$OS" in - Darwin|Linux) ;; - *) echo "Unsupported OS: $OS. Use macOS, Linux, or WSL." >&2; exit 1 ;; -esac - -# Flag to track whether Homebrew installation was used -BREW_INSTALLED=false - -# Check for required commands -if ! command_exists curl; then - echo "Error: curl is not installed." - exit 1 -fi + trap cleanup_installer_files EXIT + trap 'exit 130' INT + trap 'exit 143' TERM + # Determine the OS and architecture + OS=$(uname -s) + ARCH=$(uname -m) + + case "$OS" in + Darwin|Linux) ;; + *) echo "Unsupported OS: $OS. Use macOS, Linux, or WSL." >&2; exit 1 ;; + esac + + # Flag to track whether Homebrew installation was used + BREW_INSTALLED=false + + # Check for required commands + if ! command_exists curl; then + echo "Error: curl is not installed." + exit 1 + fi -# On macOS, prefer Homebrew installation if brew is available -if [[ "$OS" == "Darwin" ]] && command_exists brew; then - echo "Homebrew detected on macOS. Attempting to install via brew..." - if brew install shelltime/tap/shelltime; then - BREW_INSTALLED=true - echo "Successfully installed shelltime via Homebrew." - # Rename old manual-install binaries so the system uses the Homebrew version - if [ -f "$HOME/.shelltime/bin/shelltime" ]; then - mv "$HOME/.shelltime/bin/shelltime" "$HOME/.shelltime/bin/shelltime.bak" - echo "Renamed ~/.shelltime/bin/shelltime to shelltime.bak (now using Homebrew version)" - fi - if [ -f "$HOME/.shelltime/bin/shelltime-daemon" ]; then - mv "$HOME/.shelltime/bin/shelltime-daemon" "$HOME/.shelltime/bin/shelltime-daemon.bak" - echo "Renamed ~/.shelltime/bin/shelltime-daemon to shelltime-daemon.bak (now using Homebrew version)" + # On macOS, prefer Homebrew installation if brew is available + if [[ "$OS" == "Darwin" ]] && command_exists brew; then + echo "Homebrew detected on macOS. Attempting to install via brew..." + if brew install shelltime/tap/shelltime; then + BREW_INSTALLED=true + echo "Successfully installed shelltime via Homebrew." + # Rename old manual-install binaries so the system uses the Homebrew version + if [ -f "$HOME/.shelltime/bin/shelltime" ]; then + mv "$HOME/.shelltime/bin/shelltime" "$HOME/.shelltime/bin/shelltime.bak" + echo "Renamed ~/.shelltime/bin/shelltime to shelltime.bak (now using Homebrew version)" + fi + if [ -f "$HOME/.shelltime/bin/shelltime-daemon" ]; then + mv "$HOME/.shelltime/bin/shelltime-daemon" "$HOME/.shelltime/bin/shelltime-daemon.bak" + echo "Renamed ~/.shelltime/bin/shelltime-daemon to shelltime-daemon.bak (now using Homebrew version)" + fi + else + echo "Homebrew installation failed. Falling back to manual installation..." fi - else - echo "Homebrew installation failed. Falling back to manual installation..." fi -fi - -if [ "$BREW_INSTALLED" = false ]; then -CLI_FILE_NAME="https://github.com/shelltime/cli/releases/latest/download/cli_" + if [ "$BREW_INSTALLED" = false ]; then -curr_time_dir=$(mktemp -d "${TMPDIR:-/tmp}/shelltime-install.XXXXXX") -trap 'rm -rf -- "$curr_time_dir"' EXIT -cd "$curr_time_dir" + CLI_FILE_NAME="https://github.com/shelltime/cli/releases/latest/download/cli_" -URL=$(get_download_url "$CLI_FILE_NAME") + curr_time_dir=$(mktemp -d "${TMPDIR:-/tmp}/shelltime-install.XXXXXX") + cd "$curr_time_dir" -# Download the file -FILENAME=$(basename "$URL") -curl -fsSLO --connect-timeout 15 --max-time 300 "$URL" + URL=$(get_download_url "$CLI_FILE_NAME") -# Check if the download was successful -if [ ! -f "$FILENAME" ]; then - echo "Error: Failed to download $FILENAME" - exit 1 -fi - -# Extract the file -if [[ "$FILENAME" == *.zip ]]; then - unzip "$FILENAME" > /dev/null -elif [[ "$FILENAME" == *.tar.gz ]]; then - tar zxvf "$FILENAME" > /dev/null -else - echo "Unsupported file type: $FILENAME" - exit 1 -fi - -# Check if the shelltime file exists -if [ ! -f "shelltime" ]; then - echo "Error: shelltime binary not found after extraction" - exit 1 -fi + # Download the file + FILENAME=$(basename "$URL") + curl -fsSLO --connect-timeout 15 --max-time 300 "$URL" -# Check if $HOME/.shelltime/bin exists, create if not -if [ ! -d "$HOME/.shelltime/bin" ]; then - if ! mkdir -p "$HOME/.shelltime/bin"; then - echo "Error: Failed to create $HOME/.shelltime/bin directory." + # Check if the download was successful + if [ ! -f "$FILENAME" ]; then + echo "Error: Failed to download $FILENAME" exit 1 fi -fi -# Move the binary to the appropriate location -if [[ "$OS" == "Darwin" ]] || [[ "$OS" == "Linux" ]]; then - chmod 755 shelltime - mv shelltime "$HOME/.shelltime/bin/" - if [ -f "shelltime-daemon" ]; then - chmod 755 shelltime-daemon - mv shelltime-daemon "$HOME/.shelltime/bin/" + # Extract the file + if [[ "$FILENAME" == *.zip ]]; then + unzip "$FILENAME" > /dev/null + elif [[ "$FILENAME" == *.tar.gz ]]; then + tar zxvf "$FILENAME" > /dev/null else - echo "" >&2 - echo "WARNING: shelltime-daemon binary was NOT found in $FILENAME." >&2 - echo " The CLI will attempt to auto-download it on first" >&2 - echo " 'shelltime daemon install/reinstall'." >&2 - echo "" >&2 + echo "Unsupported file type: $FILENAME" + exit 1 fi -fi -# Add $HOME/.shelltime/bin to user path -if [[ "$OS" == "Darwin" ]] || [[ "$OS" == "Linux" ]]; then - # For Zsh - if [ -f "$HOME/.zshrc" ]; then - if ! grep -q '$HOME/.shelltime/bin' "$HOME/.zshrc"; then - echo '# Added by shelltime' >> "$HOME/.zshrc" - echo 'export PATH="$HOME/.shelltime/bin:$PATH"' >> "$HOME/.zshrc" + # Check if the shelltime file exists + if [ ! -f "shelltime" ]; then + echo "Error: shelltime binary not found after extraction" + exit 1 + fi + + # Check if $HOME/.shelltime/bin exists, create if not + if [ ! -d "$HOME/.shelltime/bin" ]; then + if ! mkdir -p "$HOME/.shelltime/bin"; then + echo "Error: Failed to create $HOME/.shelltime/bin directory." + exit 1 fi fi - # For Fish - if command_exists fish; then - if [ ! -d "$HOME/.config/fish" ]; then - mkdir -p "$HOME/.config/fish" + # Move the binary to the appropriate location + if [[ "$OS" == "Darwin" ]] || [[ "$OS" == "Linux" ]]; then + chmod 755 shelltime + mv shelltime "$HOME/.shelltime/bin/" + if [ -f "shelltime-daemon" ]; then + chmod 755 shelltime-daemon + mv shelltime-daemon "$HOME/.shelltime/bin/" + else + echo "" >&2 + echo "WARNING: shelltime-daemon binary was NOT found in $FILENAME." >&2 + echo " The CLI will attempt to auto-download it on first" >&2 + echo " 'shelltime daemon install/reinstall'." >&2 + echo "" >&2 fi - if [ ! -f "$HOME/.config/fish/config.fish" ]; then - touch "$HOME/.config/fish/config.fish" + fi + + # Add $HOME/.shelltime/bin to user path + if [[ "$OS" == "Darwin" ]] || [[ "$OS" == "Linux" ]]; then + # For Zsh + if [ -f "$HOME/.zshrc" ]; then + if ! grep -q '$HOME/.shelltime/bin' "$HOME/.zshrc"; then + echo '# Added by shelltime' >> "$HOME/.zshrc" + echo 'export PATH="$HOME/.shelltime/bin:$PATH"' >> "$HOME/.zshrc" + fi fi - if ! grep -q '$HOME/.shelltime/bin' "$HOME/.config/fish/config.fish"; then - echo '# Added by shelltime' >> "$HOME/.config/fish/config.fish" - echo 'fish_add_path $HOME/.shelltime/bin' >> "$HOME/.config/fish/config.fish" + + # For Fish + if command_exists fish; then + if [ ! -d "$HOME/.config/fish" ]; then + mkdir -p "$HOME/.config/fish" + fi + if [ ! -f "$HOME/.config/fish/config.fish" ]; then + touch "$HOME/.config/fish/config.fish" + fi + if ! grep -q '$HOME/.shelltime/bin' "$HOME/.config/fish/config.fish"; then + echo '# Added by shelltime' >> "$HOME/.config/fish/config.fish" + echo 'fish_add_path $HOME/.shelltime/bin' >> "$HOME/.config/fish/config.fish" + fi fi - fi - # For Bash - if [ -f "$HOME/.bashrc" ]; then - if ! grep -q '$HOME/.shelltime/bin' "$HOME/.bashrc"; then - echo '# Added by shelltime' >> "$HOME/.bashrc" - echo 'export PATH="$HOME/.shelltime/bin:$PATH"' >> "$HOME/.bashrc" + # For Bash + if [ -f "$HOME/.bashrc" ]; then + if ! grep -q '$HOME/.shelltime/bin' "$HOME/.bashrc"; then + echo '# Added by shelltime' >> "$HOME/.bashrc" + echo 'export PATH="$HOME/.shelltime/bin:$PATH"' >> "$HOME/.bashrc" + fi fi fi -fi -# Clean up + # Clean up -cd "${TMPDIR:-/tmp}" + cd "${TMPDIR:-/tmp}" -fi # end of manual installation block + fi # end of manual installation block -# Check if $HOME/.shelltime/daemon exists, create if not -if [ ! -d "$HOME/.shelltime/daemon" ]; then - if ! mkdir -p "$HOME/.shelltime/daemon"; then - echo "Warning: Failed to create $HOME/.shelltime/daemon directory. Daemon functionality may be unavailable." >&2 - return 1 + # Check if $HOME/.shelltime/daemon exists, create if not + if [ ! -d "$HOME/.shelltime/daemon" ]; then + if ! mkdir -p "$HOME/.shelltime/daemon"; then + echo "Error: Failed to create $HOME/.shelltime/daemon directory." >&2 + return 1 + fi fi -fi -# STEP 2 -# insert a preexec and postexec script to user configuration, including `zsh` and `fish` + # STEP 2 + # insert a preexec and postexec script to user configuration, including `zsh` and `fish` -# Define the path -hooks_path="$HOME/.shelltime/hooks" + # Define the path + hooks_path="$HOME/.shelltime/hooks" -# Check if the directory exists -if [ ! -d "$hooks_path" ]; then - if ! mkdir -p "$hooks_path"; then - echo "Warning: Failed to create $hooks_path directory. Shell hooks may be unavailable." >&2 - return 1 + # Check if the directory exists + if [ ! -d "$hooks_path" ]; then + if ! mkdir -p "$hooks_path"; then + echo "Error: Failed to create $hooks_path directory." >&2 + return 1 + fi fi -fi -installation_failed=false + installation_failed=false -# Process zsh.zsh -if ! process_file "zsh.zsh" "https://raw.githubusercontent.com/shelltime/installation/master/hooks/zsh.zsh"; then - installation_failed=true -fi + # Process zsh.zsh + if ! process_file "zsh.zsh" "https://raw.githubusercontent.com/shelltime/installation/master/hooks/zsh.zsh"; then + installation_failed=true + fi -# Process fish.fish -if ! process_file "fish.fish" "https://raw.githubusercontent.com/shelltime/installation/master/hooks/fish.fish"; then - installation_failed=true -fi + # Process fish.fish + if ! process_file "fish.fish" "https://raw.githubusercontent.com/shelltime/installation/master/hooks/fish.fish"; then + installation_failed=true + fi -# Process bash.bash -if ! process_file "bash-preexec.sh" "https://raw.githubusercontent.com/rcaloras/bash-preexec/master/bash-preexec.sh"; then - installation_failed=true -fi -if ! process_file "bash.bash" "https://raw.githubusercontent.com/shelltime/installation/master/hooks/bash.bash"; then - installation_failed=true -fi + # Process bash.bash + if ! process_file "bash-preexec.sh" "https://raw.githubusercontent.com/rcaloras/bash-preexec/master/bash-preexec.sh"; then + installation_failed=true + fi + if ! process_file "bash.bash" "https://raw.githubusercontent.com/shelltime/installation/master/hooks/bash.bash"; then + installation_failed=true + fi -# Add source lines to config files -if [ -f "$HOME/.zshrc" ] && [ -f "${hooks_path}/zsh.zsh" ]; then - add_source_to_config "$HOME/.zshrc" "${hooks_path}/zsh.zsh" -fi -if [ -f "$HOME/.config/fish/config.fish" ] && [ -f "${hooks_path}/fish.fish" ]; then - add_source_to_config "$HOME/.config/fish/config.fish" "${hooks_path}/fish.fish" -fi -if [ -f "$HOME/.bashrc" ] && [ -f "${hooks_path}/bash.bash" ] && [ -f "${hooks_path}/bash-preexec.sh" ]; then - add_source_to_config "$HOME/.bashrc" "${hooks_path}/bash.bash" -fi + # Add source lines to config files + if [ -f "$HOME/.zshrc" ] && [ -f "${hooks_path}/zsh.zsh" ]; then + add_source_to_config "$HOME/.zshrc" "${hooks_path}/zsh.zsh" + fi + if [ -f "$HOME/.config/fish/config.fish" ] && [ -f "${hooks_path}/fish.fish" ]; then + add_source_to_config "$HOME/.config/fish/config.fish" "${hooks_path}/fish.fish" + fi + if [ -f "$HOME/.bashrc" ] && [ -f "${hooks_path}/bash.bash" ] && [ -f "${hooks_path}/bash-preexec.sh" ]; then + add_source_to_config "$HOME/.bashrc" "${hooks_path}/bash.bash" + fi -# Reinstall daemon if shelltime is available -if command_exists shelltime; then - if ! shelltime daemon reinstall > /dev/null 2>&1; then - echo "Warning: Daemon setup failed. Run shelltime doctor after authentication." >&2 + # Reinstall daemon if shelltime is available + if command_exists shelltime; then + if ! shelltime daemon reinstall > /dev/null 2>&1; then + echo "Warning: Daemon setup failed. Run shelltime doctor after authentication." >&2 + fi fi -fi -echo "" -if [ "$installation_failed" = true ]; then - echo "Installation incomplete: some hooks could not be updated. Rerun the installer to retry." >&2 - return 1 -fi -echo "Installation complete!" -echo "" -echo "Next steps:" -echo " 1. Reload your shell: source ~/.zshrc (or ~/.bashrc / ~/.config/fish/config.fish)" -echo " 2. Run: shelltime init" -echo "" + echo "" + if [ "$installation_failed" = true ]; then + echo "Installation incomplete: some hooks could not be updated. Rerun the installer to retry." >&2 + return 1 + fi + echo "Installation complete!" + echo "" + echo "Next steps:" + echo " 1. Reload your shell: source ~/.zshrc (or ~/.bashrc / ~/.config/fish/config.fish)" + echo " 2. Run: shelltime init" + echo "" } # Also run when piped to Bash, where BASH_SOURCE is empty. diff --git a/tests/test_install.py b/tests/test_install.py index d112843..2e6e317 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -121,7 +121,7 @@ def test_legacy_and_duplicate_source_lines_are_migrated_once(self): config.write_text(f'# user config\nsource {hook}\nsource "{hook}"\n') result = self.run_script( 'add_source_to_config "$1" "$2"; add_source_to_config "$1" "$2"; ' - 'source "$1"; echo "$loads"', config, hook, + 'loads=0; source "$1"; echo "$loads"', config, hook, ) self.assertEqual(result.returncode, 0, result.stderr) self.assertEqual(result.stdout.strip(), "1") @@ -206,6 +206,47 @@ def test_failed_new_hook_is_not_sourced(self): self.assertFalse((self.hooks / "fish.fish").exists()) self.assertTrue((self.hooks / "bash.bash").exists()) + def test_config_without_trailing_newline_is_preserved(self): + config = self.home / ".bashrc" + hook = self.hooks / "bash.bash" + config.write_text("# existing config") + result = self.run_script('add_source_to_config "$1" "$2"', config, hook) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(config.read_text(), f'# existing config\nsource "{hook}"\n') + + def test_symlinked_config_target_and_permissions_are_preserved(self): + target = self.home / "shared.bashrc" + target.write_text("# shared config\n") + target.chmod(0o640) + config = self.home / ".bashrc" + config.symlink_to(target.name) + hook = self.hooks / "bash.bash" + result = self.run_script('add_source_to_config "$1" "$2"', config, hook) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertTrue(config.is_symlink()) + self.assertEqual(os.readlink(config), target.name) + self.assertEqual(target.read_text(), f'# shared config\nsource "{hook}"\n') + self.assertEqual(target.stat().st_mode & 0o777, 0o640) + + def test_interrupted_hook_download_cleans_pending_files(self): + self.enable_downloads() + original = self.hooks / "zsh.zsh" + original.write_text("working hook") + curl = self.binary_dir / "curl" + curl.write_text(curl.read_text().replace( + 'echo "# updated hook" > "$output"', 'kill -TERM "$PPID"\n exit 143', + )) + result = self.run_script() + self.assertEqual(result.returncode, 143, result.stderr) + self.assertEqual(original.read_text(), "working hook") + self.assertEqual(list(self.hooks.glob("zsh.zsh.*")), []) + self.assertEqual(list(self.root.glob("shelltime-install.*")), []) + + def test_sourcing_helpers_preserves_the_callers_exit_trap(self): + result = self.run_script('trap "echo preserved" EXIT; source "$1"', SCRIPT) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), "preserved") + if __name__ == "__main__": unittest.main() From 118f1e346f385084a2ad95b6444aa9e95a74cf11 Mon Sep 17 00:00:00 2001 From: Le He Date: Fri, 2 Oct 2026 10:34:35 +0000 Subject: [PATCH 4/5] fix(ci): create Zsh and Fish profile fixtures before installation --- .github/workflows/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 707d846..93c3d65 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -39,7 +39,7 @@ jobs: run: | mkdir -p ~/.config/fish mkdir -p ~/.zsh - touch ~/.bashrc + touch ~/.bashrc ~/.zshrc ~/.config/fish/config.fish - name: Test install script in Zsh if: matrix.shell == 'zsh' From 3d36b451cfb1f03b740856eaf0ce28127f2a8397 Mon Sep 17 00:00:00 2001 From: Le He Date: Fri, 2 Oct 2026 10:36:43 +0000 Subject: [PATCH 5/5] fix(install): preserve backups with atomic copy replacement --- install.bash | 9 ++++++++- tests/test_install.py | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/install.bash b/install.bash index f4cdae8..48b382d 100644 --- a/install.bash +++ b/install.bash @@ -62,9 +62,16 @@ process_file() { fi if [ -f "${hooks_path}/${file}" ]; then # Keep the active hook in place until the final atomic replacement. - if ! cp -p -- "${hooks_path}/${file}" "${hooks_path}/${file}.bak"; then + local pending_backup + pending_backup=$(mktemp "${hooks_path}/${file}.bak.XXXXXX") || { rm -f -- "$pending_file" return 1 + } + installer_temp_files+=("$pending_backup") + if ! cp -p -- "${hooks_path}/${file}" "$pending_backup" || + ! mv -- "$pending_backup" "${hooks_path}/${file}.bak"; then + rm -f -- "$pending_file" "$pending_backup" + return 1 fi fi if ! mv -- "$pending_file" "${hooks_path}/${file}"; then diff --git a/tests/test_install.py b/tests/test_install.py index 2e6e317..ddea250 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -114,6 +114,21 @@ def test_hook_success_updates_backup_and_sets_permissions(self): self.assertEqual(backup.read_text(), "working hook") self.assertEqual(original.stat().st_mode & 0o777, 0o644) + def test_failed_backup_copy_preserves_existing_hook_and_backup(self): + self.enable_downloads() + original = self.hooks / "bash.bash" + backup = self.hooks / "bash.bash.bak" + original.write_text("working hook") + backup.write_text("previous backup") + self.stub("cp", 'echo "partial copy" > "${@: -1}"; exit 1') + result = self.run_script( + 'hooks_path="$1"; process_file bash.bash https://example.invalid/hook', self.hooks, + ) + self.assertNotEqual(result.returncode, 0) + self.assertEqual(original.read_text(), "working hook") + self.assertEqual(backup.read_text(), "previous backup") + self.assertEqual(sorted(p.name for p in self.hooks.iterdir()), ["bash.bash", "bash.bash.bak"]) + def test_legacy_and_duplicate_source_lines_are_migrated_once(self): config = self.home / ".bashrc" hook = self.hooks / "bash.bash"