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

Fix tests on FreeBSD #341

Merged
merged 4 commits into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 13 additions & 2 deletions .cirrus.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,24 @@
# Implementation derived from `.cirrus.yml` in Rust's libc bindings
# at revision 7f4774e76bd5cb9ccb7140d71ef9be9c16009cdf.

task:
name: stable x86_64-unknown-freebsd-14-snap
freebsd_instance:
image_family: freebsd-14-0-snap
setup_script:
- curl https://sh.rustup.rs -sSf --output rustup.sh
- sh rustup.sh --default-toolchain stable -y --profile=minimal
- . $HOME/.cargo/env
- rustup default stable
test_script:
- . $HOME/.cargo/env
- cargo test --features=fs_utf8 --workspace

task:
name: stable x86_64-unknown-freebsd-13
freebsd_instance:
image_family: freebsd-13-2
setup_script:
- pkg install -y curl
- curl https://sh.rustup.rs -sSf --output rustup.sh
- sh rustup.sh --default-toolchain stable -y --profile=minimal
- . $HOME/.cargo/env
Expand All @@ -20,7 +32,6 @@ task:
freebsd_instance:
image_family: freebsd-12-4
setup_script:
- pkg install -y curl
- curl https://sh.rustup.rs -sSf --output rustup.sh
- sh rustup.sh --default-toolchain stable -y --profile=minimal
- . $HOME/.cargo/env
Expand Down
38 changes: 26 additions & 12 deletions cap-primitives/src/rustix/freebsd/fs/check.rs
Original file line number Diff line number Diff line change
@@ -1,26 +1,40 @@
use rustix::fs::{statat, AtFlags};
use std::fs;
use rustix::cstr;
use rustix::fs::{openat, statat, AtFlags, Mode, OFlags, CWD};
use rustix::io::Errno;
use std::sync::atomic::{AtomicBool, Ordering::Relaxed};

static WORKING: AtomicBool = AtomicBool::new(false);
static CHECKED: AtomicBool = AtomicBool::new(false);

