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

The left sidebar doesn't show all the functions when using the "latest" url #1590

Closed
STPR opened this issue Jan 10, 2022 · 23 comments · Fixed by rust-lang/rust#92742
Closed

Comments

@STPR
Copy link

STPR commented Jan 10, 2022

Hello,
The left sidebar doesn't show all the functions when using the "latest" url.

For example, the following url shows the 2 functions correctly:
https://docs.rs/contour_tracing/1.0.8/contour_tracing/

While this one doesn't show the function that is part of a feature:
https://docs.rs/contour_tracing/latest/contour_tracing/

@Nemo157
Copy link
Member

Nemo157 commented Jan 10, 2022

Seems to be a caching issue, https://docs.rs/contour_tracing/1.0.8/contour_tracing/sidebar-items.js has last-modified Mon, 10 Jan 2022 03:25:41 GMT while https://docs.rs/contour_tracing/latest/contour_tracing/sidebar-items.js has last-modified Tue, 03 Aug 2021 01:27:10 GMT which lines up with the last 1.0.3 build: https://docs.rs/crate/contour_tracing/1.0.3/builds.

cc @jsha, what is the caching behavior meant to be around sub-resources of latest links now?

@Nemo157
Copy link
Member

Nemo157 commented Jan 10, 2022

Actually.... why does this not have the resource suffix added onto it? Even the search-index.js has that resource suffix, so I would expect every file to 🤔

@syphar
Copy link
Member

syphar commented Jan 10, 2022

cc @jsha, what is the caching behavior meant to be around sub-resources of latest links now?

@Nemo157 that didn't change, the PR is still open #1569

@syphar
Copy link
Member

syphar commented Jan 10, 2022

I can imagine it's cached separately below the separate paths,

so the reason is probably in the missing suffix?

@syphar
Copy link
Member

syphar commented Jan 10, 2022

@Nemo157 @GuillaumeGomez could this be a rustdoc issue?

That surfaced because we serve different versions under the same URL, while statics are always cached forever?

@GuillaumeGomez
Copy link
Member

It's because of sidebar-items.js being different I think.

@syphar
Copy link
Member

syphar commented Jan 11, 2022

@GuillaumeGomez thinking about rust-lang/rust#92742 and this issue:
I believe this will only partially fix this.

I will have to think it through again, but currently I believe:

  • If two crate-releases are built with different nightly compilers, the resource-suffix and the path will lead to different file-names for actually different files.
  • if two crate-releases are built with the same nightly, the resource suffix will be the same, so under /latest/ there is a chance we will serve static assets from an older release.

Or am I missing something?

cc @jsha

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jan 11, 2022

If a crate with two different versions is built with the same compiler version, then it'll be the same issue. However, I think it'd be the same issue with search.js and search-index.js, no?

EDIT: I see there is the date followed by the compiler version. Maybe we should include the time too?

@syphar
Copy link
Member

syphar commented Jan 11, 2022

If a crate with two different versions is built with the same compiler version, then it'll be the same issue. However, I think it'd be the same issue with search.js and search-index.js, no?

Yep, would also be an issue for other release-assets.

EDIT: I see there is the date followed by the compiler version. Maybe we should include the time too?

Wouldn't really help. To my knowledge we don't update the compiler every night, but it's only updated on deploy

@GuillaumeGomez
Copy link
Member

Actually, I had in mind something like this:

[compiler version]-[crate build time]

I think it would fix this issue but the question is: would it break something?

@syphar
Copy link
Member

syphar commented Jan 11, 2022

ah ok. My thoughts were similar, but I wanted to add create-name and version. Though this would break on rebuilds ...

I need to think it through later.

@jsha
Copy link
Contributor

jsha commented Jan 11, 2022

This category of problem is supposed to be solved by SharedResource in rustdoc's write_shared.rs:

