install: don't reject existing non-regular targets as "invalid target"#13552
Open
Gooh456 wants to merge 1 commit into
Open
install: don't reject existing non-regular targets as "invalid target"#13552Gooh456 wants to merge 1 commit into
Gooh456 wants to merge 1 commit into
Conversation
Signed-off-by: Kyue <164024549+Gooh456@users.noreply.github.com>
Gooh456
force-pushed
the
fix-install-invalid-target-9934
branch
from
July 24, 2026 14:04
a3403a1 to
c9320fd
Compare
Contributor
Merging this PR will degrade performance by 3.24%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
|
GNU testsuite comparison: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
standard()'s final branch only takes the copy path fortarget.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 toInstallError::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/fullprints "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_pathhas already routed actual directories intocopy_files_into_dir, sotarget.is_dir()is guaranteed false here — broadening the condition totarget.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/devpermissions that vary by environment. A more portable regression test would create a FIFO in a temp dir via the test harness and assertinstalloverwrites it into a regular file instead of erroring — happy to add if wanted.