bug#25378: cp does not preserve SElinx context of sub folder

[Bernhard Voelker]

Hi Padraig,

only some minor remarks from my side.

On 02/21/2017 04:11 AM, Pádraig Brady wrote:
> From d17ca013f3aadcf697663830fa9ec34cba551f66 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?P=C3=A1draig=20Brady?= <address@hidden>
> Date: Mon, 20 Feb 2017 18:46:49 -0800
> Subject: [PATCH] cp: set SElinux context for --parents directories
> 
> * src/copy.h (set_process_security_ctx, set_file_security_ctx):
_____________^

minor typo: s/c/h/

> diff --git a/src/cp.c b/src/cp.c
> index 88db3a3..6e18263 100644
> --- a/src/cp.c
> +++ b/src/cp.c
> @@ -394,6 +394,8 @@ make_dir_parents_private (char const *const_dir, size_t 
> src_offset,
>  
>    *attr_list = NULL;
>  
> +  /* XXX: If all dirs present at the destination,
________^^^_____________^

why "XXX"?  Missing verb "are".


> diff --git a/tests/cp/cp-a-selinux.sh b/tests/cp/cp-a-selinux.sh
> index 19a9e64..de07406 100755
> --- a/tests/cp/cp-a-selinux.sh
> +++ b/tests/cp/cp-a-selinux.sh
> @@ -48,7 +48,6 @@ rm -f f
>  # due to recursion, and was handled incorrectly in coreutils-8.22
>  # Note standard permissions are updated for existing directories
>  # in the destination, so SELinux contexts should be updated too.
> -chmod o+rw restore/existing_dir

What was that change for?
(I don't have a SELinux system here, so the test is skipped here.)

>  mkdir -p backup/existing_dir/ || framework_failure_
>  ls -Zd backup/existing_dir > ed_ctx || fail=1
>  grep $ctx ed_ctx && framework_failure_
> @@ -57,11 +56,31 @@ chcon $ctx backup/existing_dir/file || framework_failure_
>  # Set the dir context to ensure it is reset
>  mkdir -p --context="$ctx" restore/existing_dir || framework_failure_
>  # Copy and ensure existing directories updated
> -cp -a backup/. restore/
> +cp -a backup/. restore/ || fail=1

nice catch, thanks!

>  ls -Zd restore/existing_dir > ed_ctx || fail=1
>  grep $ctx ed_ctx &&
>    { ls -lZd restore/existing_dir; fail=1; }
>  
> +# Check context preserved with directories created with --parents,
> +# which was not handled before coreutils-8.27

Do we already know the next release will be "8.27"?
Maybe it's better to change to "<= coreutils-8.26".

Thanks & have a nice day,
Berny