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

set cfg(rustdoc) when rustdoc is running on a crate #53076

Merged
merged 3 commits into from
Sep 1, 2018

Conversation

QuietMisdreavus
Copy link
Member

When using #[doc(cfg)] to document platform-specific items, it's a little cumbersome to get all the platforms' items to appear all at once. For example, the standard library adds --cfg dox to rustdoc's command line whenever it builds docs, and the documentation for #![feature(doc_cfg)] suggests using a Cargo feature to approximate the same thing. This is a little awkward, because you always need to remember to set --features dox whenever you build documentation.

This PR proposes making rustdoc set #[cfg(rustdoc)] whenever it runs on a crate, to provide an officially-sanctioned version of this that is set automatically. This way, there's a standardized way to declare that a certain version of an item is specifically when building docs.

To try to prevent the spread of this feature from happening too quickly, this PR also restricts the use of this flag to whenever #![feature(doc_cfg)] is active. I'm sure there are other uses for this, but right now i'm tying it to this feature. (If it makes more sense to give this its own feature, i can easily do that.)

r? @rust-lang/rustdoc (and maaaaaaybe someone from core? This seems a bit far-reaching...)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 4, 2018
@Mark-Simulacrum
Copy link
Member

I'll bring this up at core team triage next week if I don't forget.

@kennytm kennytm added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-core Relevant to the core team, which will review and decide on the PR/issue. labels Aug 5, 2018
@GuillaumeGomez
Copy link
Member

Awesome!

@nrc
Copy link
Member

nrc commented Aug 5, 2018

SGTM. Also note that tools attributes are nearly stable (just waiting for a PR), so you can now use attributes of the form #[rustdoc::foo] however you like (and if there are any existing rustdoc attributes we might think about changing them to that form and deprecating the old ones).

@nikomatsakis
Copy link
Contributor

cc @rust-lang/lang -- this PR is adding a #[cfg(rustdoc)] so that people can know that rustdoc is running and tweak their crate as needed. Seems fine to me but though y'all might like to be aware.

@Mark-Simulacrum
Copy link
Member

We discussed this in Core team triage and the conclusion was to cc the lang team; we felt it wasn't really our field of purview at this point.

@QuietMisdreavus QuietMisdreavus added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed T-core Relevant to the core team, which will review and decide on the PR/issue. labels Aug 10, 2018
@TimNN TimNN added I-nominated S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 14, 2018
@TimNN
Copy link
Contributor

TimNN commented Aug 14, 2018

I've added I-nominated to make sure the @rust-lang/lang team takes a look.

@joshtriplett
Copy link
Member

This makes me a little nervous, but given that people can trivially do this themselves anyway, we might as well provide a sanctioned solution.

That said...I wonder if rustdoc could somehow handle platform-specific things more automatically, rather than requiring people to use cfg(rustdoc)? rustdoc could learn to "see through" cfg, automatically document everything, and provide some readable description of the platforms an item is available on.

@QuietMisdreavus
Copy link
Member Author

@joshtriplett You have just described the oldest rustdoc issue on this tracker.

The biggest problem with "making rustdoc see through cfg" is that rustdoc isn't the one that's processing them. Rustc is. Rustdoc will happily try to show off everything that's given to it, if rustc can handle ignoring every #[cfg] attribute in a crate and still make it through macro expansion and name resolution. Making that happen is something i have called "the holy grail of rustdoc" because i'm pretty sure it will require way more work in the compiler then people are willing to deal with.

@joshtriplett
Copy link
Member

@QuietMisdreavus Ah, I see. I didn't realize that, I thought this fell under the domain of rustdoc rather than rustc.

@Centril
Copy link
Contributor

Centril commented Aug 15, 2018

Seems fine to me; especially since it is feature gated. :)

@joshtriplett
Copy link
Member

Chatted with @QuietMisdreavus about this at RustConf, and it seems reasonable. I'm going to go ahead and FCP this and see if anyone objects.

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Aug 16, 2018

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Aug 16, 2018
@onur
Copy link
Member

onur commented Aug 25, 2018

This is fine for me guys, maybe we can also set up a cfg(docsrs) for docs.rs, people have been asking me how they can differ build environment and do specific tasks for docs.rs.

@QuietMisdreavus
Copy link
Member Author

