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

Improve defaults in x.py #73964

Merged
merged 12 commits into from
Jul 28, 2020
Merged

Improve defaults in x.py #73964

merged 12 commits into from
Jul 28, 2020

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Jul 2, 2020

  • Make the default stage dependent on the subcommand
  • Don't build stage1 rustc artifacts with x.py build --stage 1. If this is what you want, use x.py build --stage 2 instead, which gives you a working libstd.
  • Change default debuginfo when debug = true from 2 to 1

I tried to fix CI to use --stage 2 everywhere it currently has no stage, but I might have missed a spot.
This does not update much of the documentation - most of it is in https://github.com/rust-lang/rustc-dev-guide/ or https://github.com/rust-lang/rust-forge and will need a separate PR.

See individual commits for a detailed rationale of each change.
See also the MCP: rust-lang/compiler-team#326

r? @Mark-Simulacrum , but anyone is free to give an opinion.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 2, 2020
@jyn514
Copy link
Member Author

jyn514 commented Jul 2, 2020

I would also like to add an option to build rustc without libstd at some point. This is not currently possible: build --stage 0 will compile rustc but won't assemble it (#73519) and I think the same is true for --stage 1.

@jyn514
Copy link
Member Author

jyn514 commented Jul 2, 2020

Looking at https://forge.rust-lang.org/compiler/mcp.html this should definitely be an MCP so marking as draft until I've gone through that process.

@jyn514 jyn514 marked this pull request as draft July 2, 2020 14:10
@jyn514 jyn514 changed the title Use sane defaults in x.py [Draft] Use sane defaults in x.py Jul 2, 2020
@Mark-Simulacrum Mark-Simulacrum added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 2, 2020
src/bootstrap/builder.rs Outdated Show resolved Hide resolved
@petrochenkov
Copy link
Contributor

The current defaults (except for the default stage) minimize the edit-build-test cycle, i.e. serve the use case "build rustc as fast as possible to run tests".

The new defaults are debug-oriented - you have some issue and build the compiler with extra info, spending extra time on producing debuginfo, assertions and logging.

The first scenario is much more common from my experience.
So I use the current default settings most of the time with debuginfo-level = 1 being the only exceptions (it's very cheap and doesn't cause any observable slowdown).

@jyn514
Copy link
Member Author

jyn514 commented Jul 2, 2020

The current defaults (except for the default stage) minimize the edit-build-test cycle, i.e. serve the use case "build rustc as fast as possible to run tests".

The new defaults are debug-oriented - you have some issue and build the compiler with extra info, spending extra time on producing debuginfo, assertions and logging.

The first scenario is much more common from my experience.
So I use the current default settings most of the time with debuginfo-level = 1 being the only exceptions (it's very cheap and doesn't cause any observable slowdown).

The debug = true change is the one I feel the least strongly about. If you think that should be kept as debug = false by default I'm fine with that as long as there's a suggestion in rustc-dev-guide to switch it on. I will note though that the people with the use case to "build rustc as fast as possible to run tests" are probably experienced contributors and know to look for debug = false in config.toml.

What do you think about the rest of the changes?

@CAD97
Copy link
Contributor

CAD97 commented Jul 2, 2020

I will note though that the people with the use case to "build rustc as fast as possible to run tests" are probably experienced contributors

I'd argue the exact opposite. People new to hacking on rustc are the most likely to use an unqualified ./x.py test in order to test their changes. And they definitely just want their test results, and don't really care what's done to get to that point.

@joshtriplett
Copy link
Member

joshtriplett commented Jul 3, 2020

