Skip to content

Support repo mapping for bazel runfiles.#1847

Closed
matts1 wants to merge 2 commits intobazelbuild:mainfrom
matts1:runfiles
Closed

Support repo mapping for bazel runfiles.#1847
matts1 wants to merge 2 commits intobazelbuild:mainfrom
matts1:runfiles

Conversation

@matts1
Copy link
Copy Markdown
Contributor

@matts1 matts1 commented Feb 24, 2023

With bazel 6 + bzlmod, bazel creates a "repo mapping" file. Note that this can be a problem even if rules_rust doesn't support bzlmod, as it can occur if a bzlmod repo includes rules_rust in its WORKSPACE.bazel file.

Discussion detailed at https://groups.google.com/g/bazel-discuss/c/DsVivJhU7Bw

@matts1
Copy link
Copy Markdown
Contributor Author

matts1 commented Mar 9, 2023

@scentini What do I need to do to get this reviewed?

@scentini
Copy link
Copy Markdown
Collaborator

scentini commented Mar 9, 2023

Hi @matts1 unfortunately I have not yet found the time to review the PR; however there seem to be some failures on CI, could you please take care of those?

@matts1 matts1 force-pushed the runfiles branch 2 times, most recently from c1d831b to b751d9e Compare March 10, 2023 00:40
@matts1
Copy link
Copy Markdown
Contributor Author

matts1 commented Mar 10, 2023

I fixed the clippy errors that were causing the failures, but there still appears to be two failures which I believe are unrelated to my code.

With bazel 6 + bzlmod, bazel creates a "repo mapping" file. Note
that this can be a problem even if rules_rust doesn't support bzlmod, as
it can occur if a bzlmod repo includes rules_rust in its WORKSPACE.bazel
file.

Discussion detailed at https://groups.google.com/g/bazel-discuss/c/DsVivJhU7Bw
let mut components = path.components();
if let Some(std::path::Component::Normal(target_local)) = components.next() {
if let Some(target_local) = target_local.to_str() {
let current_canonical = self.current_repository();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We're looking at this PR with @Wyverald, and all seems fine apart from this line; we concluded that the implementation of current_repository() is currently incorrect. We'll first need to tackle that, because it causes the next couple of lines to be wrong as well. I'll file a separate issue to describe the problem.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@scentini Has current_repository been fixed? If not, could you link to the bug that was filed so I can know when it is fixed.

matts1 added a commit to matts1/rules_rust that referenced this pull request Jan 9, 2024
This is a copy of bazelbuild#1847.
Submitting to our fork because it's unlikely to be submitted soon.

BUG=None
TEST=portage/tools/run_tests.sh

Change-Id: I1076582fb709a9eb8f1b3713507926a56c531575
@dzbarsky
Copy link
Copy Markdown
Contributor

#2566 is the latest attempt

@cerisier
Copy link
Copy Markdown
Contributor

Should this be closed ?

@dzbarsky
Copy link
Copy Markdown
Contributor

yes, this was fixed in #2566

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants