feat(gitutils): add `parse_vcs_url` for pip VCS URL parsing by tiran · Pull Request #1215 · python-wheel-build/fromager

tiran · 2026-06-24T11:15:27Z

Pull Request Description

What

Add parse_vcs_url to parse pip VCS URLs (git+https, git+ssh) into a repo clone URL and git ref. Use it in bootstrapper and sources to replace duplicated manual git+ URL parsing logic.

Why

Replace ad-hoc implementations of pip VCS url parsing with a single, well-designed, and reusable function.

PR follows CONTRIBUTING.md guidelines

coderabbitai · 2026-06-24T11:15:41Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ea5c91d5-0095-48f3-a357-cfa7535c2bdd

📥 Commits

Reviewing files that changed from the base of the PR and between abb3171 and d72f84e.

📒 Files selected for processing (6)

src/fromager/bootstrapper.py
src/fromager/gitutils.py
src/fromager/sources.py
tests/test_bootstrap.py
tests/test_gitutils.py
tests/test_sources.py

🚧 Files skipped from review as they are similar to previous changes (5)

tests/test_sources.py
tests/test_gitutils.py
src/fromager/gitutils.py
src/fromager/bootstrapper.py
src/fromager/sources.py

📝 Walkthrough

Walkthrough

gitutils.py gains a GIT_HEAD constant and a new parse_vcs_url helper for git+https, git+ssh, and git+file URLs, with validation for unsupported schemes and missing or empty refs. git_clone_fast now uses GIT_HEAD as its default ref. bootstrapper.py and sources.py replace manual urlparse-based VCS URL handling with parse_vcs_url. Tests were updated for the new parsing behavior and error message.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: adding parse_vcs_url for pip VCS URL parsing.
Description check	✅ Passed	The description accurately describes the new VCS URL parser and its use in bootstrapper and sources.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (1)

tests/test_gitutils.py (1)

79-93: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover #subdirectory= fragments in the parser tests.

parse_vcs_url() explicitly drops URL fragments before returning the clone URL, but this suite never exercises a pip-style #subdirectory= URL. That leaves a common VCS form unprotected against regressions in both callers.

Suggested test addition

 def test_parse_vcs_url() -> None:
     assert parse_vcs_url("git+https://git.test/org/project.git@v1.0") == (
         "https://git.test/org/project.git",
         "v1.0",
     )
+    assert parse_vcs_url(
+        "git+https://git.test/org/project.git@v1.0#subdirectory=src"
+    ) == (
+        "https://git.test/org/project.git",
+        "v1.0",
+    )
     # '@' in netloc must not be confused with the ref '@'
     assert parse_vcs_url("git+ssh://git@git.test/org/project.git@abc123") == (
         "ssh://git@git.test/org/project.git",
         "abc123",
     )

As per path instructions, "tests/**: Verify test actually tests the intended behavior. Check for missing edge cases."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_gitutils.py` around lines 79 - 93, Add a test case in
test_parse_vcs_url() to cover a pip-style VCS URL with a `#subdirectory`=
fragment, and assert that parse_vcs_url() still returns the cloned repo URL plus
the correct ref while dropping the fragment. Use the existing parse_vcs_url and
GIT_HEAD patterns in this test module so the new assertion verifies the fragment
is ignored without changing the returned clone URL/ref behavior.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/test_gitutils.py`:
- Around line 79-93: Add a test case in test_parse_vcs_url() to cover a
pip-style VCS URL with a `#subdirectory`= fragment, and assert that
parse_vcs_url() still returns the cloned repo URL plus the correct ref while
dropping the fragment. Use the existing parse_vcs_url and GIT_HEAD patterns in
this test module so the new assertion verifies the fragment is ignored without
changing the returned clone URL/ref behavior.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 41e8cbc8-5fc4-450a-a86b-b0796f44c7ae

📥 Commits

Reviewing files that changed from the base of the PR and between 694f04f and 0f0e286.

📒 Files selected for processing (6)

src/fromager/bootstrapper.py
src/fromager/gitutils.py
src/fromager/sources.py
tests/test_bootstrap.py
tests/test_gitutils.py
tests/test_sources.py

