From 67c857688e626f1016e8522fc3e4e4535cc23198 Mon Sep 17 00:00:00 2001 From: Byron Date: Sun, 25 Oct 2026 10:12:02 +0100 Subject: [PATCH] fix(dua-core): fall back when macOS bulk directory reads are unavailable (#417) The new implementation can have a performance impact if the bulk reads are turned off, but this can be controlled with configuration so shouldn't be a problem. Also, it's for old systems. PowerPC and macOS before 10.10 do not provide getattrlistbulk, so unconditional native directory enumeration leaves dua-core consumers with an undefined symbol at link time. Select bulk reads from the target architecture and deployment version in build.rs. Respect MACOSX_DEPLOYMENT_TARGET, otherwise query the target compiler's default; unknown versions and architectures use ordinary directory reads. Keep the existing macOS entry/metadata API and preserve requested APFS clone metadata in the fallback. Review identified an unused 64 KiB allocation per directory in fallback builds. Make that buffer zero-sized, avoiding the heap allocation while keeping the shared reader layout and fallback behavior. Also omit clock_gettime before macOS 10.12, leaving the metadata probe's existing missing-clock behavior in place. Assisted-by: GPT 6.1 Sol Co-authored-by: GPT 6.1 Sol --- .github/workflows/rust.yml | 14 +++++++ ci/check-macos-fallback.sh | 26 ++++++++++++ crates/dua-lib/build.rs | 57 ++++++++++++++++++++++++++ crates/dua-lib/src/macos/attributes.rs | 2 + crates/dua-lib/src/macos/mod.rs | 53 +++++++++++++++++++----- crates/dua-lib/src/macos/tests.rs | 16 ++++++++ crates/dua-lib/tests/build.rs | 31 ++++++++++++++ 7 files changed, 188 insertions(+), 11 deletions(-) create mode 100644 ci/check-macos-fallback.sh create mode 100644 crates/dua-lib/build.rs create mode 100644 crates/dua-lib/tests/build.rs diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 87128f71..05e15736 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -46,6 +46,20 @@ jobs: - name: clippy run: cargo clippy --workspace --all-targets --all-features -- -D warnings + build-and-test-macos-fallback: + runs-on: macos-15 + steps: + - uses: actions/checkout@v7 + - uses: Swatinem/rust-cache@v2 + - run: rustup target add x86_64-apple-darwin + - name: Test fallback for macOS before 10.10 + run: sh ci/check-macos-fallback.sh + - name: Lint fallback + env: + MACOSX_DEPLOYMENT_TARGET: '10.9' + CARGO_TARGET_DIR: target/macos-fallback + run: cargo clippy --workspace --all-targets --all-features --target x86_64-apple-darwin -- -D warnings + build-and-test-on-windows: name: Windows (nightly) runs-on: windows-latest diff --git a/ci/check-macos-fallback.sh b/ci/check-macos-fallback.sh new file mode 100644 index 00000000..403522c0 --- /dev/null +++ b/ci/check-macos-fallback.sh @@ -0,0 +1,26 @@ +#!/bin/sh +set -eu + +# An Intel deployment target predating getattrlistbulk must use ordinary directory reads. +export MACOSX_DEPLOYMENT_TARGET=10.9 +export CARGO_TARGET_DIR=target/macos-fallback +target=x86_64-apple-darwin + +cargo test --workspace --all-targets --target "$target" +cargo build -p dua-core --target "$target" +symbols=$(nm -u "$CARGO_TARGET_DIR/$target/debug/libdua_core.rlib") +if printf '%s\n' "$symbols" | grep -E '_getattrlistbulk|_clock_gettime'; then + echo 'The fallback must not link macOS APIs introduced after the deployment target.' >&2 + exit 1 +fi + +# Reusing the build directory also checks that changing the deployment target reruns build.rs. +export MACOSX_DEPLOYMENT_TARGET=10.10 +cargo test -p dua-core --target "$target" +cargo build -p dua-core --target "$target" +symbols=$(nm -u "$CARGO_TARGET_DIR/$target/debug/libdua_core.rlib") +printf '%s\n' "$symbols" | grep '_getattrlistbulk' > /dev/null +if printf '%s\n' "$symbols" | grep '_clock_gettime'; then + echo 'macOS 10.10 supports bulk reads but does not provide clock_gettime.' >&2 + exit 1 +fi diff --git a/crates/dua-lib/build.rs b/crates/dua-lib/build.rs new file mode 100644 index 00000000..383f5c1e --- /dev/null +++ b/crates/dua-lib/build.rs @@ -0,0 +1,57 @@ +use std::{env, process::Command}; + +fn main() { + println!("cargo:rerun-if-changed=build.rs"); + println!("cargo:rerun-if-env-changed=MACOSX_DEPLOYMENT_TARGET"); + println!("cargo:rerun-if-env-changed=RUSTC"); + println!("cargo:rustc-check-cfg=cfg(dua_macos_bulk)"); + println!("cargo:rustc-check-cfg=cfg(dua_macos_clock_gettime)"); + if env::var("CARGO_CFG_TARGET_OS").as_deref() != Ok("macos") { + return; + } + + let version = env::var("MACOSX_DEPLOYMENT_TARGET") + .ok() + .or_else(|| { + // Ask the target compiler, since the host OS may differ when cross-compiling. + let output = Command::new(env::var_os("RUSTC")?) + .args(["--print=deployment-target", "--target"]) + .arg(env::var_os("TARGET")?) + .output() + .ok()?; + if !output.status.success() { + return None; + } + String::from_utf8(output.stdout) + .ok()? + .trim() + .strip_prefix("MACOSX_DEPLOYMENT_TARGET=") + .map(str::to_owned) + }) + .unwrap_or_default(); + let arch = env::var("CARGO_CFG_TARGET_ARCH").unwrap_or_default(); + let (bulk, clock_gettime) = macos_api_support(&arch, &version); + if bulk { + println!("cargo:rustc-cfg=dua_macos_bulk"); + } + if clock_gettime { + println!("cargo:rustc-cfg=dua_macos_clock_gettime"); + } +} + +fn macos_api_support(arch: &str, version: &str) -> (bool, bool) { + // PowerPC never shipped with macOS 10.10; unknown compilers/targets use the fallback. + if !matches!(arch, "x86" | "x86_64" | "aarch64") { + return (false, false); + } + let mut parts = version.split('.'); + let major = parts.next().and_then(|part| part.parse::().ok()); + let minor = parts.next().unwrap_or("0").parse::().ok(); + let (Some(major), Some(minor)) = (major, minor) else { + return (false, false); + }; + if parts.any(|part| part.parse::().is_err()) { + return (false, false); + } + ((major, minor) >= (10, 10), (major, minor) >= (10, 12)) +} diff --git a/crates/dua-lib/src/macos/attributes.rs b/crates/dua-lib/src/macos/attributes.rs index fc7f2da1..24014a48 100644 --- a/crates/dua-lib/src/macos/attributes.rs +++ b/crates/dua-lib/src/macos/attributes.rs @@ -41,6 +41,7 @@ const FTS_COMMON_ATTRIBUTES: libc::attrgroup_t = libc::ATTR_CMN_RETURNED_ATTRS | libc::ATTR_CMN_FILEID; // XNU's `vfs_attrlist.c` `LIST_DIR_ATTRS` authorizes this subset with LIST_DIRECTORY alone. // Metadata attributes additionally require SEARCH, which a readable directory need not grant. +#[cfg(any(dua_macos_bulk, test))] const LIST_ONLY_ATTRIBUTES: libc::attrgroup_t = libc::ATTR_CMN_RETURNED_ATTRS | libc::ATTR_CMN_NAME | libc::ATTR_CMN_OBJTYPE @@ -97,6 +98,7 @@ pub(super) struct DataFork { pub(super) clone_id: Option, } +#[cfg(any(dua_macos_bulk, test))] pub(super) fn requested_attributes(list_only: bool, include_apfs: bool) -> libc::attrlist { let mut attributes = libc::attrlist { bitmapcount: libc::ATTR_BIT_MAP_COUNT, diff --git a/crates/dua-lib/src/macos/mod.rs b/crates/dua-lib/src/macos/mod.rs index 4f5be7e3..ef416052 100644 --- a/crates/dua-lib/src/macos/mod.rs +++ b/crates/dua-lib/src/macos/mod.rs @@ -4,27 +4,31 @@ use std::{ ffi::{CString, OsString}, fs, io, num::NonZeroU64, - os::{ - fd::{AsRawFd, OwnedFd}, - unix::{ - ffi::OsStrExt, - fs::{MetadataExt, OpenOptionsExt}, - }, - }, + os::unix::{ffi::OsStrExt, fs::MetadataExt}, path::{Path, PathBuf}, sync::Arc, time::{Duration, Instant, SystemTime}, }; +#[cfg(dua_macos_bulk)] +use std::os::{ + fd::{AsRawFd, OwnedFd}, + unix::fs::OpenOptionsExt, +}; + mod attributes; use attributes::{ AlignedBuffer, DataFork, ParsedRecord, RecordHeader, RootCloneResponse, SF_FIRMLINK, STAT_BLOCK_BYTES, VDIR, VLNK, VNON, VREG, invalid_data, parse_record, read_record_length, - requested_attributes, root_clone_attributes, + root_clone_attributes, }; -const DIRECTORY_BUFFER_BYTES: usize = 64 * 1024; +#[cfg(dua_macos_bulk)] +use attributes::requested_attributes; + +// A zero-sized buffer avoids heap allocation when every read uses the fallback. +const DIRECTORY_BUFFER_BYTES: usize = if cfg!(dua_macos_bulk) { 64 * 1024 } else { 0 }; /// A macOS filesystem entry produced from native directory metadata. pub struct Entry { @@ -236,6 +240,7 @@ impl Metadata { } pub(crate) struct ReadDir { + #[cfg(dua_macos_bulk)] directory: OwnedFd, fallback: Option, buffer: Box>, @@ -323,14 +328,19 @@ impl ReadDir { } pub(crate) fn open(path: Arc, depth: usize, options: crate::Options) -> io::Result { + #[cfg(dua_macos_bulk)] let directory: OwnedFd = fs::OpenOptions::new() .read(true) .custom_flags(libc::O_DIRECTORY) .open(&path)? .into(); Ok(Self { + #[cfg(dua_macos_bulk)] directory, + #[cfg(dua_macos_bulk)] fallback: None, + #[cfg(not(dua_macos_bulk))] + fallback: Some(fs::read_dir(&path)?), buffer: Box::new(AlignedBuffer::new()), offset: 0, remaining: 0, @@ -348,6 +358,7 @@ impl ReadDir { /// /// Returns `true` when iteration can continue, either because bulk records are available or /// because `self.fallback` was initialized. Returns `false` when the directory is exhausted. + #[cfg(dua_macos_bulk)] fn refill(&mut self) -> io::Result { loop { let list_only = self.listing_error.is_some(); @@ -421,10 +432,24 @@ impl ReadDir { } } + #[cfg(not(dua_macos_bulk))] + fn refill(&mut self) -> io::Result { + self.fallback = Some(fs::read_dir(&self.parent_path)?); + Ok(true) + } + fn fallback_entry(&self, entry: fs::DirEntry) -> Entry { let file_name = entry.file_name(); - let metadata = - fs::symlink_metadata(entry.path()).map(|metadata| Metadata::from_std(&metadata, None)); + let path = entry.path(); + let metadata = fs::symlink_metadata(&path).map(|metadata| { + let data_fork = + if self.extended_attributes && metadata.is_file() && metadata.blocks() != 0 { + clone_attributes_at(&path, &metadata) + } else { + None + }; + Metadata::from_std(&metadata, data_fork) + }); let file_type = metadata.as_ref().map_or_else( |_| { entry @@ -517,6 +542,7 @@ impl ReadDir { /// Differences between these readings count only time the worker spends executing on a CPU. /// Differences between [`std::time::Instant::now()`] readings also include time blocked on I/O /// or waiting to be scheduled. Comparing the two lets the probe distinguish execution from waiting. +#[cfg(dua_macos_clock_gettime)] fn thread_cpu_time() -> Option { let mut time = libc::timespec { tv_sec: 0, @@ -532,6 +558,11 @@ fn thread_cpu_time() -> Option { )) } +#[cfg(not(dua_macos_clock_gettime))] +fn thread_cpu_time() -> Option { + None +} + impl Iterator for ReadDir { type Item = io::Result; diff --git a/crates/dua-lib/src/macos/tests.rs b/crates/dua-lib/src/macos/tests.rs index e4ab8aef..a610b124 100644 --- a/crates/dua-lib/src/macos/tests.rs +++ b/crates/dua-lib/src/macos/tests.rs @@ -45,6 +45,7 @@ fn parallel_metadata_requires_repeated_waiting_regardless_of_clock_speed() { } #[test] +#[cfg(dua_macos_bulk)] fn cpu_bound_probe_leaves_directory_stats_for_streaming() { let directory = tempfile::tempdir().unwrap(); let child = directory.path().join("child"); @@ -75,6 +76,21 @@ fn cpu_bound_probe_leaves_directory_stats_for_streaming() { ); } +#[test] +#[cfg(not(dua_macos_bulk))] +fn unsupported_targets_start_with_ordinary_directory_reads() { + let directory = tempfile::tempdir().unwrap(); + fs::write(directory.path().join("file"), b"content").unwrap(); + let mut reader = ReadDir::open(Arc::from(directory.path()), 1, options(false)).unwrap(); + assert!(reader.fallback.is_some()); + assert!(reader.buffer.as_bytes().is_empty()); + assert!(reader.probe_metadata().is_empty()); + let entry = reader.next().unwrap().unwrap(); + assert_eq!(entry.file_name, "file"); + assert_eq!(entry.metadata.unwrap().unwrap().len(), 7); + assert!(reader.next().is_none()); +} + #[test] fn metadata_and_entries_keep_clone_tracking_compact() { assert_eq!( diff --git a/crates/dua-lib/tests/build.rs b/crates/dua-lib/tests/build.rs new file mode 100644 index 00000000..da88b935 --- /dev/null +++ b/crates/dua-lib/tests/build.rs @@ -0,0 +1,31 @@ +include!("../build.rs"); + +#[test] +fn macos_apis_follow_the_target_version_and_architecture() { + for arch in ["x86", "x86_64", "aarch64"] { + for (version, expected) in [ + ("10.9", (false, false)), + ("10.9.5", (false, false)), + ("10.10", (true, false)), + ("10.11.6", (true, false)), + ("10.12", (true, true)), + ("11", (true, true)), + ("26.0.1", (true, true)), + ("", (false, false)), + ("invalid", (false, false)), + ("10.bad", (false, false)), + ("10.12.bad", (false, false)), + ] { + assert_eq!( + macos_api_support(arch, version), + expected, + "{arch}: {version}" + ); + } + } + for arch in ["powerpc", "powerpc64", "unknown"] { + for version in ["", "10.5.8", "10.12", "26"] { + assert_eq!(macos_api_support(arch, version), (false, false)); + } + } +}