Reinstate CARGO_PRIMARY_PACKAGE (take 2) #8758

ebroto · 2020-10-06T21:15:04Z

rust-highfive · 2020-10-06T21:15:07Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

alexcrichton · 2020-10-08T21:32:51Z

Would you be ok adding documentation for this new environment variable as well as a test?

ehuss · 2020-10-08T22:29:15Z

Ignore me.
Just a minor note: It might also be worthwhile to consider a more precise name. This is only set for the root units, and isn't necessarily the root "package". For example, if you run cargo test, any binaries in a package will be built, but they aren't "primary". Or cargo build --bin foo, the binary target will be a root, but the library will not be marked as "primary".

~~Maybe CARGO_ROOT_TARGET? CARGO_ROOT_UNIT? CARGO_PRIMARY_TARGET? CARGO_ROOT_CRATE? Hm, these terms are a bit overloaded, vague, or potentially confusing. Any other ideas?~~

ehuss · 2020-10-08T22:31:20Z

Oh, wait, nevermind, I misread how that is implemented. CARGO_PRIMARY_PACKAGE is good I think.

ebroto · 2020-10-12T14:07:53Z

Would you be ok adding documentation for this new environment variable as well as a test?

Of course! Besides the doc, I've added the following tests:

in build.rs, make sure it's only set for the primary package and not for deps of any kind
in workspaces.rs, one for "simple" and one for virtual workspaces, trying some combinations of -p and --all.

Please tell me if I missed some cases or if I put them in the wrong place :)

ehuss · 2020-10-12T18:37:25Z

Thanks!

@rust-lang/cargo This will add a new, stable environment variable set when running rustc. CARGO_PRIMARY_PACKAGE indicates that this is one of the "root" packages requested on the command-line (via -p or the default package). The use case for this is primarily for clippy, so that it can know if it should emit lints. In particular, this will help with the --fix flag, so that it doesn't attempt to run clippy on other dependencies in a workspace, which can cause errors and problems. This can also help with solving rust-lang/rust-clippy#3025, though that issue will require some additional UI on the clippy side to determine whether or not it runs for all members. This should help lead towards stabilizing #8143 (RUSTC_WORKSPACE_WRAPPER) which will fix the caching problems with clippy.

I think it is OK to go straight to stable for this, though feel free to disagree. I expect that after updating clippy and testing some things out, we can move to stabilize RUSTC_WORKSPACE_WRAPPER soon as well.

@rfcbot fcp merge

rfcbot · 2020-10-12T18:37:27Z

Team member @ehuss 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.

Mark-Simulacrum · 2020-10-12T18:42:07Z

FWIW perf.rlo is also interested in this and uses a hack (cargo rustc -- --wrap-rustc-with $prog) to workaround this today; the extra argument is intercepted with a wrapper script and stripped (some other processing is also done). Once this is added I'd definitely be interested in reworking perf to rely on it over the wrap-rustc-with option.

rfcbot · 2020-10-13T04:04:00Z

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

ehuss · 2020-10-14T20:38:00Z

@bors r+

bors · 2020-10-14T20:38:01Z

📌 Commit 6a94a3b has been approved by ehuss

bors · 2020-10-14T20:38:09Z

⌛ Testing commit 6a94a3b with merge 900f981...

Reinstate CARGO_PRIMARY_PACKAGE (take 2) As discussed in [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/RUSTC_WORKSPACE_WRAPPER/near/212461098) r? `@ehuss`

bors · 2020-10-14T20:54:39Z

💔 Test failed - checks-actions

ehuss · 2020-10-14T21:01:10Z

Oh, I should have paid closer attention to the CI status. @ebroto, the tests can't check the environment variables via the verbose output, since the output is different for different platforms. Can you update the tests to a different approach? There are a variety of different options. Maybe just use env! to capture the value, and use cargo run or cargo test to run some code the verify the value is correct?

Environment variables are represented differently in differents OSes in the output. Add tests checking if the variable is set instead.

ebroto · 2020-10-14T22:48:57Z

@ehuss I've reworked the tests following your suggestion. I think it helped to simplify the implementation. Thanks!

