Add unstable --output-format option to cargo rustdoc

[charmitro]

Add unstable --output-format option to "rustdoc"

We achieved this by:

• Handle --output-format argument, accepting html or json
• A new field json in CompileMode::Doc.
• If json is passed, we append the following in
  prepare_rustdoc:
  1. -Zunstable-options
  2. --output-format=json

Fixes #12103

[rustbot]

r? @ehuss

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

[charmitro]

I'm a little confused about the -Zunstable-options etc. Any comment on that will help.

[weihanglo]

Thanks for the contribution!

Haven't read through the entire change. Just wondering if you could add tests that verify without the -Z flag Cargo will fail, for both rustdoc and doc commands.

[charmitro]

Thanks for the contribution!

Haven't read through the entire change. Just wondering if you could add tests that verify without the -Z flag Cargo will fail, for both rustdoc and doc commands.

Thanks! Added a couple more.

[weihanglo]

Thanks for the contribution. Really appreciate the effort of working on this.

However, the current implementation went a bit off the rails. There are a few parts we can discuss.

Fingerprint detection for <crate>.json

#12103 is an issue originated from #12100. The most important line in description of #12103 is missing in the current patch:

This would allow cargo to reason about it and take it into account in the fingerprinting logic.

If we look into #12100 carefully, we shall the change touched this piece of code:

cargo/src/cargo/core/compiler/context/compilation_files.rs

Lines 439 to 442 in 928b956

	let path = self
	.out_dir(unit)
	.join(unit.target.crate_name())
	.join("index.html");

This is the core logic Cargo determines where final output artifacts are located. We should do some trict here in order to make the fingerprint (the re-compile detection) takes <crate>.json into account.

In order to do so, Unit or some other types involved in the above code may need to know about --output-format. This is one non-easy part.

Where to pass the --output-format to rustdoc

On the other hand, we need a place to apppend rustdoc --output-format. The current fix found a place target_rustdoc_args, which is just a coincidence. It is for cargo rustdoc -- <args> trailing args only. Cargo should have a way to prevent contributor adding args into it (probably a way to make the semantic clearer). What we need is like --crate-type, which was also a cargo rustc -- --crate-type trailing argument and then got promoted as a normal argument. Presumbly we can follow its tracking issue to see what we can design for --output-format for rustdoc.

Tests to verify the behavior

The current added tests are good enough to verify -Z flag on-off and basic usage of `--output-format. Some cases are missing:

• Should bail when without -Z unstable-options.
• No recompile of run the same command multiple times.
• Recompile if change format.
• Check if each dependency outputs <crate>json for cargo doc.
• Check if only the requested package outputs <crate>.json for cargo rustdoc.

Focus on cargo rustdoc maybe?

Chances are that the output <crate>.json may collide with each other. Search collision_doc in this testsuite and you'll find some examples of doc collisions. We may or may not need to fix it. Maybe just make JSON collides like what HTML have done so far. However, if you do feel working on both cargo doc and cargo rustdoc are too distacting, we can focus on cargo rustdoc first. That's also the original issue author suggesting.

---

Sorry I don't have too many ideas on te implementaion side at this moment. I'll try writing down more thoughts if needed.

[ehuss]

r? @weihanglo

It looks like you have this covered, but feel free to reassign it back to me if you want.

[charmitro]

Fingerprint detection for <crate>.json

#12103 is an issue originated from #12100. The most important line in description of #12103 is missing in the current patch:

This would allow cargo to reason about it and take it into account in the fingerprinting logic.

If we look into #12100 carefully, we shall the change touched this piece of code:

cargo/src/cargo/core/compiler/context/compilation_files.rs

Lines 439 to 442 in 928b956

	let path = self
	.out_dir(unit)
	.join(unit.target.crate_name())
	.join("index.html");

This is the core logic Cargo determines where final output artifacts are located. We should do some trict here in order to make the fingerprint (the re-compile detection) takes <crate>.json into account.

In order to do so, Unit or some other types involved in the above code may need to know about --output-format. This is one non-easy part.

Is it safe to say that we should add a, e.g. json: bool or output_format: OutputFormat in CompileMode::Doc so we can determine whether the fingerprint should look for index.html or <crate>.json?

[weihanglo]

Is it safe to say that we should add a, e.g. json: bool or output_format: OutputFormat in CompileMode::Doc so we can determine whether the fingerprint should look for index.html or <crate>.json?

It's really tempting to do that! I feel a bit uncertain to add more fields onto CompileMode::Doc though I cannot find any better place than that. CompileMode is also already tracked in the fingerprint, so recompile should be automatically detected. I think it worth giving an attempt.

[weihanglo]

@ehuss do you have any input on adding a new field on CompileMode::Doc? Is there any better alternative?

[ehuss]

CompileMode::Doc seems like the correct place, especially with output computation and fingerprinting being involved. I would also expect things like calc_outputs to get updated.

[charmitro]

CompileMode::Doc seems like the correct place, especially with output computation and fingerprinting being involved. I would also expect things like calc_outputs to get updated.

Ack.

I will move forward with this and the rest of the requested changes and ping back. I will try to focus on cargo rustdoc for now and if the waters are calm I will also propose the cargo doc also.

[charmitro]

PR updated. All comments were addressed.

A major change in this revision is allowing --open with --output-format json.

[weihanglo]

Thanks for the update!

There are some nits and unresolved comments, and then we're ready to go!

• Add unstable --output-format option to cargo rustdoc #12252 (comment)
• Add unstable --output-format option to cargo rustdoc #12252 (comment)

[charmitro]

Thanks for the update!

There are some nits and unresolved comments, and then we're ready to go!

* [Add unstable `--output-format` option to  `cargo rustdoc` #12252 (comment)](https://github.com/rust-lang/cargo/pull/12252#discussion_r1448324026)

* [Add unstable `--output-format` option to  `cargo rustdoc` #12252 (comment)](https://github.com/rust-lang/cargo/pull/12252#discussion_r1448324634)

Resolved!

[weihanglo]

Great job!

Thanks a lot for the collaboration :)

@bors r+

[bors]

📌 Commit 9276399 has been approved by weihanglo

It is now in the queue for this repository.

[weihanglo]

@charmitro

Could you add a new doc entry of https://doc.rust-lang.org/nightly/cargo/reference/unstable.html for this feature?

You can take this as a reference: 4e45f58#diff-e35cef716988e9f7122a9c90479aa9204e61d1f41b094c0d183a44e0ca271eaaR83-R469

I've created the tracking issue here #13283

[charmitro]

@charmitro

Could you add a new doc entry of https://doc.rust-lang.org/nightly/cargo/reference/unstable.html for this feature?

You can take this as a reference: 4e45f58#diff-e35cef716988e9f7122a9c90479aa9204e61d1f41b094c0d183a44e0ca271eaaR83-R469

I've created the tracking issue here #13283

In this PR or create a follow-up?

[weihanglo]

This PR is now in the bors merge queue, so a follow-up would be better :)

[bors]

⌛ Testing commit 9276399 with merge f4e2eac...

[bors]

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing f4e2eac to master...