Skip to content

install: don't reject existing non-regular targets as "invalid target"#13552

Open
Gooh456 wants to merge 1 commit into
uutils:mainfrom
Gooh456:fix-install-invalid-target-9934
Open

install: don't reject existing non-regular targets as "invalid target"#13552
Gooh456 wants to merge 1 commit into
uutils:mainfrom
Gooh456:fix-install-invalid-target-9934

Conversation

@Gooh456

@Gooh456 Gooh456 commented Jul 24, 2026

Copy link
Copy Markdown

standard()'s final branch only takes the copy path for target.is_file() || is_new_file_path(&target). Both are false for a target that already exists but isn't a regular file — a device node, FIFO, or socket — so it falls to InstallError::InvalidTarget, whose string is hardcoded as "No such file or directory". That's just wrong when the file is right there; install /dev/null /dev/full prints "invalid target '/dev/full': No such file or directory" instead of attempting the overwrite and reporting why it actually failed.

By the time we reach this check, is_potential_directory_path has already routed actual directories into copy_files_into_dir, so target.is_dir() is guaranteed false here — broadening the condition to target.exists() && !target.is_dir() just lets any existing non-directory target reach the same remove-then-copy path a regular file already takes, matching GNU's behavior of attempting the overwrite and surfacing the real errno.

Fixes #9934. Note: the removal-failure path this now falls into (InstallError::NotPermitted) is worded "Operation not permitted" rather than GNU's "Permission denied" for this specific EACCES case — that's a preexisting, separate string issue in that error arm and I left it alone rather than bundle an unrelated fix in here.

Didn't add a /dev/full-based test since that depends on /dev permissions that vary by environment. A more portable regression test would create a FIFO in a temp dir via the test harness and assert install overwrites it into a regular file instead of erroring — happy to add if wanted.

Signed-off-by: Kyue <164024549+Gooh456@users.noreply.github.com>
@Gooh456
Gooh456 force-pushed the fix-install-invalid-target-9934 branch from a3403a1 to c9320fd Compare July 24, 2026 14:04
@xtqqczze

Copy link
Copy Markdown
Contributor

@codspeed-hq

codspeed-hq Bot commented Jul 24, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 3.24%

❌ 1 regressed benchmark
✅ 338 untouched benchmarks
⏩ 46 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation uniq_check_chars[(10000, "1")] 14.4 ms 14.9 ms -3.24%

Tip

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


Comparing Gooh456:fix-install-invalid-target-9934 (c9320fd) with main (b6bba4a)

Open in CodSpeed

Footnotes

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

@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skipping an intermittent issue tests/cut/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/tail-n0f (passes in this run but fails in the 'main' branch)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

install /dev/null /dev/full has wrong error message

2 participants