Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name #9365

jyn514 · 2021-04-16T15:50:12Z

Fixes #9362.
The whole point of rust-lang/rust#77802 was to allow specifying this granularly, giving a hard error defeats the point.

I didn't know how to check what targets were reverse-dependencies of build.rs, so I just unconditionally use the library name (and give a hard error for anything else, even if it's the name of one of the binaries). End-users can still opt-in with RUSTC_BOOTSTRAP=1, and no public binaries use RUSTC_BOOTSTRAP=1, so I don't think this a big deal in practice.

Script to verify all crates using RUSTC_BOOTSTRAP=1 have a library

curl https://pastebin.com/raw/fGQ97xP6 | cut -d / -f1 | grep -v shnatsel | grep -v cargo- | sed 's#-\([0-9]\)#/\1#' | xargs -i curl -s -I -L "https://docs.rs/{}/" -w "%{http_code}\n" -o/dev/null

It should output 20 200s in a row.

r? @ehuss cc @Mark-Simulacrum

I don't know what cargo's policy is for backports, but this should be backported to 1.52.

jyn514 · 2021-04-16T15:51:02Z

src/cargo/core/compiler/custom_build.rs

+                            std::env::var("RUSTC_BOOTSTRAP").map_or(false, |var| {
+                                var.split(',').any(|s| s == name)
+                            })


This is copied from https://github.com/jyn514/rust/blob/0679a4cd93db7dfd33b45cd5bd6be52a0f5e894a/compiler/rustc_feature/src/lib.rs#L86-L89.

Hmm, pkg_name makes me worried it will have dashes instead of underscores, let me test that.

Yup, it does indeed have dashes instead of underscores. @ehuss do you know how to get the --crate-name cargo will pass to rustc?

Looks like unit.target.crate_name() works.

Hmm no, that's the name of the build script, which is always build_script_build. I guess this should be different depending on what targets cargo plans to build? 😕 and there could be multiple since you can e.g. build all binaries at once.

Yea, this is maybe going to be a little more complicated.

For the common case, it is probably the library name that it wants to check. That can be obtained from unit with something like unit.pkg.targets().iter().find(|t| t.is_lib()).map(|t| t.crate_name()).

I guess that brings up the question on what to do for binaries, tests, examples, benches, etc. I'm uncertain how that should be handled, though. Maybe @joshtriplett has ideas?

I would also be ok with reverting #9181 on beta until this is fixed.

For now, maybe we should downgrade it to an error if library name matches or if there's no library target? I think erring on the side of less hard errors is better if we're not sure, it will still give an unsilencable warning.

Is there a way to find reverse dependencies that are currently being compiled? Like if someone passes cargo build --bin xyz and that runs a build script, can I get back xyz somehow?

Another alternative is to give a hard error here - since there's no library, it has no reverse dependencies. I guess that could break people running cargo install though?

petrochenkov · 2021-04-16T21:25:37Z

The whole point of rust-lang/rust#77802 was to allow specifying this granularly

The point was to allow specifying RUSTC_BOOTSTRAP only at the top level of the build, while still supporting per-crate granularity in enabling it.
I don't think allowing individual crates to quietly sneak unstable features again is a good idea.

See the original motivating example with packed_simd breakage, if the build script used RUSTC_BOOTSTRAP=packed_simd instead of RUSTC_BOOTSTRAP=1 it wouldn't help to prevent the breakage in any way.

bors · 2021-04-16T22:44:57Z

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

jyn514 · 2021-04-17T14:50:12Z

The point was to allow specifying RUSTC_BOOTSTRAP only at the top level of the build, while still supporting per-crate granularity in enabling it.
I don't think allowing individual crates to quietly sneak unstable features again is a good idea.

That's not what this change does. Right now, cargo errors even if the user specifies RUSTC_BOOTSTRAP at the top level if it's anything other than 1. This downgrades it to a warning if it would be allowed anyway.

@ehuss I wonder if a simpler approach rather than trying to figure out what the crate name will be is to just check if the top-level env variable already contains that string. Then cargo doesn't have to try and figure out if rustc will accept it or not.

jyn514 · 2021-04-17T14:57:04Z

I wonder if a simpler approach rather than trying to figure out what the crate name will be is to just check if the top-level env variable already contains that string.

Oh wait, that doesn't help if the crate is just setting =1 (which it always is in practice).

This doesn't work because it uses the name of the build script, which is always build_script_build. I'm not sure what to change it to - the name of the library crate could be different than the name of the package, and there could be multiple different crates being compiled in the same package.

… from the top-level. If there's no library, give a hard error unless features are unconditionally allowed with RUSTC_BOOTSTRAP=1.

ehuss · 2021-04-20T23:43:08Z

Ah, OK. I was going to suggest maybe checking if it is a workspace member and adjusting the error to say use RUSTC_BOOTSTRAP=1. But after taking a look at this, I think it looks pretty good. Since it has been a couple months since this landed and I don't think anyone has said anything, I'll go ahead and approve. Thanks for the quick fix!

@bors r+

@jyn514 Can you backport this to the rust-1.52.0 branch?

bors · 2021-04-20T23:43:09Z

📌 Commit 5a71496 has been approved by ehuss

bors · 2021-04-20T23:43:16Z

⌛ Testing commit 5a71496 with merge fb0130c...

bors · 2021-04-21T00:13:44Z

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

