cp: copy attributes after making subdir by rm-dr · Pull Request #6884 · uutils/coreutils

rm-dr · 2024-11-23T23:14:29Z

Closes #6875.

TODO:

: Fix test_dir_perm_race_with_preserve_mode_and_ownership.
: Add a test for the original issue described in cp -a does not copy attributes (owner, rights) of directories #6875
: Fix gnu heap test

github-actions · 2024-11-24T00:21:15Z

GNU testsuite comparison:

GNU test failed: tests/cp/cp-HL. tests/cp/cp-HL is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/cp/cp-deref. tests/cp/cp-deref is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/cp/dir-slash. tests/cp/dir-slash is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/cp/fail-perm. tests/cp/fail-perm is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/cp/link-preserve. tests/cp/link-preserve is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/cp/parent-perm-race. tests/cp/parent-perm-race is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/cp/preserve-mode. tests/cp/preserve-mode is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/cp/symlink-slash. tests/cp/symlink-slash is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/mv/dup-source. tests/mv/dup-source is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
GNU test error: tests/cp/link-heap. tests/cp/link-heap is passing on 'main'. Maybe you have to rebase?

github-actions · 2024-11-24T09:59:24Z

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

github-actions · 2024-11-24T18:41:18Z

GNU testsuite comparison:

GNU test failed: tests/cp/link-heap. tests/cp/link-heap is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

sylvestre · 2024-11-25T17:21:03Z

src/uu/cp/src/copydir.rs

+fn build_dir(
+    path: &PathBuf,
+    recursive: bool,
+    options: &Options,


why changing the order?

This is the usual convention, no?
The "primary" argument is first, all options and etc come afterwards.

maybe but if you do, it should be in a sep commit :)

Ok, done. I've reorganized commits & force-pushed.

tests/by-util/test_cp.rs

github-actions · 2024-11-25T17:45:47Z

GNU testsuite comparison:

GNU test failed: tests/cp/link-heap. tests/cp/link-heap is passing on 'main'. Maybe you have to rebase?

github-actions · 2024-11-26T17:16:35Z

GNU testsuite comparison:

GNU test failed: tests/cp/link-heap. tests/cp/link-heap is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

github-actions · 2024-11-30T07:00:49Z

GNU testsuite comparison:

GNU test error: tests/cp/link-heap. tests/cp/link-heap is passing on 'main'. Maybe you have to rebase?

github-actions · 2024-12-01T10:28:01Z

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

github-actions · 2024-12-03T06:32:42Z

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

rm-dr · 2024-12-11T17:24:57Z

This pr should be ready to be merged.
Is there anything else I need to do?

sylvestre · 2024-12-11T17:26:55Z

Sorry, i looked if skip_last wasn't implemented elsewhere and got distracted

sylvestre · 2024-12-11T17:27:24Z

src/uu/cp/src/copydir.rs

