-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
rustdoc: rename the doc parts metadata params #159415
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
I like these names (better than the previous and what I'd proposed), and I think this is a good interface given the current state of CCI. Long-term, once full metadata is implemented, I don't want rustdoc to emit HTML at all until its last invocation in a workspace (i.e. the last crate, or maybe even a step after that). However, that's obviously a ways off, so we shouldn't block CCI on that. But I would like to be forwards-compatible with it as much as possible.
So, I'm wondering if we should forbid the combination of
--write-doc-meta-dirand--read-doc-meta-dir. Right now, there is no use for it as far as I know. Notice that the original--mergeflag only had three options, not four, for this reason.To be clear, my long-term vision for rustdoc usage:
cleanand anything else needed to generate HTML laterTo implement this in a way that is backwards-compatible with CCI, we'd probably need to add a
--defer-htmlflag that switches to the mode I just described. Note that this mode would require feeding in metadata to all intermediate crates, not just the final crate like in CCI.The reason to delay HTML generation until the end is then we have complete information about the workspace and can remove all of our hacks that use JS to inject downstream implementors, etc. Rustdoc really wants to operate at the workspace level, not the crate level where it actually operators, which is why we have so much pain from cross-crate inlining and other features.
View changes since the review
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.
It sounds like you’re expecting rustdoc to perform finalization at the same time that it documents the last crate? That makes sense, and that’s why the current system lets you do
readandwrite-doc-meta-dirat the same time.It was designed that way because the build system might schedule a different crate to be documented last in a different run. For example, with Cargo, you would supply the
-pparameter. To make sure this doesn’t cause problems, you’re expected to write doc metadata for every crate in your workspace, even the one that gets documented last and used for finalization.One thing that does concern me, though, is that Cargo always does finalization as a separate step, because the finalization step isn’t parallelized. It might not make sense to worry about doing finalization and documentation at the same time, if no builds systems want to do that.
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.
Yes, exactly. I would prefer that finalization happens as a separate step (like you said Cargo does) after the last crate. In the future, once finalization will do all the HTML generation too, this would also get rid of our weird quirk where the last documented crate is considered the main one.
I suppose supporting
--readand--writetogether is fine for CCI. The future version of rustdoc that defers all HTML generation until the end will need to be gated behind a new flag anyway, so we can change the interface when that flag is passed.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.
However, if we don't need to support simultaneous documenting-last-crate and finalizing, it'd be better to save ourselves the trouble and just disallow it. I'm a bit rusty on the details of the CCI implementation, but it sounds like rustdoc already supports finalizing as a separate step like you said?
Sorry if this all seems a bit pedantic, I just want to make sure we don't paint ourselves into a corner.
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.
Yes, it's possible. I'll do that, then.
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! And thanks for taking action to make this PR, much appreciated.