Skip to content
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.

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

Support repo mapping for bazel runfiles. #1847

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

matts1
Copy link
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
Contributor Author

matts1 commented Mar 9, 2023

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

@scentini
Copy link
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
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
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
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
Contributor

#2566 is the latest attempt

@cerisier
Copy link
Contributor

Should this be closed ?

@dzbarsky
Copy link
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.

4 participants