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

rustbuild: Move compiler-builtins build logic to manifest #73374

Merged

Conversation

alexcrichton
Copy link
Member

This commit moves the compiler-builtins-specific build logic from
src/bootstrap/bin/rustc.rs into the workspace Cargo.toml's
[profile] configuration. Now that rust-lang/cargo#7253 is fixed we can
ensure that Cargo knows about debug assertions settings, and it can also
be configured to specifically disable debug assertions unconditionally
for compiler-builtins. This should improve rebuild logic when
debug-assertions settings change and also improve build-std integration
where Cargo externally now has an avenue to learn how to build
compiler-builtins as well.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 15, 2020
@ehuss
Copy link
Contributor

ehuss commented Jun 15, 2020

There will be a few more steps before this can help build-std. The root manifest is not included in the src distribution (rust-lang/wg-cargo-std-aware#27). Even if it was, Cargo would need to merge the profiles somehow (possibly just the overrides?) (cc rust-lang/wg-cargo-std-aware#28).

@alexcrichton
Copy link
Member Author

Oh it's true yeah I don't expect this to "simply by default" help build-std, but at least in this form the information is encoded in a way that build-std has any hope of reading one day!

@Mark-Simulacrum
Copy link
Member

@bors r+

One thing I've been thinking about is that more broadly, removing support for configuring these values in config.toml may make some sense. On the other hand, it does seem true that it's nice to have the .gitignore'd nature of config.toml which you can't readily get with Cargo.toml today (due to lack of cargo.toml inheritance) as far as I know.

@bors
Copy link
Contributor

bors commented Jun 25, 2020

📌 Commit 260d5cf has been approved by Mark-Simulacrum

@bors 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 Jun 25, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 26, 2020
…g-assertions, r=Mark-Simulacrum

rustbuild: Move compiler-builtins build logic to manifest

This commit moves the compiler-builtins-specific build logic from
`src/bootstrap/bin/rustc.rs` into the workspace `Cargo.toml`'s
`[profile]` configuration. Now that rust-lang/cargo#7253 is fixed we can
ensure that Cargo knows about debug assertions settings, and it can also
be configured to specifically disable debug assertions unconditionally
for compiler-builtins. This should improve rebuild logic when
debug-assertions settings change and also improve build-std integration
where Cargo externally now has an avenue to learn how to build
compiler-builtins as well.
@Manishearth
Copy link
Member

Seeing a failure in #73768 (comment) , might be y'all, but it might also be a bad interaction with a different PR

Caused by:
  could not parse input as TOML

Caused by:
  redefinition of table `profile.release.package.compiler_builtins` for key `profile.release.package.compiler_builtins` at line 46 column 1

@bors
Copy link
Contributor

bors commented Jun 28, 2020

⌛ Testing commit 260d5cf with merge 3e6c31fa36e8bdd5333aa341f8205bfb3f47ce06...

@bors
Copy link
Contributor

bors commented Jun 28, 2020

💔 Test failed - checks-azure

@bors 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 Jun 28, 2020
This commit moves the compiler-builtins-specific build logic from
`src/bootstrap/bin/rustc.rs` into the workspace `Cargo.toml`'s
`[profile]` configuration. Now that rust-lang/cargo#7253 is fixed we can
ensure that Cargo knows about debug assertions settings, and it can also
be configured to specifically disable debug assertions unconditionally
for compiler-builtins. This should improve rebuild logic when
debug-assertions settings change and also improve build-std integration
where Cargo externally now has an avenue to learn how to build
compiler-builtins as well.
@alexcrichton alexcrichton force-pushed the compiler-bulitins-debug-assertions branch from 260d5cf to 3dfbf0b Compare June 29, 2020 13:54
@alexcrichton
Copy link
Member Author

@bors: r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Jun 29, 2020

📌 Commit 3dfbf0b has been approved by Mark-Simulacrum

@bors 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 Jun 29, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 29, 2020
…g-assertions, r=Mark-Simulacrum

rustbuild: Move compiler-builtins build logic to manifest

This commit moves the compiler-builtins-specific build logic from
`src/bootstrap/bin/rustc.rs` into the workspace `Cargo.toml`'s
`[profile]` configuration. Now that rust-lang/cargo#7253 is fixed we can
ensure that Cargo knows about debug assertions settings, and it can also
be configured to specifically disable debug assertions unconditionally
for compiler-builtins. This should improve rebuild logic when
debug-assertions settings change and also improve build-std integration
where Cargo externally now has an avenue to learn how to build
compiler-builtins as well.
@bors
Copy link
Contributor

bors commented Jun 29, 2020

⌛ Testing commit 3dfbf0b with merge a1528c4...

@bors
Copy link
Contributor

bors commented Jun 30, 2020

☀️ Test successful - checks-azure
Approved by: Mark-Simulacrum
Pushing a1528c4 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 30, 2020
@bors bors merged commit a1528c4 into rust-lang:master Jun 30, 2020
tmiasko added a commit to tmiasko/rust that referenced this pull request Jul 5, 2020
Since rust-lang#73374 the rustc wrapper no longer configures debug assertions
based on RUSTC_DEBUG_ASSERTIONS environment variable.
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 6, 2020
…alfJung

Remove unused RUSTC_DEBUG_ASSERTIONS

Since rust-lang#73374 the rustc wrapper no longer configures debug assertions
based on RUSTC_DEBUG_ASSERTIONS environment variable.

r? @RalfJung
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 6, 2020
…alfJung

Remove unused RUSTC_DEBUG_ASSERTIONS

Since rust-lang#73374 the rustc wrapper no longer configures debug assertions
based on RUSTC_DEBUG_ASSERTIONS environment variable.

r? @RalfJung
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jul 6, 2020
…alfJung

Remove unused RUSTC_DEBUG_ASSERTIONS

Since rust-lang#73374 the rustc wrapper no longer configures debug assertions
based on RUSTC_DEBUG_ASSERTIONS environment variable.

r? @RalfJung
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 7, 2020
…alfJung

Remove unused RUSTC_DEBUG_ASSERTIONS

Since rust-lang#73374 the rustc wrapper no longer configures debug assertions
based on RUSTC_DEBUG_ASSERTIONS environment variable.

r? @RalfJung
@alexcrichton alexcrichton deleted the compiler-bulitins-debug-assertions branch July 23, 2020 21:20
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants