From 5687cf757e33597676575398af4bb0cefcaa0c95 Mon Sep 17 00:00:00 2001 From: Ioannis Angelakopoulos Date: Mon, 10 Nov 2025 16:17:06 -0500 Subject: [PATCH] fix: git URLs working with bootstrap-parallel Build-parallel reconstructed requirements as name==version, losing git URL info and causing invalid URL errors. Fixes: #761 Co-authored-by: Claude Signed-off-by: Ioannis Angelakopoulos --- .github/workflows/test.yaml | 2 + .mergify.yml | 6 +++ docs/how-tos/build-from-git-repo.rst | 17 +++++- e2e/test_bootstrap_parallel_git_url.sh | 60 ++++++++++++++++++++++ e2e/test_bootstrap_parallel_git_url_tag.sh | 60 ++++++++++++++++++++++ src/fromager/commands/build.py | 11 +++- src/fromager/dependency_graph.py | 13 +++++ src/fromager/sources.py | 24 ++++++++- 8 files changed, 190 insertions(+), 3 deletions(-) create mode 100755 e2e/test_bootstrap_parallel_git_url.sh create mode 100755 e2e/test_bootstrap_parallel_git_url_tag.sh diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 1506c572..7feeb5f3 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -86,6 +86,8 @@ jobs: - bootstrap_git_url - bootstrap_git_url_tag - bootstrap_parallel + - bootstrap_parallel_git_url + - bootstrap_parallel_git_url_tag - bootstrap_skip_constraints - build - build_order diff --git a/.mergify.yml b/.mergify.yml index 66bbd25e..b67756a8 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -46,6 +46,8 @@ pull_request_rules: - check-success=e2e (3.11, 1.75, bootstrap_git_url, ubuntu-latest) - check-success=e2e (3.11, 1.75, bootstrap_git_url_tag, ubuntu-latest) - check-success=e2e (3.11, 1.75, bootstrap_parallel, ubuntu-latest) + - check-success=e2e (3.11, 1.75, bootstrap_parallel_git_url, ubuntu-latest) + - check-success=e2e (3.11, 1.75, bootstrap_parallel_git_url_tag, ubuntu-latest) - check-success=e2e (3.11, 1.75, bootstrap_prerelease, ubuntu-latest) - check-success=e2e (3.11, 1.75, bootstrap_sdist_only, ubuntu-latest) - check-success=e2e (3.11, 1.75, bootstrap_skip_constraints, ubuntu-latest) @@ -88,6 +90,10 @@ pull_request_rules: - check-success=e2e (3.12, 1.75, bootstrap_git_url_tag, ubuntu-latest) - check-success=e2e (3.12, 1.75, bootstrap_parallel, macos-latest) - check-success=e2e (3.12, 1.75, bootstrap_parallel, ubuntu-latest) + - check-success=e2e (3.12, 1.75, bootstrap_parallel_git_url, macos-latest) + - check-success=e2e (3.12, 1.75, bootstrap_parallel_git_url, ubuntu-latest) + - check-success=e2e (3.12, 1.75, bootstrap_parallel_git_url_tag, macos-latest) + - check-success=e2e (3.12, 1.75, bootstrap_parallel_git_url_tag, ubuntu-latest) - check-success=e2e (3.12, 1.75, bootstrap_prerelease, macos-latest) - check-success=e2e (3.12, 1.75, bootstrap_prerelease, ubuntu-latest) - check-success=e2e (3.12, 1.75, bootstrap_sdist_only, macos-latest) diff --git a/docs/how-tos/build-from-git-repo.rst b/docs/how-tos/build-from-git-repo.rst index a1ba08dd..9d73a3f9 100644 --- a/docs/how-tos/build-from-git-repo.rst +++ b/docs/how-tos/build-from-git-repo.rst @@ -14,6 +14,15 @@ you can do the following: This will clone the ``stevedore`` repository and build the package from the local copy. +You can also use the ``bootstrap-parallel`` command for faster builds: + +.. code-block:: console + + $ fromager bootstrap-parallel stevedore @ git+https://github.com/openstack/stevedore.git + +This will perform the same operation but build wheels in parallel after the +bootstrap phase completes. + Building from a specific version -------------------------------- @@ -24,9 +33,15 @@ the ``@`` syntax to specify the version. $ fromager bootstrap stevedore @ git+https://github.com/openstack/stevedore.git@5.2.0 -This will clone the ``stevedore`` repository at the tagg ``5.2.0`` and build the +This will clone the ``stevedore`` repository at the tag ``5.2.0`` and build the package from the local copy. +Or with parallel builds: + +.. code-block:: console + + $ fromager bootstrap-parallel stevedore @ git+https://github.com/openstack/stevedore.git@5.2.0 + .. important:: Building from a git repository URL is a special case which bypasses all of diff --git a/e2e/test_bootstrap_parallel_git_url.sh b/e2e/test_bootstrap_parallel_git_url.sh new file mode 100755 index 00000000..b575fced --- /dev/null +++ b/e2e/test_bootstrap_parallel_git_url.sh @@ -0,0 +1,60 @@ +#!/bin/bash + +# Test bootstrapping from a requirement with a git+https URL witout specifying a +# version tag. + +SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" +source "$SCRIPTDIR/common.sh" + +GIT_REPO_URL="https://github.com/python-wheel-build/stevedore-test-repo.git" + +fromager \ + --debug \ + --log-file="$OUTDIR/bootstrap.log" \ + --error-log-file="$OUTDIR/fromager-errors.log" \ + --sdists-repo="$OUTDIR/sdists-repo" \ + --wheels-repo="$OUTDIR/wheels-repo" \ + --work-dir="$OUTDIR/work-dir" \ + --settings-dir="$SCRIPTDIR/changelog_settings" \ + bootstrap-parallel "stevedore @ git+${GIT_REPO_URL}" + +find "$OUTDIR/wheels-repo/" -name '*.whl' +find "$OUTDIR/sdists-repo/" -name '*.tar.gz' +ls "$OUTDIR"/work-dir/*/build.log || true + +EXPECTED_FILES=" +$OUTDIR/wheels-repo/downloads/setuptools-*.whl +$OUTDIR/wheels-repo/downloads/pbr-*.whl +$OUTDIR/wheels-repo/downloads/stevedore-*.whl + +$OUTDIR/sdists-repo/downloads/setuptools-*.tar.gz +$OUTDIR/sdists-repo/downloads/pbr-*.tar.gz + +$OUTDIR/sdists-repo/builds/stevedore-*.tar.gz +$OUTDIR/sdists-repo/builds/setuptools-*.tar.gz +$OUTDIR/sdists-repo/builds/pbr-*.tar.gz + +$OUTDIR/work-dir/build-order.json +$OUTDIR/work-dir/constraints.txt + +$OUTDIR/bootstrap.log +$OUTDIR/fromager-errors.log + +$OUTDIR/work-dir/pbr-*/build.log +$OUTDIR/work-dir/setuptools-*/build.log +$OUTDIR/work-dir/stevedore-*/build.log +" + +pass=true +for pattern in $EXPECTED_FILES; do + if [ ! -f "${pattern}" ]; then + echo "Did not find $pattern" 1>&2 + pass=false + fi +done + +$pass + +twine check "$OUTDIR"/sdists-repo/builds/*.tar.gz +twine check "$OUTDIR"/wheels-repo/downloads/*.whl + diff --git a/e2e/test_bootstrap_parallel_git_url_tag.sh b/e2e/test_bootstrap_parallel_git_url_tag.sh new file mode 100755 index 00000000..b4e4180b --- /dev/null +++ b/e2e/test_bootstrap_parallel_git_url_tag.sh @@ -0,0 +1,60 @@ +#!/bin/bash + +# Test bootstrapping from a requirement with a git+https URL and specifying a +# version tag. + +SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" +source "$SCRIPTDIR/common.sh" + +GIT_REPO_URL="https://github.com/python-wheel-build/stevedore-test-repo.git" + +fromager \ + --debug \ + --log-file="$OUTDIR/bootstrap.log" \ + --error-log-file="$OUTDIR/fromager-errors.log" \ + --sdists-repo="$OUTDIR/sdists-repo" \ + --wheels-repo="$OUTDIR/wheels-repo" \ + --work-dir="$OUTDIR/work-dir" \ + --settings-dir="$SCRIPTDIR/changelog_settings" \ + bootstrap-parallel "stevedore @ git+${GIT_REPO_URL}@5.2.0" + +find "$OUTDIR/wheels-repo/" -name '*.whl' +find "$OUTDIR/sdists-repo/" -name '*.tar.gz' +ls "$OUTDIR"/work-dir/*/build.log || true + +EXPECTED_FILES=" +$OUTDIR/wheels-repo/downloads/setuptools-*.whl +$OUTDIR/wheels-repo/downloads/pbr-*.whl +$OUTDIR/wheels-repo/downloads/stevedore-*.whl + +$OUTDIR/sdists-repo/downloads/setuptools-*.tar.gz +$OUTDIR/sdists-repo/downloads/pbr-*.tar.gz + +$OUTDIR/sdists-repo/builds/stevedore-*.tar.gz +$OUTDIR/sdists-repo/builds/setuptools-*.tar.gz +$OUTDIR/sdists-repo/builds/pbr-*.tar.gz + +$OUTDIR/work-dir/build-order.json +$OUTDIR/work-dir/constraints.txt + +$OUTDIR/bootstrap.log +$OUTDIR/fromager-errors.log + +$OUTDIR/work-dir/pbr-*/build.log +$OUTDIR/work-dir/setuptools-*/build.log +$OUTDIR/work-dir/stevedore-*/build.log +" + +pass=true +for pattern in $EXPECTED_FILES; do + if [ ! -f "${pattern}" ]; then + echo "Did not find $pattern" 1>&2 + pass=false + fi +done + +$pass + +twine check "$OUTDIR"/sdists-repo/builds/*.tar.gz +twine check "$OUTDIR"/wheels-repo/downloads/*.whl + diff --git a/src/fromager/commands/build.py b/src/fromager/commands/build.py index 34d41a2b..3467052a 100644 --- a/src/fromager/commands/build.py +++ b/src/fromager/commands/build.py @@ -661,11 +661,20 @@ def update_progressbar_cb(future: concurrent.futures.Future) -> None: logger.info("requires exclusive build") logger.info("ready to build") + # Use original top-level requirement only if it has a URL (git, etc.), + # otherwise use the resolved version to avoid building wrong versions. + top_level_req = graph.get_top_level_requirement(node) + if top_level_req and top_level_req.url: + req = top_level_req + logger.debug("using top-level requirement with URL: %s", req) + else: + req = Requirement(f"{node.canonicalized_name}=={node.version}") + future = executor.submit( _build_parallel, wkctx=wkctx, resolved_version=node.version, - req=node.requirement, + req=req, source_download_url=node.download_url, force=force, cache_wheel_server_url=cache_wheel_server_url, diff --git a/src/fromager/dependency_graph.py b/src/fromager/dependency_graph.py index c63c9db7..893de699 100644 --- a/src/fromager/dependency_graph.py +++ b/src/fromager/dependency_graph.py @@ -367,6 +367,19 @@ def get_nodes_by_name(self, req_name: str | None) -> list[DependencyNode]: def get_root_node(self) -> DependencyNode: return self.nodes[ROOT] + def get_top_level_requirement(self, node: DependencyNode) -> Requirement | None: + """Get the top-level requirement specification for a node. + + For packages that were specified as top-level requirements (e.g., with git URLs), + this returns the original requirement specification. Returns None if the node + is not a direct child of ROOT. + """ + root = self.get_root_node() + for edge in node.parents: + if edge.destination_node is root: + return edge.req + return None + def get_all_nodes(self) -> typing.Iterable[DependencyNode]: return self.nodes.values() diff --git a/src/fromager/sources.py b/src/fromager/sources.py index cc6665c1..c8a2af98 100644 --- a/src/fromager/sources.py +++ b/src/fromager/sources.py @@ -8,6 +8,7 @@ import tarfile import typing import zipfile +from urllib.parse import urlparse import resolvelib from packaging.requirements import Requirement @@ -76,11 +77,32 @@ def download_source( elif req.url: download_path = ctx.work_dir / f"{req.name}-{version}" / f"{req.name}-{version}" download_path.mkdir(parents=True, exist_ok=True) + + # Parse ref from URL if it's in the form repo@ref + url_to_clone = req.url + git_ref: str | None = None + + # Remove git+ prefix for parsing + parsing_url = url_to_clone + if parsing_url.startswith("git+"): + parsing_url = parsing_url[len("git+") :] + + parsed_url = urlparse(parsing_url) + if "@" in parsed_url.path: + # Extract the ref from the path (e.g., /path/to/repo.git@5.2.0 -> @5.2.0) + new_path, _, git_ref = parsed_url.path.rpartition("@") + # Reconstruct URL without the @ref part + url_to_clone = parsed_url._replace(path=new_path).geturl() + # Add back git+ prefix if it was present + if req.url.startswith("git+"): + url_to_clone = f"git+{url_to_clone}" + download_git_source( ctx=ctx, req=req, - url_to_clone=req.url, + url_to_clone=url_to_clone, destination_dir=download_path, + ref=git_ref, ) return download_path