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 workspaces. #3007

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

UebelAndre
Copy link
Collaborator

@UebelAndre UebelAndre commented Nov 19, 2024

This change moves the bindgen, proto, and wasm_bindgen sub-packages into individual workspaces within the extensions directory. 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_bindgen//
@rules_rust//proto/prost @rules_rust_prost//
@rules_rust//proto/protobuf @rules_rust_protobuf//
@rules_rust//wasm_bindgen @rules_rust_wasm_bindgen//

closes #2882

@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!

Copy link
Collaborator

@krasimirgg krasimirgg left a comment

Choose a reason for hiding this comment

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

Thank you! Looks good (if we manage to solve the testing CI limit situation, splitting this up more would be good too).

@UebelAndre
Copy link
Collaborator Author

UebelAndre commented Nov 25, 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!

The commit I just added splits the extensions into their own repositories but hits the following error:

https://buildkite.com/bazel/rules-rust-rustlang/builds/12747#0193646f-6c25-4faa-a809-e5e319a1c9e6

The number of tasks in one config file is limited to 80!
fatal: pipeline parsing of "(stdin)" failed: EOF

@meteorcloudy @krasimirgg there's no way to raise this is there 😅

(Plus, it would be nice to be told how many were found)

@UebelAndre
Copy link
Collaborator Author

The number of tasks in one config

I've created bazelbuild/continuous-integration#2120 as a hopeful path forward.

@meteorcloudy
Copy link
Member

presubmit is now working https://buildkite.com/bazel/rules-rust-rustlang/builds/12752

@UebelAndre
Copy link
Collaborator Author

UebelAndre commented Nov 26, 2024

presubmit is now working https://buildkite.com/bazel/rules-rust-rustlang/builds/12752

With this in place I'll go ahead and update the PR to reflect the granular workspaces change instead of rules_rust_ext. Thanks again!

@UebelAndre UebelAndre added this pull request to the merge queue Nov 26, 2024
Merged via the queue into bazelbuild:main with commit 2bad96d Nov 26, 2024
4 checks passed
@UebelAndre UebelAndre changed the title Moved rust extension rules into a separate rules_rust_ext workspace. Moved rust extension rules into a separate workspaces. Nov 26, 2024
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
5 participants