Skip to content

Add support for apparent names to --override_repository #27706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
fmeum wants to merge 11 commits into from
Closed

Add support for apparent names to --override_repository #27706

fmeum wants to merge 11 commits into from

Conversation

@fmeum
Copy link
Collaborator

@fmeum fmeum commented Nov 18, 2025 •
edited by Wyverald
Loading

The implementation has to handle command line overrides on module repos subject to non-registry overrides specially to avoid a cycle when requesting the main repo mapping.

Fixes #24617
Fixes #17128

RELNOTES: --override_repository now supports apparent repository names from the point of view of the main repository. An unknown apparent repo name will result in an error.

@github-actions github-actions bot added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. awaiting-review PR is awaiting review from an assigned reviewer labels Nov 18, 2025
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 18, 2025

@bazel-io fork 9.0.0

@fmeum
Copy link
Collaborator Author

fmeum commented Nov 18, 2025 •
edited
Loading

@bazel-io fork 8.5.0

This was referenced Nov 18, 2025
Closed
Closed
@meteorcloudy
Copy link
Member

meteorcloudy commented Nov 18, 2025

I assume this override happens after bazel module resolution? Will it also influence module extension resolution? It's better to explicitly document the behaviour.

@fmeum
Copy link
Collaborator Author

fmeum commented Nov 18, 2025

I assume this override happens after bazel module resolution? Will it also influence module extension resolution? It's better to explicitly document the behaviour.

This override happens before module resolution (which is why it needs tricks to implement). I actually don't think we have any overrides that apply after module resolution other than single_version_override with patch_cmds, so perhaps it makes more sense to document these exceptions?

@meteorcloudy
Copy link
Member

meteorcloudy commented Nov 18, 2025

Hmm, I remember --override_repository=<canonical repo name>=<path> happens after bazel module and module extension resolutions. Am I wrong or did that change with this PR? The vendor mode pin feature is also implemented the same way: https://bazel.build/external/vendor#configure-vendor-mode

If this happens before module resolution, then for bazel modules, users should just use --override_module? IMO, --override_repository with apparent repo name is only useful for module extension repos?

@fmeum
Copy link
Collaborator Author

fmeum commented Nov 18, 2025

Hmm, I remember --override_repository== happens after bazel module and module extension resolutions. Am I wrong or did that change with this PR? The vendor mode pin feature is also implemented the same way: https://bazel.build/external/vendor#configure-vendor-mode

You are indeed right and this PR doesn't change that behavior. :-) I have added additional explanations to the flag docs.

@fmeum
Copy link
Collaborator Author

fmeum commented Nov 18, 2025

Looks like this caught some bugs in our tests, will fix them later.

@fmeum fmeum force-pushed the better-override-repository branch from f20d9e2 to a04a1cf Compare November 18, 2025 19:08
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 18, 2025

CI is green. This found a few tests that didn't test what they thought they did

Wyverald
Wyverald approved these changes Nov 20, 2025
View reviewed changes
Copy link
Member

@Wyverald Wyverald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! thank you for addressing this long-standing feature request :) (I added the relevant links to the PR description)

@fmeum fmeum force-pushed the better-override-repository branch 3 times, most recently from e88c308 to 43bdb11 Compare November 21, 2025 19:54
@fmeum fmeum requested a review from a team as a code owner November 21, 2025 19:54
@fmeum fmeum requested review from mai93 and removed request for a team November 21, 2025 19:54
@bharadwaj08-one
Copy link

bharadwaj08-one commented Nov 24, 2025

@fmeum Could you please take a look at the failing checks?

@bharadwaj08-one bharadwaj08-one added awaiting-user-response Awaiting a response from the author and removed awaiting-review PR is awaiting review from an assigned reviewer labels Nov 24, 2025
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 24, 2025

@bharadwaj08-one Should be fixed now

@fmeum fmeum added awaiting-review PR is awaiting review from an assigned reviewer and removed awaiting-user-response Awaiting a response from the author labels Nov 24, 2025
@iancha1992
Copy link
Member

iancha1992 commented Nov 24, 2025

@Wyverald Is this awaiting merge now?

@meteorcloudy meteorcloudy removed the awaiting-review PR is awaiting review from an assigned reviewer label Nov 25, 2025
@meteorcloudy meteorcloudy added the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Nov 25, 2025
@meteorcloudy
Copy link
Member

meteorcloudy commented Nov 25, 2025

@Wyverald is OOO, but this is good to go.

@bharadwaj08-one
Copy link

bharadwaj08-one commented Nov 26, 2025

@fmeum Could you please resolve the conflicts?

fmeum added 11 commits November 26, 2025 13:54
@fmeum
Add support for apparent names to --override_repository
81ab8a6 
The implementation has to handle command line overrides on module repos subject to non-registry overrides specially to avoid a cycle when requesting the main repo mapping.

RELNOTES: `--override_repository` now supports apparent repository names from the point of view of the main repository. An unknown apparent repo name will result in an error.
@fmeum
Add test and docs
23b6d6e
@fmeum
Fix repo_name issue
13293a6
@fmeum
Fix null
45c3a16
@fmeum
Fix crash
4da302d 
# Conflicts:
#	src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java
@fmeum
Add docs
a5d8f00
@fmeum
Fix more bugs
a560722
@fmeum
Fix tests
13c227d
@fmeum
Attempt to fix crash
b159cae
@fmeum
Address comments
92da9b2
@fmeum
Fix test
e83cac5
@fmeum fmeum force-pushed the better-override-repository branch from dae58b7 to e83cac5 Compare November 26, 2025 12:55
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 26, 2025

@bharadwaj08-one Done

@copybara-service copybara-service bot closed this in 82c06ca Dec 8, 2025
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Dec 8, 2025
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Dec 8, 2025
@fmeum @bazel-io
Add support for apparent names to --override_repository
4a170fd 
The implementation has to handle command line overrides on module repos subject to non-registry overrides specially to avoid a cycle when requesting the main repo mapping.

Fixes bazelbuild#24617
Fixes bazelbuild#17128

RELNOTES: `--override_repository` now supports apparent repository names from the point of view of the main repository. An unknown apparent repo name will result in an error.

Closes bazelbuild#27706.

PiperOrigin-RevId: 841652194
Change-Id: Ic8d53e5c7beec0cf14a293c2d589ab08a5ba4f9d
@bazel-io bazel-io mentioned this pull request Dec 8, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Dec 9, 2025
@bazel-io @fmeum
[9.0.0] Add support for apparent names to --override_repository (#2…
598a385 
…7898)

The implementation has to handle command line overrides on module repos
subject to non-registry overrides specially to avoid a cycle when
requesting the main repo mapping.

Fixes #24617
Fixes #17128

RELNOTES: `--override_repository` now supports apparent repository names
from the point of view of the main repository. An unknown apparent repo
name will result in an error.

Closes #27706.

PiperOrigin-RevId: 841652194
Change-Id: Ic8d53e5c7beec0cf14a293c2d589ab08a5ba4f9d

Commit
82c06ca

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Wyverald Wyverald Wyverald approved these changes

@meteorcloudy meteorcloudy Awaiting requested review from meteorcloudy meteorcloudy is a code owner

@mai93 mai93 Awaiting requested review from mai93 mai93 is a code owner automatically assigned from bazelbuild/configurability

Assignees

No one assigned

Labels

team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.

Projects

None yet

Milestone

No milestone

5 participants

@fmeum @meteorcloudy @bharadwaj08-one @iancha1992 @Wyverald