I would personally like to have debug = false by default` as well. I think it makes sense to provide better build performance out of the box; most people won't run rustc through a debugger or similar.

That said, if debug with debug level 1 causes almost no slowdown, and doesn't use substantially more memory when building or linking, then leaving that on by default seems fine.

@jyn514
Copy link
Member Author

jyn514 commented Jul 3, 2020

To be clear, debug = true does not mean you can run rustc through a debugger, for that you need debuginfo = 2. Instead it adds line numbers to backtraces, enables debug assertions, and emits debug logging. Without debug = true, RUST_LOG=debug has exactly the same effect as RUST_LOG=info, which can be very confusing for new contributors.

@joshtriplett
Copy link
Member

@jyn514 Ah, I see; that seems reasonable. Thanks for the explanation.

@jyn514
Copy link
Member Author

jyn514 commented Jul 3, 2020

I removed debug = true from the MCP, see rust-lang/compiler-team#326 (comment) for discussion.

@pickfire
Copy link
Contributor

pickfire commented Jul 3, 2020

To be clear, debug = true does not mean you can run rustc through a debugger, for that you need debuginfo = 2. Instead it adds line numbers to backtraces, enables debug assertions, and emits debug logging. Without debug = true, RUST_LOG=debug has exactly the same effect as RUST_LOG=info, which can be very confusing for new contributors.

The worst thing I did was changing that, I need to rebuild (which took me a day) just to flip the debug to true.

@jyn514 jyn514 changed the title [Draft] Use sane defaults in x.py [Draft] Improve defaults in x.py Jul 7, 2020
@jyn514
Copy link
Member Author

jyn514 commented Jul 7, 2020

The failure is reproducible with

$ ./x.py test --doc src/tools/rustdoc
   Doc-tests rustdoc

running 1 test
F
failures:

---- html/markdown.rs - html::markdown (line 5) stdout ----
Test executable failed (exit code 127).

stderr:
/tmp/rustdoctestmt2Lhu/rust_out: error while loading shared libraries: libtest-762795f83cf44985.so: cannot open shared object file: No such file or directory

Investigating now.

@jyn514
Copy link
Member Author

jyn514 commented Jul 14, 2020

Yay bootstrap tests are finally passing 🎉 next to change CI to use the explicit names for things instead of relying on the defaults to be correct.

@jyn514
Copy link
Member Author

jyn514 commented Jul 14, 2020

CI is passing and using stage 2 🎉 The MCP still has 4 days until it's officially accepted, but this is ready for review.

@jyn514 jyn514 changed the title [Draft] Improve defaults in x.py Improve defaults in x.py Jul 14, 2020
@jyn514 jyn514 marked this pull request as ready for review July 14, 2020 04:00
@petrochenkov
Copy link
Contributor

petrochenkov commented Jul 14, 2020

@jyn514
Could you split the changes for different defaults into separate PRs? (One for stage 2, one for debuginfo, etc.)
Right now this PR does many different things, moves code around and doesn't maintain git history.
The PR description also went out of sync with its contents.
All of this makes a person who wants to figure out what this PR changes to do a full review.

@bors
Copy link
Contributor

bors commented Jul 28, 2020

📌 Commit da40cf8 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 Jul 28, 2020
@bors
Copy link
Contributor

bors commented Jul 28, 2020

⌛ Testing commit da40cf8 with merge 7b3a781...

@bors
Copy link
Contributor

bors commented Jul 28, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: Mark-Simulacrum
Pushing 7b3a781 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 28, 2020
@bors bors merged commit 7b3a781 into rust-lang:master Jul 28, 2020
@jyn514 jyn514 deleted the sane-defaults branch July 28, 2020 17:09
jyn514 added a commit to jyn514/measureme that referenced this pull request Aug 16, 2020
See rust-lang/rust#73964.

The choices are either to link to `stage1` with rustup or build stage 2; this recommends to build stage2 to be consistent with the existing docs.
@matklad
Copy link
Member

matklad commented Aug 30, 2020

I’ve just learned about this from 1.47 relnotes. This might mean that this might need some additional advertising for existing contributors to take advantage of. “How rustc stages work” might be an interesting post for inside Rust blog :)

@tesuji
Copy link
Contributor

tesuji commented Aug 30, 2020

Joshua is making a post: rust-lang/blog.rust-lang.org#666

jyn514 added a commit to jyn514/rustc-dev-guide that referenced this pull request Aug 31, 2020
jyn514 added a commit to jyn514/rustc-dev-guide that referenced this pull request Aug 31, 2020
jyn514 added a commit to jyn514/rustc-dev-guide that referenced this pull request Aug 31, 2020
tshepang pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Aug 31, 2020
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Nov 13, 2020
Pkgsrc changes:
 * Remove patches now integrated upstream, many related to SunOS / Illumos.
 * The LLVM fix for powerpc is also now integrated upstream.
 * Adapt those patches where the source has moved or parts are integrated.
 * The randomness patches no longer applies, and I could not find
   where those files went...
 * Provide a separate bootstrap for NetBSD/powerpc 9.0, since apparently
   the C++ ABI is different from 8.0.  Yes, this appears to be specific to
   the NetBSD powerpc ports.

Upstream changes:

Version 1.47.0 (2020-10-08)
==========================

Language
--------
- [Closures will now warn when not used.][74869]

Compiler
--------
- [Stabilized the `-C control-flow-guard` codegen option][73893], which enables
  [Control Flow Guard][1.47.0-cfg] for Windows platforms, and is ignored on
  other platforms.
- [Upgraded to LLVM 11.][73526]
- [Added tier 3\* support for the `thumbv4t-none-eabi` target.][74419]
- [Upgrade the FreeBSD toolchain to version 11.4][75204]
- [`RUST_BACKTRACE`'s output is now more compact.][75048]

\* Refer to Rust's [platform support page][forge-platform-support] for more
information on Rust's tiered platform support.

Libraries
---------
- [`CStr` now implements `Index<RangeFrom<usize>>`.][74021]
- [Traits in `std`/`core` are now implemented for arrays of any length, not just
  those of length less than 33.][74060]
- [`ops::RangeFull` and `ops::Range` now implement Default.][73197]
- [`panic::Location` now implements `Copy`, `Clone`, `Eq`, `Hash`, `Ord`,
  `PartialEq`, and `PartialOrd`.][73583]

Stabilized APIs
---------------
- [`Ident::new_raw`]
- [`Range::is_empty`]
- [`RangeInclusive::is_empty`]
- [`Result::as_deref`]
- [`Result::as_deref_mut`]
- [`Vec::leak`]
- [`pointer::offset_from`]
- [`f32::TAU`]
- [`f64::TAU`]

The following previously stable APIs have now been made const.

- [The `new` method for all `NonZero` integers.][73858]
- [The `checked_add`,`checked_sub`,`checked_mul`,`checked_neg`, `checked_shl`,
  `checked_shr`, `saturating_add`, `saturating_sub`, and `saturating_mul`
  methods for all integers.][73858]
- [The `checked_abs`, `saturating_abs`, `saturating_neg`, and `signum`  for all
  signed integers.][73858]
- [The `is_ascii_alphabetic`, `is_ascii_uppercase`, `is_ascii_lowercase`,
  `is_ascii_alphanumeric`, `is_ascii_digit`, `is_ascii_hexdigit`,
  `is_ascii_punctuation`, `is_ascii_graphic`, `is_ascii_whitespace`, and
  `is_ascii_control` methods for `char` and `u8`.][73858]

Cargo
-----
- [`build-dependencies` are now built with opt-level 0 by default.][cargo/8500]
  You can override this by setting the following in your `Cargo.toml`.
  ```toml
  [profile.release.build-override]
  opt-level = 3
  ```
- [`cargo-help` will now display man pages for commands rather just the
  `--help` text.][cargo/8456]
- [`cargo-metadata` now emits a `test` field indicating if a target has
  tests enabled.][cargo/8478]
- [`workspace.default-members` now respects `workspace.exclude`.][cargo/8485]
- [`cargo-publish` will now use an alternative registry by default if it's the
  only registry specified in `package.publish`.][cargo/8571]

Misc
----
- [Added a help button beside Rustdoc's searchbar that explains rustdoc's
  type based search.][75366]
- [Added the Ayu theme to rustdoc.][71237]

Compatibility Notes
-------------------
- [Bumped the minimum supported Emscripten version to 1.39.20.][75716]
- [Fixed a regression parsing `{} && false` in tail expressions.][74650]
- [Added changes to how proc-macros are expanded in `macro_rules!` that should
  help to preserve more span information.][73084] These changes may cause
  compiliation errors if your macro was unhygenic or didn't correctly handle
  `Delimiter::None`.
- [Moved support for the CloudABI target to tier 3.][75568]
- [`linux-gnu` targets now require minimum kernel 2.6.32 and glibc 2.11.][74163]

Internal Only
--------
- [Improved default settings for bootstrapping in `x.py`.][73964]
  You can read details about this change in the ["Changes to `x.py`
  defaults"](https://blog.rust-lang.org/inside-rust/2020/08/30/changes-to-x-py-defaults.html)
  post on the Inside Rust blog.

- [Added the `rustc-docs` component.][75560] This allows you to install
  and read the documentation for the compiler internal APIs. (Currently only
  available for `x86_64-unknown-linux-gnu`.)

[1.47.0-cfg]: https://docs.microsoft.com/en-us/windows/win32/secbp/control-flow-guard
[76980]: rust-lang/rust#76980
[75048]: rust-lang/rust#75048
[74163]: rust-lang/rust#74163
[71237]: rust-lang/rust#71237
[74869]: rust-lang/rust#74869
[73858]: rust-lang/rust#73858
[75716]: rust-lang/rust#75716
[75908]: rust-lang/rust#75908
[75516]: rust-lang/rust#75516
[75560]: rust-lang/rust#75560
[75568]: rust-lang/rust#75568
[75366]: rust-lang/rust#75366
[75204]: rust-lang/rust#75204
[74650]: rust-lang/rust#74650
[74419]: rust-lang/rust#74419
[73964]: rust-lang/rust#73964
[74021]: rust-lang/rust#74021
[74060]: rust-lang/rust#74060
[73893]: rust-lang/rust#73893
[73526]: rust-lang/rust#73526
[73583]: rust-lang/rust#73583
[73084]: rust-lang/rust#73084
[73197]: rust-lang/rust#73197
[72488]: rust-lang/rust#72488
[cargo/8456]: rust-lang/cargo#8456
[cargo/8478]: rust-lang/cargo#8478
[cargo/8485]: rust-lang/cargo#8485
[cargo/8500]: rust-lang/cargo#8500
[cargo/8571]: rust-lang/cargo#8571
[`Ident::new_raw`]:  https://doc.rust-lang.org/nightly/proc_macro/struct.Ident.html#method.new_raw
[`Range::is_empty`]: https://doc.rust-lang.org/nightly/std/ops/struct.Range.html#method.is_empty
[`RangeInclusive::is_empty`]: https://doc.rust-lang.org/nightly/std/ops/struct.RangeInclusive.html#method.is_empty
[`Result::as_deref_mut`]: https://doc.rust-lang.org/nightly/std/result/enum.Result.html#method.as_deref_mut
[`Result::as_deref`]: https://doc.rust-lang.org/nightly/std/result/enum.Result.html#method.as_deref
[`TypeId::of`]: https://doc.rust-lang.org/nightly/std/any/struct.TypeId.html#method.of
[`Vec::leak`]: https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#method.leak
[`f32::TAU`]: https://doc.rust-lang.org/nightly/std/f32/consts/constant.TAU.html
[`f64::TAU`]: https://doc.rust-lang.org/nightly/std/f64/consts/constant.TAU.html
[`pointer::offset_from`]: https://doc.rust-lang.org/nightly/std/primitive.pointer.html#method.offset_from
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. 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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.