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

Compute the right output filenames for rustdoc invocations that request the JSON format #12100

Closed

Conversation

LukeMathWalker
Copy link

@LukeMathWalker LukeMathWalker commented May 7, 2023

What does this PR try to resolve?

cargo currently assumes that all invocations of cargo rustdoc produce HTML files as outputs.
This assumption breaks down for invocations of cargo rustdoc that instruct rustdoc to produce documentation in a different format (i.e. JSON).

As a consequence, running the very same command multiple times (cargo +nightly rustdoc --lib -p <NAME> -- -Zunstable-options -wjson) recomputes the documentation every single time, instead of detecting that the pre-existing file is still fresh.

immagine

This PR fixes the logic in cargo to correctly detect the output filename when the JSON format is required.

How should we test and review this PR?

I've added a test to confirm that:

  • The output filename is what we expect (i.e. <crate name>.json);
  • The unit is marked as fresh the second time the same command is invoked.

Additional information

An alternative implementation strategy could see us adding --output-format as a cargo rustdoc option, rather than trying to parse the extra arguments being passed to rustdoc.

…compilation unit when `cargo rustdoc` has been instructed to generate JSON output rather than the conventional HTML output.
@rustbot
Copy link
Collaborator

rustbot commented May 7, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added A-layout Area: target output directory layout, naming, and organization S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 7, 2023
@ehuss
Copy link
Contributor

ehuss commented May 7, 2023

Just FYI, cargo has a policy of not parsing rustc or rustdoc flags. Generally, if there is some behavior that cargo needs to be aware of, then that needs to be lifted up as a first-class feature where cargo is driving the process (such as with its own flags). I don't offhand know how this should be designed, but I think it would be good to at least start with an issue where that design could be discussed.

@LukeMathWalker
Copy link
Author

I see, then I'll pursue the alternative I listed in the PR description—lifitng --output-format to a flag of cargo rustdoc.
I'll open an issue about it 👍🏻

@LukeMathWalker
Copy link
Author

Opened #12103

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-layout Area: target output directory layout, naming, and organization S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants