install: name the directory component that could not be created

[abendrothj]

install named the whole leading-directory path instead of the component that failed, and with -D gave no reason at all:

install -D f dangling/sub/f   cannot create directory 'dangling/sub'
install -d dangling/sub       cannot create directory 'dangling/sub': Already exists
GNU, both cases               cannot create directory 'dangling': File exists

create_dir_all_safe now returns the failing prefix along with its error, and the ancestor search walks up past files, symlink loops and unreadable directories instead of giving up with the whole path. A name that exists but can't be descended into reports EEXIST when it doesn't resolve and ENOTDIR when it resolves to a non-directory, which is what GNU prints. When the parent of the failing component can't be searched at all, that parent is named instead - the difference between r--/x, which GNU reports as r--, and r-x/x, which it reports as r-x/x. The message string gained a place for the errno.

install -d keeps creating directories path-based. I tried routing it through the same fd walk and it broke write-only directories: DirFd::open needs read permission, mkdir only needs write and execute, so install -d wx-dir/sub started failing where GNU succeeds. There's a regression test for that now. -D has had the same limitation since it moved to the fd walk, which is #14778; this PR does not change it.

Verified on Linux against GNU 9.10: -D messages match over 23 invocations covering symlinks, dangling symlinks, plain files, symlink loops, unreadable, read-only and write-only parents, trailing and doubled slashes. No GNU source was read.

Closes #14995

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[abendrothj]

Conflicts with #12715 in the two -d hunks. Whichever lands second resolves to with_umask(0, || create_dir_all_safe(&path_to_create, DEFAULT_MODE)) with show!(InstallError::CreateDirFailed(e.path, e.error)).

[github-actions]

GNU testsuite comparison:

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

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 · 3

Open (4)

• These newly added unit tests directly reference Unix-only items (libc::ENOTDIR / libc::EEXIST,… · New
• Using libc::access() here can mispredict what the process can actually do because access()… · New
• create_dir_all_safe is pub and its return type changed from io::Result<DirFd> to… · New
• This From<CreateDirError> for io::Error conversion silently discards the path context that the… · New

[codspeed]

Merging this PR will degrade performance by 17.84%

⚡ 1 improved benchmark
❌ 2 regressed benchmarks
✅ 390 untouched benchmarks
⏩ 54 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
❌	Simulation	three_39_bit_primes	346.9 ms	600.7 ms	-42.25%
❌	Simulation	five_38_bit_primes	1.8 s	2 s	-10.24%
⚡	Simulation	thirteen_39_bit_primes	9.4 s	8.8 s	+6.96%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing abendrothj:fix/install-create-dir-error-component (5867bbe) with main (cacdd8d)}

Footnotes

1. 54 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. ↩

[Copilot]

Copilot review overview

🔵 Needs a closer look

install -d still reports the full path for nested failures, and permission-sensitive naming cases lack regression coverage.

Review effort: Lite
Findings: 1 · 2

Open (3)

• These newly added unit tests directly reference Unix-only items (libc::ENOTDIR / libc::EEXIST,…
• create_dir_all_safe is pub and its return type changed from io::Result<DirFd> to…
• Using libc::access() here can mispredict what the process can actually do because access()…

Resolved since last review (1)

• This From<CreateDirError> for io::Error conversion silently discards the path context that the…

Previously missed (1)

In code that hasn't changed since last review

Report the failing path component for nested mkdir errors

src/​uu/​install/​src/​install.rs:517

install -d still passes the entire path_to_create as the error path, so a nested failure such as install -d dangling/sub (or regular/sub where regular is a file) will continue to report dangling/sub/regular/sub instead of the failing component. Keep the path-based mkdir behavior for write-only parents, but track the component that failed (or add a path-based helper that returns it) before constructing CreateDirFailed.

[Devel08]

Wouldn't that cause a TOCTOU?

[abendrothj]

It only decides which error gets printed. If the target is an existing non-directory, install stops with "Not a directory"; otherwise nothing is done with the result. If the path changes after the check, the directories are still created through create_dir_all_safe, which works through directory descriptors (mkdirat, openat with O_DIRECTORY) and refuses a file or dangling symlink at that name by itself. So a race can only change which message you get, to "cannot create directory". The check isn't new either: main already does the same exists()/is_dir() check on the -t target; this moves it after the trailing-slash trim.

[Copilot]

Copilot review overview

🟡 Changes recommended

A critical -t target-validation regression and a moderate safe-traversal issue remain unresolved.

Review effort: Lite
Findings: 2 · 2

Open (4)

• Validate -t target directories outside the -D block · New
• These newly added unit tests directly reference Unix-only items (libc::ENOTDIR / libc::EEXIST,…
• create_dir_all_safe is pub and its return type changed from io::Result<DirFd> to…
• Using libc::access() here can mispredict what the process can actually do because access()…

Previously missed (1)

In code that hasn't changed since last review

Add integration coverage for unsearchable parent error paths

src/​uucore/​src/​lib/​features/​safe_traversal.rs:630

The new blame branch is what implements the documented r--/x versus r-x/x distinction, but the added tests only cover a regular file, a dangling symlink, and the successful -d write-only case. Add an integration test for an unsearchable parent (and ideally the loop/read-only error cases described by the PR) so this error-path naming behavior cannot regress.

[Copilot]

Copilot review overview

🟡 Changes recommended

Fix -t target validation and guard chmod-dependent tests for WASI.

Review effort: Lite
Findings: 3 · 2

Open (5)

• Skip permission test on WASI · New
• Validate -t target directories outside the -D block
• These newly added unit tests directly reference Unix-only items (libc::ENOTDIR / libc::EEXIST,…
• create_dir_all_safe is pub and its return type changed from io::Result<DirFd> to…
• Using libc::access() here can mispredict what the process can actually do because access()…

[Copilot]

Copilot review overview

🔵 Needs a closer look

An outstanding documentation nit and mixed readiness signals warrant human review.

Review effort: Lite
Findings: 1

Open (1)

• Skip permission test on WASI

Resolved since last review (4)

• Validate -t target directories outside the -D block
• These newly added unit tests directly reference Unix-only items (libc::ENOTDIR / libc::EEXIST,…
• create_dir_all_safe is pub and its return type changed from io::Result<DirFd> to…
• Using libc::access() here can mispredict what the process can actually do because access()…

Previously missed (1)

In code that hasn't changed since last review

Remove GNU test path reference from PR description

tests/​by-util/​test_install.rs:2891

The PR description references the GNU test path tests/install/basic-1. Please remove that path reference and summarize the observed behavior instead; this repository must not include references to GNU source/test paths.