Skip to content

cp: better permissions mode handling with umask awareness#10904

Open
my4ng wants to merge 3 commits intouutils:mainfrom
my4ng:cp_mode
Open

cp: better permissions mode handling with umask awareness#10904
my4ng wants to merge 3 commits intouutils:mainfrom
my4ng:cp_mode

Conversation

@my4ng
Copy link
Contributor

@my4ng my4ng commented Feb 13, 2026

Fix #10787, #10862, Supersede #10859.

This PR reworks the permissions mode handling such that its behaviour is in line with GNU's implementation. Specifically:

  • set umask to !0o077, which means (1) cp can actually create and work with new directories/files which will have 700 permissions mode before their final calculated mode is set. the original umask is kept and passed around for calculation in the case of newly created and non-mode-preserving dir/files. umask is restored to original on exit.

  • for explicit non-mode-preserving, newly created dir/files, use 777 and 666 modes for calculation respectively.

  • for newly created dir/files, always set their permission mode to the correct one.

  • added 12 new tests for the combination of existing/not-existing, no-preserve/default/preserve, dir/file.

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/cp/cp-parents. tests/cp/cp-parents is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/factor/t34 is no longer failing!
Congrats! The gnu test tests/printf/printf-surprise is now passing!

@ChrisDryden
Copy link
Collaborator

Is there any way to split this PR up into something smaller? When its this big it gets pretty hard to review

@my4ng
Copy link
Contributor Author

my4ng commented Feb 13, 2026

I'm afraid not. Most changes are actually from the noise of propagating orig_umask, and the only significant changes being build_dir and handle_mode. Without either, it results in a dozen or more failed tests.

There is a GNU test regression: tests/cp/cp-parents, caused by the second to last test in that file due to created parent dir not having attributes set correctly. Will fix soon.

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/pr/bounded-memory. tests/pr/bounded-memory is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/tail/symlink (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/factor/t34 is no longer failing!
Note: The gnu test tests/tail/pipe-f is now being skipped but was previously passing.
Congrats! The gnu test tests/printf/printf-surprise is now passing!


#[test]
#[cfg(unix)]
fn test_cp_existing_no_preserve_file() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to take a closer look at the overall logic later, but do you think it would be possible to use some of the built in integ test utilities for doing chmod and for creating files to condese the amount of code here. Do you think it also would be possible to parametrize the tests a bit more too to make it smaller and easier to read?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by integ test utilities? I think it should be possible to condense the twelve into one with some for loops, but I am not sure how readable or easily usable it becomes when it fails, but I will try it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also another problem is that there would have to be cleanup in between the various test cases if condensed, which makes it even more complex. I think keep them as separate tests is the best way to go.

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/cut/bounded-memory. tests/cut/bounded-memory is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/misc/io-errors. tests/misc/io-errors is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/pr/bounded-memory. tests/pr/bounded-memory is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/factor/t10 is no longer failing!

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.

cp -a still fails with readonly directories (follow-up to #7961)

2 participants