Backport "Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name" Backports #9365, fixing #9362.

Update cargo, rls ## cargo 18 commits in 65d57e6f384c2317f76626eac116f683e2b63665..0ed318d182e465cd66071b91ac3d265af63ef8a1 2021-04-04 15:07:52 +0000 to 2021-04-23 20:54:54 +0000 - Restore crates.io's `SourceId` hash value to before (rust-lang/cargo#9397) - Fix loading `branch=master` patches in the v3 lock transition (rust-lang/cargo#9392) - Update changelog for 1.52 beta changes. (rust-lang/cargo#9396) - Fix build-std updating the index on every build. (rust-lang/cargo#9393) - Fix typo in profile docs (rust-lang/cargo#9386) - Fix disagreement about lockfile ordering on stable/nightly (rust-lang/cargo#9384) - Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name (rust-lang/cargo#9365) - Fix rust-lang/cargo#9350 (cargo build -Z help is missing options) (rust-lang/cargo#9369) - an struct -> a struct (rust-lang/cargo#9379) - Handle man pages better on Windows. (rust-lang/cargo#9378) - fix: better error message when dependency/workspace member missing (rust-lang/cargo#9368) - Fix typo in book (rust-lang/cargo#9376) - Don't re-use rustc cache when RUSTC_WRAPPER changes (rust-lang/cargo#9348) - doc: add split-debuginfo doc in config chapter (rust-lang/cargo#9372) - refactor: remove `CargoResultExt` (rust-lang/cargo#9367) - Track "CARGO" in environment fingerprint. (rust-lang/cargo#9363) - Update clippy lint allow set. (rust-lang/cargo#9356) - Fix 'suport' typo in documentation (rust-lang/cargo#9338) ## rls 3 commits in 32c0fe006dcdc13e1ca0ca31de543e4436c1299e..74d1800c25498689c5b5120a1e8495fce0cd0d0d 2021-04-12 11:21:12 +0000 to 2021-04-22 21:29:51 +0000 - Bump default integration test message timeout to 30s (rust-lang/rls#1731) - itertools = 0.9, fst = 0.4 (rust-lang/rls#1729) - Update cargo (rust-lang/rls#1728)

rust-highfive assigned ehuss Apr 16, 2021

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

jyn514 commented Apr 16, 2021

View reviewed changes

Don't give a hard error on RUSTC_BOOTSTRAP=crate_name

04fa2f2

jyn514 changed the title ~~Don't give a hard error on RUSTC_BOOTSTRAP=crate_name~~ Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name Apr 17, 2021

jyn514 force-pushed the rustc-bootstrap-crate-name branch from f264756 to 46e0c8d Compare April 17, 2021 15:33

jyn514 added 2 commits April 17, 2021 11:36

Use the library name to decide whether the override should be allowed…

236fe87

… from the top-level. If there's no library, give a hard error unless features are unconditionally allowed with RUSTC_BOOTSTRAP=1.

minor cleanup

1b36ddc

jyn514 force-pushed the rustc-bootstrap-crate-name branch 2 times, most recently from ff43674 to e9fbf53 Compare April 17, 2021 16:01

Fix tests

5a71496

jyn514 force-pushed the rustc-bootstrap-crate-name branch from e9fbf53 to 5a71496 Compare April 17, 2021 16:13

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 Apr 20, 2021

jyn514 mentioned this pull request Apr 20, 2021

Backport "Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name" #9385

Merged

bors merged commit fb0130c into rust-lang:master Apr 21, 2021

bors added a commit that referenced this pull request Apr 21, 2021

Auto merge of #9385 - jyn514:beta-bootstrap-backport, r=ehuss

6976741

Backport "Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name" Backports #9365, fixing #9362.

ehuss mentioned this pull request Apr 21, 2021

Update cargo, rls rust-lang/rust#84412

Merged

jyn514 deleted the rustc-bootstrap-crate-name branch June 2, 2021 07:10

ehuss modified the milestones: 1.53.0, 1.52.0 Feb 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name #9365

Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name #9365

jyn514 commented Apr 16, 2021 •

edited

Loading

jyn514 Apr 16, 2021

jyn514 Apr 16, 2021

jyn514 Apr 16, 2021

jyn514 Apr 16, 2021

jyn514 Apr 16, 2021

ehuss Apr 16, 2021

jyn514 Apr 16, 2021

jyn514 Apr 16, 2021

jyn514 Apr 17, 2021

jyn514 Apr 17, 2021

petrochenkov commented Apr 16, 2021 •

edited

Loading

bors commented Apr 16, 2021

jyn514 commented Apr 17, 2021

jyn514 commented Apr 17, 2021 •

edited

Loading

ehuss commented Apr 20, 2021

bors commented Apr 20, 2021

bors commented Apr 20, 2021

bors commented Apr 21, 2021

Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name #9365

Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name #9365

Conversation

jyn514 commented Apr 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petrochenkov commented Apr 16, 2021 • edited Loading

bors commented Apr 16, 2021

jyn514 commented Apr 17, 2021

jyn514 commented Apr 17, 2021 • edited Loading

ehuss commented Apr 20, 2021

bors commented Apr 20, 2021

bors commented Apr 20, 2021

bors commented Apr 21, 2021

jyn514 commented Apr 16, 2021 •

edited

Loading

petrochenkov commented Apr 16, 2021 •

edited

Loading

jyn514 commented Apr 17, 2021 •

edited

Loading