Pádraig Brady wrote:
> `sudo cp -a non-root-file copy` would not preserve capabilities.
> The attached fixes this and passes all tests.
...
> Subject: [PATCH] cp: preserve "capabilities" when also preserving file
> ownership
>
> * src/copy.c (copy_reg): Copy xattrs _after_ setting file ownership
> so that capabilities are not cleared when setting ownership.
> * tests/cp/capability: A new root test.
> * tests/Makefile.am (root_tests): Reference the new test.
> * NEWS: Mention the fix.
Good catch!
The patch looks fine.
Some tiny suggestions:
> diff --git a/NEWS b/NEWS
...
> + cp now preserves "capabilities" when also preserving file ownership.
s/when also/also when/
> ls --color once again honors the 'NORMAL' dircolors directive.
> [bug introduced in coreutils-6.11]
>
> diff --git a/src/copy.c b/src/copy.c
> index 0fa148e..4e70c21 100644
> --- a/src/copy.c
> +++ b/src/copy.c
> @@ -826,6 +826,22 @@ copy_reg (char const *src_name, char const *dst_name,
> }
> }
>
> + /* We set ownership before xattrs as changing owners will
> + clear capabilities. */
Please use an active/imperative voice:
/* Set ownership before setting xattrs, since setting ownership
clears capabilities. */
> + if (x->preserve_ownership && ! SAME_OWNER_AND_GROUP (*src_sb, sb))
> + {
> + switch (set_owner (x, dst_name, dest_desc, src_sb, *new_dst, &sb))
...
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index db1610d..a943ff3 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -23,6 +23,7 @@ root_tests = \
> cp/preserve-gid \
> cp/special-bits \
> cp/cp-mv-enotsup-xattr \
> + cp/capability \
> dd/skip-seek-past-dev \
> install/install-C-root \
> ls/capability \
> diff --git a/tests/cp/capability b/tests/cp/capability
...
> +(setcap --help) 2>&1 |grep 'usage: setcap' > /dev/null \
> + || skip_test_ "setcap utility not found"
> +(getcap --help) 2>&1 |grep 'usage: getcap' > /dev/null \
> + || skip_test_ "getcap utility not found"
> +
> +# Don't let a different umask perturb the results.
> +umask 22
It's slightly better to use this function in place of the above two lines:
working_umask_or_skip_
> +touch file || framework_failure
> +chown $NON_ROOT_USERNAME file || framework_failure
...
