refactor(downloads): centralize download helpers in new module

Create `fromager.downloads` as a single import point for all download
helpers, avoiding cyclic import issues between `sources`, `wheels`,
and `resolver`.

Move `download_url` and `extract_filename_from_url` from `sources`
and `resolver` into the new module.

Add `download_sdist` and `download_wheel` that validate filenames
with `parse_sdist_filename` / `parse_wheel_filename` before
downloading and verify archives are non-empty afterwards.

Add `download_git_source` for cloning from pip VCS URLs using
`git_clone_fast` with destination directory conflict detection.

Improve `download_url` temp file handling: use `NamedTemporaryFile`
in the target directory with `os.rename` for atomic moves.

Replace `_download_source_check` and `_download_wheel_check` with
the new `download_sdist` and `download_wheel` functions.

Co-Authored-By: Claude <claude@anthropic.com>
Signed-off-by: Christian Heimes <cheimes@redhat.com>

Author: tiran

src/fromager/commands/build.py

@@ -23,6 +23,7 @@
     clickext,
     context,
     dependency_graph,
+    downloads,
     hooks,
     metrics,
     overrides,
@@ -33,7 +34,6 @@
     wheels,
 )
 
-from .. import resolver
 from ..log import VERBOSE_LOG_FMT, ThreadLogFilter, req_ctxvar_context
 
 logger = logging.getLogger(__name__)
