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

feature-gate most Nix functions #1611

Merged
merged 4 commits into from
Dec 21, 2021
Merged

feature-gate most Nix functions #1611

merged 4 commits into from
Dec 21, 2021

Conversation

asomers
Copy link
Member

@asomers asomers commented Dec 21, 2021

Using features reduces build time and size for consumer crates. By
default all features are enabled.

Using features reduces build time and size for consumer crates.  By
default all features are enabled.
@asomers
Copy link
Member Author

asomers commented Dec 21, 2021

Supersedes #1498 . I'm creating a new PR rather than update the previous one because I didn't want to erase some of its history by force-pushing.

@asomers asomers added this to the 0.24.0 milestone Dec 21, 2021
@asomers asomers force-pushed the use-features5 branch 2 times, most recently from e4c3179 to 771c710 Compare December 21, 2021 15:18
@asomers asomers requested a review from rtzoeller December 21, 2021 15:50
@rtzoeller
Copy link
Collaborator

bors r+

@bors bors bot merged commit 1dcc582 into nix-rust:master Dec 21, 2021
@asomers asomers deleted the use-features5 branch December 21, 2021 21:08
@rtzoeller rtzoeller mentioned this pull request Jan 1, 2023
O_LARGEFILE;
/// Do not update the file last access time during `read(2)`s.
#[cfg(any(target_os = "android", target_os = "linux"))]
#[cfg_attr(docsrs, doc(cfg(all())))]
Copy link
Member

@SteveLauC SteveLauC Nov 13, 2023

Choose a reason for hiding this comment

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

Hi @asomers, looking at our source code, I am curious why this #[cfg_attr(docsrs, doc(cfg(all())))] is needed?

From my obervation:

  1. We are not enabling the doc_auto_cfg feature, which means we won't get a #[doc(cfg(xx))] with #[cfg(xxx)], so no need to overwrite a non-existing value

  2. The doc_cfg feature does not seem to work with assocaiated item:

    // src/lib.rs
    
    #![feature(doc_cfg)]
    
    #[cfg(target_os = "linux")]
    #[doc(cfg(target_os = "linux"))]
    pub struct Foo;
    
    impl Foo {
        #[cfg(target_os = "linux")]
        #[doc(cfg(target_os = "linux"))]
        pub const LINUX_SPECIFIC_CONST: usize = 1;
    }

    Run:

    $ cargo +nightly doc --target x86_64-unknown-linux-gnu --open

    That Available on Linux only. banner will be set for Foo only, not for LINUX_SPECIFIC_CONST

And some types in our code are partially annocated with this attribute, e.g., enum Request defined in src/sys/ptrace/linux.rs, which makes me confused

Copy link
Member Author

Choose a reason for hiding this comment

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

I think they're there because I originally developed the PR with the doc_auto_cfg feature (see 83ea2f3). When that's on, constants like O_NOCTTY get a "This is supported on non-Redox only" banner. Those banners are both annoyingly verbose and deceptive (see the linked commit message for the explanation for why they're deceptive). But then I must've removed doc_auto_cfg before I posted the PR, leaving the doc(cfg(all())) attributes useless. So we can remove them now.

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 this pull request may close these issues.

4 participants