install: improve directory creation #8986

mattsu2020 · 2025-10-23T14:33:27Z

Summary

Ported the refreshed install utility improvements from install-linux-update, introducing the new create_dir_all_for_install implementation with Unix nix support and retaining Windows compatibility through standard path handling.

fix this issue in unix environments
#8963

Replaces fs::create_dir_all with a custom implementation using nix on Unix systems to ensure proper mode handling for ancestor directories, with a fallback for non-Unix platforms. Adds related imports and a new test case for the functionality.

Reorganized Unix-specific imports and refactored create_dir_all_for_install to use nix crate's OwnedFd and BorrowedFd directly, simplifying file descriptor management and improving error handling for better efficiency.

- Add terms like CLOEXEC, dirfds, FDCWD, mkdirat, openat, RDONLY to jargon wordlist for cspell to avoid false positives in spell-checking for Unix file operations - Update test_install_directory_deep_path_succeeds to use #[cfg(unix)] instead of #[cfg(not(windows))] for more precise platform targeting

Replaced vector of file descriptors with a single Option<OwnedFd> in create_dir_all_for_install for improved efficiency and readability. Added AsFd trait import and OsStr for supporting changes. Also added "accum" to jargon wordlist for spell checking.

… flags Use O_PATH on Linux/Android for efficient directory opening without read permissions. Add fallback to fs::create_dir_all when mkdirat is unsupported (ENOSYS). Refine error handling to ignore EACCES and EPERM in addition to EEXIST.

- Add Unix-specific nix dependency with dir and fs features for precise file operations - Introduce create_dir_all_for_install function leveraging mkdirat and openat for efficient, low-level directory handling - Update jargon wordlist with related terms like CLOEXEC, dirfds, FDCWD, mkdirat, openat, RDONLY for spell-checking - Replace fs::create_dir_all with new function in directory creation logic, falling back on standard method if unsupported - Improves performance and compatibility for install command on Unix systems by using system calls directly

Remove duplicate uucore::libc import and reorder statements for cleaner code structure in install utility tests.

- Added std::os::fd::{AsFd, BorrowedFd} and std::os::unix::io::OwnedFd for enhanced file descriptor handling - Reorganized Unix-specific imports into a structured block for better maintainability - Included OsStr in std::ffi imports to support additional string operations on Unix systems

Removed duplicate `use std::path::Path;` import and unified `#[cfg(not(windows))]` to `#[cfg(unix)]` for `use uucore::libc;` to improve code consistency and target Unix-like systems more precisely.

github-actions · 2025-10-23T14:56:07Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

.vscode/cspell.dictionaries/jargon.wordlist.txt

delete duplicate word Co-authored-by: Daniel Hofstetter <[email protected]>

cakebaker · 2025-10-24T07:38:30Z

tests/by-util/test_install.rs


+#[test]
+#[cfg(unix)]
+fn test_install_directory_deep_path_succeeds() {


This test passes even without applying the changes to install.rs. Is that the expected behavior?

#8963
This is a test to fix the change, so it was originally designed to fail.

github-actions · 2025-10-24T07:58:29Z

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

cakebaker · 2025-10-24T08:10:01Z

tests/by-util/test_install.rs

+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let deep_rel_path = format!("./{}", "a/".repeat((libc::PATH_MAX as usize / 4) + 16));


The value (1040) you calculate as argument for repeat is too low to trigger the issue. If I run the example of the ticket with that value, it works fine and the directories are created.

increased the number of times

This change updates the test to repeat the path component up to the full libc::PATH_MAX limit, ensuring the test properly exercises the maximum allowed path depth for the install directory functionality.

…coreutils into install-only-updates

Update the deep path generation in the install test to use libc::PATH_MAX, ensuring the test accurately checks behavior at the system's maximum path length limit.

cakebaker · 2025-10-24T08:41:09Z

tests/by-util/test_install.rs

+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let deep_rel_path = format!("./{}", "a/".repeat(libc::PATH_MAX as usize));


Now the value is probably too high as the test keeps failing when applying the changes to install.rs ;-)

Refactor the path construction logic in test_install_directory_deep_path_succeeds to ensure the generated path is at least 3000 characters long while respecting PATH_MAX limits, preventing potential overflows and improving test reliability for long path scenarios.

Combine the min_repeat calculation into a single line for improved readability in the test_install_directory_deep_path_succeeds function.

Explicitly annotate min_len as usize for improved code clarity and type safety in path length calculations.

