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

crates_repository: The default_features field in crate.spec doesn't seem to work #1168

Closed
SlyMarbo opened this issue Mar 5, 2022 · 11 comments

Comments

@SlyMarbo
Copy link

SlyMarbo commented Mar 5, 2022

Hi there. I'm currently updating from crate_universe to the new crates_repository functionality introduced in #1158. The introduction of support for default_features = False is exciting, as I'm building a lot of #![no_std] code, so I often need to disable the default features to turn off the std feature.

However, setting default_features = False in a crate.spec doesn't seem to have any effect. I've had a quick look through the code and crate.spec propagates the field to the JSON it produces, but I can't find anything pulling it out of the JSON and crates still seem to be using their default features. Have I missed something? Thanks for your help.

@SlyMarbo
Copy link
Author

SlyMarbo commented Mar 5, 2022

I should add that with crate_universe, I worked around the inability to disable default features by using crate.override's features_to_remove field. There doesn't seem to be anything similar in the new crate.annotate, so I'm not sure how to work around the issue.

@UebelAndre
Copy link
Collaborator

@SlyMarbo can you check if #1174 solved the issue you were seeing with default_features = False having no effect?

Additionally, could you file a separate issue for your last comment?

@SlyMarbo
Copy link
Author

SlyMarbo commented Mar 7, 2022

@UebelAndre sadly #1174 doesn't seem to have helped. If it helps, I tried it by using commit 261f4e0. That commit also seems to have no reference to default_features, beside the field in crate.spec. I've opened #1176 for my second comment. Thanks :)
Edit: I've also tried current HEAD (b9469a0) but no effect. Still no other reference to default_features.

@UebelAndre
Copy link
Collaborator

@SlyMarbo Can you provide a repro case to test with?

@SlyMarbo
Copy link
Author

SlyMarbo commented Mar 7, 2022

I'm working on a smaller repro, but that doesn't seem to be exhibiting the issue so I'll keep debugging my main case (ProjectSerenity/firefly) and let you know how I get on.

@SlyMarbo
Copy link
Author

SlyMarbo commented Mar 7, 2022

Here's my failed reproduction, just in case that helps: SlyMarbo/sample-rules-rust-std.

@SlyMarbo
Copy link
Author

SlyMarbo commented Mar 7, 2022

I have now been able to reproduce the issue in the smaller repro. For some reason, default_features = False worked correctly with just the managed crate, but not smoltcp. I can make a branch using the previous crates_universe functionality if that would help.

@UebelAndre
Copy link
Collaborator

I have now been able to reproduce the issue in the smaller repro. For some reason, default_features = False worked correctly with just the managed crate, but not smoltcp. I can make a branch using the previous crates_universe functionality if that would help.

diff --git a/WORKSPACE b/WORKSPACE
index ec0989f..2fd1f2e 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -64,7 +64,6 @@ crates_repository(
             features = [
                 "alloc",
                 "async",
-                "default",
                 "libc",
                 "medium-ethernet",
                 "proto-dhcpv4",

This seems to work for me.

@SlyMarbo
Copy link
Author

SlyMarbo commented Mar 7, 2022

I'm the world's biggest idiot. Sorry for wasting your time. I really appreciate the help.

@SlyMarbo SlyMarbo closed this as completed Mar 7, 2022
@UebelAndre
Copy link
Collaborator

I'm the world's biggest idiot. Sorry for wasting your time. I really appreciate the help.

You're no idiot, I'd have done the exact same thing 😄

Happy the tool is working for you. Don't hesitate to report any more funny behavior!

@SlyMarbo
Copy link
Author

SlyMarbo commented Mar 7, 2022

Will do. It's been working really well for me. I've migrated completely from Cargo to Bazel and rules_rust to allow the use of other languages and simpler code generation. Thanks again 😄

@UebelAndre UebelAndre added the bug label Mar 7, 2022
SlyMarbo added a commit to ProjectSerenity/firefly that referenced this issue Mar 8, 2022
The primary change here is updating rules_rust. In the process, we
also switch from using crates_universe to the new crates_repository
functionality. This is a newer, more structured approach to managing
external crates. The most visible differences are:

- We now specify crates using `{"crate_name": crate.spec(version = "=1.2.3")}`, rather than `[crate.spec(name = "crate_name", version = "=1.2.3")]` when we specify the crates we use in rust.bzl.
- We now specify crates using `"@crates//:crate_name"`, rather than `crate("crate_name")` when we specify dependencies in BUILD files.
- There is now support for disabling default features, rather than having to do that manually.
- There is now a lockfile for external crates, including their cryptographic checksum. For now this has to be updated by using the `CARGO_BAZEL_REPIN=true` environment variable, but in due course we may fix that in update-deps.

In the process, we've also updated a few crates. The only one with
breaking changes was updating chacha20 to 0.9.0.

This also moves most of the crates that previously were fetched
using http_archive (allowing us to customise the BUILD file), rather
than using crates_universe, to now being fetched as normal with the
other crates in crates_repository. This simplifies the dependency
management somewhat. The only two crates still being fetched using
http_archive are compiler-builtins (since we don't build it using
rules_rust) and bootloader (since we both customise the BUILD file
considerably and also patch some files).

A big thank-you is due to @UebelAndre for their patience in helping
me diagnose an issue I had while upgrading. This was due to me having
the `default` feature enabled in smoltcp and wondering why disabling
default features wasn't working. See bazelbuild/rules_rust#1168 for
the details.

Signed-off-by: SlyMarbo <the.sly.marbo@googlemail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants