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

Add support for all_crate_deps when using bzlmod #2461

Merged
merged 7 commits into from
Feb 9, 2024

Conversation

AmeliasCode
Copy link
Contributor

This adds support for the all_crate_deps functionality in repositories created via bzlmod. An earlier update that added support for 3rd party crates with bzlmod intentionally left this functionality out.

Some key things that I think should be reviewed:

  • Is the method of getting crate_label_template in extensions.bzl the best approach? It requires a lot of hard-coding, and it seems like it might be possible to simplify but I'm not familiar enough with the code base to know how.
  • Is the generate_config_file method from crates_vendor.bzl the right method to use for generating the cargo-bazel config? It seems like there are two places with similar logic (here and in crates_repositories.bzl (see Add render_config attribute to crates_vendor. #1832)

Copy link

google-cla bot commented Feb 5, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@AmeliasCode
Copy link
Contributor Author

Sorry for the buildifier failures, wasn't sure exactly what command the buildkite workflow was running. Appears to be passing now! For anyone stuck in the future, try buildifier -v --warnings all --lint fix

@AmeliasCode
Copy link
Contributor Author

@illicitonion Judging by similar pull requests this seems like it might be in your arena? Could you please take a look when you have time? Thanks!

Copy link
Collaborator

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

This looks great, thanks so much for the contribution!

@illicitonion illicitonion merged commit 63a73ac into bazelbuild:main Feb 9, 2024
3 checks passed
cameron-martin added a commit to cameron-martin/rules_rust that referenced this pull request Feb 18, 2024
 bazelbuild#2461 changed the rendering of the hub repo to use canonical repository names, but incorrectly. Doing so wasn't necessary - instead we just need to do the resolution of the non-canonical label from the perspective of the hub repo rather than the consumer.

Fixes bazelbuild#2483
UebelAndre pushed a commit that referenced this pull request Feb 18, 2024
#2461 changed the rendering of the hub repo to use canonical repository
names, but incorrectly. Doing so wasn't necessary - instead we just need
to do the resolution of the non-canonical label from the perspective of
the hub repo rather than the consumer by using the `Label` constructor.

Fixes #2483
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.

2 participants