Skip to content

Conditionally enable functionality with feature flags #1498

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.

Sign up for GitHub

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

Closed
wants to merge 8 commits into from
Closed

Conditionally enable functionality with feature flags #1498

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bf93bc0
feature-gate most Nix functions
vdagonneau Mar 19, 2020
37fed26
fix CHANGELOG entry
asomers Oct 9, 2021
e11256f
Alphabetize features list
asomers Oct 9, 2021
10a160f
Restrict SealFlag to feature(fs)
asomers Oct 9, 2021
9463da3
Don't build from_libc_sockaddr in feautre(uio)
asomers Oct 9, 2021
d1a2bcd
style
asomers Oct 9, 2021
261fcb2
DO NOT MERGE fix a warning on the latest nightly
asomers Oct 16, 2021
83ea2f3
Remove target_os annotations from the docs.
asomers Oct 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions .cirrus.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ build: &BUILD
- $TOOL +$TOOLCHAIN $BUILD $ZFLAGS --target $TARGET --all-targets
- $TOOL +$TOOLCHAIN doc $ZFLAGS --no-deps --target $TARGET
- $TOOL +$TOOLCHAIN clippy $ZFLAGS --target $TARGET -- $CLIPPYFLAGS
- if [ -z "$NOHACK" ]; then $TOOL install cargo-hack; fi
- if [ -z "$NOHACK" ]; then $TOOL hack $ZFLAGS check --target $TARGET --each-feature; fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are enough features that it's not feasible to run cargo hack check --feature-powerset, but it may be useful to run it with limited depth.

$ cargo hack check --feature-powerset --depth 2

