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

'cargo doc --open' seemingly does nothing if target is different from forced-target #11728

Open
dramforever opened this issue Feb 17, 2023 · 9 comments · May be fixed by #11804
Open

'cargo doc --open' seemingly does nothing if target is different from forced-target #11728

dramforever opened this issue Feb 17, 2023 · 9 comments · May be fixed by #11804
Assignees
Labels
C-bug Category: bug Command-doc S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review Z-per-package-target Nightly: per-package-target

Comments

@dramforever
Copy link

dramforever commented Feb 17, 2023

Problem

Currently if you have a forced-target (with -Zper-package-target) that's not the default, cargo doc --open builds the docs in target/{target_name}/doc but doesn't open it, which is confusing.

Steps

$ cargo doc --open
 Documenting forced-target v0.1.0 (/home/dram/tmp/cargo-doc-forced-target)
    Finished dev [unoptimized + debuginfo] target(s) in 0.20s
$ # Nothing opens

Possible Solution(s)

cargo doc --target riscv64gc-unknown-none-elf --open works. Maybe we can have cargo doc default to that for forced-target?

I originally encountered this in a workspace. I don't really know how this should interact with workspaces, but I think it should at least work if I explicitly choose cargo doc -p forced_target or use e.g. a --bin from it

Notes

No response

Version

cargo 1.69.0-nightly (82c3bb79e 2023-02-04)
release: 1.69.0-nightly
commit-hash: 82c3bb79e3a19a5164e33819ef81bfc2c984bc56
commit-date: 2023-02-04
host: x86_64-unknown-linux-gnu
libgit2: 1.5.0 (sys:0.16.0 vendored)
libcurl: 7.86.0-DEV (sys:0.4.59+curl-7.86.0 vendored ssl:OpenSSL/1.1.1q)
os: NixOS 23.5.0 [64-bit]
@dramforever dramforever added the C-bug Category: bug label Feb 17, 2023
@weihanglo
Copy link
Member

Thanks for the report!

Sounds like a good start to push that unstable feature forward. To fix it, we need to make this path respect not only forced-target but default-target as well. I haven't yet thought about the interaction too much but the behavior definitely needs to be defined before hand-on code.

Note that now Cargo can have multiple --targrt in a single cargo command invocation. Anyone who wants to help should take care of that.

@Rustin170506
Copy link
Member

@rustbot claim

I will try to support it.

@Rustin170506
Copy link
Member

Note that now Cargo can have multiple --targrt in a single cargo command invocation. Anyone who wants to help should take care of that.

For cargo doc, we only support one target.

➜  cargo-doc-forced-target git:(main) cargo doc --target riscv64gc-unknown-none-elf  --target aarch64-apple-darwin --open      
    Finished dev [unoptimized + debuginfo] target(s) in 0.10s
error: only one `--target` argument is supported

So probably we can just keep it.

@Rustin170506

This comment was marked as off-topic.

@weihanglo
Copy link
Member

@weihanglo Also, I found forced-target should conflict with default-target, right? I think we should bail out an error if we use it at the same time. What do you think?

Hmm… I don't think so. Perhaps a warning is sufficient. However, I feel like it is out of scope of this issue and not really worth putting effort on the warning for such an unstable feature.

@Rustin170506 Rustin170506 linked a pull request Mar 6, 2023 that will close this issue
@ehuss
Copy link
Contributor

ehuss commented Mar 14, 2023

I'm wondering about what the correct behavior should be for this when there is a mixture of normal and target-specific crates. If cargo generates docs for different targets, then there isn't a way to navigate between them. From the user's perspective, the docs for the other targets are essentially invisible. (And which one gets opened is kinda arbitrary.)

I'm wondering if it would instead make sense to ignore the forced-target when generating documentation? A concern would be if there is target-specific documentation differences. But I would not expect those to be too common, and in theory #[doc(cfg)] is the long-term solution for platform-specific documentation.

What do people think?

@Rustin170506
Copy link
Member

I'm wondering about what the correct behavior should be for this when there is a mixture of normal and target-specific crates.

If we use -p to indicate a specific package, then I think we should open its docs for forced-target. Because this is consistent with the current building/generating behavior. I think us already down it correctly in #11804.
If we use -p or -workspace to indicate a bunch of packages, as you suggested in #11804 we should always try to find a document for the host target. I think it quite makes sense. Because there is nothing special about any one of those packages. Opening the first document targeting the host seems very intuitive.

I'm wondering if it would instead make sense to ignore the forced target when generating documentation.

I suppose this would be quite confusing that forced-target only works for build instead of generating documentation. Also, consider this issue, @dramforever wants to see documentation opened for the forced-target. Opening the document targeting the forced target seems also intuitive.

@dramforever
Copy link
Author

dramforever commented Mar 15, 2023

I think having all of cargo doc use the same triple is a good idea too. I don't really know how well it works though.

Also, consider this issue, @dramforever wants to see documentation opened for the forced-target.

I'd clarify that I was mostly looking for any documentation at all. IIUC documentation currently can depend on the triple but it would be pretty rare, and IMHO it's confusing to document anything this way.

I don't think, for example, that docs.rs supports docs for multiple triples? At least I've never seen anyone do this. Edit: Apparently not? I'm unsure.

@Rustin170506
Copy link
Member

Edit: Apparently not? I'm unsure.

I think it is. You can see:
image
Also, you can see in the post, you can set the target for docs.rs.

@weihanglo weihanglo added S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed E-help-wanted labels Apr 19, 2023
@Rustin170506 Rustin170506 moved this from 🏊WIP to ⏳ Await responses in 🛠 OSS Maintenance May 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug Command-doc S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review Z-per-package-target Nightly: per-package-target
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants