Skip to content

Fix Self keyword doc URL conflict on case insensitive file systems (until definitely fixed on rustdoc) #83678

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Mar 31, 2021

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Mar 30, 2021

This is just a hack to allow rustup to work on macOS and windows again to distribute std documentation (hopefully once rust-lang/rfcs#3097 or an equivalent is merged).

Fixes #80504. Prevents #83154 and rust-lang/rustup#2694 in future releases.

cc @kinnison
r? @jyn514

@GuillaumeGomez GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Mar 30, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 30, 2021
@jyn514
Copy link
Member

jyn514 commented Mar 30, 2021

@GuillaumeGomez could you double check this generates keyword.SelfTy.html? Ideally I'd like the lint I mention in rust-lang/rfcs#3097 (comment), but you don't need to add it if it's a burden, just manually checking that the list of duplicates from

build_case_insensitive_map(&krate, &options)
is empty is fine by me.

@jyn514
Copy link
Member

jyn514 commented Mar 30, 2021

error: Found non-existing keyword `SelfTy` used in `#[doc(keyword = "...")]`
    --> library/std/src/keyword_docs.rs:1316:1
     |
1316 | #[doc(keyword = "SelfTy")]
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^
     |
note: the lint level is defined here
    --> library/std/src/lib.rs:216:9
     |
216  | #![deny(rustc::existing_doc_keyword)]
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
     = help: only existing keywords are allowed in core/std

You need to update

fn is_doc_keyword(s: Symbol) -> bool {
s <= kw::Union
}
.

@jyn514
Copy link
Member

jyn514 commented Mar 30, 2021

Err actually I think I would just #[allow] the lint for now on that specific item.

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member Author

@GuillaumeGomez could you double check this generates keyword.SelfTy.html?

Sure, adding a test for it then.

Err actually I think I would just #[allow] the lint for now on that specific item.

I intended to do that. :3 (and yet another lint I added which prevents me from doing bad things. Sorry past me...)

@GuillaumeGomez GuillaumeGomez force-pushed the hack-Self-keyword-conflict branch from 9038279 to f35e587 Compare March 30, 2021 14:48
@GuillaumeGomez
Copy link
Member Author

Actually, testing the existence of a file isn't that easy. But I can confirm that the rust/build/x86_64-unknown-linux-gnu/doc/std/keyword.SelfTy.html file was generated for me.

@jyn514
Copy link
Member

jyn514 commented Mar 30, 2021

@bors r+

@rustbot label: +beta-nominated

@bors
Copy link
Collaborator

bors commented Mar 30, 2021

📌 Commit f35e587 has been approved by jyn514

@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 30, 2021
@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 Mar 30, 2021
@camelid
Copy link
Member

camelid commented Mar 30, 2021

Actually, testing the existence of a file isn't that easy.

What about something like this?

#![crate_name = "foo"]

#[doc(keyword = "banana")]
mod banana_keyword {}

// @has foo/keyword.banana.html ''

@GuillaumeGomez
Copy link
Member Author

@camelid We already check that the doc(keyword) is working. I think that @jyn514 was talking specifically about SelfTy.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 30, 2021
…nflict, r=jyn514

Fix Self keyword doc URL conflict on case insensitive file systems (until definitely fixed on rustdoc)

This is just a hack to allow rustup to work on macOS and windows again to distribute std documentation (hopefully once rust-lang/rfcs#3097 or an equivalent is merged).

Fixes rust-lang#80504. Prevents rust-lang#83154 and rust-lang/rustup#2694 in future releases.

cc `@kinnison`
r? `@jyn514`
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 30, 2021
Rollup of 6 pull requests

Successful merges:

 - rust-lang#80720 (Make documentation of which items the prelude exports more readable.)
 - rust-lang#83654 (Do not emit a suggestion that causes the E0632 error)
 - rust-lang#83671 (Add a regression test for issue-75801)
 - rust-lang#83678 (Fix Self keyword doc URL conflict on case insensitive file systems (until definitely fixed on rustdoc))
 - rust-lang#83680 (Update for loop desugaring docs)
 - rust-lang#83683 (bootstrap: don't complain about linkcheck if it is excluded)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d51fc97 into rust-lang:master Mar 31, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 31, 2021
@GuillaumeGomez GuillaumeGomez deleted the hack-Self-keyword-conflict branch March 31, 2021 08:10
@apiraino
Copy link
Contributor

apiraino commented Apr 1, 2021

beta backport decision deferred to next week

@apiraino
Copy link
Contributor

apiraino commented Apr 8, 2021

beta backport approved as per compiler meeting on Zulip

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Apr 8, 2021
@camelid
Copy link
Member

camelid commented Apr 9, 2021

Hmm, maybe we should also add a note to the SelfTy docs that say that the actual keyword is Self and it's named SelfTy because of a bug? Otherwise I imagine it could be quite confusing for readers of the docs.

ehuss pushed a commit to ehuss/rust that referenced this pull request Apr 21, 2021
…nflict, r=jyn514

Fix Self keyword doc URL conflict on case insensitive file systems (until definitely fixed on rustdoc)

This is just a hack to allow rustup to work on macOS and windows again to distribute std documentation (hopefully once rust-lang/rfcs#3097 or an equivalent is merged).

Fixes rust-lang#80504. Prevents rust-lang#83154 and rust-lang/rustup#2694 in future releases.

cc ``@kinnison``
r? ``@jyn514``
@ehuss ehuss mentioned this pull request Apr 21, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 21, 2021
[beta] Beta rollups

- Upgrade expat dependency in riscv64 to newer version. rust-lang#84394
- Fix Self keyword doc URL conflict on case insensitive file systems (until definitely fixed on rustdoc) rust-lang#83678
- Cargo:
    - Backport "Don't give a hard error when the end-user specifies RUSTC_BOOTSTRAP=crate_name" (rust-lang/cargo#9385)
    - [beta] Revert rust-lang/cargo#9133, moving to git HEAD dependencies by default (rust-lang/cargo#9383)
@ehuss ehuss removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 22, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keyword doc of self and Self points to the same file on windows.
9 participants