smoparth

LGTM

mergify · 2026-06-25T13:53:05Z

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

Queue this pull request

rd4398

This looks good! Thanks!

mergify · 2026-06-25T14:12:41Z

queue

⚠️ Configuration not compatible with a branch protection setting

Details

The branch protection setting Require branches to be up to date before merging is not compatible with draft PR checks. To keep this branch protection enabled, update your Mergify configuration to enable in-place checks: set merge_queue.max_parallel_checks: 1, set every queue rule batch_size: 1, and avoid two-step CI (make merge_conditions identical to queue_conditions). Otherwise, disable this branch protection.

mergify · 2026-06-25T14:48:46Z

queue

⚠️ Configuration not compatible with a branch protection setting

Details

The branch protection setting Require branches to be up to date before merging is not compatible with draft PR checks. To keep this branch protection enabled, update your Mergify configuration to enable in-place checks: set merge_queue.max_parallel_checks: 1, set every queue rule batch_size: 1, and avoid two-step CI (make merge_conditions identical to queue_conditions). Otherwise, disable this branch protection.

rd4398 · 2026-06-25T14:51:16Z

@mergify rebase

mergify · 2026-06-25T14:51:51Z

rebase

🛑 The pull request rule doesn't match anymore

Details

This action has been cancelled.

Add `parse_vcs_url` to parse pip VCS URLs (git+https, git+ssh) into a repo clone URL and git ref. Use it in `bootstrapper` and `sources` to replace duplicated manual git+ URL parsing logic. Co-Authored-By: Claude <claude@anthropic.com> Signed-off-by: Christian Heimes <cheimes@redhat.com>

tiran requested a review from a team as a code owner June 24, 2026 11:15

mergify Bot added the ci label Jun 24, 2026

coderabbitai Bot reviewed Jun 24, 2026

View reviewed changes

tiran mentioned this pull request Jun 24, 2026

feat(resolver): wire source_resolver into resolver and download pipelines #1202

Open

1 task

tiran force-pushed the parse-vcs-url branch 3 times, most recently from b8429f2 to a9f9e4d Compare June 25, 2026 09:41

smoparth approved these changes Jun 25, 2026

View reviewed changes

rd4398 approved these changes Jun 25, 2026

View reviewed changes

rd4398 force-pushed the parse-vcs-url branch from a9f9e4d to d72f84e Compare June 25, 2026 14:52

mergify Bot merged commit 55adcc5 into python-wheel-build:main Jun 25, 2026
38 of 39 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(gitutils): add `parse_vcs_url` for pip VCS URL parsing#1215

feat(gitutils): add `parse_vcs_url` for pip VCS URL parsing#1215
mergify[bot] merged 1 commit into
python-wheel-build:mainfrom
tiran:parse-vcs-url

tiran commented Jun 24, 2026

Uh oh!

coderabbitai Bot commented Jun 24, 2026 •

edited

Loading

Walkthrough

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Uh oh!

smoparth left a comment

Uh oh!

mergify Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

rd4398 left a comment

Uh oh!

mergify Bot commented Jun 25, 2026

Uh oh!

mergify Bot commented Jun 25, 2026

Uh oh!

rd4398 commented Jun 25, 2026

Uh oh!

mergify Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

tiran commented Jun 24, 2026

Pull Request Description

What

Why

Uh oh!

coderabbitai Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

smoparth left a comment

Choose a reason for hiding this comment

Uh oh!

mergify Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rd4398 left a comment

Choose a reason for hiding this comment

Uh oh!

mergify Bot commented Jun 25, 2026

⚠️ Configuration not compatible with a branch protection setting

Uh oh!

mergify Bot commented Jun 25, 2026

⚠️ Configuration not compatible with a branch protection setting

Uh oh!

rd4398 commented Jun 25, 2026

Uh oh!

mergify Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛑 The pull request rule doesn't match anymore

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Jun 24, 2026 •

edited

Loading

mergify Bot commented Jun 25, 2026 •

edited

Loading

mergify Bot commented Jun 25, 2026 •

edited

Loading