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': File 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.

Checked on Linux against GNU 9.12 built from source: -D messages match in 19 of 24 cases covering symlinks, dangling symlinks, plain files, symlink loops, unreadable, read-only and write-only parents, and doubled slashes. The five that differ also differ on main and are not changed here: trailing-slash targets (-D f regular/sub/, -D f dangling/sub/), an unreadable existing parent (-D f r--/f), and two -D -t cases covered by #14999. 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:

GNU test failed: tests/id/setgid. tests/id/setgid is passing on 'main'. Maybe you have to rebase?
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 2.29%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 8 improved benchmarks
❌ 16 regressed benchmarks
✅ 222 untouched benchmarks
⏩ 207 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
❌	Simulation	split_lines	8.5 ms	11.1 ms	-23.5%
❌	Simulation	split_numeric_suffix	8.7 ms	11.4 ms	-23.08%
❌	Simulation	false_consecutive_calls	294.7 ns	350.2 ns	-15.86%
❌	Simulation	check_sorted_utf8_locale	422.9 ms	492.2 ms	-14.09%
❌	Simulation	sort_numeric_utf8_locale	36.1 ms	40.7 ms	-11.41%
❌	Simulation	merge_single_file_utf8_locale	119.3 ms	133.4 ms	-10.59%
❌	Simulation	tsort_complex_dag[50000]	87.6 ms	97.1 ms	-9.79%
❌	Simulation	tsort_tree_dag[(10, 3)]	36.5 ms	39.4 ms	-7.33%
❌	Simulation	tsort_wide_dag[100000]	157.6 ms	167 ms	-5.65%
❌	Simulation	merge_pre_sorted_files	238.7 ms	252.6 ms	-5.53%
❌	Simulation	merge_pre_sorted_files_utf8_locale	237.9 ms	251.8 ms	-5.53%
❌	Simulation	tsort_linear_chain[1000000]	1.9 s	2 s	-4.75%
❌	Simulation	five_38_bit_primes	1.5 s	1.6 s	-4.43%
❌	Simulation	sort_german_de_locale	282.1 ms	295 ms	-4.4%
❌	Simulation	sort_case_sensitive[500000]	321 ms	334.9 ms	-4.14%
❌	Simulation	sort_spill_to_tmp_files_utf8_locale	321 ms	332.1 ms	-3.34%
⚡	Simulation	cut_fields_custom_delim	65.9 ms	51.9 ms	+27%
⚡	Simulation	cut_fields_tab	57.4 ms	45.5 ms	+26.23%
⚡	Simulation	three_39_bit_primes	901.6 ms	730.4 ms	+23.44%
⚡	Simulation	cut_bytes	18.1 ms	15.5 ms	+16.69%
...	...	...	...	...	...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

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 (e24d398) with main (fe26d56)}

Footnotes

1. 207 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.

[abendrothj]

Rechecked against GNU 9.12 built from the release tarball (9.10 behaves the same): this branch matches GNU in 19 of 24 -D cases. The five that differ are trailing-slash targets (-D f regular/sub/, -D f dangling/sub/), an unreadable existing parent (-D f r--/f), and two -D -t cases that #14999 covers. None of them match on main either. I've corrected the description.

[abendrothj]

Correction: with the current #12715 this is one hunk in directory(), and with_umask must not be nested there, since install already runs under it. Keep #12715's DirBuilder with DEFAULT_MODE and take this PR's error: show!(InstallError::CreateDirFailed(failed_create_dir_prefix(&path_to_create), e)), keeping the #[cfg(unix)]/#[cfg(not(unix))] split for failed.

[Copilot]

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