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

Intra-doc links not resolved in workspace with proc-macro #11628

Open
AaronKutch opened this issue Jan 25, 2023 · 3 comments
Open

Intra-doc links not resolved in workspace with proc-macro #11628

AaronKutch opened this issue Jan 25, 2023 · 3 comments
Labels
C-bug Category: bug Command-doc E-hard Experience: Hard S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@AaronKutch
Copy link

Problem

There are doc links that sometimes refuse to work, I found a reproducible way of witnessing it.

Steps

Make sure rust-analyzer is not running for reproducibility. clone doc-bug, run cargo clean and cargo doc --open at the root of the workspace. Click on the awint_macros crate in the "Crates" list and scroll all the way to the bottom of the lib docs (I can't reduce further because it causes the problem to go away). There is a broken doc link with the literal text [awint_macro_internals::awint_macro_inlawi] towards the bottom.

Then, go to the source code awint/awint_macros/src/lib.rs and make some dummy change like adding a "// hello" comment somewhere. Run just cargo doc and refresh, now the doc link is no longer broken.

Possible Solution(s)

No response

Notes

It probably has to do with cargo-doc getting confused when there are multiple transitive dependencies and multiple reexports of items. Recompiling sometimes breaks or fixes it strangely.

Version

cargo 1.69.0-nightly (985d561f0 2023-01-20)
release: 1.69.0-nightly
commit-hash: 985d561f0bb9b76ec043a2b12511790ec7a2b954
commit-date: 2023-01-20
host: x86_64-pc-windows-msvc
libgit2: 1.5.1 (sys:0.16.1 vendored)
libcurl: 7.86.0-DEV (sys:0.4.59+curl-7.86.0 vendored ssl:Schannel)
os: Windows 10.0.19045 (Windows 10 Home) [64-bit]
@AaronKutch AaronKutch added the C-bug Category: bug label Jan 25, 2023
@ehuss
Copy link
Contributor

ehuss commented Jan 25, 2023

Thanks for the report! I'm closing as a duplicate of #8487. The problem is that Cargo is not tracking the dependencies between rustdoc invocations. It allows them to run concurrently, which causes a race condition.

@ehuss ehuss closed this as not planned Won't fix, can't repro, duplicate, stale Jan 25, 2023
@AaronKutch
Copy link
Author

Ok, I feel like I have been encountering this sporadically for a while, I hope it is fixed sometime

@ehuss ehuss changed the title Intra-doc links getting borked with a sufficiently complicated workspace Intra-doc links not resolved in workspace with proc-macro Jan 31, 2023
@ehuss
Copy link
Contributor

ehuss commented Jan 31, 2023

I'm going to reopen since further investigation looks like this is a different issue. The following is a test for this scenario:

#[cargo_test]
fn virtual_ws_proc_macro() {
    // A workspace with a proc-macro with a dependency into the workspace.
    // This creates a situation where some units are "for host" and some are
    // "for target". The unit sharing code should combine them, so the doc
    // deduplication system should only end up with a single copy, and it
    // should have its dependencies wired up correctly so that intra-doc links
    // work.
    let p = project()
        .file(
            "Cargo.toml",
            r#"
                [workspace]
                members = ["pm", "bar"]
            "#,
        )
        .file(
            "pm/Cargo.toml",
            r#"
                [package]
                name = "pm"
                version = "0.1.0"
                edition = "2021"

                [lib]
                proc-macro = true

                [dependencies]
                bar = {path = "../bar"}
            "#,
        )
        .file(
            "pm/src/lib.rs",
            r#"
                use proc_macro::TokenStream;
                /// See [bar::example]
                #[proc_macro]
                pub fn links_to(input: TokenStream) -> TokenStream {
                    "".parse().unwrap()
                }
            "#,
        )
        .file("bar/Cargo.toml", &basic_manifest("bar", "0.1.0"))
        .file(
            "bar/src/lib.rs",
            r#"
                /// Example docs.
                pub fn example() {}
            "#,
        )
        .build();
    // Run check first to emphasize the chance of a race condition if things
    // aren't working correctly.
    p.cargo("check").run();
    p.cargo("doc").with_stderr("").run();
    let contents = p.read_file("target/doc/pm/macro.links_to.html");
    assert!(contents.contains(r#"<a href="../bar/fn.example.html""#));
}

The problem is that Cargo generates a build graph with a duplicate for the pm package. When the deduplication code here removes those duplicates, it deletes the edges without replacing them to the version of pm that gets documented. In detail:

The initial unit graph looks like:

bar doc target
bar check host
pm doc host
    -> bar check host
    -> bar doc host
bar doc host

After doc deduplication, it looks like:

bar doc target
bar check host
pm doc host
    -> bar check host

Here the edge from pm doc to bar doc has been removed.

This particular case can be resolved by doing doc deduplication after unit sharing and removing the "prefer target over host" code. However, that breaks situations where either the units differ by profiles (illustrated in collision_doc_profile_split), or they differ by features (illustrated in decouple_proc_macro).

The profile distinction could possibly be solved by using a dummy profile for doc units. rustdoc doesn't use profiles, so the distinction is not useful.

The feature distinction is harder. I think one solution is to prefer the target doc units over the host doc units, but instead of removing the host doc unit edges here, they need to instead be replaced by the host doc units, but only if --target was not specified (with --target, this isn't an issue since the doc directories are separate).

That's starting to get quite complicated, and I'm concerned about trying to be too clever will introduce more problems. #10241 (comment) contains some thoughts on a longer-term solution, though some of the changes suggested here (such as using a common profile) might still be worth adding.

@ehuss ehuss reopened this Jan 31, 2023
@weihanglo weihanglo added E-hard Experience: Hard S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. labels Jun 8, 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 E-hard Experience: Hard S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

3 participants