-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Update the reference #128215
Update the reference #128215
Conversation
bootstrap.py handling of submodules was removed in rust-lang#97513.
This is not used anywhere outside this module.
This felt like an important point to me.
The argument was not necessary, since it was only ever passed one value that exists in the config itself.
Although its origins were in bootstrap.py, that code in bootstrap.py no longer exists since it was removed.
I put this submodule update in the entirely wrong location. I put it in the `RustcBook` step (for generating src/doc/rustc), when it really should exist for all steps that use the `Rustbook` tool.
This adds a new method `require_and_update_submodule` to replace `update_submodule`. This new method will generate an error if the submodule doesn't actually exist. This replaces some ad-hoc checks that were performing this function. This helps ensure that a good error message is always displayed. This also adds require_and_update_all_submodules which does this for all submodules. Ideally this should not have any change other than better error messages when submodules are missing.
If the submodule is not checked out, then these tests would fail.
This updates the reference which is now using a new mdbook plugin. This requires a little extra work than a normal book because the plugin uses `rustdoc` to generate links to the standard library. It also ensures that the submodule is available for *any* command that uses rustbook, since it is now part of the rustbook workspace.
|
This PR modifies If appropriate, please update The list of allowed third-party dependencies may have been modified! You must ensure that any new dependencies have compatible licenses before merging. This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I went through the commits and left a few comments.
I tried to run x doc reference
locally, and I got this:
Finished `release` profile [optimized] target(s) in 31.00s
Rustbook (x86_64-unknown-linux-gnu) - reference
error: manifest path `mdbook-spec/Cargo.toml` does not exist
when trying to run x doc reference
for the first time, not sure what to make of it. Additional executions ended successfully (or at least it seemed like so).
One thing I also noticed is that the reference does not open automatically in the browser if I do x doc reference
or x doc src/doc/reference
, I have to use --open
(not sure if this is correct or not, from the code it looked like it might be intended to be opened by default). But this also happens on master
, so nothing to do with this PR.
This was a copy/paste mistake.
This makes it easier to call these functions without needing to form a Path.
…ired These are required 100% of the time, but they are almost always required for any command that runs Cargo in the main workspace. Ideally, initializing these two standard library submodules would be lazy and only initialized when required (see rust-lang#82653). However, it would require updating these in almost every Step (anything that runs `cargo` in the main workspace).
I misread this one. It is only checking if LLVM needs to be rebuilt. There is code below that handles the case where it is unable to compute the stamp if the source is missing.
Just trying to be a little less verbose here.
That's an unfortunate consequence of how the mdbook preprocessors are overridden. The error is ignored, and it builds successfully. I don't have a simple way to silence it right now. That's what this comment is referring to about being unable to suppress the error message. I will try to get mdbook updated in a future PR soon so that it replaces the existing preprocessor to avoid that.
That's normal. It is required to use BTW, while re-reviewing, I noticed a mistake in the |
Sorry, I did not notice this condition. Thank you! @bors r+ rollup=iffy |
…iaskrgr Rollup of 3 pull requests Successful merges: - rust-lang#125889 (Add migration lint for 2024 prelude additions) - rust-lang#128215 (Update the reference) - rust-lang#128263 (rustdoc: use strategic ThinVec/Box to shrink `clean::ItemKind`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#128215 - ehuss:update-reference, r=Kobzol Update the reference This updates the reference to use the new mdbook-spec preprocessor, which is a Cargo library inside the reference submodule. Note that this PR contains a bunch of bootstrap cleanup commits to assist with making sure the submodules are working correctly. All of the cleanup PRs should have a description in their commit. I'd be happy to move those to a separate PR if that makes review easier. The main changes for the reference are: - Move the `doc::Reference` bootstrap step out of the generic macro into a custom step. - This step needs to build rustdoc because the new mdbook-spec plugin uses rustdoc for generating links. - PATH is updated so that the rustdoc binary can be found. - rustbook now includes the mdbook-spec plugin as a dependency. - rustbook enables the mdbook-spec preprocessor. I did a bunch of testing with the various commands and setups, such as: - `submodules=true` and `submodules=false` - having all submodules deinitialized - not in a git repository However, there are probably thousands of different permutations of different commands, settings, and environments, so there is a chance I'm missing something.
This updates the reference to use the new mdbook-spec preprocessor, which is a Cargo library inside the reference submodule.
Note that this PR contains a bunch of bootstrap cleanup commits to assist with making sure the submodules are working correctly. All of the cleanup PRs should have a description in their commit. I'd be happy to move those to a separate PR if that makes review easier.
The main changes for the reference are:
doc::Reference
bootstrap step out of the generic macro into a custom step.I did a bunch of testing with the various commands and setups, such as:
submodules=true
andsubmodules=false
However, there are probably thousands of different permutations of different commands, settings, and environments, so there is a chance I'm missing something.