Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cp: makes --preserve requires = #4928

Merged
merged 1 commit into from
Jun 4, 2023
Merged

cp: makes --preserve requires = #4928

merged 1 commit into from
Jun 4, 2023

Conversation

granquet
Copy link
Contributor

@granquet granquet commented Jun 1, 2023

prevents --preserve to eat the next argument when no value is passed.

default value for --preserve is set to mode,ownership,timestamps

before the patch:
cp --preserve foo bar
error: invalid value 'foo' for '--preserve [<ATTR_LIST>...]'
[possible values: mode, ownership, timestamps, context, link, links, xattr, all]

@granquet
Copy link
Contributor Author

granquet commented Jun 1, 2023

Fixes #4924

The issue mentions tests/cp/reflink-perm which is not present in main.
So it is untested with reflink-perm, any idea where to find that test?

@cakebaker cakebaker linked an issue Jun 1, 2023 that may be closed by this pull request
cp --preserve foo bar should be accepted #4924
Closed
@sylvestre
Copy link
Contributor

it is a GNU test, sorry

@sylvestre
Copy link
Contributor

could you please add a test to make sure we don't regress ?

@granquet
Copy link
Contributor Author

granquet commented Jun 1, 2023

could you please add a test to make sure we don't regress ?

Done.
Though I'm not fond of the name of the new "test" test_cp_preserve_no_args_before_opts

It's the same test as test_cp_preserve_no_args but with --preserve before the filenames... I could modify the existing test to test both things at once or keep it as a new test?

@sylvestre
Copy link
Contributor

the name is fine :)

@github-actions
Copy link

github-actions bot commented Jun 1, 2023

GNU testsuite comparison:

Congrats! The gnu test tests/cp/reflink-perm is no longer failing!
Congrats! The gnu test tests/rm/rm2 is no longer failing!
GNU test failed: tests/tail-2/inotify-dir-recreate. tests/tail-2/inotify-dir-recreate is passing on 'main'. Maybe you have to rebase?

@sylvestre
Copy link
Contributor

sweet:
Congrats! The gnu test tests/cp/reflink-perm is no longer failing!

@sylvestre
Copy link
Contributor

but it broke many windows tests. could you please have a look ?

@granquet
Copy link
Contributor Author

granquet commented Jun 2, 2023

but it broke many windows tests. could you please have a look ?

The broken tests related to 'preserve' on windows are because "ownership" is unix only and the previous version of the patch was setting ownership as part of the default missing values no matter the operating system.

Unfortunately I don't have access to a windows machine. I hope this fixes it.

@github-actions
Copy link

github-actions bot commented Jun 2, 2023

GNU testsuite comparison:

Congrats! The gnu test tests/cp/reflink-perm is no longer failing!
Congrats! The gnu test tests/rm/rm2 is no longer failing!

@github-actions
Copy link

github-actions bot commented Jun 3, 2023

GNU testsuite comparison:

Congrats! The gnu test tests/cp/reflink-perm is no longer failing!
GNU test failed: tests/tail-2/inotify-dir-recreate. tests/tail-2/inotify-dir-recreate is passing on 'main'. Maybe you have to rebase?

@granquet @sylvestre
cp: makes --preserve requires =
ee4691f 
prevents --preserve to eat the next argument when no value is passed.

default value for --preserve is set to mode,ownership(unix only),timestamps

before the patch:
cp --preserve foo bar
error: invalid value 'foo' for '--preserve [<ATTR_LIST>...]'
  [possible values: mode, ownership, timestamps, context, link, links, xattr, all]

Signed-off-by: Guillaume Ranquet <[email protected]>
@github-actions
Copy link

github-actions bot commented Jun 4, 2023

GNU testsuite comparison:

Congrats! The gnu test tests/cp/reflink-perm is no longer failing!
GNU test failed: tests/tail-2/inotify-dir-recreate. tests/tail-2/inotify-dir-recreate is passing on 'main'. Maybe you have to rebase?

@sylvestre sylvestre merged commit 99fa504 into uutils:main Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cp --preserve foo bar should be accepted
2 participants
@granquet @sylvestre