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

Apply resource-suffix to search-index and source-files scripts as well #59776

Merged
merged 1 commit into from
Apr 14, 2019

Conversation

GuillaumeGomez
Copy link
Member

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 7, 2019
@QuietMisdreavus
Copy link
Member

If you're changing the filename of source-files.js, make sure it's being linked correctly - IIRC the way it's included as an extra_file in the layout doesn't automatically add the resource suffix.

Moreover, i feel like this changes the meaning of the resource suffix a little. So far, there's been a bit of a separation between files that received the resource suffix, and those that didn't. As far as i know, the distinction was that "static" files that didn't change when run on a different crate (rustdoc.css, main.js, the theme CSS files, etc) got the suffix, but files that were generated with information that depended on the crate (search-index.js, aliases.js, source-files.js, etc) did not. Starting with this PR, we'll start adding the resource suffix to crate-specific files, that aren't also affected by --static-root-path.

It's worth noting why this PR was called for. Docs.rs has been having issues recently with the CDN caching files like the search index on URLs like docs.rs/tokio/*/tokio/search-index.js, and serving it even when the underlying file has changed. This was proposed as a solution to make the page reference a different URL, which would cause the CDN to cache a different file.

(Though, while typing this up, i also realized that a docs.rs-side solution that would also solve that problem is to always redirect non-exact versions to the latest version before serving the file. This would solve the issue by always serving the right version of the crate in the URL, thus never having the problem of which search-index.js is being served from the URL change over time. I'm leaning toward that solution instead of changing what files we add the resource suffix to, TBH.)

@GuillaumeGomez
Copy link
Member Author

As far as I can tell, the links are done correctly. Only source-files.js has a "special" way to be generated through layout and this has been updated as well. Do you see anything else that might require changes?

@QuietMisdreavus
Copy link
Member

Oh, i totally missed the part that added the resource suffix to the layout call.

The only other thing i would add is to do the same to alises.js, since it can have the same issue. Otherwise, this looks fine. Based on rust-lang/docs.rs#316 (comment) i want to have this in anyway, since it will fix a specific problem that the "docs.rs-side solution" would not.

@GuillaumeGomez GuillaumeGomez force-pushed the apply-resource-suffix branch from 579c3ef to 4f28431 Compare April 12, 2019 17:30
@GuillaumeGomez
Copy link
Member Author

I added it for aliases.js as well.

Copy link
Member

@QuietMisdreavus QuietMisdreavus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks so much! r=me when travis is green.

@GuillaumeGomez
Copy link
Member Author

@bors: r=QuietMisdreavus

@bors
Copy link
Contributor

bors commented Apr 12, 2019

📌 Commit 4f28431 has been approved by QuietMisdreavus

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 12, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
…x, r=QuietMisdreavus

Apply resource-suffix to search-index and source-files scripts as well

Fixes rust-lang#59771.

r? @QuietMisdreavus
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
Rollup of 9 pull requests

Successful merges:

 - rust-lang#59655 (Use a proc macro to declare preallocated symbols)
 - rust-lang#59769 (compiletest normalization: preserve non-JSON lines such as ICEs)
 - rust-lang#59776 (Apply resource-suffix to search-index and source-files scripts as well)
 - rust-lang#59784 (Suggest importing macros from the crate root)
 - rust-lang#59812 (Exclude profiler-generated symbols from MSVC __imp_-symbol workaround.)
 - rust-lang#59856 (update polonius-engine)
 - rust-lang#59874 (Clean up handling of `-Z pgo-gen` commandline option.)
 - rust-lang#59890 (Don't generate empty json variables)
 - rust-lang#59911 (Revert "compile crates under test w/ -Zemit-stack-sizes")

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
…x, r=QuietMisdreavus

Apply resource-suffix to search-index and source-files scripts as well

Fixes rust-lang#59771.

r? @QuietMisdreavus
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
…x, r=QuietMisdreavus

Apply resource-suffix to search-index and source-files scripts as well

Fixes rust-lang#59771.

r? @QuietMisdreavus
bors added a commit that referenced this pull request Apr 14, 2019
Rollup of 6 pull requests

Successful merges:

 - #59776 (Apply resource-suffix to search-index and source-files scripts as well)
 - #59784 (Suggest importing macros from the crate root)
 - #59812 (Exclude profiler-generated symbols from MSVC __imp_-symbol workaround.)
 - #59874 (Clean up handling of `-Z pgo-gen` commandline option.)
 - #59890 (Don't generate empty json variables)
 - #59911 (Revert "compile crates under test w/ -Zemit-stack-sizes")

Failed merges:

r? @ghost
@bors bors merged commit 4f28431 into rust-lang:master Apr 14, 2019
@GuillaumeGomez GuillaumeGomez deleted the apply-resource-suffix branch April 14, 2019 10:00
QuietMisdreavus added a commit to QuietMisdreavus/docs.rs that referenced this pull request Apr 29, 2019
starting with rust-lang/rust#59776, the search index, aliases, and
source file list are getting the resource suffix added to their file
names. this means we can't route them from a static path any more.

since we always check the static file router first, this will still
allow serving `main.js` for rustdoc versions prior to the addition of
`--static-root-path`, and also allows rustdoc to add per-crate
javascript files without requiring docs.rs to update to handle it.

(it also allows us to start hosting things *other* than rustdoc output
as docs - e.g. mdbook output - in the future without changing the
routing table massively.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--resource-suffix option isn't applied to search-index.js
4 participants