Misc improvements (conda#1191)

Author: lrandersson

constructor/briefcase.py

@@ -360,7 +360,7 @@ def remove(self, *, ignore_errors: bool = True) -> None:
             # delattr on a cached_property may raise on some versions / edge cases
             pass
 
-    def prepare(self) -> tuple:
+    def prepare(self) -> None:
         """Prepares the payload.
 
         Directory structure created during preparation:
@@ -369,9 +369,10 @@ def prepare(self) -> tuple:
             └── <EXTERNAL_PACKAGE_PATH>/     (external_dir: contains the payload archive and conda exe)
                 └── base/                    (base_dir: represents the base conda environment)
                     └── pkgs/                (pkgs_dir: staging area for conda package distributions)
+
+        Note: base_dir and pkgs_dir are removed after archiving.
         """
-        root = self.root
-        external_dir = root / EXTERNAL_PACKAGE_PATH
+        external_dir = self.root / EXTERNAL_PACKAGE_PATH
         external_dir.mkdir(parents=True, exist_ok=True)
 
         # Note that the directory name "base" is also explicitly defined in `run_installation.bat`
@@ -380,9 +381,9 @@ def prepare(self) -> tuple:
 
         pkgs_dir = base_dir / "pkgs"
         pkgs_dir.mkdir()
-        # Render the template files and add them to the necessary config field
+
         self.render_templates()
-        self.write_pyproject_toml(root, external_dir)
+        self.write_pyproject_toml(self.root, external_dir)
 
         preconda.write_files(self.info, base_dir)
         preconda.copy_extra_files(self.info.get("extra_files", []), external_dir)
@@ -392,7 +393,6 @@ def prepare(self) -> tuple:
         archive_path = self.make_archive(base_dir, external_dir)
         if not archive_path.exists():
             raise RuntimeError(f"Unexpected error, failed to create archive: {archive_path}")
-        return (root, external_dir, base_dir, pkgs_dir)
 
     def make_archive(self, src: Path, dst: Path) -> Path:
         """Create an archive of the directory 'src'.
@@ -522,40 +522,42 @@ def _stage_conda(self, external_dir: Path) -> None:
 
 def create(info, verbose=False):
     if not IS_WINDOWS:
-        raise Exception(f"Invalid platform '{sys.platform}'. MSI installers require Windows.")
+        raise OSError(f"Invalid platform '{sys.platform}'. MSI installers require Windows.")
 
     if not info.get("_conda_exe_supports_logging"):
-        raise Exception("MSI installers require conda-standalone with logging support.")
-
-    # MSI installers always use conda-standalone for uninstallation.
-    # This ensures proper cleanup of conda init, environments, and shortcuts
-    # via the `conda constructor uninstall` command.
-    info["uninstall_with_conda_exe"] = True
-
-    payload = Payload(info)
-    payload.prepare()
+        raise ValueError("MSI installers require conda-standalone with logging support.")
 
+    # Check briefcase exists before doing any work
     briefcase = Path(sysconfig.get_path("scripts")) / "briefcase.exe"
     if not briefcase.exists():
         raise FileNotFoundError(
             f"Dependency 'briefcase' does not seem to be installed.\nTried: {briefcase}"
         )
 
-    logger.info("Building MSI installer")
-    run(
-        [briefcase, "package"] + (["-v"] if verbose else []),
-        cwd=payload.root,
-        check=True,
-    )
-
-    dist_dir = payload.root / "dist"
-    msi_paths = list(dist_dir.glob("*.msi"))
-    if len(msi_paths) != 1:
-        raise RuntimeError(f"Found {len(msi_paths)} MSI files in {dist_dir}, expected 1.")
+    # MSI installers always use conda-standalone for uninstallation.
+    # This ensures proper cleanup of conda init, environments, and shortcuts
+    # via the `conda constructor uninstall` command.
+    info["uninstall_with_conda_exe"] = True
 
-    outpath = Path(info["_outpath"])
-    outpath.unlink(missing_ok=True)
-    shutil.move(msi_paths[0], outpath)
+    payload = Payload(info)
+    try:
+        payload.prepare()
+
+        logger.info("Building MSI installer")
+        run(
+            [briefcase, "package"] + (["-v"] if verbose else []),
+            cwd=payload.root,
+            check=True,
+        )
 
-    if not info.get("_debug"):
-        payload.remove()
+        dist_dir = payload.root / "dist"
+        msi_paths = list(dist_dir.glob("*.msi"))
+        if len(msi_paths) != 1:
+            raise RuntimeError(f"Found {len(msi_paths)} MSI files in {dist_dir}, expected 1.")
+
+        outpath = Path(info["_outpath"])
+        outpath.unlink(missing_ok=True)
+        shutil.move(msi_paths[0], outpath)
+    finally:
+        if not info.get("_debug"):
+            payload.remove()

tests/test_briefcase.py

@@ -198,15 +198,13 @@ def test_payload_layout():
     """
     info = mock_info.copy()
     payload = Payload(info)
-    prepared_payload = payload.prepare()
+    payload.prepare()
 
-    root = prepared_payload[0]
-    external_dir = root / "external"
-    # The second item in prepared_payload is the 'external' directory
-    assert external_dir.is_dir() and external_dir == prepared_payload[1]
+    external_dir = payload.root / "external"
+    assert external_dir.is_dir()
 
-    base_dir = root / "external" / "base"
-    pkgs_dir = root / "external" / "base" / "pkgs"
+    base_dir = payload.root / "external" / "base"
+    pkgs_dir = payload.root / "external" / "base" / "pkgs"
     archive_path = external_dir / payload.archive_name
     # Since archiving removes the directory 'base_dir' and its contents
     assert not base_dir.exists()
@@ -241,20 +239,21 @@ def test_payload_remove():
     """Test removing the payload."""
     info = mock_info.copy()
     payload = Payload(info)
-    prepared_payload = payload.prepare()
+    payload.prepare()
+    root = payload.root
 
-    assert prepared_payload[0].is_dir()
+    assert root.is_dir()
     payload.remove()
-    assert not prepared_payload[0].is_dir()
+    assert not root.is_dir()
 
 
 @pytest.mark.skipif(sys.platform != "win32", reason="Windows only")
 def test_payload_pyproject_toml():
     """Test that the pyproject.toml file is created when the payload is prepared."""
     info = mock_info.copy()
     payload = Payload(info)
-    prepared_payload = payload.prepare()
-    pyproject_toml = prepared_payload[0] / "pyproject.toml"
+    payload.prepare()
+    pyproject_toml = payload.root / "pyproject.toml"
     assert pyproject_toml.is_file()
 
 
@@ -263,8 +262,9 @@ def test_payload_conda_exe():
     """Test that conda-standalone is prepared."""
     info = mock_info.copy()
     payload = Payload(info)
-    prepared_payload = payload.prepare()
-    conda_exe = prepared_payload[1] / "_conda.exe"  # The second item is the 'external' directory
+    payload.prepare()
+    external_dir = payload.root / "external"
+    conda_exe = external_dir / "_conda.exe"
     assert conda_exe.is_file()