ehuss · 2020-10-14T23:07:41Z

Thanks!
@bors r+

bors · 2020-10-14T23:07:43Z

📌 Commit fb02ade has been approved by ehuss

bors · 2020-10-14T23:07:50Z

⌛ Testing commit fb02ade with merge 12db56c...

bors · 2020-10-14T23:37:42Z

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

Update cargo 11 commits in 9d1a4863abd9237dbf9d1b74c78632b6a205f6bb..12db56cdedbc2c26a9aa18f994c0188cdcc67df5 2020-10-05 18:29:52 +0000 to 2020-10-14 23:07:45 +0000 - Reinstate CARGO_PRIMARY_PACKAGE (take 2) (rust-lang/cargo#8758) - Add actionable help message for --features (rust-lang/cargo#8773) - Fix panic in `cargo doc` with -Zfeatures=itarget (rust-lang/cargo#8777) - Update git2. (rust-lang/cargo#8778) - Document RUSTFMT environment variable (rust-lang/cargo#8767) - Update crossbeam-utils requirement from 0.7 to 0.8 (rust-lang/cargo#8769) - Update toml dependency (rust-lang/cargo#8772) - Mark proc-macro crates (rust-lang/cargo#8765) - cargo-tree: mention special target `all` in CLI help text (rust-lang/cargo#8766) - Bump to 0.50.0, update changelog (rust-lang/cargo#8764) - Update deprecated GitHub add-path in workflows. (rust-lang/cargo#8760)

Add --no-deps option to avoid running on path dependencies in workspaces Since rust-lang/cargo#8758 has hit nightly, this allows us to address the second bullet point and [the concern related to `--fix`](rust-lang/cargo#8143 (comment)) in the [RUSTC_WORKSPACE_WRAPPER tracking issue](rust-lang/cargo#8143). As a reminder stabilizing that env var will solve #4612 (Clippy not running after `cargo check` in stable) and would allow to stabilize the `--fix` option in Clippy. changelog: Add `--no-deps` option to avoid running on path dependencies in workspaces Fixes #3025

See also rust-lang/cargo#8758 and rust-lang/rust-clippy#6188

Pkgsrc changes: * Adjust patches, convert tabs to spaces so that tests pass. * Remove patches which are no longer needed (upstream changed) * Minor adjustments for SunOS, e.g. disable stack probes. * Adjust cargo checksum patching accordingly. * Remove commented-out use of PATCHELF on NetBSD, which doesn't work anyway... Upstream changes: Version 1.49.0 (2020-12-31) ============================ Language ----------------------- - [Unions can now implement `Drop`, and you can now have a field in a union with `ManuallyDrop<T>`.][77547] - [You can now cast uninhabited enums to integers.][76199] - [You can now bind by reference and by move in patterns.][76119] This allows you to selectively borrow individual components of a type. E.g. ```rust #[derive(Debug)] struct Person { name: String, age: u8, } let person = Person { name: String::from("Alice"), age: 20, }; // `name` is moved out of person, but `age` is referenced. let Person { name, ref age } = person; println!("{} {}", name, age); ``` Compiler ----------------------- - [Added tier 1\* support for `aarch64-unknown-linux-gnu`.][78228] - [Added tier 2 support for `aarch64-apple-darwin`.][75991] - [Added tier 2 support for `aarch64-pc-windows-msvc`.][75914] - [Added tier 3 support for `mipsel-unknown-none`.][78676] - [Raised the minimum supported LLVM version to LLVM 9.][78848] - [Output from threads spawned in tests is now captured.][78227] - [Change os and vendor values to "none" and "unknown" for some targets][78951] \* Refer to Rust's [platform support page][forge-platform-support] for more information on Rust's tiered platform support. Libraries ----------------------- - [`RangeInclusive` now checks for exhaustion when calling `contains` and indexing.][78109] - [`ToString::to_string` now no longer shrinks the internal buffer in the default implementation.][77997] - [`ops::{Index, IndexMut}` are now implemented for fixed sized arrays of any length.][74989] Stabilized APIs --------------- - [`slice::select_nth_unstable`] - [`slice::select_nth_unstable_by`] - [`slice::select_nth_unstable_by_key`] The following previously stable methods are now `const`. - [`Poll::is_ready`] - [`Poll::is_pending`] Cargo ----------------------- - [Building a crate with `cargo-package` should now be independently reproducible.][cargo/8864] - [`cargo-tree` now marks proc-macro crates.][cargo/8765] - [Added `CARGO_PRIMARY_PACKAGE` build-time environment variable.] [cargo/8758] This variable will be set if the crate being built is one the user selected to build, either with `-p` or through defaults. - [You can now use glob patterns when specifying packages & targets.][cargo/8752] Compatibility Notes ------------------- - [Demoted `i686-unknown-freebsd` from host tier 2 to target tier 2 support.][78746] - [Macros that end with a semi-colon are now treated as statements even if they expand to nothing.][78376] - [Rustc will now check for the validity of some built-in attributes on enum variants.][77015] Previously such invalid or unused attributes could be ignored. - Leading whitespace is stripped more uniformly in documentation comments, which may change behavior. You read [this post about the changes][rustdoc-ws-post] for more details. - [Trait bounds are no longer inferred for associated types.][79904] Internal Only ------------- These changes provide no direct user facing benefits, but represent significant improvements to the internals and overall performance of rustc and related tools. - [rustc's internal crates are now compiled using the `initial-exec` Thread Local Storage model.][78201] - [Calculate visibilities once in resolve.][78077] - [Added `system` to the `llvm-libunwind` bootstrap config option.][77703] - [Added `--color` for configuring terminal color support to bootstrap.][79004] [75991]: rust-lang/rust#75991 [78951]: rust-lang/rust#78951 [78848]: rust-lang/rust#78848 [78746]: rust-lang/rust#78746 [78376]: rust-lang/rust#78376 [78228]: rust-lang/rust#78228 [78227]: rust-lang/rust#78227 [78201]: rust-lang/rust#78201 [78109]: rust-lang/rust#78109 [78077]: rust-lang/rust#78077 [77997]: rust-lang/rust#77997 [77703]: rust-lang/rust#77703 [77547]: rust-lang/rust#77547 [77015]: rust-lang/rust#77015 [76199]: rust-lang/rust#76199 [76119]: rust-lang/rust#76119 [75914]: rust-lang/rust#75914 [74989]: rust-lang/rust#74989 [79004]: rust-lang/rust#79004 [78676]: rust-lang/rust#78676 [79904]: rust-lang/rust#79904 [cargo/8864]: rust-lang/cargo#8864 [cargo/8765]: rust-lang/cargo#8765 [cargo/8758]: rust-lang/cargo#8758 [cargo/8752]: rust-lang/cargo#8752 [`slice::select_nth_unstable`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable [`slice::select_nth_unstable_by`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by [`slice::select_nth_unstable_by_key`]: https://doc.rust-lang.org/nightly/std/primitive.slice.html#method.select_nth_unstable_by_key [`hint::spin_loop`]: https://doc.rust-lang.org/stable/std/hint/fn.spin_loop.html [`Poll::is_ready`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_ready [`Poll::is_pending`]: https://doc.rust-lang.org/stable/std/task/enum.Poll.html#method.is_pending [rustdoc-ws-post]: https://blog.guillaume-gomez.fr/articles/2020-11-11+New+doc+comment+handling+in+rustdoc