@@ -495,7 +495,7 @@ def _is_wheel_built(
         pbi = wkctx.package_build_info(req)
         build_tag_from_settings = pbi.build_tag(resolved_version)
         build_tag = build_tag_from_settings if build_tag_from_settings else (0, "")
-        wheel_basename = resolver.extract_filename_from_url(url)
+        wheel_basename = downloads.extract_filename_from_url(url)
         _, _, build_tag_from_name, _ = parse_wheel_filename(wheel_basename)
         existing_build_tag = build_tag_from_name if build_tag_from_name else (0, "")
         if (

src/fromager/downloads.py

@@ -0,0 +1,204 @@
+from __future__ import annotations
+
+import logging
+import os
+import pathlib
+import tarfile
+import tempfile
+import typing
+import zipfile
+from urllib.parse import unquote, urlparse
+
+from packaging.utils import parse_sdist_filename, parse_wheel_filename
+
+from . import gitutils
+from .http_retry import RETRYABLE_EXCEPTIONS, retry_on_exception
+from .request_session import session
+
+logger = logging.getLogger(__name__)
+
+
+def extract_filename_from_url(url: str) -> str:
+    """Extract filename from URL and decode it."""
+    path = urlparse(url).path
+    filename = os.path.basename(path)
+    return unquote(filename)
+
+
+def download_url(
+    *,
+    destination_dir: pathlib.Path,
+    url: str,
+    destination_filename: str | None = None,
+) -> pathlib.Path:
+    """Download a URL to destination_dir, returning the local path.
+
+    Returns immediately if the file already exists. Downloads to a
+    temporary file in the same directory first and uses :func:`os.rename`
+    for an atomic move on success, avoiding partial files. Retries on
+    transient network errors.
+
+    .. versionadded:: 0.90.0
+    """
+    basename = (
+        destination_filename if destination_filename else extract_filename_from_url(url)
+    )
+    outfile = destination_dir / basename
+    logger.debug(
+        "looking for %s %s",
+        outfile,
+        "(exists)" if outfile.exists() else "(not there)",
+    )
+    if outfile.exists():
+        logger.debug("already have %s", outfile)
+        return outfile
+
+    destination_dir.mkdir(parents=True, exist_ok=True)
+
+    @retry_on_exception(
+        exceptions=RETRYABLE_EXCEPTIONS,
+        max_attempts=5,
+        backoff_factor=1.5,
+        max_backoff=120.0,
+    )
+    def _download_with_retry() -> pathlib.Path:
+        """Download to a temporary file, then atomically rename."""
+        logger.debug("reading from %s", url)
+        # NamedTemporaryFile in the same directory ensures os.rename() is
+        # atomic (same filesystem). delete=False so we control cleanup.
+        # Temp filename looks like ".tmp.foo-1.2.tar.gz.abc123def".
+        tmp = tempfile.NamedTemporaryFile(
+            dir=destination_dir,
+            prefix=f".tmp.{basename}.",
+            delete=False,
+        )
+        temp_path = pathlib.Path(tmp.name)
+        try:
+            with session.get(url, stream=True) as r:
+                r.raise_for_status()
+                logger.debug("writing to %s", temp_path)
+                for chunk in r.iter_content(chunk_size=64 * 1024):
+                    if chunk:
+                        tmp.write(chunk)
+            tmp.close()
+            # Atomic rename on the same filesystem
+            os.rename(temp_path, outfile)
+            logger.info("saved %s", outfile)
+            return outfile
+        except BaseException:
+            tmp.close()
+            temp_path.unlink(missing_ok=True)
+            raise
+
+    result: pathlib.Path = _download_with_retry()
+    return result
+
+
+def download_sdist(
+    *,
+    destination_dir: pathlib.Path,
+    url: str,
+    destination_filename: str | None = None,
+) -> pathlib.Path:
+    """Download a source distribution and verify it.
+
+    Accepts ``.tar.gz`` and ``.zip`` sdists. Validates the filename
+    with :func:`packaging.utils.parse_sdist_filename` before
+    downloading and checks that the archive is non-empty afterwards.
+
+    .. versionadded:: 0.90.0
+    """
+    basename = (
+        destination_filename if destination_filename else extract_filename_from_url(url)
+    )
+    # Validates name, version, and extension (.tar.gz or .zip);
+    # raises InvalidSdistFilename on malformed names.
+    parse_sdist_filename(basename)
+
+    filepath = download_url(
+        destination_dir=destination_dir,
+        url=url,
+        destination_filename=basename,
+    )
+
+    if basename.endswith(".tar.gz"):
+        with tarfile.open(filepath, mode="r:gz") as tar:
+            if tar.next() is None:
+                raise tarfile.TarError(f"empty tar file: {filepath}")
+    elif basename.endswith(".zip"):
+        with zipfile.ZipFile(filepath) as zf:
+            if not zf.namelist():
+                raise zipfile.BadZipFile(f"empty zip file: {filepath}")
+    else:
+        typing.assert_never(basename)  # type: ignore[arg-type]
+
+    return filepath
+
+
+def download_wheel(
+    *,
+    destination_dir: pathlib.Path,
+    url: str,
+    destination_filename: str | None = None,
+) -> pathlib.Path:
+    """Download a wheel and verify it.
+
+    Validates that the filename is a valid wheel name (using
+    :func:`packaging.utils.parse_wheel_filename`) before downloading,
+    then checks that the zip archive is non-empty.
+
+    .. versionadded:: 0.90.0
+    """
+    basename = (
+        destination_filename if destination_filename else extract_filename_from_url(url)
+    )
+    # Validates name, version, build tag, tags, and .whl extension;
+    # raises InvalidWheelFilename on malformed names.
+    parse_wheel_filename(basename)
+
+    filepath = download_url(
+        destination_dir=destination_dir,
+        url=url,
+        destination_filename=basename,
+    )
+
+    with zipfile.ZipFile(filepath) as zf:
+        if not zf.namelist():
+            raise zipfile.BadZipFile(f"empty wheel file: {filepath}")
+
+    return filepath
+
+
+def download_git_source(
+    *,
+    destination_dir: pathlib.Path,
+    vcs_url: str,
+    require_ref: bool = True,
+) -> pathlib.Path:
+    """Clone a git repository from a pip VCS URL.
+
+    Parses *vcs_url* with :func:`~fromager.gitutils.parse_vcs_url`
+    and clones using :func:`~fromager.gitutils.git_clone_fast`
+    with recursive submodules.
+
+    Raises ``FileExistsError`` if the destination directory already
+    exists.
+
+    .. versionadded:: 0.90.0
+    """
+    repo_url, ref = gitutils.parse_vcs_url(vcs_url, require_ref=require_ref)
+
+    # git clone fails when the target directory is not empty.
+    try:
+        destination_dir.mkdir(parents=True, exist_ok=False)
+    except FileExistsError:
+        raise FileExistsError(
+            f"destination directory {destination_dir} already exists, "
+            f"cannot clone {vcs_url}"
+        ) from None
+    gitutils.git_clone_fast(
+        output_dir=destination_dir,
+        repo_url=repo_url,
+        ref=ref,
+    )
+    return destination_dir

src/fromager/gitutils.py

@@ -1,11 +1,16 @@
+from __future__ import annotations
+
 import logging
 import pathlib
 import typing
 from urllib.parse import urlparse
 
 from packaging.requirements import Requirement
 
-from fromager import context, external_commands
+from fromager import external_commands
+
+if typing.TYPE_CHECKING:
+    from fromager import context
 
 logger = logging.getLogger(__name__)
 

src/fromager/resolver.py

@@ -14,7 +14,7 @@
 from collections.abc import Iterable
 from operator import attrgetter
 from platform import python_version
-from urllib.parse import quote, unquote, urlparse
+from urllib.parse import quote, urlparse
 
 import pypi_simple
 import resolvelib
@@ -33,6 +33,7 @@
 from . import overrides
 from .candidate import Candidate, Cooldown
 from .constraints import Constraints
+from .downloads import extract_filename_from_url as extract_filename_from_url
 from .extras_provider import ExtrasProvider
 from .http_retry import RETRYABLE_EXCEPTIONS, retry_on_exception
 from .request_session import session
@@ -204,13 +205,6 @@ def _compute_max_age_cutoff(
     return bootstrap_time - ctx.max_release_age
 
 
-def extract_filename_from_url(url: str) -> str:
-    """Extract filename from URL and decode it."""
-    path = urlparse(url).path
-    filename = os.path.basename(path)
-    return unquote(filename)
-
-
 class LogReporter(resolvelib.BaseReporter):
     """Report resolution events