+                // (Note that there can be more than one! We might step out of
+                // `./a/b/c` into `./a/`, in which case we'll need to fix the
+                // permissions of both `./a/b/c` and `./a/b`, in that order.)
+                if direntry.file_type().is_dir() {


would it make sense to move this into a function?
it is starting to be a bit long

hm, I don't think so? That code is tightly integrated with the rest of copy_directory, and most of the lines are comments.

github-actions · 2024-12-11T17:51:42Z

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

rm-dr · 2025-01-05T18:32:03Z

Hm, cp-i.sh from gnu tests is failing. Looks like it's the last two lines:

> touch c d
> cargo run -- cp -b --update=none c d
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.16s
     Running `/uutils/coreutils/target/debug/coreutils cp -b --update=none c d`
> cp -b --update=none c d
cp: --backup is mutually exclusive with -n or --update=none-fail
Try 'cp --help' for more information.

This manual check fails on main and on my branch (which is rebased on latest main).
I don't think this is related to the changes in this pr, and I'm not sure why CI isn't catching this everywhere else.

sylvestre · 2025-01-05T18:34:37Z

yes, cp-i fails https://uutils.github.io/coreutils/docs/test_coverage.html

rm-dr · 2025-01-05T18:35:36Z

Huh, I think it's only reason this PR hasn't been passing tests?
Shouldn't cp -i be ignored?
Let's see what happens this time.

rm-dr · 2025-01-05T19:10:10Z

Looks like it worked!

sylvestre · 2025-01-05T21:12:05Z

about cp-i.sh #7082
thanks for the investigation!

rm-dr marked this pull request as draft November 23, 2024 23:14

rm-dr changed the title ~~opy attributes after making subdir~~ Copy attributes after making subdir Nov 23, 2024

rm-dr force-pushed the 6875-cp-a branch 2 times, most recently from 898a027 to 6676cde Compare November 23, 2024 23:56

rm-dr force-pushed the 6875-cp-a branch from 6676cde to 8e98620 Compare November 24, 2024 01:30

rm-dr force-pushed the 6875-cp-a branch from 8e98620 to bb673a9 Compare November 24, 2024 17:09

rm-dr marked this pull request as ready for review November 24, 2024 17:09

rm-dr force-pushed the 6875-cp-a branch from bb673a9 to 3a4c4b7 Compare November 24, 2024 17:12

cakebaker changed the title ~~Copy attributes after making subdir~~ cp: copy attributes after making subdir Nov 25, 2024

rm-dr force-pushed the 6875-cp-a branch from 3a4c4b7 to 25090b0 Compare November 25, 2024 17:11

sylvestre reviewed Nov 25, 2024

View reviewed changes

tests/by-util/test_cp.rs Outdated Show resolved Hide resolved

rm-dr force-pushed the 6875-cp-a branch from 25090b0 to 34a10c6 Compare November 26, 2024 16:50

rm-dr marked this pull request as draft November 26, 2024 16:50

rm-dr force-pushed the 6875-cp-a branch from 34a10c6 to 8e1965f Compare November 30, 2024 06:22

rm-dr marked this pull request as ready for review November 30, 2024 06:23

rm-dr requested a review from sylvestre November 30, 2024 06:23

rm-dr force-pushed the 6875-cp-a branch 4 times, most recently from 82de5b9 to 62c33d4 Compare November 30, 2024 06:33

rm-dr force-pushed the 6875-cp-a branch from 62c33d4 to 63c100a Compare November 30, 2024 17:06

rm-dr force-pushed the 6875-cp-a branch from 63c100a to 1c62fbf Compare December 3, 2024 02:47

sylvestre force-pushed the 6875-cp-a branch from 1c62fbf to f54095f Compare December 4, 2024 13:05

rm-dr force-pushed the 6875-cp-a branch 2 times, most recently from 80ef881 to c8f7fdc Compare December 11, 2024 17:24

sylvestre reviewed Dec 11, 2024

View reviewed changes

rm-dr added 3 commits January 5, 2025 10:22

cp: add subdir permissions test

3306a06

cp: minor cleanup

9d6ae1c

cp: fix subdir permissions

aac753e

rm-dr force-pushed the 6875-cp-a branch from c8f7fdc to aac753e Compare January 5, 2025 18:23

sylvestre merged commit 49ae68d into uutils:main Jan 5, 2025
65 checks passed

rm-dr deleted the 6875-cp-a branch January 6, 2025 00:29

moonfruit mentioned this pull request Jan 20, 2025

uutils-selected 0.0.29 moonfruit/homebrew-tap#200

Closed

Uh oh!

Conversation

rm-dr commented Nov 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 24, 2024

Uh oh!

github-actions bot commented Nov 24, 2024

Uh oh!

github-actions bot commented Nov 24, 2024

Uh oh!

sylvestre Nov 25, 2024

Choose a reason for hiding this comment

Uh oh!

rm-dr Nov 26, 2024

Choose a reason for hiding this comment

Uh oh!

sylvestre Dec 2, 2024

Choose a reason for hiding this comment

Uh oh!

rm-dr Dec 3, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Nov 25, 2024

Uh oh!

github-actions bot commented Nov 26, 2024

Uh oh!

github-actions bot commented Nov 30, 2024

Uh oh!

github-actions bot commented Dec 1, 2024

Uh oh!

github-actions bot commented Dec 3, 2024

Uh oh!

rm-dr commented Dec 11, 2024

Uh oh!

sylvestre commented Dec 11, 2024

Uh oh!

sylvestre Dec 11, 2024

Choose a reason for hiding this comment

Uh oh!

rm-dr Jan 5, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 11, 2024

Uh oh!

rm-dr commented Jan 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sylvestre commented Jan 5, 2025

Uh oh!

rm-dr commented Jan 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rm-dr commented Jan 5, 2025

Uh oh!

sylvestre commented Jan 5, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rm-dr commented Nov 23, 2024 •

edited

Loading

rm-dr commented Jan 5, 2025 •

edited

Loading

rm-dr commented Jan 5, 2025 •

edited

Loading