rust-highfive assigned ehuss Oct 6, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 6, 2020

ehuss added the T-cargo Team: Cargo label Oct 12, 2020

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

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 Oct 13, 2020

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 Oct 14, 2020

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

ebroto added 4 commits October 14, 2020 23:35

Reinstate CARGO_PRIMARY_PACKAGE

6a0a35c

Add documentation for CARGO_PRIMARY_PACKAGE

f7ddbed

Add test for CARGO_PRIMARY_PACKAGE

0f20b2e

Add tests for CARGO_PRIMARY_PACKAGE (workspaces)

c12aed2

Rework tests to avoid using -vv

fb02ade

Environment variables are represented differently in differents OSes in the output. Add tests checking if the variable is set instead.

ebroto force-pushed the cargo_primary_package branch from 6a94a3b to fb02ade Compare October 14, 2020 22:46

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 Oct 14, 2020

ebroto mentioned this pull request Oct 14, 2020

Suggestion: run clippy for specific package in workspace rust-lang/rust-clippy#2518

Closed

bors merged commit 12db56c into rust-lang:master Oct 14, 2020

ebroto deleted the cargo_primary_package branch October 14, 2020 23:38

ehuss mentioned this pull request Oct 15, 2020

Update cargo rust-lang/rust#77952

Merged

