buildRustCrate: set CARGO_FEATURE_* when running the build script #90193
Conversation
Cargo sets `CARGO_FEATURE_*` for all features when running a build script: https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts Some crates have build scripts (e.g. openblas-src) that rely on the feature variables being properly set. Since we now need several representations of features, this change also updates `createFeatures` to be a list of features, rather than `rustc` feature arguments. `configureCrate` and `buildCrate` then build the required representations as-needed. Fixes NixOS#68978
danieldk
force-pushed
the
build-rs-feature-env
branch
from
4a74e4f
to
fe50bab
Compare
| # Features should be set as environment variable for build scripts: | ||
| # https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts | ||
| for feature in ${envFeatures}; do | ||
| export CARGO_FEATURE_$feature=1 |
There was a problem hiding this comment.
are features allowed to contain whitespaces? Is this the sanest way to set them?
There was a problem hiding this comment.
are features allowed to contain whitespaces?
Yes, they are, though I don’t believe anybody actually uses features with spaces in them. Still a worthwhile corner-case to handle.
There was a problem hiding this comment.
Value is hardcoded to 1 in cargo
There was a problem hiding this comment.
Yes, they are, though I don’t believe anybody actually uses features with spaces in them. Still a worthwhile corner-case to handle.
I didn't treat any other cases, since Cargo does not do anything besides uppercasing and replacing dashes either:
(Unless the features are pre-processed somewhere else.)
There was a problem hiding this comment.
Interesting, Cargo will set the environment variable with a space in it. So I guess we'll have to do that too.
There was a problem hiding this comment.
Currently features with spaces seem to fail in other places. Once a feature contains a space, it is also incorrectly passed to rustc and already fails when compiling the build script.
I guess supporting features with spaces would be better to handle in a separate PR.
There was a problem hiding this comment.
Yeah, I agree. Is anyone going to do that? ;)
There was a problem hiding this comment.
I have created an issue #90605 . I'll try to have a look at it one of the next few days.
|
@ofborg build buildRustCrateTests |
Motivation for this change
Cargo sets
CARGO_FEATURE_*for all features when running a buildscript:
https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts
Some crates have build scripts (e.g. openblas-src) that rely on the
feature variables being properly set.
Since we now need several representations of features, this change
also updates
createFeaturesto be a list of features, rather thanrustcfeature arguments.configureCrateandbuildCratethenbuild the required representations as-needed.
Fixes #68978
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)