From c5e3a28704ef35e68b750b72a02e6bf642d65973 Mon Sep 17 00:00:00 2001 From: Shanmukh Pawan Date: Fri, 23 Oct 2026 12:25:31 -0400 Subject: [PATCH] fix(sources): prepare Git checkouts by download kind Carry DownloadedSource.kind through bootstrap and build entry points. Unpack ordinary archives; prepare clean Git checkouts in place so patches, project overrides, and Rust vendoring run. Keep prepare_source override arguments and path-only callers compatible. Document the public Git helper and no-cleanup behavior, and reject dirty nested submodules before preparation. Reject archive input paths that cleanup would remove before extraction, preserving the input and reporting an actionable error. Closes: #1205 Co-Authored-By: Codex Signed-off-by: Shanmukh Pawan --- docs/concepts/bootstrapper-architecture.rst | 7 +- docs/concepts/hooks-and-overrides.rst | 6 + docs/concepts/resolver-architecture.rst | 31 +- docs/reference/hooks.rst | 35 +- docs/using.md | 16 +- src/fromager/bootstrapper/_bootstrapper.py | 9 +- src/fromager/bootstrapper/_prepare_source.py | 12 +- src/fromager/commands/build.py | 11 +- src/fromager/commands/step.py | 67 ++- src/fromager/packagesettings/_resolver.py | 9 + src/fromager/sources.py | 133 ++++- tests/test_bootstrapper.py | 67 ++- tests/test_commands.py | 179 +++++- tests/test_packagesettings_resolver.py | 128 +++- tests/test_sources.py | 597 +++++++++++++++++++ 15 files changed, 1251 insertions(+), 56 deletions(-) diff --git a/docs/concepts/bootstrapper-architecture.rst b/docs/concepts/bootstrapper-architecture.rst index 80a3532b2..37d7d5493 100644 --- a/docs/concepts/bootstrapper-architecture.rst +++ b/docs/concepts/bootstrapper-architecture.rst @@ -57,7 +57,8 @@ the contract every phase must satisfy: * - ``Start`` - Add to dependency graph; deduplicate already-seen packages * - ``PrepareSource`` - - Download source or prebuilt wheel; read build-system deps + - Download source or prebuilt wheel; prepare archives or checkouts; + read build-system deps * - ``PrepareBuild`` - Install build-system deps; discover backend and sdist deps * - ``Build`` @@ -67,6 +68,10 @@ the contract every phase must satisfy: * - ``Complete`` - Clean up build directories (terminal phase) +.. versionchanged:: 0.102.0 + ``PrepareSource`` passes the downloaded artifact kind through preparation, + so Git checkouts can be prepared without unpacking them. + Each phase wraps a ``WorkItem`` dataclass that accumulates per-package state (resolved version, build environment, build result, etc.) as it moves through the pipeline. diff --git a/docs/concepts/hooks-and-overrides.rst b/docs/concepts/hooks-and-overrides.rst index 9d5274995..6b048bbf2 100644 --- a/docs/concepts/hooks-and-overrides.rst +++ b/docs/concepts/hooks-and-overrides.rst @@ -62,6 +62,12 @@ arguments. download results with explicit artifact kinds. See :ref:`download_source_hook` for the contract, legacy path defaults, and a migration example. +.. versionchanged:: 0.102.0 + + ``prepare_source`` selects an archive or Git default using the downloaded + artifact kind. Package overrides remain active. See :ref:`prepare_source_hook` + for their arguments and the public Git checkout helper. + Global Hooks ------------ diff --git a/docs/concepts/resolver-architecture.rst b/docs/concepts/resolver-architecture.rst index 0da851d7c..4702b957b 100644 --- a/docs/concepts/resolver-architecture.rst +++ b/docs/concepts/resolver-architecture.rst @@ -113,15 +113,34 @@ settings are used. Hook downloads accept explicit artifact kinds and adapt legacy paths using profile-specific defaults. If no download hook exists, the profile selects the standard sdist or wheel downloader and its destination directory. Hook errors -propagate with context instead of triggering a fallback. The typed result makes -the artifact kind available for source preparation; consuming it in preparation -and integrating prebuilt detection into the bootstrapper are separate steps. +propagate with context instead of triggering a fallback. In-process callers +pass the downloaded path and kind to preparation: ``sdist`` and ``tarball`` +are unpacked, while ``git_checkout`` uses the checkout directory. The checkout +uses the same double-nested work directory pattern as an unpacked archive; +the directory names can differ for normalized package names. +The default Git preparer rejects a checkout with local changes before applying +patches, project overrides, and Rust vendoring. Git status does not prove that the +checkout is the original clone or preserve it after preparation. Build metadata +is written after preparation but is not used to choose the preparation path. +With ``--no-cleanup``, downloading to an existing checkout fails and preserves +its files for inspection. Enable cleanup or use a new work directory to +download and prepare a fresh checkout. Git status cannot detect changes to +patch or settings files outside the checkout. If preparation left the checkout +clean, a manual second ``prepare-source`` call runs the preparation steps again. +The separate ``step prepare-source`` invocation accepts ``--source-kind`` and +``--source-path`` because it cannot retrieve the earlier Python download result. +``step download-source-archive --output-format json`` exposes that result's +actual path and kind without changing its default path-only output. +Without ``--source-kind``, it keeps the legacy archive behavior. +Integrating prebuilt detection into the bootstrapper remains a separate step. See :ref:`download_source_hook` for the return contract and compatibility rules. .. versionchanged:: 0.102.0 - Configured source resolvers also select the sdist build method. Git checkout - resolvers use PEP 517 by default; hook sdist resolvers require an explicit - ``build_sdist: pep517`` or ``build_sdist: tarball``. PyPI sdist resolvers + Explicit ``DownloadKind.git_checkout`` results can be prepared without + unpacking an archive. Configured source resolvers also select the sdist + build method. Git checkout resolvers use PEP 517 by default; hook sdist + resolvers require an explicit ``build_sdist: pep517`` or + ``build_sdist: tarball``. PyPI sdist resolvers repack a tarball. Both configured strategies take precedence over an installed ``build_sdist`` override and log a warning when it is ignored. Legacy settings continue to use the override. diff --git a/docs/reference/hooks.rst b/docs/reference/hooks.rst index c92149684..5479a9937 100644 --- a/docs/reference/hooks.rst +++ b/docs/reference/hooks.rst @@ -298,17 +298,32 @@ Source hooks the configured hook profiles; packages using legacy settings still require a bare ``Path``. -.. autofromagerhook:: default_prepare_source - - The ``prepare_source()`` function is responsible for setting up a tree - of source files in a format that is ready to be built. The default - implementation unpacks the source archive and applies patches. +.. _prepare_source_hook: - The arguments are the ``WorkContext``, the ``Requirement`` being - evaluated, the ``Path`` to the source archive, and the version. +.. autofromagerhook:: default_prepare_source - The return value should be the ``Path`` to the root of the source tree, - ideally inside the ``ctx.work_dir`` directory. + The ``prepare_source()`` function sets up a source tree for building. It + selects ``default_prepare_source()`` for archives or + ``default_prepare_git_checkout()`` for a Git checkout according to the + downloaded ``DownloadKind``. Both defaults apply patches and other project + preparation. + + In-process callers pass the ``path`` and ``kind`` from ``DownloadedSource`` + to ``prepare_source()``. The Python function accepts a source ``Path`` and + optional ``download_kind``; path-only callers default to ``sdist``. + A configured ``source:`` resolver does not disable the package + ``prepare_source`` override. The override receives the ``WorkContext``, + ``Requirement``, source ``Path``, and version, but not ``download_kind``. + An archive-specific override can call ``sources.default_prepare_source()``; + a Git-specific override can call + ``sources.default_prepare_git_checkout()`` before its own changes. + + The override returns the source root ``Path`` or a ``(Path, bool)`` pair. + Git checkouts must use the double-nested package/version layout under + ``ctx.work_dir`` so the later bootstrap and step commands can find them. + + .. versionchanged:: 0.102.0 + Passing ``DownloadKind.git_checkout`` prepares a clean Git checkout. .. autofromagerhook:: default_build_sdist @@ -328,6 +343,8 @@ Source helper functions The following helper functions are available in the ``fromager.sources`` module for use in custom source hooks: +.. autofunction:: fromager.sources.default_prepare_git_checkout + .. autofunction:: fromager.sources.ensure_pkg_info .. autofunction:: fromager.sources.pep517_build_sdist diff --git a/docs/using.md b/docs/using.md index 7439ed529..7ffab10e8 100644 --- a/docs/using.md +++ b/docs/using.md @@ -256,9 +256,19 @@ package index and downloads it. It will be common to run this step with `pypi.org`, but for truly isolated and reproducible builds a private index server is more robust. -The `step prepare-source` command unpacks the source archive -downloaded from the previous step and applies any patches (refer to -[customization](customization.md) for details about patching). +The `step prepare-source` command unpacks a source archive and applies any +patches (refer to [customization](customization.md) for details about patching). +It defaults to the archive kind for existing scripts. For a Git checkout, pass +`--source-kind git_checkout`. Pass `--source-path` when the downloaded source +cannot be found at its standard location. Git checkouts must use the standard +double-nested work directory layout so later build steps can find them. The +download command's optional +`--output-format json` prints both `path` and `kind` when a hook can return +different artifact kinds. Its default output remains the path alone. Git +preparation applies patches to the selected checkout itself, so use a +disposable clone for `--source-path`. Git downloads do not reuse an existing +checkout with `--no-cleanup`; the files remain available for inspection. Enable +cleanup or use a new work directory to download it again. The `step prepare-build` command creates a virtualenv with the build dependencies for building the wheel. It expects a `--wheel-server-url` diff --git a/src/fromager/bootstrapper/_bootstrapper.py b/src/fromager/bootstrapper/_bootstrapper.py index a22464b56..c524324c7 100644 --- a/src/fromager/bootstrapper/_bootstrapper.py +++ b/src/fromager/bootstrapper/_bootstrapper.py @@ -482,7 +482,7 @@ def _download_source( req: Requirement, resolved_version: Version, source_url: str, - ) -> pathlib.Path: + ) -> packagesettings.DownloadedSource: """Download source for a package.""" result: packagesettings.DownloadedSource = sources.download_source( ctx=self.ctx, @@ -490,20 +490,21 @@ def _download_source( version=resolved_version, download_url=source_url, ) - return result.path + return result def _prepare_source( self, req: Requirement, resolved_version: Version, - source_filename: pathlib.Path, + downloaded_source: packagesettings.DownloadedSource, ) -> pathlib.Path: """Prepare (unpack/patch) source for building.""" result: pathlib.Path = sources.prepare_source( ctx=self.ctx, req=req, - source_filename=source_filename, + source_filename=downloaded_source.path, version=resolved_version, + download_kind=downloaded_source.kind, ) return result diff --git a/src/fromager/bootstrapper/_prepare_source.py b/src/fromager/bootstrapper/_prepare_source.py index 67df15469..76f5212de 100644 --- a/src/fromager/bootstrapper/_prepare_source.py +++ b/src/fromager/bootstrapper/_prepare_source.py @@ -29,7 +29,7 @@ def _bg_prepare_source( resolved_version: Version, source_url: str, ) -> PreparedSourceData: - """Background-safe source download+unpack: no Bootstrapper state accessed.""" + """Background-safe source download and preparation without Bootstrapper state.""" # Thread-safe: _seen_requirements in the main thread prevents the same # package from being submitted to the thread pool more than once. # Paths from download_source() and prepare_source() include {name}-{version}, @@ -43,11 +43,15 @@ def _bg_prepare_source( sdist_root_dir=unpacked / unpacked.stem, cached_wheel_filename=cached_wheel, ) - source_filename = sources.download_source( + downloaded_source = sources.download_source( ctx=ctx, req=req, version=resolved_version, download_url=source_url - ).path + ) sdist_root_dir = sources.prepare_source( - ctx=ctx, req=req, source_filename=source_filename, version=resolved_version + ctx=ctx, + req=req, + source_filename=downloaded_source.path, + version=resolved_version, + download_kind=downloaded_source.kind, ) return PreparedSourceData(sdist_root_dir=sdist_root_dir) diff --git a/src/fromager/commands/build.py b/src/fromager/commands/build.py index 1fe34257c..70d549484 100644 --- a/src/fromager/commands/build.py +++ b/src/fromager/commands/build.py @@ -384,25 +384,26 @@ def _build( # If we get here and still don't have a wheel filename, then we need to # build the wheel. if not wheel_filename: - source_filename = sources.download_source( + downloaded_source = sources.download_source( ctx=wkctx, req=req, version=resolved_version, download_url=source_download_url, - ).path + ) logger.debug( - "saved sdist of version %s from %s to %s", + "saved source of version %s from %s to %s", resolved_version, source_download_url, - source_filename, + downloaded_source.path, ) # Prepare source source_root_dir = sources.prepare_source( ctx=wkctx, req=req, - source_filename=source_filename, + source_filename=downloaded_source.path, version=resolved_version, + download_kind=downloaded_source.kind, ) # Build environment diff --git a/src/fromager/commands/step.py b/src/fromager/commands/step.py index 95280934e..f9ea3ef04 100644 --- a/src/fromager/commands/step.py +++ b/src/fromager/commands/step.py @@ -1,3 +1,4 @@ +import json import logging import pathlib @@ -10,6 +11,7 @@ clickext, context, finders, + packagesettings, server, sources, wheels, @@ -26,17 +28,25 @@ def step() -> None: @step.command() +@click.option( + "--output-format", + type=click.Choice(["path", "json"]), + default="path", + show_default=True, + help="Print the path or a JSON object with path and artifact kind.", +) @click.argument("dist_name") @click.argument("dist_version", type=clickext.PackageVersion()) @click.argument("sdist_server_url") @click.pass_obj def download_source_archive( wkctx: context.WorkContext, + output_format: str, dist_name: str, dist_version: Version, sdist_server_url: str, ) -> None: - """download the source code archive for one version of one package + """download source for one version of one package DIST_NAME is the name of a distribution @@ -50,22 +60,52 @@ def download_source_archive( source_url, version = sources.resolve_source( ctx=wkctx, req=req, sdist_server_url=sdist_server_url ) - filename = sources.download_source( + downloaded_source = sources.download_source( ctx=wkctx, req=req, version=version, download_url=source_url, - ).path + ) requirement_ctxvar.reset(token) - print(filename) + if output_format == "json": + print( + json.dumps( + { + "path": str(downloaded_source.path), + "kind": downloaded_source.kind.value, + } + ) + ) + else: + print(downloaded_source.path) @step.command() +@click.option( + "--source-kind", + type=click.Choice( + [ + packagesettings.DownloadKind.sdist.value, + packagesettings.DownloadKind.tarball.value, + packagesettings.DownloadKind.git_checkout.value, + ] + ), + default=packagesettings.DownloadKind.sdist.value, + show_default=True, + help="Kind returned by the source download step.", +) +@click.option( + "--source-path", + type=click.Path(exists=True, path_type=pathlib.Path), + help="Downloaded source path; Git checkouts must use the standard work directory layout.", +) @click.argument("dist_name") @click.argument("dist_version", type=clickext.PackageVersion()) @click.pass_obj def prepare_source( wkctx: context.WorkContext, + source_kind: str, + source_path: pathlib.Path | None, dist_name: str, dist_version: Version, ) -> None: @@ -78,22 +118,35 @@ def prepare_source( """ req = Requirement(f"{dist_name}=={dist_version}") token = requirement_ctxvar.set(req) + download_kind = packagesettings.DownloadKind(source_kind) sdists_downloads = pathlib.Path(wkctx.sdists_repo) / "downloads" - source_filename = finders.find_sdist( - wkctx, wkctx.sdists_downloads, req, str(dist_version) + checkout = ( + wkctx.work_dir / f"{req.name}-{dist_version}" / f"{req.name}-{dist_version}" ) + source_filename: pathlib.Path | None + if source_path is not None: + source_filename = source_path + elif download_kind is packagesettings.DownloadKind.git_checkout: + source_filename = checkout if checkout.is_dir() else None + else: + source_filename = finders.find_sdist( + wkctx, wkctx.sdists_downloads, req, str(dist_version) + ) if source_filename is None: + if download_kind is packagesettings.DownloadKind.git_checkout: + raise RuntimeError(f"Cannot find Git checkout for {req} at {checkout}") dir_contents: list[str] = [] for ext in ["*.tar.gz", "*.zip"]: dir_contents.extend(str(e) for e in wkctx.sdists_downloads.glob(ext)) raise RuntimeError( - f"Cannot find sdist for {req.name} version {dist_version} in {sdists_downloads} among {dir_contents}" + f"Cannot find source for {req.name} version {dist_version} in {sdists_downloads} among {dir_contents}" ) source_root_dir = sources.prepare_source( ctx=wkctx, req=req, source_filename=source_filename, version=dist_version, + download_kind=download_kind, ) requirement_ctxvar.reset(token) print(source_root_dir) diff --git a/src/fromager/packagesettings/_resolver.py b/src/fromager/packagesettings/_resolver.py index ac57756ac..363f536f8 100644 --- a/src/fromager/packagesettings/_resolver.py +++ b/src/fromager/packagesettings/_resolver.py @@ -7,6 +7,7 @@ import logging import pathlib import re +import shutil import typing import pydantic @@ -156,6 +157,14 @@ def _download( / f"{req.name}-{candidate.version}" / f"{req.name}-{candidate.version}" ) + if destination_dir.parent.exists(): + if ctx.cleanup: + shutil.rmtree(destination_dir.parent) + elif destination_dir.exists(): + raise FileExistsError( + f"Git checkout {destination_dir} already exists; " + "enable cleanup or use a new work directory to download it again" + ) path = downloads.download_git_source( destination_dir=destination_dir, vcs_url=candidate.url, diff --git a/src/fromager/sources.py b/src/fromager/sources.py index efd2f20ea..ec36a4782 100644 --- a/src/fromager/sources.py +++ b/src/fromager/sources.py @@ -5,6 +5,7 @@ import logging import pathlib import shutil +import subprocess import tarfile import typing import warnings @@ -293,6 +294,9 @@ def unpack_source( source_filename: pathlib.Path, ) -> tuple[pathlib.Path, bool]: """Extracts a downloaded source archive (.tar.gz or .zip file) into a standardized directory.""" + if source_filename.is_dir(): + raise ValueError(f"{source_filename} is a directory, not a source archive") + # sdist names are less standardized and the names of the directories they # contain are also not very standard. Force the names into a predictable # form based on the override module name for the requirement. @@ -304,6 +308,17 @@ def unpack_source( unpack_dir = ctx.work_dir / expected_name if unpack_dir.exists(): if ctx.cleanup: + resolved_unpack_dir = unpack_dir.resolve() + # Deleting a parent can also break a path that uses ".." or a symlink. + source_paths = (source_filename, *source_filename.parents) + if any( + path.resolve().is_relative_to(resolved_unpack_dir) + for path in source_paths + ): + raise ValueError( + f"source archive path {source_filename} overlaps {unpack_dir}; " + "use an archive path outside the directory being cleaned" + ) logger.debug("cleaning up %s", unpack_dir) shutil.rmtree(unpack_dir) else: @@ -412,18 +427,43 @@ def prepare_source( req: Requirement, source_filename: pathlib.Path, version: Version, + download_kind: packagesettings.DownloadKind = packagesettings.DownloadKind.sdist, ) -> pathlib.Path: - """Unpack and prepare source for building. - - Calls ``find_and_invoke`` which may call a package-specific override. - The plugin may return a ``Path`` or a ``(Path, bool)`` tuple; both - forms are handled. + """Prepare an archive or checkout according to its downloaded kind. + + Path-only callers default to an sdist. Package-specific ``prepare_source`` + overrides may replace the selected default, including when a source + resolver is configured. Overrides receive the original path-based + arguments and may return a ``Path`` or a ``(Path, bool)`` pair. + Git-specific overrides can call ``default_prepare_git_checkout()``. + + Args: + ctx: Work context for the build. + req: Package requirement being prepared. + source_filename: Downloaded archive or checkout path. + version: Resolved package version. + download_kind: Artifact kind; defaults to an sdist. + + Returns: + Path to the prepared source root. + + .. versionchanged:: 0.102.0 + Pass ``download_kind`` to prepare a Git checkout. """ logger.info(f"preparing source for {req} from {source_filename}") + match download_kind: + case packagesettings.DownloadKind.sdist | packagesettings.DownloadKind.tarball: + default_fn = default_prepare_source + case packagesettings.DownloadKind.git_checkout: + default_fn = default_prepare_git_checkout + case _: + raise ValueError( + f"cannot prepare source with download kind {download_kind}" + ) prepare_source_details = overrides.find_and_invoke( req.name, "prepare_source", - default_prepare_source, + default_fn, ctx=ctx, req=req, source_filename=source_filename, @@ -450,7 +490,7 @@ def default_prepare_source( source_filename: pathlib.Path, version: Version, ) -> tuple[pathlib.Path, bool]: - """Unpack, modify, and check sdist sources + """Unpack an archive, then prepare newly extracted source. Calls :func:`~fromager.sources.prepare_new_source` by default. """ @@ -460,7 +500,79 @@ def default_prepare_source( version=version, source_filename=source_filename, ) - if is_new: + return _prepare_source_root( + ctx, req, source_root_dir, version, should_prepare=is_new + ) + + +def default_prepare_git_checkout( + ctx: context.WorkContext, + req: Requirement, + source_filename: pathlib.Path, + version: Version, +) -> tuple[pathlib.Path, bool]: + """Prepare a clean Git checkout in place. + + Reject tracked, untracked, or ignored local changes in the checkout and its + initialized submodules before applying patches, project overrides, and Rust + vendoring. A clean status does not prove where the checkout came from or + that it will remain unchanged after preparation. + + Args: + ctx: Work context for the build. + req: Package requirement being prepared. + source_filename: Path to the Git checkout. + version: Resolved package version. + + Returns: + The checkout path and ``True`` because preparation ran. + + .. versionadded:: 0.102.0 + """ + if not source_filename.is_dir() or not (source_filename / ".git").exists(): + raise ValueError(f"{source_filename} is not a Git checkout") + status_command = ( + "git", + "status", + "--porcelain=v1", + "--untracked-files=all", + "--ignored=matching", + "--ignore-submodules=none", + ) + try: + changes = external_commands.run(status_command, cwd=str(source_filename)) + if not changes: + # A parent repository can hide changes inside ignored submodules. + changes = external_commands.run( + [ + "git", + "submodule", + "--quiet", + "foreach", + "--recursive", + " ".join(status_command), + ], + cwd=str(source_filename), + ) + except subprocess.CalledProcessError as err: + raise ValueError(f"cannot inspect Git checkout {source_filename}") from err + if changes: + raise ValueError( + f"Git checkout {source_filename} has local changes; " + "download a fresh checkout before preparing" + ) + return _prepare_source_root(ctx, req, source_filename, version, should_prepare=True) + + +def _prepare_source_root( + ctx: context.WorkContext, + req: Requirement, + source_root_dir: pathlib.Path, + version: Version, + should_prepare: bool, +) -> tuple[pathlib.Path, bool]: + """Run requested preparation and scan the resulting source tree.""" + if should_prepare: prepare_new_source( ctx=ctx, req=req, @@ -476,7 +588,7 @@ def default_prepare_source( ", ".join(str(f) for f in sorted(potential_issues)), ) - return source_root_dir, is_new + return source_root_dir, should_prepare def prepare_new_source( @@ -491,7 +603,8 @@ def prepare_new_source( - apply project overrides from settings - vendor Rust dependencies - :func:`~default_prepare_source` runs this function when the sources are new. + :func:`~default_prepare_source` and + :func:`~default_prepare_git_checkout` run this function for new sources. """ patch_source(ctx, source_root_dir, req, version) pyproject.apply_project_override( diff --git a/tests/test_bootstrapper.py b/tests/test_bootstrapper.py index 00b8a0044..0d775339c 100644 --- a/tests/test_bootstrapper.py +++ b/tests/test_bootstrapper.py @@ -1,6 +1,7 @@ import json import logging import pathlib +import subprocess import typing from unittest.mock import Mock, patch @@ -1200,6 +1201,7 @@ def test_bg_prepare_source_log_prefix_includes_version( req=req, source_filename=source_filename, version=version, + download_kind=DownloadKind.sdist, ) finally: logging.setLogRecordFactory(old_factory) @@ -1210,10 +1212,42 @@ def test_bg_prepare_source_log_prefix_includes_version( ) -def test_bootstrapper_download_source_returns_downloaded_path( +def test_bg_prepare_source_accepts_git_checkout(tmp_context: WorkContext) -> None: + req = Requirement("mypkg==1.2.3") + version = Version("1.2.3") + checkout = tmp_context.work_dir / "mypkg-1.2.3" / "mypkg-1.2.3" + checkout.mkdir(parents=True) + subprocess.run(["git", "init", str(checkout)], check=True, capture_output=True) + + with ( + patch( + "fromager.bootstrapper._cache.find_cached_wheel", return_value=(None, None) + ), + patch( + "fromager.sources.download_source", + return_value=DownloadedSource(checkout, DownloadKind.git_checkout), + ), + patch("fromager.sources.prepare_new_source") as prepare_new_source, + ): + result = _bg_prepare_source( + ctx=tmp_context, + cache_wheel_server_url=None, + req=req, + resolved_version=version, + source_url="git+https://code.test/org/mypkg.git@v1.2.3", + ) + + assert result.sdist_root_dir == checkout + prepare_new_source.assert_called_once_with( + ctx=tmp_context, req=req, source_root_dir=checkout, version=version + ) + assert (checkout.parent / "build-meta.json").is_file() + + +def test_bootstrapper_download_source_preserves_download_kind( tmp_context: WorkContext, ) -> None: - """Return only the path from the resolver download result.""" + """Keep the artifact kind for the legacy Bootstrapper helper.""" bt = bootstrapper.Bootstrapper(tmp_context) req = Requirement("mypkg==1.2.3") version = Version("1.2.3") @@ -1226,7 +1260,7 @@ def test_bootstrapper_download_source_returns_downloaded_path( ) as download_source: result = bt._download_source(req, version, source_url) - assert result == source_filename + assert result == DownloadedSource(source_filename, DownloadKind.sdist) download_source.assert_called_once_with( ctx=tmp_context, req=req, @@ -1235,6 +1269,33 @@ def test_bootstrapper_download_source_returns_downloaded_path( ) +def test_bootstrapper_prepare_source_passes_download_kind( + tmp_context: WorkContext, +) -> None: + bt = bootstrapper.Bootstrapper(tmp_context) + req = Requirement("mypkg==1.2.3") + version = Version("1.2.3") + downloaded = DownloadedSource( + tmp_context.work_dir / "mypkg-1.2.3" / "mypkg-1.2.3", + DownloadKind.git_checkout, + ) + + with patch( + "fromager.bootstrapper._bootstrapper.sources.prepare_source", + return_value=downloaded.path, + ) as prepare_source: + result = bt._prepare_source(req, version, downloaded) + + assert result == downloaded.path + prepare_source.assert_called_once_with( + ctx=tmp_context, + req=req, + source_filename=downloaded.path, + version=version, + download_kind=DownloadKind.git_checkout, + ) + + def test_bg_prepare_prebuilt_log_prefix_includes_version( tmp_context: WorkContext, caplog: pytest.LogCaptureFixture, diff --git a/tests/test_commands.py b/tests/test_commands.py index 17572993b..6b4ae406c 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -1,4 +1,6 @@ +import json import pathlib +import subprocess import typing from unittest.mock import Mock, patch @@ -34,7 +36,7 @@ def test_download_source_archive_prints_downloaded_path( tmp_context: WorkContext, tmp_path: pathlib.Path, ) -> None: - """Print only the path when the download helper returns a tuple.""" + """Keep the path-only output for existing scripts.""" expected = tmp_path / "pkg-1.0.tar.gz" with ( @@ -66,11 +68,181 @@ def test_download_source_archive_prints_downloaded_path( assert result.output == f"{expected}\n" -def test_build_passes_downloaded_path_to_prepare_source( +def test_download_source_archive_json_includes_kind( + cli_runner: CliRunner, + tmp_context: WorkContext, + tmp_path: pathlib.Path, +) -> None: + checkout = tmp_path / "pkg-1.0" + + with ( + patch.object( + step.sources, + "resolve_source", + return_value=("git+https://code.test/pkg.git@v1.0", Version("1.0")), + ), + patch.object( + step.sources, + "download_source", + return_value=packagesettings.DownloadedSource( + checkout, packagesettings.DownloadKind.git_checkout + ), + ), + ): + result = cli_runner.invoke( + step.step, + [ + "download-source-archive", + "--output-format", + "json", + "pkg", + "1.0", + "https://pkg.test/simple", + ], + obj=tmp_context, + ) + + assert result.exit_code == 0, result.output + assert json.loads(result.output) == { + "path": str(checkout), + "kind": "git_checkout", + } + + +def test_step_prepare_source_prepares_clean_git_checkout( + cli_runner: CliRunner, tmp_context: WorkContext +) -> None: + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + subprocess.run(["git", "init", str(checkout)], check=True, capture_output=True) + + with patch.object(step.sources, "prepare_new_source") as prepare_new_source: + result = cli_runner.invoke( + step.step, + ["prepare-source", "--source-kind", "git_checkout", "pkg", "1.0"], + obj=tmp_context, + ) + + assert result.exit_code == 0, result.output + assert result.output == f"{checkout}\n" + prepare_new_source.assert_called_once_with( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + source_root_dir=checkout, + version=Version("1.0"), + ) + assert (checkout.parent / "build-meta.json").is_file() + + +def test_step_prepare_source_kind_selects_checkout_over_archive( + cli_runner: CliRunner, tmp_context: WorkContext +) -> None: + archive = tmp_context.sdists_downloads / "pkg-1.0.tar.gz" + archive.write_bytes(b"old archive") + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + with patch.object( + step.sources, "prepare_source", return_value=checkout + ) as prepare_source: + result = cli_runner.invoke( + step.step, + ["prepare-source", "--source-kind", "git_checkout", "pkg", "1.0"], + obj=tmp_context, + ) + + assert result.exit_code == 0 + assert result.output == f"{checkout}\n" + prepare_source.assert_called_once_with( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + source_filename=checkout, + version=Version("1.0"), + download_kind=packagesettings.DownloadKind.git_checkout, + ) + + +def test_step_prepare_source_defaults_to_archive_kind( + cli_runner: CliRunner, tmp_context: WorkContext +) -> None: + archive = tmp_context.sdists_downloads / "pkg-1.0.tar.gz" + archive.write_bytes(b"archive") + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + + with patch.object( + step.sources, "prepare_source", return_value=checkout + ) as prepare_source: + result = cli_runner.invoke( + step.step, ["prepare-source", "pkg", "1.0"], obj=tmp_context + ) + + assert result.exit_code == 0, result.output + prepare_source.assert_called_once_with( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + source_filename=archive, + version=Version("1.0"), + download_kind=packagesettings.DownloadKind.sdist, + ) + + +def test_step_prepare_source_accepts_explicit_hook_path( + cli_runner: CliRunner, tmp_context: WorkContext, tmp_path: pathlib.Path +) -> None: + checkout = tmp_path / "hook-checkout" + checkout.mkdir() + + with patch.object( + step.sources, "prepare_source", return_value=checkout + ) as prepare_source: + result = cli_runner.invoke( + step.step, + [ + "prepare-source", + "--source-kind", + "git_checkout", + "--source-path", + str(checkout), + "pkg", + "1.0", + ], + obj=tmp_context, + ) + + assert result.exit_code == 0, result.output + prepare_source.assert_called_once_with( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + source_filename=checkout, + version=Version("1.0"), + download_kind=packagesettings.DownloadKind.git_checkout, + ) + + +def test_step_prepare_source_keeps_archive_inside_cleanup_dir( + cli_runner: CliRunner, tmp_context: WorkContext +) -> None: + """Reject an archive that cleanup would delete before opening it.""" + archive = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0.tar.gz" + archive.parent.mkdir(parents=True) + archive.write_bytes(b"original archive") + + result = cli_runner.invoke( + step.step, + ["prepare-source", "--source-path", str(archive), "pkg", "1.0"], + obj=tmp_context, + ) + + assert isinstance(result.exception, ValueError) + assert "overlaps" in str(result.exception) + assert archive.read_bytes() == b"original archive" + + +def test_build_passes_downloaded_kind_to_prepare_source( tmp_context: WorkContext, tmp_path: pathlib.Path, ) -> None: - """Pass only the downloaded path into source preparation.""" + """Pass the downloaded path and kind into source preparation.""" req = Requirement("pkg==1.0") version = Version("1.0") source_url = "https://pkg.test/pkg-1.0.tar.gz" @@ -119,5 +291,6 @@ def test_build_passes_downloaded_path_to_prepare_source( req=req, source_filename=source_filename, version=version, + download_kind=packagesettings.DownloadKind.sdist, ) assert result.wheel_filename == tmp_context.wheels_downloads / wheel_filename.name diff --git a/tests/test_packagesettings_resolver.py b/tests/test_packagesettings_resolver.py index 8a942d12c..749285231 100644 --- a/tests/test_packagesettings_resolver.py +++ b/tests/test_packagesettings_resolver.py @@ -4,6 +4,7 @@ import datetime import pathlib import re +import subprocess import textwrap import typing from unittest import mock @@ -15,7 +16,7 @@ from packaging.version import Version from wheel.wheelfile import WheelFile -from fromager import resolver +from fromager import resolver, sources from fromager.candidate import Candidate, Cooldown from fromager.context import WorkContext from fromager.packagesettings._resolver import ( @@ -407,6 +408,131 @@ def test_download(self, mock_dl: mock.MagicMock, tmp_context: WorkContext) -> No vcs_url=_CANDIDATE_GIT.url, ) + def test_download_rejects_existing_checkout_without_cleanup( + self, tmp_context: WorkContext + ) -> None: + tmp_context.cleanup = False + checkout = ( + tmp_context.work_dir / f"{_REQ.name}-{_VERSION}" / f"{_REQ.name}-{_VERSION}" + ) + checkout.mkdir(parents=True) + (checkout / ".git").mkdir() + (checkout / "keep.txt").write_text("prepared") + + with pytest.raises(FileExistsError, match="enable cleanup"): + _parse(self.YAML).download(tmp_context, _REQ, _CANDIDATE_GIT) + + assert (checkout / "keep.txt").read_text() == "prepared" + + def test_download_cleans_checkout_before_reclone( + self, tmp_context: WorkContext + ) -> None: + checkout = ( + tmp_context.work_dir / f"{_REQ.name}-{_VERSION}" / f"{_REQ.name}-{_VERSION}" + ) + checkout.mkdir(parents=True) + (checkout / "old.txt").write_text("old") + (checkout.parent / "build-meta.json").write_text("old") + + def clone(*, destination_dir: pathlib.Path, vcs_url: str) -> pathlib.Path: + assert vcs_url == _CANDIDATE_GIT.url + destination_dir.mkdir(parents=True) + (destination_dir / "new.txt").write_text("new") + return destination_dir + + with mock.patch("fromager.downloads.download_git_source", side_effect=clone): + result = _parse(self.YAML).download(tmp_context, _REQ, _CANDIDATE_GIT) + + assert result == DownloadedSource(checkout, DownloadKind.git_checkout) + assert (checkout / "new.txt").read_text() == "new" + assert not (checkout / "old.txt").exists() + assert not (checkout.parent / "build-meta.json").exists() + + def test_download_preserves_non_checkout_on_collision( + self, tmp_context: WorkContext + ) -> None: + tmp_context.cleanup = False + checkout = ( + tmp_context.work_dir / f"{_REQ.name}-{_VERSION}" / f"{_REQ.name}-{_VERSION}" + ) + checkout.mkdir(parents=True) + (checkout / "keep.txt").write_text("existing source") + meta_file = checkout.parent / "build-meta.json" + meta_file.write_text("existing metadata") + + with pytest.raises(FileExistsError, match="already exists"): + _parse(self.YAML).download(tmp_context, _REQ, _CANDIDATE_GIT) + + assert (checkout / "keep.txt").read_text() == "existing source" + assert meta_file.read_text() == "existing metadata" + + def test_local_git_checkout_prepares_and_reclones( + self, tmp_context: WorkContext, tmp_path: pathlib.Path + ) -> None: + repo = tmp_path / "repo" + repo.mkdir() + subprocess.run(["git", "init", str(repo)], check=True, capture_output=True) + (repo / "pyproject.toml").write_text("[build-system]\nrequires = []\n") + subprocess.run(["git", "-C", str(repo), "add", "."], check=True) + subprocess.run( + [ + "git", + "-C", + str(repo), + "-c", + "user.name=Test", + "-c", + "user.email=test@pkg.test", + "commit", + "-m", + "Initial source", + ], + check=True, + capture_output=True, + ) + subprocess.run(["git", "-C", str(repo), "tag", "v1.2.3"], check=True) + candidate = Candidate( + name=_REQ.name, + version=_VERSION, + url=f"git+file://{repo}@refs/tags/v1.2.3", + ) + source_resolver = _parse(self.YAML) + tmp_context.cleanup = False + package_dir = tmp_context.work_dir / f"{_REQ.name}-{_VERSION}" + (package_dir / "other-dir").mkdir(parents=True) + (package_dir / "build-meta.json").write_text("stale") + + downloaded = source_resolver.download(tmp_context, _REQ, candidate) + assert (package_dir / "other-dir").is_dir() + assert (package_dir / "build-meta.json").read_text() == "stale" + prepared = sources.prepare_source( + ctx=tmp_context, + req=_REQ, + source_filename=downloaded.path, + version=_VERSION, + download_kind=downloaded.kind, + ) + (prepared / "keep.txt").write_text("prepared") + with pytest.raises(FileExistsError, match="already exists"): + source_resolver.download(tmp_context, _REQ, candidate) + + assert downloaded.kind is DownloadKind.git_checkout + assert prepared == downloaded.path + assert (prepared / ".git").is_dir() + assert (prepared / "keep.txt").read_text() == "prepared" + assert sources.read_build_meta(prepared.parent)["source-filename"] == str( + prepared + ) + + tmp_context.cleanup = True + recloned = source_resolver.download(tmp_context, _REQ, candidate) + + assert recloned.path == prepared + assert (recloned.path / ".git").is_dir() + assert not (recloned.path / "keep.txt").exists() + assert not (package_dir / "other-dir").exists() + assert not (recloned.path.parent / "build-meta.json").exists() + # -- Git source resolvers ---------------------------------------------------- diff --git a/tests/test_sources.py b/tests/test_sources.py index 2816db5b3..1059c8156 100644 --- a/tests/test_sources.py +++ b/tests/test_sources.py @@ -1,5 +1,6 @@ import datetime import pathlib +import subprocess import sys import tarfile import zipfile @@ -788,6 +789,99 @@ def test_unpack_source_unknown_extension( ) +@pytest.mark.parametrize("cleanup", [True, False]) +def test_unpack_source_rejects_directory_before_cleanup( + tmp_context: context.WorkContext, + cleanup: bool, +) -> None: + req = Requirement("pkg==1.0") + version = Version("1.0") + tmp_context.cleanup = cleanup + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + (checkout / "keep.txt").write_text("original") + + with pytest.raises(ValueError, match="is a directory, not a source archive"): + sources.unpack_source( + ctx=tmp_context, req=req, version=version, source_filename=checkout + ) + + assert (checkout / "keep.txt").read_text() == "original" + + +def test_unpack_source_rejects_archive_path_through_cleanup_dir( + tmp_context: context.WorkContext, +) -> None: + """A path using the soon-to-be-deleted directory stops working after cleanup.""" + unpack_dir = tmp_context.work_dir / "pkg-1.0" + unpack_dir.mkdir() + archive = tmp_context.work_dir / "pkg-1.0.tar.gz" + archive.write_bytes(b"original archive") + source_path = unpack_dir / ".." / archive.name + + with pytest.raises(ValueError, match="overlaps"): + sources.unpack_source( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + version=Version("1.0"), + source_filename=source_path, + ) + + assert unpack_dir.is_dir() + assert archive.read_bytes() == b"original archive" + + +def test_unpack_source_rejects_symlink_into_cleanup_dir( + tmp_context: context.WorkContext, +) -> None: + """Reject an external symlink whose archive target would be deleted.""" + unpack_dir = tmp_context.work_dir / "pkg-1.0" + unpack_dir.mkdir() + archive = unpack_dir / "pkg-1.0.tar.gz" + archive.write_bytes(b"original archive") + source_path = tmp_context.work_dir / "pkg-1.0.tar.gz" + source_path.symlink_to(archive) + + with pytest.raises(ValueError, match="overlaps"): + sources.unpack_source( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + version=Version("1.0"), + source_filename=source_path, + ) + + assert archive.read_bytes() == b"original archive" + + +def test_unpack_source_replaces_existing_tree_from_external_archive( + tmp_context: context.WorkContext, + tmp_path: pathlib.Path, +) -> None: + """Keep the normal cleanup path for an archive outside the work tree.""" + unpack_dir = tmp_context.work_dir / "pkg-1.0" + unpack_dir.mkdir() + (unpack_dir / "stale.txt").write_text("old output") + content_dir = tmp_path / "content" / "pkg-1.0" + content_dir.mkdir(parents=True) + (content_dir / "setup.py").write_text("# setup") + archive = tmp_path / "pkg-1.0.tar.gz" + with tarfile.open(archive, "w:gz") as tar: + tar.add(content_dir, arcname=content_dir.name) + + source_root, is_new = sources.unpack_source( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + version=Version("1.0"), + source_filename=archive, + ) + + assert is_new is True + assert source_root == unpack_dir / "pkg-1.0" + assert (source_root / "setup.py").read_text() == "# setup" + assert not (unpack_dir / "stale.txt").exists() + assert archive.is_file() + + def test_unpack_source_renames_mismatched_dir( tmp_context: context.WorkContext, tmp_path: pathlib.Path, @@ -1021,6 +1115,509 @@ def test_prepare_source_plugin_returns_path( assert result == source_dir +def test_prepare_source_keeps_override_with_configured_resolver( + tmp_context: context.WorkContext, +) -> None: + req = Requirement("pkg==1.0") + version = Version("1.0") + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + downloaded = packagesettings.DownloadedSource( + path=checkout, kind=packagesettings.DownloadKind.git_checkout + ) + source_resolver = Mock() + source_resolver.download.return_value = downloaded + + def prepare_override( + ctx: context.WorkContext, + req: Requirement, + source_filename: pathlib.Path, + version: Version, + ) -> pathlib.Path: + assert ctx is tmp_context + assert req == Requirement("pkg==1.0") + assert source_filename == checkout + assert version == Version("1.0") + return checkout + + with ( + patch.object( + packagesettings.PackageBuildInfo, + "source_resolver", + new_callable=PropertyMock, + return_value=source_resolver, + ), + patch( + "fromager.overrides.find_override_method", return_value=prepare_override + ) as find_override_method, + ): + result = sources.download_source( + ctx=tmp_context, + req=req, + version=version, + download_url="git+https://code.test/org/pkg.git@v1.0", + ) + prepared = sources.prepare_source( + ctx=tmp_context, + req=req, + source_filename=result.path, + version=version, + download_kind=result.kind, + ) + + assert result is downloaded + assert prepared == checkout + source_resolver.download.assert_called_once_with( + tmp_context, + req, + Candidate( + name=req.name, + version=version, + url="git+https://code.test/org/pkg.git@v1.0", + ), + ) + find_override_method.assert_called_once_with(req.name, "prepare_source") + assert (checkout.parent / "build-meta.json").is_file() + + +def test_prepare_source_git_checkout_rejects_dirty_retry( + tmp_context: context.WorkContext, +) -> None: + req = Requirement("pkg==1.0") + version = Version("1.0") + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + subprocess.run(["git", "init", str(checkout)], check=True, capture_output=True) + + def add_prepared_file(**_: object) -> None: + (checkout / "prepared.txt").write_text("patched") + + with patch( + "fromager.sources.prepare_new_source", side_effect=add_prepared_file + ) as prepare_new_source: + result = sources.prepare_source( + ctx=tmp_context, + req=req, + source_filename=checkout, + version=version, + download_kind=packagesettings.DownloadKind.git_checkout, + ) + with pytest.raises(ValueError, match="has local changes"): + sources.prepare_source( + ctx=tmp_context, + req=req, + source_filename=checkout, + version=version, + download_kind=packagesettings.DownloadKind.git_checkout, + ) + + assert result == checkout + assert (checkout / "prepared.txt").read_text() == "patched" + prepare_new_source.assert_called_once_with( + ctx=tmp_context, req=req, source_root_dir=checkout, version=version + ) + assert sources.read_build_meta(checkout.parent) == { + "req": "pkg==1.0", + "source-filename": str(checkout), + "version": "1.0", + } + + +def test_prepare_source_git_checkout_rejects_ignored_files( + tmp_context: context.WorkContext, +) -> None: + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + subprocess.run(["git", "init", str(checkout)], check=True, capture_output=True) + (checkout / ".gitignore").write_text("build/\n") + subprocess.run(["git", "-C", str(checkout), "add", ".gitignore"], check=True) + subprocess.run( + [ + "git", + "-C", + str(checkout), + "-c", + "user.name=Test", + "-c", + "user.email=test@pkg.test", + "commit", + "-m", + "Ignore build outputs", + ], + check=True, + capture_output=True, + ) + (checkout / "build").mkdir() + (checkout / "build" / "generated.txt").write_text("generated") + + with patch("fromager.sources.prepare_new_source") as prepare_new_source: + with pytest.raises(ValueError, match="has local changes"): + sources.prepare_source( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + source_filename=checkout, + version=Version("1.0"), + download_kind=packagesettings.DownloadKind.git_checkout, + ) + + prepare_new_source.assert_not_called() + + +@pytest.mark.parametrize("change_kind", ["tracked", "ignored"]) +def test_prepare_source_git_checkout_rejects_nested_submodule_changes( + tmp_context: context.WorkContext, + tmp_path: pathlib.Path, + change_kind: str, +) -> None: + def run_git(repo: pathlib.Path, *args: str) -> str: + result = subprocess.run( + ["git", "-C", str(repo), *args], + check=True, + capture_output=True, + text=True, + ) + return result.stdout + + leaf = tmp_path / "leaf" + leaf.mkdir() + run_git(leaf, "init") + (leaf / "tracked.txt").write_text("original") + (leaf / ".gitignore").write_text("build/\n") + run_git(leaf, "add", ".") + run_git( + leaf, + "-c", + "user.name=Test", + "-c", + "user.email=test@pkg.test", + "commit", + "-m", + "Initial leaf", + ) + + middle = tmp_path / "middle" + middle.mkdir() + run_git(middle, "init") + run_git( + middle, + "-c", + "protocol.file.allow=always", + "submodule", + "add", + str(leaf), + "leaf", + ) + run_git(middle, "config", "-f", ".gitmodules", "submodule.leaf.ignore", "all") + run_git(middle, "add", ".") + run_git( + middle, + "-c", + "user.name=Test", + "-c", + "user.email=test@pkg.test", + "commit", + "-m", + "Initial middle", + ) + + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + run_git(checkout, "init") + run_git( + checkout, + "-c", + "protocol.file.allow=always", + "submodule", + "add", + str(middle), + "middle", + ) + run_git(checkout, "config", "-f", ".gitmodules", "submodule.middle.ignore", "all") + run_git(checkout, "add", ".") + run_git( + checkout, + "-c", + "user.name=Test", + "-c", + "user.email=test@pkg.test", + "commit", + "-m", + "Initial checkout", + ) + run_git( + checkout, + "-c", + "protocol.file.allow=always", + "submodule", + "update", + "--init", + "--recursive", + ) + + nested_checkout = checkout / "middle" / "leaf" + if change_kind == "tracked": + (nested_checkout / "tracked.txt").write_text("changed") + else: + (nested_checkout / "build").mkdir() + (nested_checkout / "build" / "generated.txt").write_text("generated") + + assert ( + run_git( + checkout, + "status", + "--porcelain=v1", + "--untracked-files=all", + "--ignored=matching", + ) + == "" + ) + with patch("fromager.sources.prepare_new_source") as prepare_new_source: + with pytest.raises(ValueError, match="has local changes"): + sources.prepare_source( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + source_filename=checkout, + version=Version("1.0"), + download_kind=packagesettings.DownloadKind.git_checkout, + ) + + prepare_new_source.assert_not_called() + + +def test_prepare_source_checkout_ignores_stale_build_metadata( + tmp_context: context.WorkContext, +) -> None: + req = Requirement("pkg==1.0") + version = Version("1.0") + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + subprocess.run(["git", "init", str(checkout)], check=True, capture_output=True) + (checkout.parent / "build-meta.json").write_text("stale") + + with patch("fromager.sources.prepare_new_source") as prepare_new_source: + result = sources.prepare_source( + ctx=tmp_context, + req=req, + source_filename=checkout, + version=version, + download_kind=packagesettings.DownloadKind.git_checkout, + ) + + assert result == checkout + prepare_new_source.assert_called_once() + assert sources.read_build_meta(checkout.parent)["source-filename"] == str(checkout) + + +def test_prepare_source_git_checkout_runs_preparation_steps( + tmp_context: context.WorkContext, +) -> None: + req = Requirement("pkg==1.0") + version = Version("1.0") + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + subprocess.run(["git", "init", str(checkout)], check=True, capture_output=True) + + with ( + patch("fromager.sources.patch_source") as patch_source, + patch( + "fromager.sources.pyproject.apply_project_override" + ) as apply_project_override, + patch("fromager.sources.vendor_rust.vendor_rust") as vendor_rust, + ): + result = sources.prepare_source( + ctx=tmp_context, + req=req, + source_filename=checkout, + version=version, + download_kind=packagesettings.DownloadKind.git_checkout, + ) + + assert result == checkout + patch_source.assert_called_once_with(tmp_context, checkout, req, version) + apply_project_override.assert_called_once_with( + ctx=tmp_context, req=req, sdist_root_dir=checkout + ) + vendor_rust.assert_called_once_with( + req, tmp_context.package_build_info(req).build_dir(checkout) + ) + + +def test_prepare_source_rejects_non_git_directory( + tmp_context: context.WorkContext, +) -> None: + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + + with pytest.raises(ValueError, match="is not a Git checkout"): + sources.prepare_source( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + source_filename=checkout, + version=Version("1.0"), + download_kind=packagesettings.DownloadKind.git_checkout, + ) + + +def test_prepare_source_git_kind_rejects_archive_file( + tmp_context: context.WorkContext, +) -> None: + archive = tmp_context.sdists_downloads / "pkg-1.0.tar.gz" + archive.write_bytes(b"archive") + + with pytest.raises(ValueError, match="is not a Git checkout"): + sources.prepare_source( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + source_filename=archive, + version=Version("1.0"), + download_kind=packagesettings.DownloadKind.git_checkout, + ) + + assert archive.read_bytes() == b"archive" + + +def test_prepare_source_path_only_rejects_checkout_without_removing_it( + tmp_context: context.WorkContext, +) -> None: + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + (checkout / "keep.txt").write_text("original") + + with pytest.raises(ValueError, match="is a directory, not a source archive"): + sources.prepare_source( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + source_filename=checkout, + version=Version("1.0"), + ) + + assert (checkout / "keep.txt").read_text() == "original" + + +@pytest.mark.parametrize( + "download_kind", + [packagesettings.DownloadKind.sdist, packagesettings.DownloadKind.tarball], +) +def test_prepare_source_archive_still_unpacks_and_prepares( + tmp_context: context.WorkContext, + tmp_path: pathlib.Path, + download_kind: packagesettings.DownloadKind, +) -> None: + req = Requirement("pkg==1.0") + version = Version("1.0") + content = tmp_path / "content" / "pkg-1.0" + content.mkdir(parents=True) + (content / "pyproject.toml").write_text("[build-system]\n") + archive = tmp_path / "pkg-1.0.tar.gz" + with tarfile.open(archive, "w:gz") as tar: + tar.add(content, arcname="pkg-1.0") + + with patch("fromager.sources.prepare_new_source") as prepare_new_source: + result = sources.prepare_source( + ctx=tmp_context, + req=req, + source_filename=archive, + version=version, + download_kind=download_kind, + ) + + expected = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + assert result == expected + assert (result / "pyproject.toml").is_file() + prepare_new_source.assert_called_once_with( + ctx=tmp_context, req=req, source_root_dir=expected, version=version + ) + assert sources.read_build_meta(result.parent)["source-filename"] == str(archive) + + +def test_prepare_source_checkout_keeps_override_contract( + tmp_context: context.WorkContext, +) -> None: + req = Requirement("pkg==1.0") + version = Version("1.0") + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + + with patch( + "fromager.sources.overrides.find_and_invoke", return_value=(checkout, True) + ) as invoke: + result = sources.prepare_source( + ctx=tmp_context, + req=req, + source_filename=checkout, + version=version, + download_kind=packagesettings.DownloadKind.git_checkout, + ) + + assert result == checkout + invoke.assert_called_once_with( + req.name, + "prepare_source", + sources.default_prepare_git_checkout, + ctx=tmp_context, + req=req, + source_filename=checkout, + version=version, + ) + assert (checkout.parent / "build-meta.json").is_file() + + +def test_prepare_source_git_override_calls_public_default( + tmp_context: context.WorkContext, +) -> None: + req = Requirement("pkg==1.0") + version = Version("1.0") + checkout = tmp_context.work_dir / "pkg-1.0" / "pkg-1.0" + checkout.mkdir(parents=True) + subprocess.run(["git", "init", str(checkout)], check=True, capture_output=True) + + def prepare_override( + ctx: context.WorkContext, + req: Requirement, + source_filename: pathlib.Path, + version: Version, + ) -> tuple[pathlib.Path, bool]: + assert ctx is tmp_context + assert req == Requirement("pkg==1.0") + assert source_filename == checkout + assert version == Version("1.0") + return sources.default_prepare_git_checkout(ctx, req, source_filename, version) + + with ( + patch("fromager.overrides.find_override_method", return_value=prepare_override), + patch("fromager.sources.prepare_new_source") as prepare_new_source, + ): + result = sources.prepare_source( + ctx=tmp_context, + req=req, + source_filename=checkout, + version=version, + download_kind=packagesettings.DownloadKind.git_checkout, + ) + + assert result == checkout + prepare_new_source.assert_called_once_with( + ctx=tmp_context, req=req, source_root_dir=checkout, version=version + ) + assert (checkout.parent / "build-meta.json").is_file() + + +def test_prepare_source_rejects_prebuilt_wheel_kind( + tmp_context: context.WorkContext, +) -> None: + with patch("fromager.sources.overrides.find_and_invoke") as invoke: + with pytest.raises(ValueError, match="cannot prepare source"): + sources.prepare_source( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + source_filename=tmp_context.wheels_prebuilt / "pkg-1.0.whl", + version=Version("1.0"), + download_kind=packagesettings.DownloadKind.prebuilt_wheel, + ) + + invoke.assert_not_called() + + @patch("fromager.sources.write_build_meta") @patch("fromager.overrides.find_and_invoke") def test_prepare_source_plugin_returns_tuple(