ebroto mentioned this pull request Oct 17, 2020

Add --no-deps option to avoid running on path dependencies in workspaces rust-lang/rust-clippy#6188

Merged

rfcbot added finished-final-comment-period FCP complete to-announce and removed final-comment-period FCP — a period for last comments before action is taken labels Oct 23, 2020

fpoli added a commit to viperproject/prusti-dev that referenced this pull request Dec 10, 2020

Use CARGO_PRIMARY_PACKAGE to skip dependencies

c3aa699

See also rust-lang/cargo#8758 and rust-lang/rust-clippy#6188

fpoli mentioned this pull request Dec 10, 2020

Use CARGO_PRIMARY_PACKAGE to skip dependencies viperproject/prusti-dev#316

Merged

fpoli added a commit to viperproject/prusti-dev that referenced this pull request Dec 14, 2020

Use CARGO_PRIMARY_PACKAGE to skip dependencies

e5a4477

See also rust-lang/cargo#8758 and rust-lang/rust-clippy#6188

ehuss mentioned this pull request Dec 14, 2020

Stabilize RUSTC_WORKSPACE_WRAPPER #8976

Merged

ehuss added this to the 1.49.0 milestone Feb 6, 2022

ghost mentioned this pull request May 30, 2022

Expose rust-version through env var #10713

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reinstate CARGO_PRIMARY_PACKAGE (take 2) #8758

Reinstate CARGO_PRIMARY_PACKAGE (take 2) #8758

ebroto commented Oct 6, 2020

rust-highfive commented Oct 6, 2020

alexcrichton commented Oct 8, 2020

ehuss commented Oct 8, 2020 •

edited

Loading

ehuss commented Oct 8, 2020

ebroto commented Oct 12, 2020

ehuss commented Oct 12, 2020

rfcbot commented Oct 12, 2020 •

edited by joshtriplett

Loading

Mark-Simulacrum commented Oct 12, 2020

rfcbot commented Oct 13, 2020

ehuss commented Oct 14, 2020

bors commented Oct 14, 2020

bors commented Oct 14, 2020

bors commented Oct 14, 2020

ehuss commented Oct 14, 2020

ebroto commented Oct 14, 2020

ehuss commented Oct 14, 2020

bors commented Oct 14, 2020

bors commented Oct 14, 2020

bors commented Oct 14, 2020

Reinstate CARGO_PRIMARY_PACKAGE (take 2) #8758

Reinstate CARGO_PRIMARY_PACKAGE (take 2) #8758

Conversation

ebroto commented Oct 6, 2020

rust-highfive commented Oct 6, 2020

alexcrichton commented Oct 8, 2020

ehuss commented Oct 8, 2020 • edited Loading

ehuss commented Oct 8, 2020

ebroto commented Oct 12, 2020

ehuss commented Oct 12, 2020

rfcbot commented Oct 12, 2020 • edited by joshtriplett Loading

Mark-Simulacrum commented Oct 12, 2020

rfcbot commented Oct 13, 2020

ehuss commented Oct 14, 2020

bors commented Oct 14, 2020

bors commented Oct 14, 2020

bors commented Oct 14, 2020

ehuss commented Oct 14, 2020

ebroto commented Oct 14, 2020

ehuss commented Oct 14, 2020

bors commented Oct 14, 2020

bors commented Oct 14, 2020

bors commented Oct 14, 2020

ehuss commented Oct 8, 2020 •

edited

Loading

rfcbot commented Oct 12, 2020 •

edited by joshtriplett

Loading