@onur That should be fairly easy to set up, since you can add cfg flags via the command-line. We'd just need to add --cfg docsrs to the rustdoc flags whenever we call rustdoc over there and that should work out.

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Aug 27, 2018
@rfcbot
Copy link

rfcbot commented Aug 27, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@joshtriplett
Copy link
Member

Note: for this issue, I think it's appropriate to just treat the rfcbot coordination above as a poll for consensus, rather than seeking "FCP" specifically; I think the rustdoc team can simply proceed with this change at their discretion at this point.

@joshtriplett joshtriplett removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). I-nominated labels Aug 27, 2018
@QuietMisdreavus QuietMisdreavus added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 27, 2018
@@ -645,7 +645,7 @@ where R: 'static + Send,
paths.add_path(s, ErrorOutputType::default());
}
let mut cfgs = matches.opt_strs("cfg");
cfgs.push("rustdoc");
cfgs.push("rustdoc".to_string());
Copy link
Member

Choose a reason for hiding this comment

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

Can you please squash this into the patch that introduced these lines?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@GuillaumeGomez
Copy link
Member

Since everyone agreed, let's get it in then. Thanks @QuietMisdreavus!

@bors: r+

@bors
Copy link
Contributor

bors commented Sep 1, 2018

📌 Commit ad2169c has been approved by GuillaumeGomez

@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 Sep 1, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Sep 1, 2018
…laumeGomez

set cfg(rustdoc) when rustdoc is running on a crate

When using `#[doc(cfg)]` to document platform-specific items, it's a little cumbersome to get all the platforms' items to appear all at once. For example, the standard library adds `--cfg dox` to rustdoc's command line whenever it builds docs, and the documentation for `#![feature(doc_cfg)]` suggests using a Cargo feature to approximate the same thing. This is a little awkward, because you always need to remember to set `--features dox` whenever you build documentation.

This PR proposes making rustdoc set `#[cfg(rustdoc)]` whenever it runs on a crate, to provide an officially-sanctioned version of this that is set automatically. This way, there's a standardized way to declare that a certain version of an item is specifically when building docs.

To try to prevent the spread of this feature from happening too quickly, this PR also restricts the use of this flag to whenever `#![feature(doc_cfg)]` is active. I'm sure there are other uses for this, but right now i'm tying it to this feature. (If it makes more sense to give this its own feature, i can easily do that.)
bors added a commit that referenced this pull request Sep 1, 2018
Rollup of 9 pull requests

Successful merges:

 - #53076 (set cfg(rustdoc) when rustdoc is running on a crate)
 - #53622 (cleanup: Add main functions to some UI tests)
 - #53769 (Also link Clippy repo in the CONTRIBUTING.md file)
 - #53774 (Add rust-gdbgui script.)
 - #53781 (bench: libcore: fix build failure of any.rs benchmark (use "dyn Any"))
 - #53782 (Make Arc cloning mechanics clearer in module docs)
 - #53790 (Add regression test for issue #52060)
 - #53801 (Prevent duplicated impl on foreign types)
 - #53850 (Nuke the `const_to_allocation` query)
@bors
Copy link
Contributor

bors commented Sep 1, 2018

⌛ Testing commit ad2169c with merge 839d99c...

@bors bors merged commit ad2169c into rust-lang:master Sep 1, 2018
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Sep 6, 2018
jyn514 added a commit to jyn514/rust that referenced this pull request Sep 29, 2020
This was added in rust-lang#53076 because
several dependencies were using `cfg(dox)` instead of `cfg(rustdoc)`.
I ran `rg 'cfg\(dox\)'` on the source tree with no matches, so I think
this is now safe to remove.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 1, 2020
Remove --cfg dox from rustdoc.rs

This was added in rust-lang#53076 because
several dependencies were using `cfg(dox)` instead of `cfg(rustdoc)` (now `cfg(doc)`).
I ran `rg 'cfg\(dox\)'` on the source tree with no matches, so I think
this is now safe to remove.

r? @Mark-Simulacrum
cc @QuietMisdreavus :)
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 3, 2020
Remove --cfg dox from rustdoc.rs

This was added in rust-lang#53076 because
several dependencies were using `cfg(dox)` instead of `cfg(rustdoc)` (now `cfg(doc)`).
I ran `rg 'cfg\(dox\)'` on the source tree with no matches, so I think
this is now safe to remove.

r? `@Mark-Simulacrum`
cc `@QuietMisdreavus` :)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue. 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.