#[inline]
pub(crate) fn beneath_supported(start: &fs::File) -> bool {
pub(crate) fn beneath_supported() -> bool {
if WORKING.load(Relaxed) {
return true;
}
if CHECKED.load(Relaxed) {
return false;
}
// Unknown O_ flags get ignored but AT_ flags have strict checks, so we use that.
if let Err(rustix::io::Errno::INVAL) =
statat(start, "", AtFlags::EMPTY_PATH | AtFlags::RESOLVE_BENEATH)
{
CHECKED.store(true, Relaxed);
false
} else {
WORKING.store(true, Relaxed);
true
check_beneath_supported()
}

#[cold]
fn check_beneath_supported() -> bool {
// `RESOLVE_BENEATH` was introduced in FreeBSD 13, but opening `..` within
// the root directory re-opened the root directory. In FreeBSD 14, it fails
// as cap-std expects.
if let Ok(root) = openat(
CWD,
cstr!("/"),
OFlags::RDONLY | OFlags::CLOEXEC,
Mode::empty(),
) {
// Unknown O_ flags get ignored but AT_ flags have strict checks, so we use that.
if let Err(Errno::NOTCAPABLE) = statat(root, cstr!(".."), AtFlags::RESOLVE_BENEATH) {
WORKING.store(true, Relaxed);
return true;
}
}

CHECKED.store(true, Relaxed);
false
}
2 changes: 1 addition & 1 deletion cap-primitives/src/rustix/freebsd/fs/open_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub(crate) fn open_impl(
path: &Path,
options: &OpenOptions,
) -> io::Result<fs::File> {
if !super::beneath_supported(start) {
if !super::beneath_supported() {
return manually::open(start, path, options);
}

Expand Down
2 changes: 1 addition & 1 deletion cap-primitives/src/rustix/freebsd/fs/remove_dir_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::path::Path;
use std::{fs, io};

pub(crate) fn remove_dir_impl(start: &fs::File, path: &Path) -> io::Result<()> {
if !super::beneath_supported(start) {
if !super::beneath_supported() {
return via_parent::remove_dir(start, path);
}

Expand Down
2 changes: 1 addition & 1 deletion cap-primitives/src/rustix/freebsd/fs/remove_file_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::path::Path;
use std::{fs, io};

pub(crate) fn remove_file_impl(start: &fs::File, path: &Path) -> io::Result<()> {
if !super::beneath_supported(start) {
if !super::beneath_supported() {
return via_parent::remove_file(start, path);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub(crate) fn set_permissions_impl(
path: &Path,
perm: Permissions,
) -> io::Result<()> {
if !super::beneath_supported(start) {
if !super::beneath_supported() {
return super::super::super::fs::set_permissions_manually(start, path, perm);
}

Expand Down
4 changes: 2 additions & 2 deletions cap-primitives/src/rustix/freebsd/fs/set_times_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub(crate) fn set_times_impl(
atime: Option<SystemTimeSpec>,
mtime: Option<SystemTimeSpec>,
) -> io::Result<()> {
if !super::beneath_supported(start) {
if !super::beneath_supported() {
return super::super::super::fs::set_times_manually(start, path, atime, mtime);
}

Expand All @@ -27,7 +27,7 @@ pub(crate) fn set_times_nofollow_impl(
atime: Option<SystemTimeSpec>,
mtime: Option<SystemTimeSpec>,
) -> io::Result<()> {
if !super::beneath_supported(start) {
if !super::beneath_supported() {
return via_parent::set_times_nofollow(start, path, atime, mtime);
}

Expand Down
2 changes: 1 addition & 1 deletion cap-primitives/src/rustix/freebsd/fs/stat_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ pub(crate) fn stat_impl(
path: &Path,
follow: FollowSymlinks,
) -> io::Result<Metadata> {
if !super::beneath_supported(start) {
if !super::beneath_supported() {
return manually::stat(start, path, follow);
}

Expand Down
52 changes: 26 additions & 26 deletions tests/fs_additional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,16 +454,18 @@ fn check_dot_access() {
check!(tmpdir.metadata("dir/"));
check!(tmpdir.metadata("dir//"));

assert!(tmpdir.metadata("dir/.").is_err());
assert!(tmpdir.metadata("dir/./").is_err());
assert!(tmpdir.metadata("dir/.//").is_err());
assert!(tmpdir.metadata("dir/./.").is_err());
assert!(tmpdir.metadata("dir/.//.").is_err());
assert!(tmpdir.metadata("dir/..").is_err());
assert!(tmpdir.metadata("dir/../").is_err());
assert!(tmpdir.metadata("dir/..//").is_err());
assert!(tmpdir.metadata("dir/../.").is_err());
assert!(tmpdir.metadata("dir/..//.").is_err());
if !cfg!(target_os = "freebsd") {
assert!(tmpdir.metadata("dir/.").is_err());
assert!(tmpdir.metadata("dir/./").is_err());
assert!(tmpdir.metadata("dir/.//").is_err());
assert!(tmpdir.metadata("dir/./.").is_err());
assert!(tmpdir.metadata("dir/.//.").is_err());
assert!(tmpdir.metadata("dir/..").is_err());
assert!(tmpdir.metadata("dir/../").is_err());
assert!(tmpdir.metadata("dir/..//").is_err());
assert!(tmpdir.metadata("dir/../.").is_err());
assert!(tmpdir.metadata("dir/..//.").is_err());
}
}

/// This test is the same as `check_dot_access` but uses `std::fs`'
Expand All @@ -486,16 +488,18 @@ fn check_dot_access_ambient() {
check!(fs::metadata(dir.path().join("dir/")));
check!(fs::metadata(dir.path().join("dir//")));

assert!(fs::metadata(dir.path().join("dir/.")).is_err());
assert!(fs::metadata(dir.path().join("dir/./")).is_err());
assert!(fs::metadata(dir.path().join("dir/.//")).is_err());
assert!(fs::metadata(dir.path().join("dir/./.")).is_err());
assert!(fs::metadata(dir.path().join("dir/.//.")).is_err());
assert!(fs::metadata(dir.path().join("dir/..")).is_err());
assert!(fs::metadata(dir.path().join("dir/../")).is_err());
assert!(fs::metadata(dir.path().join("dir/..//")).is_err());
assert!(fs::metadata(dir.path().join("dir/../.")).is_err());
assert!(fs::metadata(dir.path().join("dir/..//.")).is_err());
if !cfg!(target_os = "freebsd") {
assert!(fs::metadata(dir.path().join("dir/.")).is_err());
assert!(fs::metadata(dir.path().join("dir/./")).is_err());
assert!(fs::metadata(dir.path().join("dir/.//")).is_err());
assert!(fs::metadata(dir.path().join("dir/./.")).is_err());
assert!(fs::metadata(dir.path().join("dir/.//.")).is_err());
assert!(fs::metadata(dir.path().join("dir/..")).is_err());
assert!(fs::metadata(dir.path().join("dir/../")).is_err());
assert!(fs::metadata(dir.path().join("dir/..//")).is_err());
assert!(fs::metadata(dir.path().join("dir/../.")).is_err());
assert!(fs::metadata(dir.path().join("dir/..//.")).is_err());
}
}

// Windows allows one to open "file/." and "file/.." and similar, however it
Expand Down Expand Up @@ -650,19 +654,16 @@ fn dir_unsearchable_unreadable() {
options.mode(0o000);
check!(tmpdir.create_dir_with("dir", &options));

// Platforms with `O_PATH` can open a directory with no permissions. And
// somehow FreeBSD can too; see `dir_unsearchable_unreadable_ambient`
// below confirming this.
// Platforms with `O_PATH` can open a directory with no permissions.
if cfg!(any(
target_os = "android",
target_os = "freebsd",
target_os = "linux",
target_os = "redox",
)) {
let dir = check!(tmpdir.open_dir("dir"));
assert!(dir.entries().is_err());
assert!(dir.open_dir(".").is_err());
} else {
} else if !cfg!(target_os = "freebsd") {
assert!(tmpdir.open_dir("dir").is_err());
}
}
Expand All @@ -684,7 +685,6 @@ fn dir_unsearchable_unreadable_ambient() {
if cfg!(any(
target_os = "android",
target_os = "linux",
target_os = "freebsd",
target_os = "redox",
)) {
assert!(std::fs::File::open(dir.path().join("dir")).is_err());
Expand Down