Add --inject_repository and fix crash on overridden non-existent repo
#23791
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fmeum
requested review from
a team,
lberki,
Wyverald and
meteorcloudy
as code owners
fmeum
requested review from
katre
and removed request for
a team,
lberki and
katre
github-actions
bot
added
team-Configurability
fmeum
commented
Sep 27, 2024
| if (repositoryName.isMain()) { | ||
| repoMapping = | ||
| repoMapping.withAdditionalMappings( | ||
| repositoryOverrides.stream().collect(toMap(RepositoryName::getName, name -> name))); |
There was a problem hiding this comment.
This is a bit unfortunate as the resulting repo will have a canonical repo name that is also a valid apparent name. We could prevent that by branching on whether the name provided on the command line is a valid apparent name or not:
- if it isn't, but is a valid canonical name, assume that it overrides an existing repo
- if it is, generate a synthetic canonical repo name such as
_main+_overrides+<name>for it.
What do you think? Would the resulting consistency be worth the increased complexity?
There was a problem hiding this comment.
(tests are failing because REPOSITORY_OVERRIDES would now need to be injected in more tests, I can do that once we have agreed on the functionality)
There was a problem hiding this comment.
could we maybe call the flag --inject_repository, similar to inject_repo vs override_repo? There's a nice bit of symmetry there, at least superficially... Then we could also say --inject_repository=X=foo always creates the repo with the canonical repo name of +_injected+X, or something.
There was a problem hiding this comment.
I completely rewrote this PR. It now implements --inject_repository through a simulated use_repo_rule call for a local_repository, which guarantees behavior identical to that of a regular module extension repo.
sgowroji
added
awaiting-user-response
fmeum
force-pushed
the
22691-override-repository
branch
from
711ef17 to
273012d
Compare
RELNOTES: The new `--inject_repository` flag can be used to add new repositories via the CLI with `--noenable_workspace`. Such repositories behave as if they were declared by `local_repository` via `use_repo_rule` in the root module.
fmeum
force-pushed
the
22691-override-repository
branch
from
273012d to
1d97bcb
Compare
fmeum
changed the title
--override_repository to add repos with --noenable_workspace
fmeum
changed the title
--inject_repository and fix crash on overridden non-existent repo
|
There are a number of test failures referencing files under |
Wyverald
added
awaiting-review
Wyverald
approved these changes
Nov 7, 2024
src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunction.java
Show resolved
Hide resolved
Co-authored-by: Xùdōng Yáng <wyverald@gmail.com>
fmeum
force-pushed
the
22691-override-repository
branch
from
b380d82 to
8ed5074
Compare
fmeum
force-pushed
the
22691-override-repository
branch
from
8ed5074 to
a55e001
Compare
|
@bazel-io fork 8.0.0 |
|
Tests finally pass :-) |
Wyverald
added
awaiting-PR-merge
github-actions
bot
removed
the
awaiting-PR-merge
Wyverald
pushed a commit
that referenced
this pull request
Nov 12, 2024
Suggest the new flag instead of crashing when `--override_repository` is applied to a non-existent repo with `--noenable_workspace`. Fixes #22691 RELNOTES: The new `--inject_repository` flag can be used to add new repositories via the CLI with `--enable_bzlmod`. Such repositories behave as if they were declared by `local_repository` via `use_repo_rule` in the root module. Closes #23791. PiperOrigin-RevId: 695848897 Change-Id: I92ed25261c92d07f289815fcf6a65485ff43f373
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 12, 2024
…stent repo (#24301) Suggest the new flag instead of crashing when `--override_repository` is applied to a non-existent repo with `--noenable_workspace`. Fixes #22691 RELNOTES: The new `--inject_repository` flag can be used to add new repositories via the CLI with `--enable_bzlmod`. Such repositories behave as if they were declared by `local_repository` via `use_repo_rule` in the root module. Closes #23791. PiperOrigin-RevId: 695848897 Change-Id: I92ed25261c92d07f289815fcf6a65485ff43f373 Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
|
@bazel-io fork 7.5.0 |
ramil-bitrise
pushed a commit
to bitrise-io/bazel
that referenced
this pull request
Dec 18, 2024
Suggest the new flag instead of crashing when `--override_repository` is applied to a non-existent repo with `--noenable_workspace`. Fixes bazelbuild#22691 RELNOTES: The new `--inject_repository` flag can be used to add new repositories via the CLI with `--enable_bzlmod`. Such repositories behave as if they were declared by `local_repository` via `use_repo_rule` in the root module. Closes bazelbuild#23791. PiperOrigin-RevId: 695848897 Change-Id: I92ed25261c92d07f289815fcf6a65485ff43f373
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Suggest the new flag instead of crashing when
--override_repositoryis applied to a non-existent repo with--noenable_workspace.Fixes #22691
RELNOTES: The new
--inject_repositoryflag can be used to add new repositories via the CLI with--enable_bzlmod. Such repositories behave as if they were declared bylocal_repositoryviause_repo_rulein the root module.