Replaces manual ceiling division with `div_ceil` for cleaner and more idiomatic code in the deep path test.

Simplify the min_repeat assignment by consolidating it into a single line, improving code readability and consistency in the install utility tests.

github-actions · 2025-10-24T09:25:33Z

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

cakebaker · 2025-10-24T09:28:02Z

tests/by-util/test_install.rs

+    let unit_len = "a/".len();
+    let prefix_len = "./".len();
+    let min_len: usize = 3000; // request a path of at least 3000 characters
+    let max_repeat = (libc::PATH_MAX as usize - prefix_len) / unit_len;
+    let min_repeat = min_len.saturating_sub(prefix_len).div_ceil(unit_len).max(1);
+    let repeat_count = std::cmp::min(max_repeat, min_repeat);
+    let deep_rel_path = format!("./{}", "a/".repeat(repeat_count));


I would keep it simple as it was before :) Why not use a fixed value like 3000 as argument for repeat?

An error occurred in the macOS environment, so hard-coding 3000 was not acceptable.

Refactored the test logic in `test_install_directory_deep_path_succeeds` to accurately calculate the maximum repeatable path segments while accounting for the base directory length and PATH_MAX constraints. Added a panic for insufficient space and a debug assertion to ensure the absolute path stays within limits, improving test reliability for long paths.

- Update import conditions: OsStrExt for unix, OsStringExt for linux to improve platform compatibility - Chain method calls in min_repeat calculation for code conciseness and readability

Replaces a panic with an assert in the `test_install_directory_deep_path_succeeds` function to handle cases where the temporary directory path exceeds PATH_MAX, improving test reliability and error reporting consistency.

Handle ENAMETOOLONG errors by using file descriptor-based chmod via nix crate, ensuring compatibility with paths exceeding system limits on Unix systems.

Reordered the import statement for nix::fcntl to place OFlag before open and openat. Reformatted long lines, including expressions for base_fd, error creation, and openat calls, to improve readability and adhere to code style guidelines.

codspeed-hq · 2025-10-24T10:31:15Z

CodSpeed Performance Report

Merging #8986 will not alter performance

_{Comparing mattsu2020:install-only-updates (7206c29) with main (6cf5b06)}

Summary

✅ 123 untouched
⏩ 5 skipped¹

5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

- Removed platform-specific node_open_flags functions to reduce code duplication - Introduced last_component tracking to properly handle the final path component - Updated CurDir and ParentDir handling to set last_component when it's the last element - Simplified Normal component logic to only open directories for non-final components - Use last_component for fchmod operation, defaulting to "." if no component is found, improving reliability for edge cases in long path chmod operations

Added std::os::fd::AsFd import under unix-specific conditions to enable file descriptor-based operations in the install utility's mode handling.

Added AsRawFd trait import alongside AsFd in mode.rs to enable access to raw file descriptors, supporting enhanced file handling operations in the install utility.

Removed the unused `std::fs` import and repositioned `std::path::Path` after the conditional block for better import organization in the mode module.

Added dirfd, ENAMETOOLONG, and fchmodat to the VS Code cspell dictionary to prevent false positives during spell checking of technical code terms.

github-actions · 2025-10-24T11:14:54Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

Adjust path calculation in test_install_directory_deep_path_succeeds to subtract a safety margin when PATH_MAX is 1024 or less, preventing potential buffer overflow issues in long path scenarios.

github-actions · 2025-10-24T11:53:41Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

…nstall-only-updates

…ll-only-updates

This reverts commit 483a1f2.

github-actions · 2025-11-10T11:19:22Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

mattsu2020 added 9 commits October 23, 2025 22:48

refactor(uu/install): streamline Unix directory creation with nix

6460982

Reorganized Unix-specific imports and refactored create_dir_all_for_install to use nix crate's OwnedFd and BorrowedFd directly, simplifying file descriptor management and improving error handling for better efficiency.

refactor(tests): reorganize imports in test_install.rs

a8bab29

Remove duplicate uucore::libc import and reorder statements for cleaner code structure in install utility tests.

refactor: clean up imports in test_install.rs

d28c545

Removed duplicate `use std::path::Path;` import and unified `#[cfg(not(windows))]` to `#[cfg(unix)]` for `use uucore::libc;` to improve code consistency and target Unix-like systems more precisely.

mattsu2020 changed the title ~~Improve install directory creation~~ install: improve directory creation Oct 24, 2025