diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index ae39cac3..0d00803a 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -67,6 +67,7 @@ jobs: - build - build_order - build_steps + - build_settings - override - prebuilt_wheels_alt_server - report_missing_dependency diff --git a/.mergify.yml b/.mergify.yml index 0501c36b..a8bfc6a9 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -50,6 +50,10 @@ pull_request_rules: - check-success=e2e (3.11, 1.75, build) - check-success=e2e (3.12, 1.75, build) + - check-success=e2e (3.10, 1.75, build_settings) + - check-success=e2e (3.11, 1.75, build_settings) + - check-success=e2e (3.12, 1.75, build_settings) + - check-success=e2e (3.10, 1.75, build_order) - check-success=e2e (3.11, 1.75, build_order) - check-success=e2e (3.12, 1.75, build_order) diff --git a/docs/customization.md b/docs/customization.md index c615c744..1612e249 100644 --- a/docs/customization.md +++ b/docs/customization.md @@ -427,13 +427,18 @@ def post_build( ## Customizations using settings.yaml -To use predefined urls to download sources from, instead of overriding the entire `download_source` function, a mapping of package to download source url can be provided directly in settings.yaml: +To use predefined urls to download sources from, instead of overriding the entire `download_source` function, a mapping of package to download source url can be provided directly in settings.yaml. Optionally the downloaded sdist can be renamed. Both the url and the destination filename support templating. The only supported template variable is `version` - it is replaced by the version returned by the resolver. + +Additionally, the source distribution index server used by the package resolver can be overriden for a particular package. The resolver can also be told to whether include wheels or sdist sources while trying to resolve the package. Templating is not supported here. ```yaml -download_source: - torch: - url: "https://github.com/pytorch/pytorch/releases/download/v${version}/pytorch-v${version}.tar.gz" - rename_to: "torch-${version}.tar.gz" +packages: + torch: + download_source: + url: "https://github.com/pytorch/pytorch/releases/download/v${version}/pytorch-v${version}.tar.gz" + destination_filename: "torch-${version}.tar.gz" + resolver_dist: + sdist_server_url: "https://pypi.org/simple" + include_wheels: true + include_sdist: false ``` - -User can define a predefined url for a package from which all its sources will be downloaded from. Optionally they can rename the downloaded sdist to whatever they want. The only supported template variable is `version` - it is replaced by the version returned by the resolver. diff --git a/e2e/build_settings.yaml b/e2e/build_settings.yaml new file mode 100644 index 00000000..487d0567 --- /dev/null +++ b/e2e/build_settings.yaml @@ -0,0 +1,9 @@ +packages: + stevedore: + download_source: + url: https://files.pythonhosted.org/packages/e7/c1/b210bf1071c96ecfcd24c2eeb4c828a2a24bf74b38af13896d02203b1eec/stevedore-${version}.tar.gz + destination_filename: stevedore-custom-${version}.tar.gz + resolver_dist: + sdist_server_url: "https://pypi.org/simple" + include_sdist: false + include_wheels: true diff --git a/e2e/test_build_settings.sh b/e2e/test_build_settings.sh new file mode 100755 index 00000000..0ac7a164 --- /dev/null +++ b/e2e/test_build_settings.sh @@ -0,0 +1,76 @@ +#!/bin/bash +# -*- indent-tabs-mode: nil; tab-width: 2; sh-indentation: 2; -*- + +# Test to show we can build a single wheel, if the dependencies are +# available. + +SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" + +set -x +set -e +set -o pipefail + +# Bootstrap to create the build order file. +OUTDIR="$(dirname "$SCRIPTDIR")/e2e-output" + +# What are we building? +DIST="stevedore" +VERSION="5.2.0" + +# Recreate output directory +rm -rf "$OUTDIR" +mkdir -p "$OUTDIR/build-logs" + +# Set up virtualenv with the CLI and dependencies. +tox -e e2e -n -r +source ".tox/e2e/bin/activate" +pip install e2e/post_build_hook + +# Bootstrap the test project +fromager \ + --sdists-repo="$OUTDIR/sdists-repo" \ + --wheels-repo="$OUTDIR/wheels-repo" \ + --work-dir="$OUTDIR/work-dir" \ + bootstrap "${DIST}==${VERSION}" + +# Remove traces of stevedore +rm "$OUTDIR/wheels-repo/downloads/${DIST}"* +rm "$OUTDIR/sdists-repo/downloads/${DIST}"* +rm -r "$OUTDIR/work-dir/${DIST}"* + +# Rebuild the wheel mirror without stevedore +pypi-mirror create -d "$OUTDIR/wheels-repo/downloads/" -m "$OUTDIR/wheels-repo/simple/" + +# Rebuild the wheel +fromager \ + --log-file="$OUTDIR/build.log" \ + --sdists-repo="$OUTDIR/sdists-repo" \ + --wheels-repo="$OUTDIR/wheels-repo" \ + --work-dir="$OUTDIR/work-dir" \ + --settings-file="$SCRIPTDIR/build_settings.yaml" \ + build "${DIST}" "${VERSION}" "unreachable_url" + +EXPECTED_FILES=" +wheels-repo/build/stevedore-5.2.0-py3-none-any.whl +sdists-repo/downloads/stevedore-custom-5.2.0.tar.gz +sdists-repo/builds/stevedore-5.2.0.tar.gz +sdists-repo/builds/test-output-file.txt +build.log +" + +pass=true +for f in $EXPECTED_FILES; do + if [ ! -f "$OUTDIR/$f" ]; then + echo "FAIL: Did not find $OUTDIR/$f" 1>&2 + pass=false + fi +done + +if $pass; then + if ! grep -q "${DIST}==${VERSION}" $OUTDIR/sdists-repo/builds/test-output-file.txt; then + echo "FAIL: Did not find content in post-build hook output file" 1>&2 + pass=false + fi +fi + +$pass diff --git a/src/fromager/commands/list_overrides.py b/src/fromager/commands/list_overrides.py index 1cdee5b9..5548d903 100644 --- a/src/fromager/commands/list_overrides.py +++ b/src/fromager/commands/list_overrides.py @@ -10,6 +10,6 @@ def list_overrides( ): """List all of the packages with overrides in the current configuration.""" for name in overrides.list_all( - wkctx.patches_dir, wkctx.envs_dir, wkctx.settings.download_source() + wkctx.patches_dir, wkctx.envs_dir, wkctx.settings.package() ): print(name) diff --git a/src/fromager/overrides.py b/src/fromager/overrides.py index ce646a5c..16f288aa 100644 --- a/src/fromager/overrides.py +++ b/src/fromager/overrides.py @@ -150,7 +150,7 @@ def find_override_method(distname: str, method: str) -> typing.Callable: def list_all( patches_dir: pathlib.Path, envs_dir: pathlib.Path, - download_source: dict[str, dict[str, str]], + settings: dict[str, dict], test: bool = False, ): exts = _get_extensions() @@ -187,7 +187,7 @@ def env_projects(): yield item.stem def projects_with_predefined_download_source(): - yield from download_source + yield from settings # Use canonicalize_name() to ensure we can correctly remove duplicate # entries from the return list. diff --git a/src/fromager/settings.py b/src/fromager/settings.py index 20d0ed65..a4446928 100644 --- a/src/fromager/settings.py +++ b/src/fromager/settings.py @@ -17,18 +17,58 @@ def pre_built(self, variant: str) -> set[str]: names = p.get(variant) or [] return set(overrides.pkgname_to_override_module(n) for n in names) - def download_source(self) -> dict[str, dict[str, str]]: - return self._data.get("download_source") or {} - - def sdist_download_url(self, pkg: str) -> str | None: - p = self.download_source() - download_source = p.get(pkg) or {} - return download_source.get("url") - - def sdist_local_filename(self, pkg: str) -> str | None: - p = self.download_source() - download_source = p.get(pkg) or {} - return download_source.get("rename_to") + def package(self) -> dict[str, dict[str, str]]: + return self._return_value_or_default(self._data.get("packages"), {}) + + def download_source_url(self, pkg: str, default: str | None = None) -> str | None: + download_source = self._get_package_download_source_settings(pkg) + return self._return_value_or_default(download_source.get("url"), default) + + def download_source_destination_filename( + self, pkg: str, default: str | None = None + ) -> str | None: + download_source = self._get_package_download_source_settings(pkg) + return self._return_value_or_default( + download_source.get("destination_filename"), default + ) + + def resolver_sdist_server_url( + self, pkg: str, default: str | None = None + ) -> str | None: + resolve_dist = self._get_package_resolver_dist_settings(pkg) + return self._return_value_or_default( + resolve_dist.get("sdist_server_url"), default + ) + + def resolver_include_wheels( + self, pkg: str, default: bool | None = None + ) -> bool | None: + resolve_dist = self._get_package_resolver_dist_settings(pkg) + return self._return_value_or_default( + resolve_dist.get("include_wheels"), default + ) + + def resolver_include_sdist( + self, pkg: str, default: bool | None = None + ) -> bool | None: + resolve_dist = self._get_package_resolver_dist_settings(pkg) + return self._return_value_or_default(resolve_dist.get("include_sdist"), default) + + def _get_package_settings(self, pkg: str) -> dict[str, dict[str, str]]: + p = self.package() + return self._return_value_or_default(p.get(pkg), {}) + + def _get_package_download_source_settings(self, pkg: str) -> dict[str, str]: + p = self._get_package_settings(pkg) + return self._return_value_or_default(p.get("download_source"), {}) + + def _get_package_resolver_dist_settings(self, pkg: str) -> dict[str, str]: + p = self._get_package_settings(pkg) + return self._return_value_or_default(p.get("resolver_dist"), {}) + + def _return_value_or_default(self, value, default): + # can't use the "or" method since certain values can be false. Need to explicitly check for None + return value if value is not None else default def _parse(content: str) -> Settings: diff --git a/src/fromager/sources.py b/src/fromager/sources.py index 4874617e..843f7594 100644 --- a/src/fromager/sources.py +++ b/src/fromager/sources.py @@ -141,23 +141,26 @@ def default_download_source( sdist_server_url: str, ) -> tuple[pathlib.Path, str, str]: "Download the requirement and return the name of the output path." - url_template = ctx.settings.sdist_download_url(req.name) - rename_to_template = ctx.settings.sdist_local_filename(req.name) - - # don't include sdists if the user wants to download source from a predefined url - include_sdists = url_template is None - include_wheels = not include_sdists + include_sdists = ctx.settings.resolver_include_sdist(req.name, True) + include_wheels = ctx.settings.resolver_include_wheels(req.name, False) + override_sdist_server_url = ctx.settings.resolver_sdist_server_url( + req.name, sdist_server_url + ) org_url, version = resolve_dist( - ctx, req, sdist_server_url, include_sdists, include_wheels + ctx, req, override_sdist_server_url, include_sdists, include_wheels ) - url = _resolve_template(url_template, req, version) or org_url + dest_filename_template = ctx.settings.download_source_destination_filename(req.name) + destination_filename = _resolve_template(dest_filename_template, req, version) + url_template = ctx.settings.download_source_url(req.name, org_url) + url = _resolve_template(url_template, req, version) logger.debug(f"{req.name}: using {url} instead of {org_url}") - rename_to = _resolve_template(rename_to_template, req, version) + source_filename = _download_source_check( + ctx.sdists_downloads, url, destination_filename + ) - source_filename = _download_source_check(ctx.sdists_downloads, url, rename_to) logger.debug( f"{req.name}: have source for {req} version {version} in {source_filename}" ) @@ -167,9 +170,9 @@ def default_download_source( # Helper method to check whether .zip /.tar / .tgz is able to extract and check its content. # It will throw exception if any other file is encountered. Eg: index.html def _download_source_check( - destination_dir: pathlib.Path, url: str, rename_to: str | None = None + destination_dir: pathlib.Path, url: str, destination_filename: str | None = None ) -> str: - source_filename = download_url(destination_dir, url, rename_to) + source_filename = download_url(destination_dir, url, destination_filename) if source_filename.suffix == ".zip": source_file_contents = zipfile.ZipFile(source_filename).namelist() if not source_file_contents: @@ -187,9 +190,13 @@ def _download_source_check( def download_url( - destination_dir: pathlib.Path, url: str, rename_to: str | None = None + destination_dir: pathlib.Path, url: str, destination_filename: str | None = None ) -> pathlib.Path: - basename = rename_to if rename_to else os.path.basename(urlparse(url).path) + basename = ( + destination_filename + if destination_filename + else os.path.basename(urlparse(url).path) + ) outfile = pathlib.Path(destination_dir) / basename logger.debug( "looking for %s %s", outfile, "(exists)" if outfile.exists() else "(not there)" diff --git a/tests/test_overrides.py b/tests/test_overrides.py index 6bd7a9cd..18106239 100644 --- a/tests/test_overrides.py +++ b/tests/test_overrides.py @@ -126,9 +126,9 @@ def test_list_all(tmp_path: pathlib.Path): project_env2 = variant_dir / "fromager_test.env" project_env2.write_text("VAR1=VALUE1\nVAR2=VALUE2") # duplicate - download_source = { - "project-with-download-source": {"url": "url"}, - "another-project-with-download-source": {"url": "url"}, + settings = { + "project-with-download-source": {"download_source": {"url": "url"}}, + "project-with-resolver-dist": {"resolver_dist": {"include_wheels": "true"}}, } expected = [ @@ -137,14 +137,14 @@ def test_list_all(tmp_path: pathlib.Path): "project-with-env", "fromager-test", "project-with-download-source", - "another-project-with-download-source", + "project-with-resolver-dist", ] expected.sort() packages = overrides.list_all( patches_dir=patches_dir, envs_dir=env_dir, - download_source=download_source, + settings=settings, test=True, ) diff --git a/tests/test_settings.py b/tests/test_settings.py index 03ca013a..12693fbf 100644 --- a/tests/test_settings.py +++ b/tests/test_settings.py @@ -32,23 +32,57 @@ def test_with_pre_built(): def test_with_download_source(): s = settings._parse( textwrap.dedent(""" - download_source: + packages: foo: - url: url - rename_to: rename + download_source: + url: url + destination_filename: rename """) ) - assert s.sdist_local_filename("foo") == "rename" - assert s.sdist_local_filename("bar") is None - assert s.sdist_download_url("foo") == "url" - assert s.sdist_download_url("bar") is None + assert s.download_source_destination_filename("foo") == "rename" + assert s.download_source_destination_filename("bar") is None + assert s.download_source_url("foo") == "url" + assert s.download_source_url("bar") is None def test_no_download_source(): s = settings._parse( textwrap.dedent(""" - pre_built: + packages: + """) + ) + assert s.download_source_destination_filename("foo") is None + assert s.download_source_url("foo") is None + + +def test_with_resolver_dist(): + s = settings._parse( + textwrap.dedent(""" + packages: + foo: + resolver_dist: + sdist_server_url: url + include_sdist: true + include_wheels: false + """) + ) + assert type(s.resolver_include_sdist("foo")) is bool + assert s.resolver_include_sdist("foo") + assert type(s.resolver_include_wheels("foo")) is bool + assert not s.resolver_include_wheels("foo") + assert s.resolver_sdist_server_url("foo") == "url" + + +def test_no_resolver_dist(): + s = settings._parse( + textwrap.dedent(""" + packages: + foo: + download_source: + url: url + destination_filename: rename """) ) - assert s.sdist_local_filename("foo") is None - assert s.sdist_download_url("foo") is None + assert s.resolver_include_sdist("foo") is None + assert s.resolver_include_wheels("foo") is None + assert s.resolver_sdist_server_url("foo") is None diff --git a/tests/test_sources.py b/tests/test_sources.py index 0008e56b..7065e628 100644 --- a/tests/test_sources.py +++ b/tests/test_sources.py @@ -38,52 +38,53 @@ def test_resolve_template_with_no_matching_template(): @patch("fromager.sources.resolve_dist") @patch("fromager.sources._download_source_check") -@patch.object(settings.Settings, "sdist_download_url") -@patch.object(settings.Settings, "sdist_local_filename") -def test_default_download_source_no_predefined_url( - sdist_local_filename: typing.Callable, - sdist_download_url: typing.Callable, +@patch.object(settings.Settings, "download_source_url") +@patch.object(settings.Settings, "download_source_destination_filename") +def test_default_download_source_from_settings( + download_source_destination_filename: typing.Callable, + download_source_url: typing.Callable, download_source_check: typing.Callable, resolve_dist: typing.Callable, tmp_context: context.WorkContext, ): resolve_dist.return_value = ("url", "1.0") download_source_check.return_value = pathlib.Path("filename.zip") - sdist_download_url.return_value = None - sdist_local_filename.return_value = None + download_source_url.return_value = "predefined_url-${version}" + download_source_destination_filename.return_value = "foo-${version}" req = Requirement("foo==1.0") sdist_server_url = sources.PYPI_SERVER_URL sources.default_download_source(tmp_context, req, sdist_server_url) resolve_dist.assert_called_with(tmp_context, req, sdist_server_url, True, False) - download_source_check.assert_called_with(tmp_context.sdists_downloads, "url", None) + download_source_check.assert_called_with( + tmp_context.sdists_downloads, "predefined_url-1.0", "foo-1.0" + ) @patch("fromager.sources.resolve_dist") @patch("fromager.sources._download_source_check") -@patch.object(settings.Settings, "sdist_download_url") -@patch.object(settings.Settings, "sdist_local_filename") -def test_default_download_source_with_predefined_url( - sdist_local_filename: typing.Callable, - sdist_download_url: typing.Callable, +@patch.object(settings.Settings, "resolver_include_sdist") +@patch.object(settings.Settings, "resolver_include_wheels") +@patch.object(settings.Settings, "resolver_sdist_server_url") +def test_default_download_source_with_predefined_resolve_dist( + resolver_sdist_server_url: typing.Callable, + resolver_include_wheels: typing.Callable, + resolver_include_sdist: typing.Callable, download_source_check: typing.Callable, resolve_dist: typing.Callable, tmp_context: context.WorkContext, ): resolve_dist.return_value = ("url", "1.0") download_source_check.return_value = pathlib.Path("filename") - sdist_download_url.return_value = "predefined_url-${version}" - sdist_local_filename.return_value = "foo-${version}" + resolver_include_sdist.return_value = False + resolver_include_wheels.return_value = True + resolver_sdist_server_url.return_value = "url" req = Requirement("foo==1.0") - sdist_server_url = sources.PYPI_SERVER_URL - sources.default_download_source(tmp_context, req, sdist_server_url) + sources.default_download_source(tmp_context, req, sources.PYPI_SERVER_URL) - resolve_dist.assert_called_with(tmp_context, req, sdist_server_url, False, True) - download_source_check.assert_called_with( - tmp_context.sdists_downloads, "predefined_url-1.0", "foo-1.0" - ) + resolve_dist.assert_called_with(tmp_context, req, "url", False, True) @patch("fromager.sources.default_download_source")