On 15/07/2026 22:56, Mateusz Nosek wrote:
Hi Pádraig,
Good point, thanks. v2 leaves setdefaultfilecon() inside
change_attributes(). Updated patch attached.
I began to wonder if need_copy() should also do a more rigorous check
for the "selinux_enabled && use_default_selinux_context" case, to see
if the destination already has the default context. But that's
slightly out of scope of this patch, so just FYI.
Thanks,
Mateusz
On Wed, Jul 15, 2026 at 1:18 PM Pádraig Brady <[email protected]> wrote:
On 14/07/2026 23:30, Mateusz Nosek wrote:
Hi,
I attach a patch for the "install" utility: "install -C" was still
calling chown/chmod on the destination even when need_copy() had
already determined it matches (mode, owner, group, content), which
could needlessly fail, e.g. on an immutable file.
This is my first patch to coreutils, so apologies for any mistakes in
the submission process -- happy to fix anything you point out.
This makes sense I think.
You moved setdefaultfilecon() out of this consideration.
I'd be inclined to leave it where it was and treat the same way,
because the comparison also compares selinux context,
and also if the file is already present it probably has an appropriate context
anyway.
I've pushed that with a NEWS entry.
I also pushed the attached as a follow up
to make cleanup of chattr +i files in tests more robust.
Marking this as done.
thanks!
Padraig
From dbed1698350ba1ec06505e35f37265f9f0c69dc0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Thu, 16 Jul 2026 12:14:59 +0100
Subject: [PATCH] tests: use more robust cleanup of immutable files
* tests/rm/empty-immutable-skip.sh: Use cleanup_() to
remove the immutable attribute, so that false failures
or stale files do not result from e.g. a killed test run.
---
tests/install/install-C-immutable.sh | 4 ++--
tests/rm/empty-immutable-skip.sh | 3 ++-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/tests/install/install-C-immutable.sh b/tests/install/install-C-immutable.sh
index 3b0cbf4d2..0bdb97997 100755
--- a/tests/install/install-C-immutable.sh
+++ b/tests/install/install-C-immutable.sh
@@ -43,6 +43,8 @@ g1=1
echo test > a || framework_failure_
+cleanup_() { chattr -i b; }
+
# Install once to create the destination, then make it immutable.
ginstall -Cv -m0644 -o$u1 -g$g1 a b || framework_failure_
chattr +i b || framework_failure_
@@ -53,6 +55,4 @@ chattr +i b || framework_failure_
# it would actually change.
ginstall -Cv -m0644 -o$u1 -g$g1 a b || fail=1
-chattr -i b
-
Exit $fail
diff --git a/tests/rm/empty-immutable-skip.sh b/tests/rm/empty-immutable-skip.sh
index 5b8ea6606..a70d61967 100755
--- a/tests/rm/empty-immutable-skip.sh
+++ b/tests/rm/empty-immutable-skip.sh
@@ -36,11 +36,12 @@ if test $chattr_i_works = 0; then
skip_ "chattr +i doesn't work on this file system"
fi
+cleanup_() { chattr -i empty; }
+
mkdir empty || framework_failure_
touch x y || framework_failure_
chattr +i empty || framework_failure_
rm -rf empty x y
{ test -f x || test -f y; } && fail=1
-chattr -i empty
Exit $fail
--
2.55.0
