feat(build-script): Pass CARGO_CFG_FEATURE #14902

epage · 2024-12-06T16:04:12Z

What does this PR try to resolve?

This may look redundant with CARGO_FEATURE_<CASE_CONVERTED_NAME>=1
except that doesn't provide a lossless way of getting the names, e.g. for
forwarding to child builds like tests that need to build examples. which clap does to test examples. Maintaining that manually is easy to mess up.

This also makes things more consistent as users
conditionalize on features through cfg and this even fits with what
the CARGO_CFG_ docs say:

For each configuration option of the package being built, this
environment variable will contain the value of the configuration, where
is the name of the configuration uppercased and having -
translated to _. Boolean configurations are present if they are set, and
not present otherwise. Configurations with multiple values are joined to
a single variable with the values delimited by , (comma). This includes values
built-in to the compiler (which can be seen with rustc --print=cfg) and
values set by build scripts and extra flags passed to rustc (such as
those defined in RUSTFLAGS). Some examples of what these variables are:

Fixes #3702

How should we test and review this PR?

Additional information

rustbot · 2024-12-06T16:04:18Z

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

epage · 2024-12-06T16:04:23Z

@rfcbot fcp merge

rfcbot · 2024-12-06T16:04:38Z

Team member @epage has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

epage · 2024-12-06T16:04:42Z

crates/build-rs/src/input.rs

@@ -102,6 +102,13 @@ mod cfg {
    // those disabled with #[cfg(any())] don't seem meaningfully useful
    // but we list all cfg that are default known to check-cfg

+    /// Each activated feature of the package being built
+    #[doc = requires_msrv!("1.85")]


This will need to be updated based on when this gets merged

It'd probably be beneficial to set up a CURRENT_RUST_VERSION placeholder like is used for stabilization in rustc

Yes, we only recently started tracking these situations more regularly in 1.81 and haven't looked at updating our processes yet. So far, this has been working out ok.

epage · 2024-12-06T16:17:04Z

#14901 will perform the version bump for build-rs

src/doc/src/reference/environment-variables.md

crates/build-rs-test-lib/build.rs

epage · 2024-12-09T18:28:36Z

CC @CAD97

### What does this PR try to resolve? My previous audit mostly focused on env variables and missed that `cargo::error` hadn't been added yet. This also includes other clean up along the way to #14902 and fixing of some env variable handling that I'm working towards. ### How should we test and review this PR? ### Additional information

This may look redundant with `CARGO_FEATURE_<CASE_CONVERTED_NAME>=1` except that doesn't provide a lossless way of getting the names, e.g. for forwarding for child builds like tests that need to build examples. This also makes things more consistent as users conditionalize on features through `cfg` and this even fits with what the `CARGO_CFG_` docs say: > For each configuration option of the package being built, this > environment variable will contain the value of the configuration, where > <cfg> is the name of the configuration uppercased and having - > translated to _. Boolean configurations are present if they are set, and > not present otherwise. Configurations with multiple values are joined to > a single variable with the values delimited by ,. This includes values > built-in to the compiler (which can be seen with rustc --print=cfg) and > values set by build scripts and extra flags passed to rustc (such as > those defined in RUSTFLAGS). Some examples of what these variables are: Fixes rust-lang#3702

rfcbot · 2024-12-10T15:25:41Z

🔔 This is now entering its final comment period, as per the review above. 🔔

tests/testsuite/build_script.rs

epage · 2024-12-10T18:42:56Z

As for the syntax for multiple values, the docs do say

Configurations with multiple values are joined to a single variable with the values delimited by ,.

weihanglo

Looks good. Thank you!

I cannot think of any reason this may break existing code, so will merge it now. If there is anything bad happening, we can revert then.

weihanglo · 2024-12-10T20:59:18Z

---- ssh::known_host_works stdout ----
SSH_AGENT_PID=79472
thread 'ssh::known_host_works' panicked at tests/testsuite/ssh.rs:187:10:
called `Option::unwrap()` on a `None` value

Spurious network error?

Update cargo 18 commits in 20a443231846b81c7b909691ec3f15eb173f2b18..7847c03965260b5dcc8d93218d6af295a717abb6 2024-12-06 21:56:56 +0000 to 2024-12-13 18:06:39 +0000 - fix(base): Support bases in patches in virtual manifests (rust-lang/cargo#14931) - fix(resolver): Report invalid index entries (rust-lang/cargo#14927) - feat: Implement `--depth workspace` for `cargo tree` command (rust-lang/cargo#14928) - fix(resolver): In errors, show rejected versions over alt versions (rust-lang/cargo#14923) - fix: emit_serialized_unit_graph uses the configured shell (rust-lang/cargo#14926) - fix(script): Don't override the release profile (rust-lang/cargo#14925) - feature(SourceId): use stable hash from rustc-stable-hash (rust-lang/cargo#14917) - fix(resolver): Don't report all versions as rejected (rust-lang/cargo#14921) - fix(resolver): Report unmatched versions, rather than saying no package (rust-lang/cargo#14897) - fix(build-rs): Implicitly report rerun-if-env-changed for input (rust-lang/cargo#14911) - a faster hash for ActivationsKey (rust-lang/cargo#14915) - feat(build-script): Pass CARGO_CFG_FEATURE (rust-lang/cargo#14902) - fix(build-rs): Correctly refer to the item in assert (rust-lang/cargo#14913) - chore: update auto-label to include build-rs crate (rust-lang/cargo#14912) - refactor: use Path::push to construct remap-path-prefix (rust-lang/cargo#14908) - feat(build-rs): Add the 'error' directive (rust-lang/cargo#14910) - fix(build-std): determine root crates by target spec `std:bool` (rust-lang/cargo#14899) - SemVer: Add section on RPIT capturing (rust-lang/cargo#14849)

Update cargo 19 commits in 20a443231846b81c7b909691ec3f15eb173f2b18..769f622e12db0001431d8ae36d1093fb8727c5d9 2024-12-06 21:56:56 +0000 to 2024-12-14 04:27:35 +0000 - test(build-std): dont require rustup (rust-lang/cargo#14933) - fix(base): Support bases in patches in virtual manifests (rust-lang/cargo#14931) - fix(resolver): Report invalid index entries (rust-lang/cargo#14927) - feat: Implement `--depth workspace` for `cargo tree` command (rust-lang/cargo#14928) - fix(resolver): In errors, show rejected versions over alt versions (rust-lang/cargo#14923) - fix: emit_serialized_unit_graph uses the configured shell (rust-lang/cargo#14926) - fix(script): Don't override the release profile (rust-lang/cargo#14925) - feature(SourceId): use stable hash from rustc-stable-hash (rust-lang/cargo#14917) - fix(resolver): Don't report all versions as rejected (rust-lang/cargo#14921) - fix(resolver): Report unmatched versions, rather than saying no package (rust-lang/cargo#14897) - fix(build-rs): Implicitly report rerun-if-env-changed for input (rust-lang/cargo#14911) - a faster hash for ActivationsKey (rust-lang/cargo#14915) - feat(build-script): Pass CARGO_CFG_FEATURE (rust-lang/cargo#14902) - fix(build-rs): Correctly refer to the item in assert (rust-lang/cargo#14913) - chore: update auto-label to include build-rs crate (rust-lang/cargo#14912) - refactor: use Path::push to construct remap-path-prefix (rust-lang/cargo#14908) - feat(build-rs): Add the 'error' directive (rust-lang/cargo#14910) - fix(build-std): determine root crates by target spec `std:bool` (rust-lang/cargo#14899) - SemVer: Add section on RPIT capturing (rust-lang/cargo#14849)

epage added the T-cargo Team: Cargo label Dec 6, 2024

rustbot assigned ehuss Dec 6, 2024

rustbot added A-build-scripts Area: build.rs scripts A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 6, 2024

rfcbot added proposed-final-comment-period An FCP proposal has started, but not yet signed off. disposition-merge FCP with intent to merge labels Dec 6, 2024

epage commented Dec 6, 2024

View reviewed changes

weihanglo reviewed Dec 6, 2024

View reviewed changes

src/doc/src/reference/environment-variables.md Show resolved Hide resolved

crates/build-rs-test-lib/build.rs Outdated Show resolved Hide resolved

epage force-pushed the cfg-feature branch 3 times, most recently from 8147bb3 to fb1794a Compare December 9, 2024 15:19

epage mentioned this pull request Dec 9, 2024

feat(build-rs): Add the 'error' directive #14910

Merged

epage force-pushed the cfg-feature branch from fb1794a to f4ef735 Compare December 9, 2024 18:39

CAD97 approved these changes Dec 9, 2024

View reviewed changes

rfcbot added final-comment-period FCP — a period for last comments before action is taken and removed proposed-final-comment-period An FCP proposal has started, but not yet signed off. labels Dec 10, 2024

weihanglo reviewed Dec 10, 2024

View reviewed changes

tests/testsuite/build_script.rs Outdated Show resolved Hide resolved

epage force-pushed the cfg-feature branch 2 times, most recently from 69f1c3f to 8e90ce9 Compare December 10, 2024 18:42

weihanglo approved these changes Dec 10, 2024

View reviewed changes

weihanglo added this pull request to the merge queue Dec 10, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 10, 2024

weihanglo added this pull request to the merge queue Dec 10, 2024

Merged via the queue into rust-lang:master with commit a451139 Dec 10, 2024
20 checks passed

epage deleted the cfg-feature branch December 10, 2024 21:59

weihanglo mentioned this pull request Dec 13, 2024

Update cargo rust-lang/rust#134278

Merged

rustbot added this to the 1.85.0 milestone Dec 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(build-script): Pass CARGO_CFG_FEATURE #14902

feat(build-script): Pass CARGO_CFG_FEATURE #14902

epage commented Dec 6, 2024 •

edited by Muscraft

Loading

rustbot commented Dec 6, 2024

epage commented Dec 6, 2024

rfcbot commented Dec 6, 2024 •

edited by weihanglo

Loading

epage Dec 6, 2024

CAD97 Dec 9, 2024

epage Dec 9, 2024

epage commented Dec 6, 2024

epage commented Dec 9, 2024

rfcbot commented Dec 10, 2024

epage commented Dec 10, 2024

weihanglo left a comment

weihanglo commented Dec 10, 2024

feat(build-script): Pass CARGO_CFG_FEATURE #14902

feat(build-script): Pass CARGO_CFG_FEATURE #14902

Conversation

epage commented Dec 6, 2024 • edited by Muscraft Loading

What does this PR try to resolve?

How should we test and review this PR?

Additional information

rustbot commented Dec 6, 2024

epage commented Dec 6, 2024

rfcbot commented Dec 6, 2024 • edited by weihanglo Loading

epage Dec 6, 2024

Choose a reason for hiding this comment

CAD97 Dec 9, 2024

Choose a reason for hiding this comment

epage Dec 9, 2024

Choose a reason for hiding this comment

epage commented Dec 6, 2024

epage commented Dec 9, 2024

rfcbot commented Dec 10, 2024

epage commented Dec 10, 2024

weihanglo left a comment

Choose a reason for hiding this comment

weihanglo commented Dec 10, 2024

epage commented Dec 6, 2024 •

edited by Muscraft

Loading

rfcbot commented Dec 6, 2024 •

edited by weihanglo

Loading