diff --git a/src/fromager/bootstrapper/_bootstrapper.py b/src/fromager/bootstrapper/_bootstrapper.py index a12e6753..d2cd7ffe 100644 --- a/src/fromager/bootstrapper/_bootstrapper.py +++ b/src/fromager/bootstrapper/_bootstrapper.py @@ -465,7 +465,8 @@ def _download_source( source_url: str, ) -> pathlib.Path: """Download source for a package.""" - result: pathlib.Path = sources.download_source( + result: pathlib.Path + result, _ = sources.download_source( ctx=self.ctx, req=req, version=resolved_version, diff --git a/src/fromager/bootstrapper/_prepare_source.py b/src/fromager/bootstrapper/_prepare_source.py index a0ba1da7..6076fdd2 100644 --- a/src/fromager/bootstrapper/_prepare_source.py +++ b/src/fromager/bootstrapper/_prepare_source.py @@ -43,7 +43,7 @@ def _bg_prepare_source( sdist_root_dir=unpacked / unpacked.stem, cached_wheel_filename=cached_wheel, ) - source_filename = sources.download_source( + source_filename, _ = sources.download_source( ctx=ctx, req=req, version=resolved_version, download_url=source_url ) sdist_root_dir = sources.prepare_source( diff --git a/src/fromager/commands/build.py b/src/fromager/commands/build.py index 2e33f8f0..831741bf 100644 --- a/src/fromager/commands/build.py +++ b/src/fromager/commands/build.py @@ -388,7 +388,7 @@ 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( + source_filename, _ = sources.download_source( ctx=wkctx, req=req, version=resolved_version, diff --git a/src/fromager/commands/step.py b/src/fromager/commands/step.py index 5df91ff6..197d06a1 100644 --- a/src/fromager/commands/step.py +++ b/src/fromager/commands/step.py @@ -50,7 +50,7 @@ def download_source_archive( source_url, version = sources.resolve_source( ctx=wkctx, req=req, sdist_server_url=sdist_server_url ) - filename = sources.download_source( + filename, _ = sources.download_source( ctx=wkctx, req=req, version=version, diff --git a/src/fromager/sources.py b/src/fromager/sources.py index 742741fd..fa915435 100644 --- a/src/fromager/sources.py +++ b/src/fromager/sources.py @@ -31,6 +31,7 @@ tarballs, vendor_rust, ) +from .candidate import Candidate from .requirements_file import RequirementType, SourceType if typing.TYPE_CHECKING: @@ -84,9 +85,20 @@ def download_source( req: Requirement, version: Version, download_url: str, -) -> pathlib.Path: +) -> tuple[pathlib.Path, packagesettings.DownloadKind]: + """Download a resolved source and return its path and artifact kind. + + Configured source resolvers receive a minimal candidate. Legacy downloads + are adapted to the sdist kind for compatibility. + """ logger.info(f"downloading source for {req}") + pbi = ctx.package_build_info(req) + source_resolver = pbi.source_resolver + if source_resolver is not None: + candidate = Candidate(name=req.name, version=version, url=download_url) + return source_resolver.download(ctx, req, candidate) + source_path = overrides.find_and_invoke( req.name, "download_source", @@ -102,7 +114,7 @@ def download_source( raise ValueError( f"expected a Path back to downloaded source. got {source_path}" ) - return source_path + return source_path, packagesettings.DownloadKind.sdist def get_source_provider( diff --git a/tests/test_bootstrapper.py b/tests/test_bootstrapper.py index 1f3b2516..03cf7be6 100644 --- a/tests/test_bootstrapper.py +++ b/tests/test_bootstrapper.py @@ -35,6 +35,7 @@ ) from fromager.bootstrapper._work_item import WorkItem from fromager.context import WorkContext +from fromager.packagesettings import DownloadKind from fromager.requirements_file import RequirementType, SourceType @@ -964,6 +965,7 @@ def test_bg_prepare_source_log_prefix_includes_version( logging.setLogRecordFactory(log.FromagerLogRecord) req = Requirement("mypkg==1.2.3") version = Version("1.2.3") + source_filename = pathlib.Path("mypkg-1.2.3.tar.gz") messages: list[str] = [] try: @@ -975,12 +977,15 @@ def test_bg_prepare_source_log_prefix_includes_version( ), patch( "fromager.sources.download_source", - return_value=pathlib.Path("mypkg-1.2.3.tar.gz"), + return_value=( + source_filename, + DownloadKind.sdist, + ), ), patch( "fromager.sources.prepare_source", return_value=pathlib.Path(tmp_context.work_dir / "mypkg-1.2.3"), - ), + ) as prepare_source, log.req_ctxvar_context(req, version), ): _bg_prepare_source( @@ -997,6 +1002,12 @@ def test_bg_prepare_source_log_prefix_includes_version( for r in caplog.records if r.name.startswith("fromager.bootstrapper") ] + prepare_source.assert_called_once_with( + ctx=tmp_context, + req=req, + source_filename=source_filename, + version=version, + ) finally: logging.setLogRecordFactory(old_factory) @@ -1006,6 +1017,31 @@ def test_bg_prepare_source_log_prefix_includes_version( ) +def test_bootstrapper_download_source_returns_downloaded_path( + tmp_context: WorkContext, +) -> None: + """Return only the path from the resolver download result.""" + bt = bootstrapper.Bootstrapper(tmp_context) + req = Requirement("mypkg==1.2.3") + version = Version("1.2.3") + source_url = "https://pkg.test/mypkg-1.2.3.tar.gz" + source_filename = pathlib.Path("mypkg-1.2.3.tar.gz") + + with patch( + "fromager.bootstrapper._bootstrapper.sources.download_source", + return_value=(source_filename, DownloadKind.sdist), + ) as download_source: + result = bt._download_source(req, version, source_url) + + assert result == source_filename + download_source.assert_called_once_with( + ctx=tmp_context, + req=req, + version=version, + download_url=source_url, + ) + + 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 5678d3d9..1f3ba7cd 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -1,8 +1,15 @@ +import pathlib import typing +from unittest.mock import Mock, patch import click +from click.testing import CliRunner +from packaging.requirements import Requirement +from packaging.version import Version -from fromager.commands import bootstrap, build +from fromager import packagesettings +from fromager.commands import bootstrap, build, step +from fromager.context import WorkContext def get_option_names(cmd: click.Command) -> typing.Iterable[str]: @@ -20,3 +27,93 @@ def test_bootstrap_parallel_options() -> None: expected.discard("test_mode") assert set(get_option_names(bootstrap.bootstrap_parallel)) == expected + + +def test_download_source_archive_prints_downloaded_path( + cli_runner: CliRunner, + tmp_context: WorkContext, + tmp_path: pathlib.Path, +) -> None: + """Print only the path when the download helper returns a tuple.""" + expected = tmp_path / "pkg-1.0.tar.gz" + + with ( + patch.object( + step.sources, + "resolve_source", + return_value=("https://pkg.test/pkg-1.0.tar.gz", Version("1.0")), + ), + patch.object( + step.sources, + "download_source", + return_value=(expected, packagesettings.DownloadKind.sdist), + ), + ): + result = cli_runner.invoke( + step.step, + [ + "download-source-archive", + "pkg", + "1.0", + "https://pkg.test/simple", + ], + obj=tmp_context, + ) + + assert result.exit_code == 0 + assert result.output == f"{expected}\n" + + +def test_build_passes_downloaded_path_to_prepare_source( + tmp_context: WorkContext, + tmp_path: pathlib.Path, +) -> None: + """Pass only the downloaded path into source preparation.""" + req = Requirement("pkg==1.0") + version = Version("1.0") + source_url = "https://pkg.test/pkg-1.0.tar.gz" + source_filename = tmp_path / "pkg-1.0.tar.gz" + source_root = tmp_path / "pkg-1.0" + sdist_filename = tmp_path / "rebuilt" / "pkg-1.0.tar.gz" + wheel_filename = tmp_path / "pkg-1.0-py3-none-any.whl" + build_env = Mock() + + with ( + patch.object(build.wheels, "get_wheel_server_urls", return_value=[]), + patch.object( + build.sources, + "download_source", + return_value=(source_filename, packagesettings.DownloadKind.sdist), + ), + patch.object( + build.sources, + "prepare_source", + return_value=source_root, + ) as prepare_source, + patch.object( + build.build_environment, + "prepare_build_environment", + return_value=build_env, + ), + patch.object(build.sources, "build_sdist", return_value=sdist_filename), + patch.object(build.wheels, "build_wheel", return_value=wheel_filename), + patch.object(build.hooks, "run_post_build_hooks"), + patch.object(tmp_context, "clean_build_dirs"), + patch.object(build.server, "update_wheel_mirror"), + ): + result = build._build( + wkctx=tmp_context, + resolved_version=version, + req=req, + source_download_url=source_url, + force=True, + cache_wheel_server_url=None, + ) + + prepare_source.assert_called_once_with( + ctx=tmp_context, + req=req, + source_filename=source_filename, + version=version, + ) + assert result.wheel_filename == tmp_context.wheels_downloads / wheel_filename.name diff --git a/tests/test_sources.py b/tests/test_sources.py index e6913573..b29b2603 100644 --- a/tests/test_sources.py +++ b/tests/test_sources.py @@ -11,7 +11,7 @@ from packaging.version import Version from fromager import context, packagesettings, resolver, sources -from fromager.candidate import Cooldown +from fromager.candidate import Candidate, Cooldown from fromager.requirements_file import RequirementType @@ -861,10 +861,73 @@ def test_download_source_regular_package( download_url="https://pkg.test/pkg-1.0.tar.gz", ) - assert result == expected + assert result == (expected, packagesettings.DownloadKind.sdist) mock_invoke.assert_called_once() +def test_download_source_uses_configured_source_resolver( + tmp_context: context.WorkContext, + tmp_path: pathlib.Path, +) -> None: + """Dispatch configured source downloads with a minimal candidate.""" + expected = tmp_path / "pkg-1.0.tar.gz" + source_resolver = packagesettings.PyPISDistResolver(provider="pypi-sdist") + req = Requirement("pkg==1.0") + version = Version("1.0") + download_url = "https://pkg.test/pkg-1.0.tar.gz" + with ( + patch.object( + packagesettings.PyPISDistResolver, + "_download", + return_value=(expected, packagesettings.DownloadKind.sdist), + ) as mock_download, + patch.object( + packagesettings.PackageBuildInfo, + "source_resolver", + new_callable=PropertyMock, + return_value=source_resolver, + ), + patch("fromager.sources.overrides.find_and_invoke") as mock_invoke, + ): + result = sources.download_source( + ctx=tmp_context, req=req, version=version, download_url=download_url + ) + + expected_candidate = Candidate(name=req.name, version=version, url=download_url) + assert result == (expected, packagesettings.DownloadKind.sdist) + mock_download.assert_called_once_with( + tmp_context, + req, + expected_candidate, + packagesettings.DownloadKind.sdist, + ) + mock_invoke.assert_not_called() + + +def test_download_source_returns_configured_result_without_validation( + tmp_context: context.WorkContext, +) -> None: + """Leave configured resolver result validation to the follow-up phase.""" + configured_result = "resolver-owned-result" + source_resolver = Mock() + source_resolver.download.return_value = configured_result + + with patch.object( + packagesettings.PackageBuildInfo, + "source_resolver", + new_callable=PropertyMock, + return_value=source_resolver, + ): + result = sources.download_source( + ctx=tmp_context, + req=Requirement("pkg==1.0"), + version=Version("1.0"), + download_url="https://pkg.test/pkg-1.0.tar.gz", + ) + + assert result == configured_result + + @patch("fromager.overrides.find_and_invoke", return_value="not-a-path") def test_download_source_invalid_return_raises( mock_invoke: Mock,