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

[BUG]: Some Rust crates haven't built for the newer Rust versions #4404

Open
partouf opened this issue Dec 7, 2022 · 14 comments
Open

[BUG]: Some Rust crates haven't built for the newer Rust versions #4404

partouf opened this issue Dec 7, 2022 · 14 comments
Labels

Comments

@partouf
Copy link
Contributor

partouf commented Dec 7, 2022

Describe the bug

For example serde 1.0.137 is giving this error on build:

    Updating crates.io index
 Downloading crates ...
  Downloaded serde_derive v1.0.137
   Compiling proc-macro2 v1.0.47
   Compiling unicode-ident v1.0.5
   Compiling quote v1.0.21
   Compiling syn v1.0.103
   Compiling serde_derive v1.0.137
   Compiling serde v1.0.137 (/tmp/staging/72613033-27e1-4af7-a747-9f25b5182220/source_serde_1.0.137)
error[E0554]: `#![feature]` may not be used on the stable release channel
  --> src/lib.rs:94:35
   |
94 | #![cfg_attr(feature = "unstable", feature(never_type))]
   |                                   ^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0554`.
error: could not compile `serde` due to previous error

I wonder if it's something that we're doing wrong during build, or that we simply have to use a newer version because this feature just changed between Rust 1.62 and 1.63.

Although it has not changed in the subsequent versions of Serde, the code here is still the same https://github.com/serde-rs/serde/blob/master/serde/src/lib.rs#L87

So maybe this is where we're hitting the "all features" on issue. (we first build with all features, if that fails, we're supposed to build without features)
So maybe that's somehow blocked now due to the build having been registered as failed and it doesn't try to build again...

Will need to do some testing.

Some other time

Steps to reproduce

Expected behavior

rand_core has built without problems

Reproduction link

Not applicable

Screenshots

Not applicable

Operating System

No response

Browser version

No response

@partouf partouf added the bug label Dec 7, 2022
@Elizafox
Copy link

Encountering this as well: https://godbolt.org/z/3WMn4xKKo

@cbiffle
Copy link

cbiffle commented Jul 24, 2023

Hi! It appears that serde has an unstable feature that causes it to rely on features only present in the nightly compiler (unstable features). This effectively restricts the range of compiler versions the crate will build with, because unstable features (by definition) change from compiler revision to compiler revision until the community feels like they're fully baked.

It is probably not a good idea to do an all-features build of serde as a result of this. You may hit this with other crates.

Out of curiosity, why are you doing an all-features build in the first place? I've never seen this done for anything but docs generation (but I assume you have your reasons).

Edited to add: your summary here is on to something:

we simply have to use a newer version because this feature just changed between Rust 1.62 and 1.63.

The other thing you'd need to do is use a nightly build of the compiler -- the error message mentions that it's "stable release channel," which means the unstable features are unavailable. This means you can't use Rust 1.62 or 1.63 because those are stable releases. You could get this feature to build by using a nightly build from somewhere in the 1.63 range if this is something you need.

@partouf
Copy link
Contributor Author

partouf commented Jul 24, 2023

Hi! It appears that serde has an unstable feature that causes it to rely on features only present in the nightly compiler (unstable features). This effectively restricts the range of compiler versions the crate will build with, because unstable features (by definition) change from compiler revision to compiler revision until the community feels like they're fully baked.

It is probably not a good idea to do an all-features build of serde as a result of this. You may hit this with other crates.

Out of curiosity, why are you doing an all-features build in the first place? I've never seen this done for anything but docs generation (but I assume you have your reasons).

Thanks for your message, totally understand about unstable. But we had to introduce it because of other features that don't fall under stable/unstable but are related to things that the user might want to be using.

Here we compile first with --all-features and if that fails, with none
https://github.com/compiler-explorer/infra/blob/main/bin/lib/rust_library_builder.py#L446

We have a caching mechanism that might be preventing us to rebuild without --all-features, so that's one bug we'll need to fix.

But there might also be something like 'we want to build with all the features except "unstable" ..' - would there be something like that?

@partouf
Copy link
Contributor Author

partouf commented Jul 24, 2023

Perhaps also applicable; if we can supply RUSTC_BOOTSTRAP=1 as environment variable (is that possible with cargo?), would that ruin later linking to the library if the user compilation did not supply that envvar... Or could it still compile/link then..

@partouf
Copy link
Contributor Author

partouf commented Jul 24, 2023

Context relating to features: from what I've read https://serde.rs/feature-flags.html#derive "derive" is used often. That's why we went for --all-features.. But we might have to consider listing desired features per crate in our configuration (although that's going to be a pain) just to avoid it also trying to build "unstable" features...

@cbiffle
Copy link

cbiffle commented Jul 24, 2023

Crates aren't under any obligation to make sure every feature even builds at all, much less on every operating system. You could try to heuristically detect features that won't work in your environment, e.g. by looking for particular English names, but it won't work in the general case.

Normally in Rust we let the user specify the feature set. Is that an option here, instead of choosing for everyone?

@cbiffle
Copy link

cbiffle commented Jul 24, 2023

... considering your approach more, I'm curious how you're doing feature unification if you're choosing features for each crate individually.

@partouf
Copy link
Contributor Author

partouf commented Jul 24, 2023

Crates aren't under any obligation to make sure every feature even builds at all, much less on every operating system. You could try to heuristically detect features that won't work in your environment, e.g. by looking for particular English names, but it won't work in the general case.

Normally in Rust we let the user specify the feature set. Is that an option here, instead of choosing for everyone?

We pre-build the libraries. Having user choose their own set and build the libraries with the user provided features will simply take too long both user-time and cpu-time. We're still a website after all...

@cbiffle
Copy link

cbiffle commented Jul 24, 2023

Fair!

Compiling with default features might be a good compromise. It sounds like you're trying to force everything on, and if that fails, turning on none (which takes out serde derive among others). Rust crates have a notion of a default feature set, which appears to be what the rust playground uses to build libraries (their use case is the most similar to yours I can think of).

@partouf
Copy link
Contributor Author

partouf commented Jul 24, 2023

Fair!

Compiling with default features might be a good compromise. It sounds like you're trying to force everything on, and if that fails, turning on none (which takes out serde derive among others). Rust crates have a notion of a default feature set, which appears to be what the rust playground uses to build libraries (their use case is the most similar to yours I can think of).

Ah yes, this list https://github.com/rust-lang/rust-playground/blob/main/compiler/base/Cargo.toml

It's likely the way to go.. it's going to hurt maintaining that list.. but maybe it's the only way

@cbiffle
Copy link

cbiffle commented Jul 24, 2023

Well, you don't need that list to do default features. You could try just not passing --no-default-features in your current build.

That said, they generate that list with a tool that's in that same repo (top-crates) -- I bet you could run the tool if you wanted a nice list.

@partouf
Copy link
Contributor Author

partouf commented Jul 24, 2023

We don't actually pass --no-default-features... but I think with testing I found that some needed features were still missing.. I don't remember. But if that would be good enough, we can certainly leave out the all-features step and focus on getting the normal build to work.

@cbiffle
Copy link

cbiffle commented Jul 24, 2023

Ah, my mistake, I had expected that serde would have derive in its default feature set -- and it does not.

I would say, then, consider lifting the crate list generation tool / canned feature set from the Rust Playground, so that you don't have to maintain your own. While their set of crates is somewhat conservative, it's enough for a lot of purposes.

@cbiffle
Copy link

cbiffle commented Jul 24, 2023

That said, they generate that list with a tool that's in that same repo (top-crates) -- I bet you could run the tool if you wanted a nice list.

Now that I'm at an actual keyboard - if you wanted to mirror the Playground behavior, you could use this:

https://github.com/rust-lang/rust-playground/blob/main/top-crates/src/main.rs

This uses stats from the package manager to do a "popularity contest" of crates. It's playground-specific, but only in that it generates a build file for a package named "playground." It looks like the output path can be overridden. So I bet this would work out of the box.

One of the features it supports is a custom package metadata key, package.metadata.playground. (The package.metadata TOML table is a place where you can put arbitrary additional stuff -- it's used to e.g. control which features are turned on for docs generation on the docs.rs website.) This is how they get serde's derive feature turned on: serde asks them to! https://github.com/serde-rs/serde/blob/master/serde/Cargo.toml#L25

So, if you were to either use the tool or just consume the package.metadata.playground, you'd get a good chunk of this behavior for free, and any updates in downstream crates to support the playground would also update your configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants