Add unstable `rustc-check-cfg` build script output by Urgau · Pull Request #10539 · rust-lang/cargo

Urgau · 2022-04-07T11:31:00Z

This PR adds a new build script output as unstable behind -Zcheck-cfg=output: rustc-check-cfg.

What does this PR try to resolve?

This PR add a way to add to use the unstable --check-cfg command line option of rustc and rustdoc.

It was discover in Bump bootstrap compiler to 1.61.0 beta that rustc_llvm sets some custom cfg from a build script and because --check-cfg=values() is globally enable in the Rust codebase that cause the compilation to fail. For now no values are checked in stage 0 for the entire codebase which is a shame and should be fixed with the addition of this feature.

How should we test and review this PR?

Commits are separated in: implementation, tests and doc.

Testing should simply be done by adding a valid cargo:rustc-check-cfg in a build script.
Watch the added tests or doc to have an example.

Additional information

This PR is also the logical next step after -Zcheck-cfg-features.

rust-highfive · 2022-04-07T11:31:03Z

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

bors · 2022-04-12T15:18:09Z

☔ The latest upstream changes (presumably #10486) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2022-04-28T04:04:02Z

☔ The latest upstream changes (presumably #10609) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2022-04-28T20:19:05Z

☔ The latest upstream changes (presumably #10611) made this pull request unmergeable. Please resolve the merge conflicts.

ehuss

This doesn't seem to pass the --check-cfg flag to rustdoc, was that intentional?

src/doc/src/reference/build-scripts.md

src/cargo/core/compiler/custom_build.rs

tests/testsuite/build_script.rs

Urgau · 2022-05-03T20:58:24Z

This doesn't seem to pass the --check-cfg flag to rustdoc, was that intentional?

No, not at all. Fixed and added a test for it.

I also fixed or responded to the review comments.
The PR should be ready for another review.

Urgau · 2022-05-07T09:51:33Z

I've updated the PR to use the improvements of #10566.
The tests are now in the check_cfg module, they are now also activated on Windows and this addition is now gated under -Zcheck-cfg=output (ie. the output of the build script, not sure that a great name).

@rustbot ready

src/cargo/core/compiler/custom_build.rs

tests/testsuite/check_cfg.rs

src/doc/src/reference/unstable.md

ehuss · 2022-05-18T17:04:20Z

src/cargo/core/compiler/mod.rs

I'd like to see a little bit of cleanup to add the --cfg flag handling in rustc and rustdoc. It looks like there is a bit of duplication and inconsistency around handling build-script flags (like --cfg, --check-cfg, and env).

A rough outline of the changes I'm thinking:

Add a new function to cargo::core::compiler that will handle adding these common flags. I think it might have the signature of something like fn add_custom_flags(cmd: &mut ProcessBuilder, build_script_outputs: &BuildScriptOutputs, metadata: Option<Metadata>)

Move all of add_custom_env to that function.

Move all of --cfg flag handling to that function.

Move all of --check-cfg flag handling to that function.

Use that function in both rustc and rustdoc removing the duplicate code.

How does that sound?

@ehuss Sound good. Do you want to do it in this PR or as a follow-up ? (I feel like it doesn't really make sense to do it in this PR, but your call)

Here is good because it is adding the --check-cfg arg.

Done (expect for doc-tests because of it's specificity that make it unable to use the new function).

ehuss · 2022-05-20T00:55:19Z

Thanks!

@bors r+

bors · 2022-05-20T00:55:21Z

📌 Commit f2a1e43 has been approved by ehuss

bors · 2022-05-20T00:55:28Z

⌛ Testing commit f2a1e43 with merge c8c6e33...

bors · 2022-05-20T01:56:01Z

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing c8c6e33 to master...

Update cargo 10 commits in a4c1cd0eb6b18082a7e693f5a665548fe1534be4..39ad1039d9e3e1746177bf5d134af4c164f95528 2022-05-20 00:55:25 +0000 to 2022-05-25 00:50:02 +0000 * doc: discuss build script instruction order (rust-lang/cargo#10600) * Require http-registry URLs to end with a '/' (rust-lang/cargo#10698) * No printing executable names when running tests and benchmarks with json message format (rust-lang/cargo#10691) * Restore proper error for crate not in local reg (rust-lang/cargo#10683) * Update libcurl (rust-lang/cargo#10696) * Fixed small typos (rust-lang/cargo#10693) * fix bugs with `workspace` key and `update_toml` (rust-lang/cargo#10685) * Bump to 0.64.0, update changelog (rust-lang/cargo#10687) * List C compiler as a build dependency in README (rust-lang/cargo#10678) * Add unstable `rustc-check-cfg` build script output (rust-lang/cargo#10539) r? `@ehuss`

…weihanglo Fix panic when running cargo tree on a package with a cross compiled bindep ### What does this PR try to resolve? This is an attempt to close out `@rukai's` [PR](#13207 (comment)) for #12358 and #10593 by adjusting the new integration test and resolving merge conflicts. I have also separated the changes into atomic commits as per [previous review](#13207 (comment)). ### How should we test and review this PR? The integration test that has been edited here is sufficient, plus the new integration test that confirms a more specific case where `cargo tree` throws an error. ### Additional information I have confirmed the test `artifact_dep_target_specified` fails on master branch and succeeds on this branch. The first commit fixes the panic and the integration test. Commits 2 and 3 add other tests that confirm behaviour mentioned in related issues. Commits: 1. [Fix panic when running cargo tree on a package with a cross compiled bindep](5c5ea78) - fixes some panics and changes the integration test to succeed 2. [test: cargo tree panic on artifact dep target deactivated](ed294ab) - adds test to confirm the behaviour for the specific panic from [#10539 (comment)](#10593 (comment))

rust-highfive assigned ehuss Apr 7, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 7, 2022

weihanglo mentioned this pull request Apr 11, 2022

Tracking Issue for rustc --check-cfg integration #10554

Closed

2 tasks

Urgau force-pushed the check-cfg-build-script branch from 3679ccc to 637a7a5 Compare April 12, 2022 16:31

Urgau force-pushed the check-cfg-build-script branch from 637a7a5 to ca984f6 Compare April 28, 2022 08:47

Urgau force-pushed the check-cfg-build-script branch from ca984f6 to 9d5c359 Compare April 29, 2022 13:32

ehuss reviewed May 3, 2022

View reviewed changes

src/doc/src/reference/build-scripts.md Outdated Show resolved Hide resolved

src/cargo/core/compiler/custom_build.rs Outdated Show resolved Hide resolved

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

Urgau force-pushed the check-cfg-build-script branch from 9d5c359 to 76fff18 Compare May 3, 2022 20:51

Urgau force-pushed the check-cfg-build-script branch 2 times, most recently from 100429f to 63ee29f Compare May 7, 2022 09:43

ehuss reviewed May 18, 2022

View reviewed changes

Urgau added 3 commits May 18, 2022 21:26

Add support for cargo:rustc-check-cfg in build script

d16631f

Add tests for cargo:rustc-check-cfg

805f5c2

Add doc for -Zextra-check-cfg and rustc-check-cfg

276c6d1

Urgau force-pushed the check-cfg-build-script branch from 63ee29f to 276c6d1 Compare May 18, 2022 21:17

Factor custom flags management to one function

f2a1e43

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 20, 2022

bors merged commit c8c6e33 into rust-lang:master May 20, 2022

bors mentioned this pull request May 20, 2022

Add fallible units for scrape-examples feature #10596

Closed

Urgau mentioned this pull request May 20, 2022

Tracking Issue for RFC 3013: Checking conditional compilation at compile time rust-lang/rust#82450

Closed

3 tasks

arlosi mentioned this pull request May 25, 2022

Update cargo rust-lang/rust#97403

Merged

ehuss added this to the 1.63.0 milestone Jun 1, 2022

Comments

Conversation

Urgau commented Apr 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR try to resolve?

How should we test and review this PR?

Additional information

Uh oh!

rust-highfive commented Apr 7, 2022

Uh oh!

bors commented Apr 12, 2022

Uh oh!

bors commented Apr 28, 2022

Uh oh!

bors commented Apr 28, 2022

Uh oh!

ehuss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Urgau commented May 3, 2022

Uh oh!

Urgau commented May 7, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ehuss May 18, 2022

Choose a reason for hiding this comment

Uh oh!

Urgau May 18, 2022

Choose a reason for hiding this comment

Uh oh!

ehuss May 18, 2022

Choose a reason for hiding this comment

Uh oh!

Urgau May 18, 2022

Choose a reason for hiding this comment

Uh oh!

ehuss commented May 20, 2022

Uh oh!

bors commented May 20, 2022

Uh oh!

bors commented May 20, 2022

Uh oh!

bors commented May 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Urgau commented Apr 7, 2022 •

edited

Loading