From 9b93b2c7b9ab1aed1c9d32ff15b7941be19acbe8 Mon Sep 17 00:00:00 2001 From: nadavy Date: Sun, 21 Jun 2026 16:25:04 +0300 Subject: [PATCH] fix(self-update): use bash for installer and show download progress bar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two QoL improvements to the Unix self-update flow: - Invoke install.sh with bash instead of /bin/sh. On macOS, /bin/sh is bash 3.2 running in POSIX compatibility mode, where the echo builtin does not recognize the -e flag and prints it literally — prefixing every colored output line with `-e`. Using bash (via shutil.which with /bin/bash fallback, raising FileNotFoundError if neither exists) respects the script's shebang and produces clean output. - Replace curl --silent with --progress-bar across all download paths in install.sh (unauthenticated, GitHub API, and direct URL with auth) so users get visual feedback during the binary download. Windows (install.ps1) is unaffected: Invoke-WebRequest renders its own progress bar natively and PowerShell is already invoked explicitly. Co-Authored-By: Claude Sonnet 4.6 --- install.sh | 8 +++---- src/apm_cli/commands/self_update.py | 7 +++++- .../test_commands_config_coverage.py | 2 +- tests/unit/test_update_command.py | 24 +++++++++++++------ 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/install.sh b/install.sh index 9bae948b4..05cfd47a6 100755 --- a/install.sh +++ b/install.sh @@ -435,7 +435,7 @@ trap 'rm -rf "$TMP_DIR"' EXIT echo -e "${YELLOW}Downloading APM...${NC}" # Try downloading without authentication first (for public repos) -if curl -L --fail --silent --show-error "$DOWNLOAD_URL" -o "$TMP_DIR/$DOWNLOAD_BINARY"; then +if curl -L --fail --progress-bar "$DOWNLOAD_URL" -o "$TMP_DIR/$DOWNLOAD_BINARY"; then echo -e "${GREEN}[+] Download successful${NC}" else # If unauthenticated download fails, try with authentication if available. @@ -448,14 +448,14 @@ else # For private repositories, use GitHub API with proper headers if [ -n "$ASSET_URL" ]; then echo -e "${BLUE}Using GitHub API for private repository access...${NC}" - if curl -L --fail --silent --show-error \ + if curl -L --fail --progress-bar \ -H "Authorization: token $AUTH_HEADER_VALUE" \ -H "Accept: application/octet-stream" \ "$ASSET_URL" -o "$TMP_DIR/$DOWNLOAD_BINARY"; then echo -e "${GREEN}[+] Download successful via GitHub API${NC}" else echo -e "${BLUE}GitHub API download failed, trying direct URL with auth...${NC}" - if curl -L --fail --silent --show-error -H "Authorization: token $AUTH_HEADER_VALUE" "$DOWNLOAD_URL" -o "$TMP_DIR/$DOWNLOAD_BINARY"; then + if curl -L --fail --progress-bar -H "Authorization: token $AUTH_HEADER_VALUE" "$DOWNLOAD_URL" -o "$TMP_DIR/$DOWNLOAD_BINARY"; then echo -e "${GREEN}[+] Download successful with authentication${NC}" else echo -e "${RED}Error: Failed to download APM CLI even with authentication${NC}" @@ -476,7 +476,7 @@ else fi else echo -e "${BLUE}No API URL available, trying direct URL with auth...${NC}" - if curl -L --fail --silent --show-error -H "Authorization: token $AUTH_HEADER_VALUE" "$DOWNLOAD_URL" -o "$TMP_DIR/$DOWNLOAD_BINARY"; then + if curl -L --fail --progress-bar -H "Authorization: token $AUTH_HEADER_VALUE" "$DOWNLOAD_URL" -o "$TMP_DIR/$DOWNLOAD_BINARY"; then echo -e "${GREEN}[+] Download successful with authentication${NC}" else if [ -n "$APM_RELEASE_BASE_URL" ]; then diff --git a/src/apm_cli/commands/self_update.py b/src/apm_cli/commands/self_update.py index bb8b3b9df..00286a502 100644 --- a/src/apm_cli/commands/self_update.py +++ b/src/apm_cli/commands/self_update.py @@ -108,7 +108,12 @@ def _get_installer_run_command(script_path: str) -> list[str]: raise FileNotFoundError("PowerShell executable not found in PATH") return [powershell_path, "-ExecutionPolicy", "Bypass", "-File", script_path] - shell_path = "/bin/sh" if os.path.exists("/bin/sh") else "sh" + shell_path = shutil.which("bash") + if not shell_path: + if os.path.exists("/bin/bash"): + shell_path = "/bin/bash" + else: + raise FileNotFoundError("bash executable not found; cannot run installer script") return [shell_path, script_path] diff --git a/tests/integration/test_commands_config_coverage.py b/tests/integration/test_commands_config_coverage.py index cbea904b8..712153dd5 100644 --- a/tests/integration/test_commands_config_coverage.py +++ b/tests/integration/test_commands_config_coverage.py @@ -1462,7 +1462,7 @@ def test_get_installer_run_command_unix(self) -> None: with patch.object(su, "_is_windows_platform", return_value=False): cmd = su._get_installer_run_command("/tmp/install.sh") - assert "sh" in cmd[0] or cmd[0] == "/bin/sh" + assert "bash" in cmd[0] assert "/tmp/install.sh" in cmd def test_get_update_installer_suffix_unix(self) -> None: diff --git a/tests/unit/test_update_command.py b/tests/unit/test_update_command.py index 959febcd5..49f3a0d44 100644 --- a/tests/unit/test_update_command.py +++ b/tests/unit/test_update_command.py @@ -125,7 +125,7 @@ def test_update_uses_shell_installer_on_unix( with ( patch.object(update_module.sys, "platform", "darwin"), - patch("apm_cli.commands.self_update.os.path.exists", return_value=True), + patch.object(update_module.shutil, "which", return_value="/usr/bin/bash"), ): result = self.runner.invoke(cli, ["self-update"]) @@ -135,7 +135,7 @@ def test_update_uses_shell_installer_on_unix( self.assertTrue(mock_get.call_args.args[0].endswith("apm-unix")) mock_run.assert_called_once() run_command = mock_run.call_args.args[0] - self.assertEqual(run_command[0], "/bin/sh") + self.assertEqual(run_command[0], "/usr/bin/bash") self.assertEqual(run_command[1][-3:], ".sh") mock_chmod.assert_called_once() # Regression guard for issue #894: the installer must be spawned with @@ -186,21 +186,31 @@ def test_manual_update_command_unix(self): self.assertIn("aka.ms/apm-unix", command) self.assertIn("curl", command) - def test_installer_run_command_unix_bin_sh_exists(self): + def test_installer_run_command_unix_bash_found(self): + with ( + patch.object(update_module.sys, "platform", "linux"), + patch.object(update_module.shutil, "which", return_value="/usr/bin/bash"), + ): + cmd = update_module._get_installer_run_command("/tmp/install.sh") + self.assertEqual(cmd, ["/usr/bin/bash", "/tmp/install.sh"]) + + def test_installer_run_command_unix_fallback_to_bin_bash(self): with ( patch.object(update_module.sys, "platform", "linux"), + patch.object(update_module.shutil, "which", return_value=None), patch.object(update_module.os.path, "exists", return_value=True), ): cmd = update_module._get_installer_run_command("/tmp/install.sh") - self.assertEqual(cmd, ["/bin/sh", "/tmp/install.sh"]) + self.assertEqual(cmd, ["/bin/bash", "/tmp/install.sh"]) - def test_installer_run_command_unix_fallback_to_sh(self): + def test_installer_run_command_unix_no_bash_raises(self): with ( patch.object(update_module.sys, "platform", "linux"), + patch.object(update_module.shutil, "which", return_value=None), patch.object(update_module.os.path, "exists", return_value=False), ): - cmd = update_module._get_installer_run_command("/tmp/install.sh") - self.assertEqual(cmd, ["sh", "/tmp/install.sh"]) + with self.assertRaises(FileNotFoundError): + update_module._get_installer_run_command("/tmp/install.sh") def test_installer_run_command_windows_powershell_not_found(self): with (