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

Moved rust extension rules into a separate rules_rust_ext workspace. #3007

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

Conversation

UebelAndre
Copy link
Collaborator

@UebelAndre UebelAndre commented Nov 19, 2024

This change moves the bindgen, proto, and wasm_bindgen sub-packages into extensions which is isolated into it's own workspace called rules_rust_ext. The intent is improve ease of maintenance of both core and extension Rust rules by ensuring changes to extensions have no impact on the core rules. Core rules should never depend on extensions.

Load statements should be updated according to the following table:

before after
@rules_rust//bindgen @rules_rust_ext//bindgen
@rules_rust//proto/prost @rules_rust_ext//prost
@rules_rust//proto/protobuf @rules_rust_ext//protobuf
@rules_rust//wasm_bindgen @rules_rust_ext//wasm_bindgen

closes #2882

@UebelAndre UebelAndre marked this pull request as ready for review November 21, 2024 16:01
@UebelAndre UebelAndre requested review from illicitonion and krasimirgg and removed request for illicitonion November 21, 2024 16:01
@UebelAndre
Copy link
Collaborator Author

@illicitonion @krasimirgg @hlopko I would love your input on this change. It's a massive breaking change but I think a better path forward to help ensure the protobuf/rules_cc headache from #2998 doesn't impact the core rules again.

@krasimirgg
Copy link
Collaborator

Thank you I think it looks very reasonable, but also I'm not working in an environment where rules_rust+bzlmod is exercised, so it's hard to flag potential non-trivial issues with this. I just asked a few folks to take a look at this and share their thoughts.

@konkers
Copy link
Contributor

konkers commented Nov 22, 2024

Curious about the motivation to put them all into a single @rules_rust_ext module. I would have naively expected them to have separate modules (i.e. @rules_rust_bindgen, @rules_rust_prost, etc.) That way a project would have more fined grained control over it's dependencies.

@UebelAndre
Copy link
Collaborator Author

UebelAndre commented Nov 22, 2024

Curious about the motivation to put them all into a single @rules_rust_ext module. I would have naively expected them to have separate modules (i.e. @rules_rust_bindgen, @rules_rust_prost, etc.) That way a project would have more fined grained control over it's dependencies.

I started down that path and hit the 80 job cap for BazelCI. For each extension I would want to test them on Linux, Linux RBE, MacOS, and Windows, and to have each product be it's own workspace didn't seem possible. Though @meteorcloudy maybe there's a way to raise that cap?

edit:
But the trade I took here was to at least be able to isolate ruless_rust from bulky external dependencies. If there's a path to granular modules that doesn't involve super slow or no CI, I'm happy to take it!

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.

Happy to do this, equally happy to have a per-extension module if that's practical :) Thanks for doing the work!

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.

Shard rules_rust into multiple bzlmod modules
4 participants