Skip to content

Add more cfg aliases #2205

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

Merged
merged 9 commits into from
Nov 25, 2023
Merged

Add more cfg aliases #2205

merged 9 commits into from
Nov 25, 2023

Conversation

asomers
Copy link
Member

@asomers asomers commented Nov 24, 2023

Add cfg aliases for linux_android, bsd, and freebsdlike. Use them in many places, though not everywhere they could theoretically be used.

Fixes #2188

What does this PR do

Checklist:

  • I have read CONTRIBUTING.md
  • I have written necessary tests and rustdoc comments
  • A change log has been added if this PR modifies nix's API

@asomers asomers requested a review from SteveLauC November 24, 2023 00:34
Add cfg aliases for linux_android, bsd, and freebsdlike.  Use them in
many places, though not everywhere they could theoretically be used.

Fixes nix-rust#2188
@SteveLauC
Copy link
Member

Should we also do this for Solaris and Illumos?

cfg_aliases! {
    illumos: { target_os = "illumos" },
    solaris: { target_os = "solaris" },
        
    solarish: { any(solaris, illumos) }
}

BTW, we should document this in probably CONVENTIONS.md (or CONTRIBUTING.md?) so that our contributors can be aware of these aliases and use them in PRs

@asomers asomers requested a review from SteveLauC November 24, 2023 15:03
@Jan561
Copy link
Contributor

Jan561 commented Nov 24, 2023

Are other config-aliases, like an alias mmsg for

#[cfg(all(
    feature = "socket",
    feature = "uio",
    any(
        target_os = "linux",
        target_os = "android",
        target_os = "freebsd",
        target_os = "netbsd",
    ),
))]

desirable for the future? Or should they just stay platform-related for the time being?

@asomers
Copy link
Member Author

asomers commented Nov 24, 2023

Are other config-aliases, like an alias mmsg for

#[cfg(all(
    feature = "socket",
    feature = "uio",
    any(
        target_os = "linux",
        target_os = "android",
        target_os = "freebsd",
        target_os = "netbsd",
    ),
))]

desirable for the future? Or should they just stay platform-related for the time being?

I think we can add them on an as-needed basis.

apple_targets,
target_os = "openbsd",
target_os = "netbsd",
bsd,
target_os = "redox",
target_os = "aix",
target_os = "illumos",
Copy link
Member

Choose a reason for hiding this comment

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

Another place where we can use solarish

apple_targets,
target_os = "openbsd",
target_os = "netbsd",
bsd,
target_os = "redox",
target_os = "aix",
target_os = "illumos",
Copy link
Member

Choose a reason for hiding this comment

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

Should be: solarish

@SteveLauC
Copy link
Member

I just checked that the changes made in this PR are good.

But there are also still cfgs that haven't been updated, we should handle them as well, but not in this PR, instead, we should probably split the remaining job into several PRs, which will make code review easier.

For #2188, let's turn it into a tracking issue, once the tasks in it are all done, then we can close it.


For the requested changes, I have committed 2 with the GitHub UI, please ignore the left ones and handle them in other PRs

@SteveLauC SteveLauC added this pull request to the merge queue Nov 25, 2023
Merged via the queue into nix-rust:master with commit 2afff81 Nov 25, 2023
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.

Tracking Issue: Replace big cfg! chunks with shorter aliases
3 participants