Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass --cfg=bootstrap for proc macros built by stage0 #83433

Merged
merged 1 commit into from
Jun 7, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Mar 24, 2021

Cargo has a bug where it ignores RUSTFLAGS when building proc macro
crates (rust-lang/cargo#4423).
However, sometimes rustc_macro needs to have conditional
compilation when there are breaking changes to the libproc_macro API
(see for example #83363). Previously, this wasn't possible, because the
crate couldn't tell the difference between stage 0 and stage 1.

Another alternative is to unconditionally build rustc_macros with the
master libstd instead of the beta one (i.e. use --sysroot stage0-sysroot), but that led to strange and maddening errors:

error[E0460]: found possibly newer version of crate `std` which `synstructure` depends on
 --> compiler/rustc_macros/src/lib.rs:5:5
  |
5 | use synstructure::decl_derive;
  |     ^^^^^^^^^^^^
  |
  = note: perhaps that crate needs to be recompiled?
  = note: the following crate versions were found:
          crate `std`: /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-b3602c301b71cc3d.rmeta
          crate `synstructure`: /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/stage0-rustc/release/deps/libsynstructure-74ee66863479e972.rmeta
error[E0460]: found possibly newer version of crate `std` which `proc_macro2` depends on
  --> /home/joshua/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/tracing-attributes-0.1.13/src/lib.rs:90:5
   |
90 | use proc_macro2::TokenStream;
   |     ^^^^^^^^^^^
   |
   = note: perhaps that crate needs to be recompiled?
   = note: the following crate versions were found:
           crate `std`: /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-b3602c301b71cc3d.rmeta
           crate `proc_macro2`: /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/stage0-rustc/release/deps/libproc_macro2-a83c1f01610c129e.rlib

r? @Mark-Simulacrum cc @jhpratt

@jyn514 jyn514 added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-proc-macros Area: Procedural macros A-contributor-roadblock Area: Makes things more difficult for new or seasoned contributors to Rust labels Mar 24, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 24, 2021
@jyn514 jyn514 changed the title Pass --cfg=bootstrap for proc_macros built by stage0 Pass --cfg=bootstrap for proc macros built by stage0 Mar 24, 2021
@@ -126,6 +130,13 @@ fn main() {
}
}

if is_proc_macro && stage == "0" {
// Cargo has a bug where it doesn't pass RUSTFLAGS to proc_macros:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I.. wouldn't call this a bug :) More like 'unexpected behavior'. rust-lang/cargo#4423 seems like the more appropriate issue to cite, though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, yes, that's the one I meant to link to. The issue title isn't quite right though - we're neither cross-compiling nor building a build script.

@rust-log-analyzer

This comment has been minimized.

@jyn514 jyn514 force-pushed the cfg-bootstrap-macro branch 2 times, most recently from 37104e1 to 2535828 Compare March 24, 2021 02:21
@jyn514
Copy link
Member Author

jyn514 commented Mar 24, 2021

Hmm, this doesn't actually work - when I pass --all-targets to x.py, cargo doesn't pass --crate-type proc-macro :/ I'm not sure how to fix this.

#83363 (comment)

@Mark-Simulacrum
Copy link
Member

I unfortunately likely won't have too much time to help debug that problem, but I suspect it's either the same problem as noted in the comments (i.e., target-specific magic) or perhaps a separate Cargo bug that could be fixed. I imagine it should be reproducible outside rustbuild though, in which case you could file a new issue on rust-lang/cargo :)

@jyn514
Copy link
Member Author

jyn514 commented Apr 7, 2021

I suspect it's either the same problem as noted in the comments (i.e., target-specific magic) or perhaps a separate Cargo bug that could be fixed.

I don't think this is actually a bug - passing --crate-type=proc-macro breaks unit tests:

$ RUSTFLAGS=--crate-type=proc-macro cargo test -q
error: test failed, to rerun pass '--lib'

Caused by:
  could not execute process `/home/joshua/.local/lib/cargo/target/debug/deps/libproc_macro_example-65ddfb6f3c001ad1.so --quiet` (never executed)

I guess I could special case --crate-name rustc_macros instead? But that seems awfully fragile :/

@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 23, 2021
@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 5, 2021
@crlf0710 crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 22, 2021
Cargo ignores RUSTFLAGS when building proc macro crates. However,
sometimes rustc_macro needs to have conditional compilation when there
are breaking changes to the `libproc_macro` API (see for example
tell the difference between stage 0 and stage 1.

Another alternative is to unconditionally build rustc_macros with the
master libstd instead of the beta one (i.e. use `--sysroot
stage0-sysroot`), but that led to strange and maddening errors:

```
error[E0460]: found possibly newer version of crate `std` which `proc_macro2` depends on
  --> /home/joshua/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/tracing-attributes-0.1.13/src/lib.rs:90:5
   |
90 | use proc_macro2::TokenStream;
   |     ^^^^^^^^^^^
   |
   = note: perhaps that crate needs to be recompiled?
   = note: the following crate versions were found:
           crate `std`: /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/stage0-sysroot/lib/rustlib/x86_64-unknown-linux-gnu/lib/libstd-b3602c301b71cc3d.rmeta
           crate `proc_macro2`: /home/joshua/rustc2/build/x86_64-unknown-linux-gnu/stage0-rustc/release/deps/libproc_macro2-a83c1f01610c129e.rlib
```
@jyn514 jyn514 force-pushed the cfg-bootstrap-macro branch from 2535828 to dc30258 Compare June 5, 2021 02:02
@jyn514
Copy link
Member Author

jyn514 commented Jun 5, 2021

This should be ready to go; I tested by rebasing #83363 on top of this and running x.py check --stage 1 and x.py check --all-targets.

@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 5, 2021
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 6, 2021

📌 Commit dc30258 has been approved by Mark-Simulacrum

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 6, 2021
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 6, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 7, 2021
…laumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#83433 (Pass --cfg=bootstrap for proc macros built by stage0)
 - rust-lang#84940 (Don't run sanity checks for `x.py setup`)
 - rust-lang#85912 (Use `Iterator::any` and `filter_map` instead of open-coding them)
 - rust-lang#85965 (Remove dead code from `LocalAnalyzer`)
 - rust-lang#86010 (Fix two ICEs in the parser)
 - rust-lang#86040 (Fix display for search results)
 - rust-lang#86058 (Remove `_`  from E0121 diagnostic suggestions)
 - rust-lang#86077 (Fix corrected example in E0759.md)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 85fec06 into rust-lang:master Jun 7, 2021
@rustbot rustbot added this to the 1.54.0 milestone Jun 7, 2021
@jyn514 jyn514 deleted the cfg-bootstrap-macro branch June 7, 2021 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new or seasoned contributors to Rust A-proc-macros Area: Procedural macros S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants