diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index e9203347a..fc203ceec 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -153,7 +153,7 @@ jobs: AZURE_SIGNTOOL_KEY_VAULT_URL: ${{ secrets.AZURE_SIGNTOOL_KEY_VAULT_URL }} CONSTRUCTOR_EXAMPLES_KEEP_ARTIFACTS: "${{ runner.temp }}/examples_artifacts" CONSTRUCTOR_SIGNTOOL_PATH: "C:/Program Files (x86)/Windows Kits/10/bin/10.0.26100.0/x86/signtool.exe" - CONSTRUCTOR_VERBOSE: 1 + CONSTRUCTOR_VERBOSE: 0 run: | rm -rf coverage.json pytest -vv --cov=constructor --cov-branch tests/test_examples.py diff --git a/constructor/briefcase.py b/constructor/briefcase.py index fa43187c1..bedb2b5be 100644 --- a/constructor/briefcase.py +++ b/constructor/briefcase.py @@ -4,6 +4,7 @@ import functools import logging +import os import re import shutil import sys @@ -249,6 +250,10 @@ class Payload: archive_name: str = "payload.tar.gz" conda_exe_name: str = "_conda.exe" + # There might be other ways we want to enable `add_debug_logging`, but it has proven + # very useful at least for the CI environment. + add_debug_logging: bool = bool(os.environ.get("CI")) and os.environ.get("CI") != "0" + @functools.cached_property def root(self) -> Path: """Create root upon first access and cache it.""" @@ -332,6 +337,8 @@ def render_templates(self) -> list[str: TemplateFile]: context: dict[str, str] = { "archive_name": self.archive_name, "conda_exe_name": self.conda_exe_name, + "add_debug": self.add_debug_logging, + "register_envs": str(self.info.get("register_envs", True)).lower(), } # Render the templates now using jinja and the defined context diff --git a/constructor/briefcase/pre_uninstall.bat b/constructor/briefcase/pre_uninstall.bat index eb0bd983e..900d12a41 100644 --- a/constructor/briefcase/pre_uninstall.bat +++ b/constructor/briefcase/pre_uninstall.bat @@ -1,9 +1,53 @@ -set "INSTDIR=%cd%" +@echo {{ 'on' if add_debug else 'off' }} +setlocal + +{%- macro error_block(message, code) -%} +echo [ERROR] {{ message }} +{%- if add_debug %} +>> "%LOG%" echo [ERROR] {{ message }} +{%- endif %} +exit /b {{ code }} +{%- endmacro -%} + +rem Assign INSTDIR and normalize the path +set "INSTDIR=%~dp0.." +for %%I in ("%INSTDIR%") do set "INSTDIR=%%~fI" + set "BASE_PATH=%INSTDIR%\base" set "PREFIX=%BASE_PATH%" set "CONDA_EXE=%INSTDIR%\{{ conda_exe_name }}" set "PAYLOAD_TAR=%INSTDIR%\{{ archive_name }}" +{%- if add_debug %} +rem Get the name of the install directory +for %%I in ("%INSTDIR%") do set "APPNAME=%%~nxI" +set "LOG=%TEMP%\%APPNAME%-preuninstall.log" + +echo ==== pre_uninstall start ==== >> "%LOG%" +echo SCRIPT=%~f0 >> "%LOG%" +echo CWD=%CD% >> "%LOG%" +echo INSTDIR=%INSTDIR% >> "%LOG%" +echo BASE_PATH=%BASE_PATH% >> "%LOG%" +echo CONDA_EXE=%CONDA_EXE% >> "%LOG%" +echo PAYLOAD_TAR=%PAYLOAD_TAR% >> "%LOG%" +"%CONDA_EXE%" --version >> "%LOG%" 2>&1 +{%- endif %} + +{%- set redir = ' >> "%LOG%" 2>&1' if add_debug else '' %} +{%- set dump_and_exit = 'type "%LOG%" & exit /b %errorlevel%' if add_debug else 'exit /b %errorlevel%' %} + +rem Consistency checks +if not exist "%CONDA_EXE%" ( + {{ error_block('CONDA_EXE not found: "%CONDA_EXE%"', 10) }} +) rem Recreate an empty payload tar. This file was deleted during installation but the rem MSI installer expects it to exist. type nul > "%PAYLOAD_TAR%" +if errorlevel 1 ( + {{ error_block('Failed to create "%PAYLOAD_TAR%"', '%errorlevel%') }} +) + +"%CONDA_EXE%" constructor uninstall --prefix "%BASE_PATH%"{{ redir }} +if errorlevel 1 ( {{ dump_and_exit }} ) + +exit /b 0 diff --git a/constructor/briefcase/run_installation.bat b/constructor/briefcase/run_installation.bat index 2eca7b7f7..85e8bbd4f 100644 --- a/constructor/briefcase/run_installation.bat +++ b/constructor/briefcase/run_installation.bat @@ -1,22 +1,75 @@ -set "INSTDIR=%cd%" +@echo {{ 'on' if add_debug else 'off' }} +setlocal + +{%- macro error_block(message, code) -%} +echo [ERROR] {{ message }} +{%- if add_debug %} +>> "%LOG%" echo [ERROR] {{ message }} +{%- endif %} +exit /b {{ code }} +{%- endmacro -%} + +rem Assign INSTDIR and normalize the path +set "INSTDIR=%~dp0.." +for %%I in ("%INSTDIR%") do set "INSTDIR=%%~fI" + set "BASE_PATH=%INSTDIR%\base" set "PREFIX=%BASE_PATH%" set "CONDA_EXE=%INSTDIR%\{{ conda_exe_name }}" set "PAYLOAD_TAR=%INSTDIR%\{{ archive_name }}" -echo "Unpacking payload..." -"%CONDA_EXE%" constructor extract --prefix "%INSTDIR%" --tar-from-stdin < "%PAYLOAD_TAR%" -"%CONDA_EXE%" constructor --prefix "%BASE_PATH%" --extract-conda-pkgs - +set CONDA_EXTRA_SAFETY_CHECKS=no set CONDA_PROTECT_FROZEN_ENVS=0 -set "CONDA_ROOT_PREFIX=%BASE_PATH%" +set CONDA_REGISTER_ENVS={{ register_envs }} set CONDA_SAFETY_CHECKS=disabled -set CONDA_EXTRA_SAFETY_CHECKS=no +set "CONDA_ROOT_PREFIX=%BASE_PATH%" set "CONDA_PKGS_DIRS=%BASE_PATH%\pkgs" -"%CONDA_EXE%" install --offline --file "%BASE_PATH%\conda-meta\initial-state.explicit.txt" -yp "%BASE_PATH%" +{%- if add_debug %} +rem Get the name of the install directory +for %%I in ("%INSTDIR%") do set "APPNAME=%%~nxI" +set "LOG=%TEMP%\%APPNAME%-postinstall.log" + +echo ==== run_installation start ==== >> "%LOG%" +echo SCRIPT=%~f0 >> "%LOG%" +echo CWD=%CD% >> "%LOG%" +echo INSTDIR=%INSTDIR% >> "%LOG%" +echo BASE_PATH=%BASE_PATH% >> "%LOG%" +echo CONDA_EXE=%CONDA_EXE% >> "%LOG%" +echo PAYLOAD_TAR=%PAYLOAD_TAR% >> "%LOG%" +{%- endif %} + +{%- set redir = ' >> "%LOG%" 2>&1' if add_debug else '' %} +{%- set dump_and_exit = 'type "%LOG%" & exit /b %errorlevel%' if add_debug else 'exit /b %errorlevel%' %} + +rem Consistency checks +if not exist "%CONDA_EXE%" ( + {{ error_block('CONDA_EXE not found: "%CONDA_EXE%"', 10) }} +) +if not exist "%PAYLOAD_TAR%" ( + {{ error_block('PAYLOAD_TAR not found: "%PAYLOAD_TAR%"', 11) }} +) + +echo Unpacking payload... +rem "%CONDA_EXE%" constructor extract --prefix "%INSTDIR%" --tar-from-stdin < "%PAYLOAD_TAR%"{{ redir }} +"%CONDA_EXE%" constructor --prefix "%INSTDIR%" --extract-tarball < "%PAYLOAD_TAR%"{{ redir }} +if errorlevel 1 ( {{ dump_and_exit }} ) + +rem "%CONDA_EXE%" constructor --prefix "%BASE_PATH%" --extract-conda-pkgs{{ redir }} +"%CONDA_EXE%" constructor --prefix "%BASE_PATH%" --extract-conda-pkgs{{ redir }} +if errorlevel 1 ( {{ dump_and_exit }} ) + +if not exist "%BASE_PATH%" ( + {{ error_block('"%BASE_PATH%" not found!', 12) }} +) + +"%CONDA_EXE%" install --offline --file "%BASE_PATH%\conda-meta\initial-state.explicit.txt" -yp "%BASE_PATH%"{{ redir }} +if errorlevel 1 ( {{ dump_and_exit }} ) rem Delete the payload to save disk space. rem A truncated placeholder of 0 bytes is recreated during uninstall rem because MSI expects the file to be there to clean the registry. del "%PAYLOAD_TAR%" +if errorlevel 1 ( {{ dump_and_exit }} ) + +exit /b 0 diff --git a/examples/register_envs/construct.yaml b/examples/register_envs/construct.yaml index 31d72c9f6..760757a05 100644 --- a/examples/register_envs/construct.yaml +++ b/examples/register_envs/construct.yaml @@ -3,7 +3,7 @@ name: RegisterEnvs version: 1.0.0 -installer_type: {{ "exe" if os.name == "nt" else "all" }} +installer_type: all channels: - https://repo.anaconda.com/pkgs/main/ specs: diff --git a/tests/test_briefcase.py b/tests/test_briefcase.py index e57911b43..5143d5f80 100644 --- a/tests/test_briefcase.py +++ b/tests/test_briefcase.py @@ -238,10 +238,12 @@ def test_payload_conda_exe(): @pytest.mark.skipif(sys.platform != "win32", reason="Windows only") -def test_payload_templates_are_rendered(): +@pytest.mark.parametrize("debug_logging", [True, False]) +def test_payload_templates_are_rendered(debug_logging): """Test that templates are rendered when the payload is prepared.""" info = mock_info.copy() payload = Payload(info) + payload.add_debug_logging = debug_logging rendered_templates = payload.render_templates() assert len(rendered_templates) == 2 # There should be at least two files for f in rendered_templates: @@ -250,3 +252,26 @@ def test_payload_templates_are_rendered(): assert "{{" not in text and "}}" not in text assert "{%" not in text and "%}" not in text assert "{#" not in text and "#}" not in text + + +@pytest.mark.skipif(sys.platform != "win32", reason="Windows only") +@pytest.mark.parametrize("debug_logging", [True, False]) +def test_templates_debug_mode(debug_logging): + """Test that debug logging affects template generation.""" + info = mock_info.copy() + payload = Payload(info) + payload.add_debug_logging = debug_logging + rendered_templates = payload.render_templates() + assert len(rendered_templates) == 2 # There should be at least two files + + for f in rendered_templates: + assert f.dst.is_file() + + with open(f.dst) as open_file: + lines = open_file.readlines() + + # Check the first line. + expected = "@echo on\n" if debug_logging else "@echo off\n" + assert lines[0] == expected + # If debug_logging is True, we expect to find %LOG%, otherwise not. + assert debug_logging == any("%LOG%" in line for line in lines) diff --git a/tests/test_examples.py b/tests/test_examples.py index a8315eb5c..d12f4602d 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -1,6 +1,5 @@ from __future__ import annotations -import ctypes import getpass import json import os @@ -10,6 +9,7 @@ import time import warnings import xml.etree.ElementTree as ET +from dataclasses import dataclass from datetime import timedelta from functools import cache from pathlib import Path @@ -338,28 +338,99 @@ def _sentinel_file_checks(example_path, install_dir): ) -def is_admin() -> bool: - try: - return ctypes.windll.shell32.IsUserAnAdmin() - except Exception: - return False - - def calculate_msi_install_path(installer: Path) -> Path: """This is a temporary solution for now since we cannot choose the install location ourselves. Installers are named --Windows-x86_64.msi. """ dir_name = installer.name.replace("-Windows-x86_64.msi", "").replace("-", " ") - if is_admin(): - root_dir = Path(os.environ.get("PROGRAMFILES", r"C:\Program Files")) - else: - local_dir = os.environ.get("LOCALAPPDATA", str(Path.home() / r"AppData\Local")) - root_dir = Path(local_dir) / "Programs" + local_dir = os.environ.get("LOCALAPPDATA", str(Path.home() / r"AppData\Local")) + root_dir = Path(local_dir) / "Programs" + root_dir.mkdir(parents=True, exist_ok=True) - assert root_dir.is_dir() # Sanity check to avoid strange unexpected errors + assert root_dir.is_dir() # Consistency check to avoid strange unexpected errors return Path(root_dir) / dir_name +def handle_exception_and_error_out( + failure: InstallationFailure | UninstallationFailure, original_exception: BaseException +) -> None: + """Print failure context (including logs) and re-raise with exception chaining.""" + print(failure.read_text()) + raise failure from original_exception + + +def _read_briefcase_log_tail(path: Path, last_digits: int) -> str: + """Helper function to read logs from installers created with briefcase. + The encoding can vary between the different logs. + """ + if not path or not path.exists(): + return f"(log not found: {path})" + + # Try UTF-16 first (MSI logs), fallback to UTF-8 + try: + text = path.read_text(encoding="utf-16", errors="replace") + except UnicodeError: + text = path.read_text(encoding="utf-8", errors="replace") + + return text[last_digits:] + + +@dataclass +class InstallationFailure(RuntimeError): + cmd: list[str] + returncode: int + msi_log: Path | None = None + post_install_log: Path | None = None + + def read_text(self, last_digits: int = -15000) -> str: + parts = [ + f"Command: {self.cmd}", + f"Return code: {self.returncode}", + ] + + if self.post_install_log: + parts.append( + f"\n=== MSI LOG POST INSTALL: {self.post_install_log} ===\n" + + _read_briefcase_log_tail(self.post_install_log, last_digits) + ) + + if self.msi_log: + parts.append( + f"\n=== MSI LOG: {self.msi_log} ===\n" + + _read_briefcase_log_tail(self.msi_log, last_digits) + ) + + return "\n".join(parts) + + +@dataclass +class UninstallationFailure(RuntimeError): + cmd: list[str] + returncode: int + msi_log: Path | None = None + pre_uninstall_log: Path | None = None + + def read_text(self, last_digits: int = -15000) -> str: + parts = [ + f"Command: {self.cmd}", + f"Return code: {self.returncode}", + ] + + if self.pre_uninstall_log: + parts.append( + f"\n=== MSI LOG PRE UNINSTALL: {self.pre_uninstall_log} ===\n" + + _read_briefcase_log_tail(self.pre_uninstall_log, last_digits) + ) + + if self.msi_log: + parts.append( + f"\n=== MSI LOG: {self.msi_log} ===\n" + + _read_briefcase_log_tail(self.msi_log, last_digits) + ) + + return "\n".join(parts) + + def _run_installer_msi( installer: Path, install_dir: Path, @@ -388,22 +459,28 @@ def _run_installer_msi( "/qn", ] - log_path = Path(os.environ.get("TEMP")) / (install_dir.name + ".log") + # Prepare logging + post_install_log = Path(os.environ.get("TEMP")) / (install_dir.name + "-postinstall.log") + log_path = Path(os.environ.get("TEMP")) / (install_dir.name + "-install.log") + for log_file in [post_install_log, log_path]: + if log_file.exists(): + os.remove(log_file) cmd.extend(["/L*V", str(log_path)]) + + # Run installer and handle errors/logs if necessary try: process = _execute(cmd, installer_input=installer_input, timeout=timeout, check=check) except subprocess.CalledProcessError as e: - if log_path.exists(): - # When running on the CI system, it tries to decode a UTF-16 log file as UTF-8, - # therefore we need to specify encoding before printing. - print(f"\n=== MSI LOG {log_path} START ===") - print( - log_path.read_text(encoding="utf-16", errors="replace")[-15000:] - ) # last 15k chars - print(f"\n=== MSI LOG {log_path} END ===") - raise e - if check: - print("A check for MSI Installers not yet implemented") + handle_exception_and_error_out( + InstallationFailure( + cmd=cmd, + returncode=e.returncode, + msi_log=log_path, + post_install_log=post_install_log, + ), + original_exception=e, + ) + return process @@ -419,13 +496,32 @@ def _run_uninstaller_msi( str(installer), "/qn", ] - process = _execute(cmd, timeout=timeout, check=check) + + # Prepare logging + pre_uninstall_log = Path(os.environ.get("TEMP")) / (install_dir.name + "-preuninstall.log") + log_path = Path(os.environ.get("TEMP")) / (install_dir.name + "-uninstall.log") + for log_file in [pre_uninstall_log, log_path]: + if log_file.exists(): + os.remove(log_file) + cmd.extend(["/L*V", str(log_path)]) + + try: + process = _execute(cmd, installer_input=None, timeout=timeout, check=check) + except subprocess.CalledProcessError as e: + handle_exception_and_error_out( + UninstallationFailure( + cmd=cmd, + returncode=e.returncode, + msi_log=log_path, + post_install_log=pre_uninstall_log, + ), + original_exception=e, + ) + if check: # TODO: # Check log and if there are remaining files, similar to the exe installers pass - # This is temporary until uninstallation works fine - shutil.rmtree(str(install_dir), ignore_errors=True) return process @@ -1051,8 +1147,6 @@ def test_register_envs(tmp_path, request): """Verify that 'register_envs: False' results in the environment not being registered.""" input_path = _example_path("register_envs") for installer, install_dir in create_installer(input_path, tmp_path): - if installer.suffix == ".msi": - raise NotImplementedError("Test for 'register_envs' not yet implemented for MSI") _run_installer(input_path, installer, install_dir, request=request) environments_txt = Path("~/.conda/environments.txt").expanduser().read_text() assert str(install_dir) not in environments_txt @@ -1622,6 +1716,8 @@ def test_not_in_installed_menu_list_(tmp_path, request, no_registry): input_path = _example_path("register_envs") # The specific example we use here is not important options = ["/InstallationType=JustMe", f"/NoRegistry={no_registry}"] for installer, install_dir in create_installer(input_path, tmp_path): + if installer.suffix == ".msi": + continue _run_installer( input_path, installer, @@ -1632,17 +1728,17 @@ def test_not_in_installed_menu_list_(tmp_path, request, no_registry): options=options, ) - # Use the installer file name for the registry search - installer_file_name_parts = Path(installer).name.split("-") - name = installer_file_name_parts[0] - version = installer_file_name_parts[1] - partial_name = f"{name} {version}" + # Use the installer file name for the registry search + installer_file_name_parts = Path(installer).name.split("-") + name = installer_file_name_parts[0] + version = installer_file_name_parts[1] + partial_name = f"{name} {version}" - is_in_installed_apps_menu = _is_program_installed(partial_name) - _run_uninstaller_exe(install_dir) + is_in_installed_apps_menu = _is_program_installed(partial_name) + _run_uninstaller_exe(install_dir) - # If no_registry=0 we expect is_in_installed_apps_menu=True - # If no_registry=1 we expect is_in_installed_apps_menu=False - assert is_in_installed_apps_menu == (no_registry == 0), ( - f"Unable to find program '{partial_name}' in the 'Installed apps' menu" - ) + # If no_registry=0 we expect is_in_installed_apps_menu=True + # If no_registry=1 we expect is_in_installed_apps_menu=False + assert is_in_installed_apps_menu == (no_registry == 0), ( + f"Unable to find program '{partial_name}' in the 'Installed apps' menu" + )