I found one issue (and possibly more, but I can't seem to get cargo hack check to not fail fast) by setting the depth to 2.

The following command fails:

$ cargo check --no-default-features --features mount,socket

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. This became an error only after PR #1546, though it was probably wrong anyway. I'm not sure why I put that part about uio in there.

Running cargo doc --no-default-features revealed some items which were potentially missed when adding things to features.

nix::fcntl::SealFlag, for instance, seems like it should be restricted to the fs feature.

Good catch. I only checked the no-features docs on FreeBSD, where SealFlag doesn't exist. What about unistd::pipe? It doesn't seem essential, like read and write, but it also doesn't seem related to any other feature and I didn't want to create a new feature just for it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed it doesn't make sense to have a dedicated feature for it.

Given the options of sticking it in a feature now and potentially moving it to a different feature later (because we aren't happy with the location), or leaving it unconditionally present for now and potentially moving it to a feature later, I prefer the second option. Both would be breaking changes, but the second feels less confusing to me.

I think you should leave it as essential for now.


# Tests that do require executing the binaries
test: &TEST
Expand Down Expand Up @@ -50,14 +52,16 @@ task:
- cargo build --target i686-unknown-freebsd
- cargo doc --no-deps --target i686-unknown-freebsd
- cargo test --target i686-unknown-freebsd
i386_feature_script:
- . $HOME/.cargo/env
- cargo hack check --each-feature --target i686-unknown-freebsd
before_cache_script: rm -rf $CARGO_HOME/registry/index

# Test OSX in a full VM
task:
matrix:
- name: OSX x86_64
env:
TARGET: x86_64-apple-darwin
name: OSX x86_64
env:
TARGET: x86_64-apple-darwin
osx_instance:
image: catalina-xcode
setup_script:
Expand Down Expand Up @@ -192,13 +196,17 @@ task:
# https://github.com/rust-embedded/cross/issues/535
- name: iOS aarch64
env:
# cargo hack tries to invoke the iphonesimulator SDK for iOS
NOHACK: 1
TARGET: aarch64-apple-ios
# Rustup only supports cross-building from arbitrary hosts for iOS at
# 1.49.0 and above. Below that it's possible to cross-build from an OSX
# host, but OSX VMs are more expensive than Linux VMs.
TOOLCHAIN: 1.49.0
- name: iOS x86_64
env:
# cargo hack tries to invoke the iphonesimulator SDK for iOS
NOHACK: 1
TARGET: x86_64-apple-ios
TOOLCHAIN: 1.49.0
# Cross testing on powerpc fails with "undefined reference to renameat2".
Expand All @@ -222,6 +230,8 @@ task:
TARGET: x86_64-unknown-netbsd
- name: Redox x86_64
env:
# cargo-hack needs a newer compiler
NOHACK: 1
TARGET: x86_64-unknown-redox
# Redox requires a nightly compiler.
# If stuff breaks, change nightly to the date in the toolchain_*
Expand Down Expand Up @@ -269,6 +279,6 @@ task:
image: rustlang/rust:nightly
setup_script:
- cargo update -Zminimal-versions
script:
check_script:
- cargo check
before_cache_script: rm -rf $CARGO_HOME/registry/index
11 changes: 11 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,17 @@
All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](https://semver.org/).

## [Unreleased] - ReleaseDate
### Added

- Added fine-grained features flags. Most Nix functionality can now be
conditionally enabled. By default, all features are enabled.
(#[1498](https://github.com/nix-rust/nix/pull/1498))

### Changed
### Fixed
### Removed

## [0.23.0] - 2021-09-28
### Added

Expand Down
43 changes: 43 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ categories = ["os::unix-apis"]
include = ["src/**/*", "LICENSE", "README.md", "CHANGELOG.md"]

[package.metadata.docs.rs]
rustdoc-args = ["--cfg", "docsrs"]
targets = [
"x86_64-unknown-linux-gnu",
"aarch64-linux-android",
Expand All @@ -32,6 +33,48 @@ cfg-if = "1.0"
[target.'cfg(not(target_os = "redox"))'.dependencies]
memoffset = "0.6.3"

[features]
default = [
"acct", "aio", "dir", "env", "event", "features", "fs",
"hostname", "inotify", "ioctl", "kmod", "mman", "mount", "mqueue",
"net", "personality", "poll", "process", "pthread", "ptrace", "quota",
"reboot", "resource", "sched", "signal", "socket", "term", "time",
"ucontext", "uio", "users", "zerocopy",
]

acct = []
aio = []
dir = ["fs"]
env = []
event = []
features = []
fs = []
hostname = []
inotify = []
ioctl = []
kmod = []
mman = []
mount = ["uio"]
mqueue = ["fs"]
net = ["socket"]
personality = []
poll = []
pthread = []
ptrace = ["process"]
quota = []
process = []
reboot = []
resource = []
sched = ["process"]
signal = ["process"]
socket = []
term = []
time = []
ucontext = ["signal"]
uio = []
users = ["features"]
zerocopy = ["fs", "uio"]

[target.'cfg(target_os = "dragonfly")'.build-dependencies]
cc = "1"

Expand Down
43 changes: 17 additions & 26 deletions src/dir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::os::unix::io::{AsRawFd, IntoRawFd, RawFd};
use std::ptr;
use std::ffi;
use crate::sys;
use cfg_if::cfg_if;

#[cfg(target_os = "linux")]
use libc::{dirent64 as dirent, readdir64_r as readdir_r};
Expand Down Expand Up @@ -186,34 +187,24 @@ pub enum Type {

impl Entry {
/// Returns the inode number (`d_ino`) of the underlying `dirent`.
#[cfg(any(target_os = "android",
target_os = "emscripten",
target_os = "fuchsia",
target_os = "haiku",
target_os = "illumos",
target_os = "ios",
target_os = "l4re",
target_os = "linux",
target_os = "macos",
target_os = "solaris"))]
pub fn ino(&self) -> u64 {
self.0.d_ino as u64
}

/// Returns the inode number (`d_fileno`) of the underlying `dirent`.
#[cfg(not(any(target_os = "android",
target_os = "emscripten",
target_os = "fuchsia",
target_os = "haiku",
target_os = "illumos",
target_os = "ios",
target_os = "l4re",
target_os = "linux",
target_os = "macos",
target_os = "solaris")))]
#[allow(clippy::useless_conversion)] // Not useless on all OSes
pub fn ino(&self) -> u64 {
u64::from(self.0.d_fileno)
cfg_if! {
if #[cfg(any(target_os = "android",
target_os = "emscripten",
target_os = "fuchsia",
target_os = "haiku",
target_os = "illumos",
target_os = "ios",
target_os = "l4re",
target_os = "linux",
target_os = "macos",
target_os = "solaris"))] {
self.0.d_ino as u64
} else {
u64::from(self.0.d_fileno)
}
}
}

/// Returns the bare file name of this directory entry without any other leading path component.
Expand Down
Loading