Implement support for optional crates enabled with dep: features #1885
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
UebelAndre
requested changes
Mar 20, 2023
For sure, I first wanted to get #1884 merged, which I'm fixing right now. |
jhchabran
added a commit
to sourcegraph/sourcegraph
that referenced
this pull request
Mar 21, 2023
…9698) This PR updates `rules_rust` to `v0.19.0`, as we're using Rust `1.68.0` which is only supported in that version. I suspect that's what caused the strange errors when repinning. Quoting @tjdevries explanation on the fix: > Pinned explicitly with features that match the features > required by rocket. Once bazel rules correctly roll up all the features, > we can remove this, but until then, this works just fine for building > with bazel (and we rarely update rocket, so this is fine). The whole issue are coming from: bazelbuild/rules_rust#1884 and bazelbuild/rules_rust#1885 which are addressing the problem upstream. --- Still, this now circles back to a compilation error what we've previously seen. To reproduce it ``` bazel build //docker-images/syntax-highlighter/... ``` Which in turn leads to the following errors: ``` ERROR: /private/var/tmp/_bazel_tech/3eea80c6015362974b7d423d1f30cb62/external/crate_index__futures-util-0.3.21/BUILD.bazel:22:13: Compiling Rust rlib futures_util v0.3.21 (180 files) [for tool] failed: (Exit 1): process_wrapper failed: error executing command (from target @crate_index__futures-util-0.3.21//:futures_util) bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/external/rules_rust/util/process_wrapper/process_wrapper --env-file ... (remaining 82 arguments skipped) Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging error[E0432]: unresolved imports `futures_task::waker`, `futures_task::ArcWake` --> external/crate_index__futures-util-0.3.21/src/stream/stream/flatten_unordered.rs:21:20 | 21 | use futures_task::{waker, ArcWake}; | ^^^^^ ^^^^^^^ no `ArcWake` in the root | | | no `waker` in the root | help: a similar name exists in the module (notice the capitalization): `Waker` error[E0432]: unresolved import `futures_task::ArcWake` --> external/crate_index__futures-util-0.3.21/src/task/mod.rs:23:9 | 23 | pub use futures_task::ArcWake; | ^^^^^^^^^^^^^^^^^^^^^ no `ArcWake` in the root error[E0432]: unresolved import `futures_task::waker` --> external/crate_index__futures-util-0.3.21/src/task/mod.rs:27:9 | 27 | pub use futures_task::waker; | ^^^^^^^^^^^^^^----- | | | | | help: a similar name exists in the module (notice the capitalization): `Waker` | no `waker` in the root error[E0432]: unresolved imports `futures_task::waker_ref`, `futures_task::WakerRef` --> external/crate_index__futures-util-0.3.21/src/task/mod.rs:31:24 | 31 | pub use futures_task::{waker_ref, WakerRef}; | ^^^^^^^^^ ^^^^^^^^ no `WakerRef` in the root | | | no `waker_ref` in the root error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0432`. ERROR: /private/var/tmp/_bazel_tech/3eea80c6015362974b7d423d1f30cb62/external/crate_index__futures-util-0.3.21/BUILD.bazel:22:13: Compiling Rust rlib futures_util v0.3.21 (180 files) failed: (Exit 1): process_wrapper failed: error executing command (from target @crate_index__futures-util-0.3.21//:futures_util) bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/external/rules_rust/util/process_wrapper/process_wrapper --env-file ... (remaining 82 arguments skipped) Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging error[E0432]: unresolved imports `futures_task::waker`, `futures_task::ArcWake` --> external/crate_index__futures-util-0.3.21/src/stream/stream/flatten_unordered.rs:21:20 | 21 | use futures_task::{waker, ArcWake}; | ^^^^^ ^^^^^^^ no `ArcWake` in the root | | | no `waker` in the root | help: a similar name exists in the module (notice the capitalization): `Waker` error[E0432]: unresolved import `futures_task::ArcWake` --> external/crate_index__futures-util-0.3.21/src/task/mod.rs:23:9 | 23 | pub use futures_task::ArcWake; | ^^^^^^^^^^^^^^^^^^^^^ no `ArcWake` in the root error[E0432]: unresolved import `futures_task::waker` --> external/crate_index__futures-util-0.3.21/src/task/mod.rs:27:9 | 27 | pub use futures_task::waker; | ^^^^^^^^^^^^^^----- | | | | | help: a similar name exists in the module (notice the capitalization): `Waker` | no `waker` in the root error[E0432]: unresolved imports `futures_task::waker_ref`, `futures_task::WakerRef` --> external/crate_index__futures-util-0.3.21/src/task/mod.rs:31:24 | 31 | pub use futures_task::{waker_ref, WakerRef}; | ^^^^^^^^^ ^^^^^^^^ no `WakerRef` in the root | | | no `waker_ref` in the root error: aborting due to 4 previous errors ``` ## Test plan <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles --> Green CI. ## App preview: - [Web](https://sg-web-jh-bzl-fix-rust.onrender.com/search) Check out the [client app preview documentation](https://docs.sourcegraph.com/dev/how-to/client_pr_previews) to learn more. --------- Co-authored-by: TJ DeVries <devries.timothyj@gmail.com> Co-authored-by: Jason Bedard <jason@aspect.dev>
|
@UebelAndre I added an integration test using |
Merged
|
CI is finally green after fixing conflicts and merging #1893 🎉 |
|
@illicitonion JFYI this was the feature I asked you about on Slack last week if you also wanna give it a look. |
UebelAndre
requested changes
Mar 22, 2023
UebelAndre
approved these changes
Mar 23, 2023
nyurik
pushed a commit
to nyurik/rules_rust
that referenced
this pull request
Mar 28, 2023
…elbuild#1885) * Implement support for optional crates enabled with dep: features * Fix triple constraints for iOS and watchOS (bazelbuild#1888) * Clippy * Use Rust 1.60 in CI * Fix clippy * Use unit test instead of integration test * Reset CI config * Simplify tests * Update crate_universe/src/metadata/dependency.rs --------- Co-authored-by: UebelAndre <github@uebelandre.com>
burmudar
pushed a commit
to sourcegraph/pr-auditor
that referenced
this pull request
Jul 18, 2023
…9698) This PR updates `rules_rust` to `v0.19.0`, as we're using Rust `1.68.0` which is only supported in that version. I suspect that's what caused the strange errors when repinning. Quoting @tjdevries explanation on the fix: > Pinned explicitly with features that match the features > required by rocket. Once bazel rules correctly roll up all the features, > we can remove this, but until then, this works just fine for building > with bazel (and we rarely update rocket, so this is fine). The whole issue are coming from: bazelbuild/rules_rust#1884 and bazelbuild/rules_rust#1885 which are addressing the problem upstream. --- Still, this now circles back to a compilation error what we've previously seen. To reproduce it ``` bazel build //docker-images/syntax-highlighter/... ``` Which in turn leads to the following errors: ``` ERROR: /private/var/tmp/_bazel_tech/3eea80c6015362974b7d423d1f30cb62/external/crate_index__futures-util-0.3.21/BUILD.bazel:22:13: Compiling Rust rlib futures_util v0.3.21 (180 files) [for tool] failed: (Exit 1): process_wrapper failed: error executing command (from target @crate_index__futures-util-0.3.21//:futures_util) bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/external/rules_rust/util/process_wrapper/process_wrapper --env-file ... (remaining 82 arguments skipped) Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging error[E0432]: unresolved imports `futures_task::waker`, `futures_task::ArcWake` --> external/crate_index__futures-util-0.3.21/src/stream/stream/flatten_unordered.rs:21:20 | 21 | use futures_task::{waker, ArcWake}; | ^^^^^ ^^^^^^^ no `ArcWake` in the root | | | no `waker` in the root | help: a similar name exists in the module (notice the capitalization): `Waker` error[E0432]: unresolved import `futures_task::ArcWake` --> external/crate_index__futures-util-0.3.21/src/task/mod.rs:23:9 | 23 | pub use futures_task::ArcWake; | ^^^^^^^^^^^^^^^^^^^^^ no `ArcWake` in the root error[E0432]: unresolved import `futures_task::waker` --> external/crate_index__futures-util-0.3.21/src/task/mod.rs:27:9 | 27 | pub use futures_task::waker; | ^^^^^^^^^^^^^^----- | | | | | help: a similar name exists in the module (notice the capitalization): `Waker` | no `waker` in the root error[E0432]: unresolved imports `futures_task::waker_ref`, `futures_task::WakerRef` --> external/crate_index__futures-util-0.3.21/src/task/mod.rs:31:24 | 31 | pub use futures_task::{waker_ref, WakerRef}; | ^^^^^^^^^ ^^^^^^^^ no `WakerRef` in the root | | | no `waker_ref` in the root error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0432`. ERROR: /private/var/tmp/_bazel_tech/3eea80c6015362974b7d423d1f30cb62/external/crate_index__futures-util-0.3.21/BUILD.bazel:22:13: Compiling Rust rlib futures_util v0.3.21 (180 files) failed: (Exit 1): process_wrapper failed: error executing command (from target @crate_index__futures-util-0.3.21//:futures_util) bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/external/rules_rust/util/process_wrapper/process_wrapper --env-file ... (remaining 82 arguments skipped) Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging error[E0432]: unresolved imports `futures_task::waker`, `futures_task::ArcWake` --> external/crate_index__futures-util-0.3.21/src/stream/stream/flatten_unordered.rs:21:20 | 21 | use futures_task::{waker, ArcWake}; | ^^^^^ ^^^^^^^ no `ArcWake` in the root | | | no `waker` in the root | help: a similar name exists in the module (notice the capitalization): `Waker` error[E0432]: unresolved import `futures_task::ArcWake` --> external/crate_index__futures-util-0.3.21/src/task/mod.rs:23:9 | 23 | pub use futures_task::ArcWake; | ^^^^^^^^^^^^^^^^^^^^^ no `ArcWake` in the root error[E0432]: unresolved import `futures_task::waker` --> external/crate_index__futures-util-0.3.21/src/task/mod.rs:27:9 | 27 | pub use futures_task::waker; | ^^^^^^^^^^^^^^----- | | | | | help: a similar name exists in the module (notice the capitalization): `Waker` | no `waker` in the root error[E0432]: unresolved imports `futures_task::waker_ref`, `futures_task::WakerRef` --> external/crate_index__futures-util-0.3.21/src/task/mod.rs:31:24 | 31 | pub use futures_task::{waker_ref, WakerRef}; | ^^^^^^^^^ ^^^^^^^^ no `WakerRef` in the root | | | no `waker_ref` in the root error: aborting due to 4 previous errors ``` ## Test plan <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles --> Green CI. ## App preview: - [Web](https://sg-web-jh-bzl-fix-rust.onrender.com/search) Check out the [client app preview documentation](https://docs.sourcegraph.com/dev/how-to/client_pr_previews) to learn more. --------- Co-authored-by: TJ DeVries <devries.timothyj@gmail.com> Co-authored-by: Jason Bedard <jason@aspect.dev>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Skip crates that are marked as
optional = trueand not enabled with adep:feature.For example, the
matrix-sdkuses this extensively (see: https://github.com/matrix-org/matrix-rust-sdk/blob/main/crates/matrix-sdk/Cargo.toml) and this patch makes it work withcargo-bazel.