From 6ef5c971ca9cf6ebd429ddb40e030ed3fe0d09af Mon Sep 17 00:00:00 2001 From: Gavin Date: Sat, 23 May 2026 17:37:38 -0400 Subject: [PATCH] fix: improve Pieces OS installer architecture detection and diagnostics --- src/pieces_os_client/wrapper/installation.py | 232 ++++++++++++++--- tests/test_installation.py | 250 +++++++++++++++++++ 2 files changed, 448 insertions(+), 34 deletions(-) create mode 100644 tests/test_installation.py diff --git a/src/pieces_os_client/wrapper/installation.py b/src/pieces_os_client/wrapper/installation.py index 6fa9afc8..d10304ba 100644 --- a/src/pieces_os_client/wrapper/installation.py +++ b/src/pieces_os_client/wrapper/installation.py @@ -1,5 +1,6 @@ import sys import subprocess +import platform from enum import Enum from tempfile import gettempdir import re @@ -26,14 +27,36 @@ class TerminalEventType(Enum): ERROR = 'ERROR' class DownloadModel: - def __init__(self, state: DownloadState, terminal_event: TerminalEventType , bytes_received: int = 0, total_bytes: int=0, percent: float=0): + def __init__( + self, + state: DownloadState, + terminal_event: TerminalEventType, + bytes_received: int = 0, + total_bytes: int = 0, + percent: float = 0, + message: Optional[str] = None, + error: Optional[str] = None, + error_code: Optional[str] = None, + command: Optional[List[str]] = None, + return_code: Optional[int] = None, + url: Optional[str] = None, + ): self.bytes_received = bytes_received self.total_bytes = total_bytes self.percent = percent self.state = state self.terminal_event = terminal_event + self.message = message + self.error = error + self.error_code = error_code + self.command = command + self.return_code = return_code + self.url = url class PosInstaller: + MACOS_INTEL_PACKAGE_SLUG = 'pkg-pos-launch-only' + MACOS_ARM_PACKAGE_SLUG = 'pkg-pos-launch-only-arm64' + def __init__(self, callback: Optional[Callable[[DownloadModel], None]], product: str): self.platform = self.detect_platform() self.download_process = None @@ -45,15 +68,57 @@ def __init__(self, callback: Optional[Callable[[DownloadModel], None]], product: self.product = product - def update_progress(self, bytes_received: int = 0, total_bytes: int = 0): + def update_progress( + self, + bytes_received: int = 0, + total_bytes: int = 0, + message: Optional[str] = None, + error: Optional[str] = None, + error_code: Optional[str] = None, + command: Optional[List[str]] = None, + return_code: Optional[int] = None, + url: Optional[str] = None, + ): if self.progress_update_callback: if total_bytes == 0: percent = 0 else: percent = (bytes_received/total_bytes)*100 - progress = DownloadModel(self.state, self.terminal_event, bytes_received, total_bytes, percent) + progress = DownloadModel( + self.state, + self.terminal_event, + bytes_received, + total_bytes, + percent, + message=message, + error=error, + error_code=error_code, + command=command, + return_code=return_code, + url=url, + ) self.progress_update_callback(progress) + def _emit_failure( + self, + error_code: str, + message: str, + error: Optional[str] = None, + command: Optional[List[str]] = None, + return_code: Optional[int] = None, + url: Optional[str] = None, + ) -> None: + self.state = DownloadState.FAILED + self.terminal_event = TerminalEventType.ERROR + self.update_progress( + message=message, + error=error or message, + error_code=error_code, + command=command, + return_code=return_code, + url=url, + ) + @staticmethod def detect_platform() -> PlatformEnum: if sys.platform == 'win32': @@ -82,9 +147,11 @@ def _start_download(self): elif self.platform == PlatformEnum.Macos: self.download_macos() except Exception as e: - print(f"Error: {e}") - self.state = DownloadState.FAILED - self.update_progress() + self._emit_failure( + 'INSTALLER_UNHANDLED_ERROR', + f'Unexpected installer error: {e}', + error=str(e), + ) def download_linux(self): self.print('Starting POS download for Linux.') @@ -98,21 +165,63 @@ def download_linux(self): exit 1 fi ''' - self.execute_command('bash', '-c', [command], self.extract_linux_regex) + return self.execute_command('bash', '-c', [command], self.extract_linux_regex) def download_macos(self): self.print('Starting POS download for Macos.') - arch = 'arm64' if sys.maxsize > 2**32 else 'x86_64' - pkg_url = f'https://builds.pieces.app/stages/production/macos_packaging/pkg-pos-launch-only-{arch}/download?product={self.product}&download=true' + try: + package_slug = self._resolve_macos_package_slug() + except ValueError as e: + self._emit_failure( + 'MACOS_UNSUPPORTED_ARCHITECTURE', + str(e), + error=str(e), + ) + return False + + pkg_url = f'https://builds.pieces.app/stages/production/macos_packaging/{package_slug}/download?product={self.product}&download=true' tmp_pkg_path = "/tmp/Pieces-OS-Launch.pkg" - self.install_using_web(pkg_url, tmp_pkg_path) + return self.install_using_web(pkg_url, tmp_pkg_path) def download_windows(self): self.print('Starting POS download for Windows.') pkg_url = f'https://builds.pieces.app/stages/production/os_server/windows-exe/download?download=true&product={self.product}' tmp_pkg_path = f"{gettempdir()}\\Pieces-OS.exe" - self.install_using_web(pkg_url, tmp_pkg_path) + return self.install_using_web(pkg_url, tmp_pkg_path) + + @staticmethod + def _detect_macos_cpu_architecture() -> str: + return platform.machine().lower() + + @staticmethod + def _is_apple_silicon_hardware() -> bool: + if sys.platform != 'darwin': + return False + + try: + output = subprocess.check_output( + ['/usr/sbin/sysctl', '-n', 'hw.optional.arm64'], + stderr=subprocess.DEVNULL, + timeout=2, + ) + return output.decode('utf-8').strip() == '1' + except (subprocess.SubprocessError, OSError, UnicodeDecodeError): + return False + + @classmethod + def _resolve_macos_package_slug(cls, machine: Optional[str] = None) -> str: + architecture = (machine or cls._detect_macos_cpu_architecture()).lower().replace('-', '_') + + if architecture in ('arm64', 'aarch64'): + return cls.MACOS_ARM_PACKAGE_SLUG + + if architecture in ('x86_64', 'amd64', 'i386', 'i686'): + if architecture in ('x86_64', 'amd64') and cls._is_apple_silicon_hardware(): + return cls.MACOS_ARM_PACKAGE_SLUG + return cls.MACOS_INTEL_PACKAGE_SLUG + + raise ValueError(f'Unsupported macOS CPU architecture: {architecture or "unknown"}') def install_using_web(self, pkg_url: str, tmp_pkg_path: str) -> bool: BUFFER_SIZE = 65536 @@ -149,17 +258,35 @@ def install_using_web(self, pkg_url: str, tmp_pkg_path: str) -> bool: self.update_progress(downloaded_size, file_size) self.print(f'Downloaded {downloaded_size} of {file_size}') - self.state = DownloadState.COMPLETED - self.update_progress() self.print(f'Download completed. Opening {tmp_pkg_path}.') if sys.platform == 'win32': - subprocess.run(['start', tmp_pkg_path], shell=True) + command = ['start', tmp_pkg_path] + result = subprocess.run(command, shell=True) else: - subprocess.run(['open', tmp_pkg_path]) + command = ['open', tmp_pkg_path] + result = subprocess.run(command) + + return_code = getattr(result, 'returncode', 0) + if return_code != 0: + self._emit_failure( + 'INSTALLER_LAUNCH_FAILED', + f'Installer launch command failed with exit code {return_code}.', + command=command, + return_code=return_code, + url=pkg_url, + ) + return False + + self.state = DownloadState.COMPLETED + self.update_progress(url=pkg_url, message=f'Download completed. Opened {tmp_pkg_path}.') return True except Exception as e: - self.state = DownloadState.FAILED - self.update_progress() + self._emit_failure( + 'DOWNLOAD_FAILED', + f'Error downloading POS: {e}', + error=str(e), + url=pkg_url, + ) self.print(f'Error downloading POS: {e}') return False @@ -179,43 +306,80 @@ def extract_linux_regex(self, line) -> Optional[Tuple[int, int]]: return bytes_downloaded, total_bytes def execute_command(self, shell: str, command: str, args: List[str], callback: Optional[Callable[[str], Tuple[int, int]]]) -> bool: + command_parts = [shell, command] + args + stderr_lines = [] try: self.print(f'Spawning process: {shell} {command} {args}') self.download_process = subprocess.Popen( - [shell, command] + args, + command_parts, stdout=subprocess.PIPE, stderr=subprocess.PIPE ) - while True: - out = self.download_process.stdout - err = self.download_process.stderr + def decode_line(raw_line) -> str: + if isinstance(raw_line, bytes): + return raw_line.decode('utf-8', errors='replace').strip() + return str(raw_line).strip() - if out: + def read_stdout() -> None: + while True: + raw_line = self.download_process.stdout.readline() + if not raw_line: + break + line = decode_line(raw_line) + if not line: + continue self.state = DownloadState.DOWNLOADING self.terminal_event = TerminalEventType.OUTPUT try: - bytes_received, total_bytes = callback(out.readline().decode('utf-8')) + bytes_received, total_bytes = callback(line) if callback else (0, 0) self.print(f'Downloaded {bytes_received} of {total_bytes}') self.update_progress(bytes_received, total_bytes) except Exception as e: self.print(f"Could not match pattern: {e}", file=sys.stderr) - if err: - self.terminal_event = TerminalEventType.ERROR - self.update_progress(bytes_received=0, total_bytes=0) - self.print(err.readline().decode('utf-8'), file=sys.stderr) - - if self.download_process.poll() is not None: - break + def read_stderr() -> None: + while True: + raw_line = self.download_process.stderr.readline() + if not raw_line: + break + line = decode_line(raw_line) + if not line: + continue + stderr_lines.append(line) + + stdout_thread = threading.Thread(target=read_stdout, daemon=True) + stderr_thread = threading.Thread(target=read_stderr, daemon=True) + stdout_thread.start() + stderr_thread.start() + + return_code = self.download_process.wait() + stdout_thread.join() + stderr_thread.join() + if return_code != 0: + stderr_text = "\n".join(stderr_lines) + message = f'Command failed with exit code {return_code}.' + if stderr_text: + message = f'{message} stderr: {stderr_text}' + self._emit_failure( + 'COMMAND_FAILED', + message, + error=message, + command=command_parts, + return_code=return_code, + ) + return False - self.download_process.wait() self.print('Process completed.') - return self.download_process.returncode == 0 + return True except Exception as e: self.print(f'Error executing command: {e}') - self.state = DownloadState.FAILED - self.update_progress() + self._emit_failure( + 'COMMAND_EXECUTION_FAILED', + f'Error executing command: {e}', + error=str(e), + command=command_parts, + ) return False diff --git a/tests/test_installation.py b/tests/test_installation.py new file mode 100644 index 00000000..33bed03a --- /dev/null +++ b/tests/test_installation.py @@ -0,0 +1,250 @@ +import importlib.util +import io +from pathlib import Path +import subprocess +import sys +import unittest +from unittest.mock import MagicMock, mock_open, patch + +INSTALLATION_PATH = ( + Path(__file__).resolve().parents[1] + / "src" + / "pieces_os_client" + / "wrapper" + / "installation.py" +) + +spec = importlib.util.spec_from_file_location("installation_under_test", INSTALLATION_PATH) +installation = importlib.util.module_from_spec(spec) +sys.modules["installation_under_test"] = installation +spec.loader.exec_module(installation) + +DownloadModel = installation.DownloadModel +DownloadState = installation.DownloadState +PosInstaller = installation.PosInstaller +TerminalEventType = installation.TerminalEventType + + +class TestPosInstallerMacosArchitecture(unittest.TestCase): + def test_arm_cpu_uses_arm_launch_only_slug(self): + self.assertEqual( + PosInstaller._resolve_macos_package_slug("arm64"), + PosInstaller.MACOS_ARM_PACKAGE_SLUG, + ) + self.assertEqual( + PosInstaller._resolve_macos_package_slug("aarch64"), + PosInstaller.MACOS_ARM_PACKAGE_SLUG, + ) + + def test_intel_cpu_uses_intel_launch_only_slug(self): + with patch.object(PosInstaller, "_is_apple_silicon_hardware", return_value=False): + self.assertEqual( + PosInstaller._resolve_macos_package_slug("x86_64"), + PosInstaller.MACOS_INTEL_PACKAGE_SLUG, + ) + self.assertEqual( + PosInstaller._resolve_macos_package_slug("amd64"), + PosInstaller.MACOS_INTEL_PACKAGE_SLUG, + ) + + def test_rosetta_intel_python_on_apple_silicon_uses_arm_slug(self): + with patch.object(installation.sys, "platform", "darwin"): + with patch.object(installation.subprocess, "check_output", return_value=b"1\n"): + self.assertEqual( + PosInstaller._resolve_macos_package_slug("x86_64"), + PosInstaller.MACOS_ARM_PACKAGE_SLUG, + ) + + def test_unknown_macos_architecture_fails_before_download(self): + events = [] + installer = PosInstaller(events.append, "TEST_PRODUCT") + + with patch.object(PosInstaller, "_detect_macos_cpu_architecture", return_value="sparc"): + with patch.object(installer, "install_using_web") as install_using_web: + result = installer.download_macos() + + self.assertFalse(result) + install_using_web.assert_not_called() + self.assertEqual(events[-1].state, DownloadState.FAILED) + self.assertEqual(events[-1].terminal_event, TerminalEventType.ERROR) + self.assertEqual(events[-1].error_code, "MACOS_UNSUPPORTED_ARCHITECTURE") + self.assertIn("sparc", events[-1].error) + + def test_download_macos_uses_resolved_package_slug_and_tmp_path(self): + installer = PosInstaller(None, "TEST_PRODUCT") + + with patch.object( + PosInstaller, + "_resolve_macos_package_slug", + return_value=PosInstaller.MACOS_ARM_PACKAGE_SLUG, + ): + with patch.object(installer, "install_using_web", return_value=True) as install_using_web: + result = installer.download_macos() + + self.assertTrue(result) + pkg_url, tmp_pkg_path = install_using_web.call_args.args + self.assertIn("macos_packaging/pkg-pos-launch-only-arm64/download", pkg_url) + self.assertIn("product=TEST_PRODUCT", pkg_url) + self.assertEqual(tmp_pkg_path, "/tmp/Pieces-OS-Launch.pkg") + + +class TestInstallerDiagnostics(unittest.TestCase): + def test_download_model_keeps_existing_fields_and_adds_optional_diagnostics(self): + model = DownloadModel( + DownloadState.DOWNLOADING, + TerminalEventType.PROMPT, + 5, + 10, + 50, + ) + + self.assertEqual(model.bytes_received, 5) + self.assertEqual(model.total_bytes, 10) + self.assertEqual(model.percent, 50) + self.assertEqual(model.state, DownloadState.DOWNLOADING) + self.assertEqual(model.terminal_event, TerminalEventType.PROMPT) + self.assertIsNone(model.message) + self.assertIsNone(model.error) + self.assertIsNone(model.error_code) + self.assertIsNone(model.command) + self.assertIsNone(model.return_code) + self.assertIsNone(model.url) + + diagnostic = DownloadModel( + DownloadState.FAILED, + TerminalEventType.ERROR, + message="failed", + error="boom", + error_code="DOWNLOAD_FAILED", + command=["open", "/tmp/Pieces-OS-Launch.pkg"], + return_code=1, + url="https://example.invalid/package", + ) + + self.assertEqual(diagnostic.message, "failed") + self.assertEqual(diagnostic.error, "boom") + self.assertEqual(diagnostic.error_code, "DOWNLOAD_FAILED") + self.assertEqual(diagnostic.command, ["open", "/tmp/Pieces-OS-Launch.pkg"]) + self.assertEqual(diagnostic.return_code, 1) + self.assertEqual(diagnostic.url, "https://example.invalid/package") + + def test_urlopen_exception_surfaces_failed_callback_diagnostics(self): + events = [] + installer = PosInstaller(events.append, "TEST_PRODUCT") + + with patch.object(installation.urllib.request, "urlopen", side_effect=RuntimeError("network down")): + result = installer.install_using_web("https://example.invalid/package", "/tmp/package.pkg") + + self.assertFalse(result) + self.assertEqual(events[-1].state, DownloadState.FAILED) + self.assertEqual(events[-1].terminal_event, TerminalEventType.ERROR) + self.assertEqual(events[-1].error_code, "DOWNLOAD_FAILED") + self.assertEqual(events[-1].error, "network down") + self.assertEqual(events[-1].url, "https://example.invalid/package") + + def test_launcher_failure_surfaces_command_return_code_and_url(self): + events = [] + installer = PosInstaller(events.append, "TEST_PRODUCT") + response = MagicMock() + response.info.return_value.get.return_value = "3" + response.read.side_effect = [b"abc", b""] + + with patch.object(installation.urllib.request, "urlopen", return_value=response): + with patch("builtins.open", mock_open()): + with patch.object(installation.sys, "platform", "darwin"): + with patch.object( + installation.subprocess, + "run", + return_value=subprocess.CompletedProcess(["open", "/tmp/package.pkg"], 1), + ): + result = installer.install_using_web( + "https://example.invalid/package", + "/tmp/package.pkg", + ) + + self.assertFalse(result) + self.assertEqual(events[-1].state, DownloadState.FAILED) + self.assertEqual(events[-1].error_code, "INSTALLER_LAUNCH_FAILED") + self.assertEqual(events[-1].command, ["open", "/tmp/package.pkg"]) + self.assertEqual(events[-1].return_code, 1) + self.assertEqual(events[-1].url, "https://example.invalid/package") + + def test_linux_stderr_and_nonzero_exit_are_callback_diagnostics(self): + events = [] + call_order = [] + installer = PosInstaller(events.append, "TEST_PRODUCT") + process = MagicMock() + process.stdout = io.BytesIO(b"") + process.stderr = io.BytesIO(b"pkexec is not available\n") + process.wait.side_effect = lambda: call_order.append("wait") or 1 + installer.progress_update_callback = lambda event: ( + call_order.append("callback"), + events.append(event), + ) + + with patch.object(installation.subprocess, "Popen", return_value=process): + result = installer.execute_command( + "bash", + "-c", + ["echo error >&2; exit 1"], + lambda line: (0, 0), + ) + + self.assertFalse(result) + self.assertEqual(call_order, ["wait", "callback"]) + self.assertEqual(len(events), 1) + self.assertEqual(events[-1].state, DownloadState.FAILED) + self.assertEqual(events[-1].terminal_event, TerminalEventType.ERROR) + self.assertEqual(events[-1].error_code, "COMMAND_FAILED") + self.assertEqual(events[-1].return_code, 1) + self.assertIn("pkexec is not available", events[-1].message) + self.assertIn("pkexec is not available", events[-1].error) + self.assertEqual(events[-1].command, ["bash", "-c", "echo error >&2; exit 1"]) + + def test_execute_command_preserves_stdout_progress_callbacks(self): + events = [] + installer = PosInstaller(events.append, "TEST_PRODUCT") + process = MagicMock() + process.stdout = io.BytesIO(b"download progress\n") + process.stderr = io.BytesIO(b"") + process.wait.return_value = 0 + + with patch.object(installation.subprocess, "Popen", return_value=process): + result = installer.execute_command( + "bash", + "-c", + ["echo download progress"], + lambda line: (25, 100), + ) + + self.assertTrue(result) + self.assertEqual(len(events), 1) + self.assertEqual(events[0].state, DownloadState.DOWNLOADING) + self.assertEqual(events[0].terminal_event, TerminalEventType.OUTPUT) + self.assertEqual(events[0].bytes_received, 25) + self.assertEqual(events[0].total_bytes, 100) + + def test_execute_command_stderr_only_zero_exit_does_not_emit_error_callback(self): + events = [] + installer = PosInstaller(events.append, "TEST_PRODUCT") + process = MagicMock() + process.stdout = io.BytesIO(b"") + process.stderr = io.BytesIO(b"benign warning\n") + process.wait.return_value = 0 + + with patch.object(installation.subprocess, "Popen", return_value=process): + result = installer.execute_command( + "bash", + "-c", + ["echo warning >&2"], + lambda line: (0, 0), + ) + + self.assertTrue(result) + self.assertFalse( + any(event.terminal_event == TerminalEventType.ERROR for event in events) + ) + + +if __name__ == "__main__": + unittest.main()