Skip to content

install: don't trim the root away when creating leading dirs - #13998

Open
MsfPablo wants to merge 1 commit into
uutils:mainfrom
MsfPablo:install-root-parent
Open

install: don't trim the root away when creating leading dirs#13998
MsfPablo wants to merge 1 commit into
uutils:mainfrom
MsfPablo:install-root-parent

Conversation

@MsfPablo

Copy link
Copy Markdown
Contributor

Fixes #13232.

For install -D src /dest, the parent to create is /. The trailing-separator loop stripped separators unconditionally, and since / is nothing but a separator it came out as an empty path. "".exists() is false, so install decided the parent was missing, tried to create it, and failed:

$ install -D -m755 file /file
install: /file: chmod failed with error No such file or directory (os error 2)
install: failed to chmod '/file'

The loop now stops while one separator is left. / stays /, which exists and is a directory, so the copy proceeds — matching the documented -D behaviour, where the only leading component of /file is a root that already exists.

I pulled the trimming into a trim_trailing_separators helper so the root boundary is unit-testable, and switched its byte conversion to uucore::os_str_from_bytes; the inline version used OsStrExt::from_bytes, which is behind a #[cfg(unix)] import in this file.

Verified against the reproducer: before, the chmod ENOENT above; after, install gets far enough to report the real reason for the destination being unwritable, and the source is left alone.

Tests: unit tests for the helper, covering /, //, ordinary trailing slashes, and the no-op cases. The end-to-end case needs a writable /, so I did not add an integration test for it — the existing 104 install tests still pass.

With -D and a single-component absolute destination, the parent to
create is /. The trailing-separator trimming ran that down to an empty
path, which exists() reports as absent, so install tried to create it,
failed with ENOENT, and left the destination unwritten.

Stop trimming at the root and move the logic into a helper so the
boundary can be tested directly.

Fixes uutils#13232
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/rm/isatty (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/cut/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/date/resolution (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/seq/seq-epipe is now being skipped but was previously passing.

@sylvestre

Copy link
Copy Markdown
Contributor

Please add a test in test_install.rs

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 -D file /file fails and loses the file when the destination's parent is /

2 participants