enum SharedResource<'a> {
    /// This file will never change, no matter what toolchain is used to build it.
    ///
    /// It does not have a resource suffix.
    Unversioned { name: &'static str },
    /// This file may change depending on the toolchain.
    ///
    /// It has a resource suffix.
    ToolchainSpecific { basename: &'static str },
    /// This file may change for any crate within a build, or based on the CLI arguments.
    ///
    /// This differs from normal invocation-specific files because it has a resource suffix.
    InvocationSpecific { basename: &'a str },
}

In this case I think sidebar-items.js should be InvocationSpecific. However, sidebar-items.js is not generated as a SharedResource in write_shared.rs. It is generated in render/context.rs by directly writing to a file. I think we need to change it to an InvocationSpecific item.

@Nemo157
Copy link
Member

Nemo157 commented Jan 11, 2022

It seems like all invocation-specific files need an additional suffix identifying this invocation, either provided on the command line along with the toolchain-suffix, or auto-generated by rustdoc.

@jsha
Copy link
Contributor

jsha commented Jan 11, 2022

Right now the invocation-specific file format appears to be:

search-20220110-1.60.0-nightly-89b9f7b28.js

What does that last part (89b9f7b28) identify?

@GuillaumeGomez
Copy link
Member

rustdoc commit hash I think.

@syphar
Copy link
Member

syphar commented Jan 11, 2022

It seems like all invocation-specific files need an additional suffix identifying this invocation, either provided on the command line along with the toolchain-suffix, or auto-generated by rustdoc.

Exactly this. But without changing the suffix for toolchain specific files, otherwise these won't be cached across releases any more.

rustdoc commit hash I think.

yes: https://github.com/syphar/docs.rs/blob/a9d9025f84537aa47d2aedac2862b57d99d4807d/src/utils/rustc_version.rs#L7-L23

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 11, 2022
…ar-items, r=notriddle

Add missing suffix for sidebar-items script path

Fixes rust-lang/docs.rs#1590.

r? `@syphar`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 11, 2022
…ar-items, r=notriddle

Add missing suffix for sidebar-items script path

Fixes rust-lang/docs.rs#1590.

r? ``@syphar``
@syphar
Copy link
Member

syphar commented Jan 13, 2022

@GuillaumeGomez since rust-lang/rust#92742 is merged and I'm new to that part of the process: which nightly will this feature be in?

Then I would trigger a rebuild for this release here.

For the general suffix issue I would create a new follow-up issue where we discuss details and approaches. IMO after the fix rust-lang/rust#92742 this can only lead to problems when one crate has multiple releases within one day.

@GuillaumeGomez
Copy link
Member

Normally, nothing needs to be changed in docs.rs since it's simply part of something we already use.

For the general suffix issue I would create a new follow-up issue where we discuss details and approaches. IMO after the fix rust-lang/rust#92742 this can only lead to problems when one crate has multiple releases within one day.

It's exactly the same current issue we have. Maybe adding the crate version into the suffix would enough.

@syphar
Copy link
Member

syphar commented Jan 13, 2022

Normally, nothing needs to be changed in docs.rs since it's simply part of something we already use.

I'm not sure I follow? I would like to rebuild this specific release when docs.rs has the bugfix in its nightly.

For the general suffix issue I would create a new follow-up issue where we discuss details and approaches. IMO after the fix rust-lang/rust#92742 this can only lead to problems when one crate has multiple releases within one day.

It's exactly the same current issue we have. Maybe adding the crate version into the suffix would enough.

Not really, at least not without changing more things. Adding the crate version to the suffix would also add it to the referenced toolchain-specific static files, which are built in a separate step ( add-essential-files) and served from a different location (the root path instead of the crate/release path).

@GuillaumeGomez
Copy link
Member

I'm not sure I follow? I would like to rebuild this specific release when docs.rs has the bugfix in its nightly.

Sorry, misunderstood. So nightly is built "every night" (but doesn't precise in which timezone, see in https://doc.rust-lang.org/book/appendix-07-nightly-rust.html). The PR was merged 2 days ago, so it should in the nightly from yesterday. I assume new crates were published yesterday so you can check if the sidebar-items.js now has its suffix.

@syphar
Copy link
Member

syphar commented Jan 13, 2022

I just queued a rebuild for contour_tracing 1.0.8.

For this crate we should be fine again, I'll try to wrap up the bigger topic separately.

@syphar
Copy link
Member

syphar commented Jan 13, 2022

@STPR could you check if this issue is fixed for you now?

Then I'll close this issue, after I created a new one for the more general (and older) problem.

@STPR
Copy link
Author

STPR commented Jan 13, 2022

@syphar
Yes. It seems fixed to me.
Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants