Move CrossBuildEnvManager._download to common.download_and_unpack_achive (#82)

Split from #81, non functional.

Author: hoodmane

pyodide_build/common.py

@@ -10,13 +10,15 @@
 import sys
 import textwrap
 import tomllib
+import warnings
 import zipfile
 from collections import deque
 from collections.abc import Generator, Iterable, Iterator, Mapping
 from contextlib import contextmanager
 from pathlib import Path
-from tempfile import TemporaryDirectory
+from tempfile import NamedTemporaryFile, TemporaryDirectory
 from typing import Any, NoReturn
+from urllib.request import urlopen
 from zipfile import ZipFile
 
 from packaging.tags import Tag
@@ -447,3 +449,45 @@ def to_bool(value: str) -> bool:
     Convert a string to a boolean value. Useful for parsing environment variables.
     """
     return value.lower() not in {"", "0", "false", "no", "off"}
+
+
+def download_and_unpack_archive(url: str, path: Path, descr: str) -> None:
+    """
+    Download the cross-build environment from the given URL and extract it to the given path.
+
+    Parameters
+    ----------
+    url
+        URL to download the cross-build environment from.
+    path
+        Path to extract the cross-build environment to.
+        If the path already exists, raise an error.
+    """
+    logger.info("Downloading %s from %s", descr, url)
+
+    if path.exists():
+        raise FileExistsError(f"Path {path} already exists")
+
+    try:
+        resp = urlopen(url)
+        data = resp.read()
+    except Exception as e:
+        raise ValueError(f"Failed to download {descr} from {url}") from e
+
+    # FIXME: requests makes a verbose output (see: https://github.com/pyodide/pyodide/issues/4810)
+    # r = requests.get(url)
+
+    # if r.status_code != 200:
+    #     raise ValueError(
+    #         f"Failed to download cross-build environment from {url} (status code: {r.status_code})"
+    #     )
+
+    with NamedTemporaryFile(suffix=".tar") as f:
+        f_path = Path(f.name)
+        f_path.write_bytes(data)
+        with warnings.catch_warnings():
+            # Python 3.12-3.13 emits a DeprecationWarning when using shutil.unpack_archive without a filter,
+            # but filter doesn't work well for zip files, so we suppress the warning until we find a better solution.
+            # https://github.com/python/cpython/issues/112760
+            warnings.simplefilter("ignore")
+            shutil.unpack_archive(str(f_path), path)

pyodide_build/tests/test_xbuildenv.py

@@ -2,6 +2,7 @@
 
 import pytest
 
+from pyodide_build.common import download_and_unpack_archive
 from pyodide_build.xbuildenv import CrossBuildEnvManager, _url_to_version
 
 
@@ -91,24 +92,20 @@ def test_current_version(self, tmp_path):
         assert manager.current_version == "0.25.0"
 
     def test_download(self, tmp_path, dummy_xbuildenv_url):
-        manager = CrossBuildEnvManager(tmp_path)
-
         download_path = tmp_path / "test"
-        manager._download(dummy_xbuildenv_url, download_path)
+        download_and_unpack_archive(dummy_xbuildenv_url, download_path, "")
 
         assert download_path.exists()
         assert (download_path / "xbuildenv").exists()
         assert (download_path / "xbuildenv" / "pyodide-root").exists()
 
     def test_download_path_exists(self, tmp_path):
-        manager = CrossBuildEnvManager(tmp_path)
-
         download_path = tmp_path / "test"
         download_path.mkdir()
 
         with pytest.raises(FileExistsError, match="Path .* already exists"):
-            manager._download(
-                "https://example.com/xbuildenv-0.25.0.tar.bz2", download_path
+            download_and_unpack_archive(
+                "https://example.com/xbuildenv-0.25.0.tar.bz2", download_path, ""
             )
 
     def test_find_latest_version(self, tmp_path, fake_xbuildenv_releases_compatible):
@@ -224,7 +221,7 @@ def test_install_cross_build_packages(
         manager = CrossBuildEnvManager(tmp_path)
 
         download_path = tmp_path / "test"
-        manager._download(dummy_xbuildenv_url, download_path)
+        download_and_unpack_archive(dummy_xbuildenv_url, download_path, "")
 
         xbuildenv_root = download_path / "xbuildenv"
         xbuildenv_pyodide_root = xbuildenv_root / "pyodide-root"
@@ -248,7 +245,7 @@ def test_create_package_index(self, tmp_path, dummy_xbuildenv_url):
         manager = CrossBuildEnvManager(tmp_path)
 
         download_path = tmp_path / "test"
-        manager._download(dummy_xbuildenv_url, download_path)
+        download_and_unpack_archive(dummy_xbuildenv_url, download_path, "")
 
         xbuildenv_root = download_path / "xbuildenv"
         xbuildenv_pyodide_root = xbuildenv_root / "pyodide-root"

pyodide_build/xbuildenv.py

@@ -1,14 +1,12 @@
 import json
 import shutil
 import subprocess
-import warnings
 from pathlib import Path
-from tempfile import NamedTemporaryFile
-from urllib.request import urlopen
 
 from pyodide_lock import PyodideLockSpec
 
 from pyodide_build import build_env
+from pyodide_build.common import download_and_unpack_archive
 from pyodide_build.create_package_index import create_package_index
 from pyodide_build.logger import logger
 from pyodide_build.xbuildenv_releases import (
@@ -194,7 +192,9 @@ def install(
                 download_path,
             )
         else:
-            self._download(download_url, download_path)
+            download_and_unpack_archive(
+                download_url, download_path, "Pyodide cross-build environment"
+            )
 
         try:
             # there is an redundant directory "xbuildenv" inside the xbuildenv archive
@@ -240,49 +240,6 @@ def _find_latest_version(self) -> str:
 
         return latest.version
 
-    def _download(self, url: str, path: Path) -> None:
-        """
-        Download the cross-build environment from the given URL and extract it to the given path.
-
-        Parameters
-        ----------
-        url
-            URL to download the cross-build environment from.
-        path
-            Path to extract the cross-build environment to.
-            If the path already exists, raise an error.
-        """
-        logger.info("Downloading Pyodide cross-build environment from %s", url)
-
-        if path.exists():
-            raise FileExistsError(f"Path {path} already exists")
-
-        try:
-            resp = urlopen(url)
-            data = resp.read()
-        except Exception as e:
-            raise ValueError(
-                f"Failed to download cross-build environment from {url}"
-            ) from e
-
-        # FIXME: requests makes a verbose output (see: https://github.com/pyodide/pyodide/issues/4810)
-        # r = requests.get(url)
-
-        # if r.status_code != 200:
-        #     raise ValueError(
-        #         f"Failed to download cross-build environment from {url} (status code: {r.status_code})"
-        #     )
-
-        with NamedTemporaryFile(suffix=".tar") as f:
-            f_path = Path(f.name)
-            f_path.write_bytes(data)
-            with warnings.catch_warnings():
-                # Python 3.12-3.13 emits a DeprecationWarning when using shutil.unpack_archive without a filter,
-                # but filter doesn't work well for zip files, so we suppress the warning until we find a better solution.
-                # https://github.com/python/cpython/issues/112760
-                warnings.simplefilter("ignore")
-                shutil.unpack_archive(str(f_path), path)
-
     def _install_cross_build_packages(
         self, xbuildenv_root: Path, xbuildenv_pyodide_root: Path
     ) -> None: