Thank you for your comments. Here are my responses: 1. Regarding running as root: my plan is indeed to run `chgrp` as root inside the chroot. Since the chroot environment isolates all operations to a temporary test directory, I believe it is safe. I considered `chroot --userspec=...`, but given that the entire filesystem is under our control, the root‑inside‑chroot approach should be acceptable.
2. About testing both `chown` and `chgrp`: I agree they are essentially the same. However, several coreutils commands support `--preserve-root` and `--no-preserve-root`, and in theory they all need testing. I think having separate tests for each command makes the coverage clearer and easier to maintain. 3. On the use of `command -v` and `cp --parents`: the source binary we copy is the one built during the current compilation (from `src/`), so we don't need to use `command -v` to locate it. The current approach directly uses that built binary. 4. For the `|| true` question: you're right – most of them are redundant. I have removed them in the updated patch. Please find the revised patch attached, which includes the removal of these unnecessary constructs. Let me know if you have any further questions. Note: I am not a native English speaker; the content above was translated with the help of AI, and I fully understad those. Thank you for your understanding. Thanks, Zuo Hongsheng 主 题：Re: bug#81502: [PATCH] tests: chgrp: test --preserve-root within a chroot env 日 期：2026年07月27日19:12 发件人：Pádraig Brady 收件人：81502,Pádraig Brady On 27/07/2026 04:03, zuohsh wrote: > * tests/local.mk: reference the new test. > * tests/chgrp/preserve-root.sh: the new file. > --- > +print_ver_ chgrp > +require_root_ OK so you're proposing this as root to support chroot, rather than have chgrp operate as root? In that case I think chroot --userspec=... would be safer. Also a general point is that chown and chgrp are essentially the same, so it would be good to test both. Actually we already have an existing https://github.com/coreutils/coreutils/blob/master/tests/chown/preserve -root.sh so it would be good to mention a quick summary of what extra this tests. > + > +# Create a safe chroot environment for testing > +chroot_dir="$PWD/chroot_test" > +mkdir -p "$chroot_dir"/{bin,lib,lib64,etc,dev,tmp} || framework_failure_ > +# Copy essential files for chroot to work > +cp /bin/sh "$chroot_dir/bin/" 2>/dev/null || \ > + cp /usr/bin/sh "$chroot_dir/bin/sh" 2>/dev/null || framework_failure_ > + > +# Copy required libraries if any > +ldd $chroot_dir/bin/sh 2>/dev/null | grep -o '/lib[^ ]*' | while read lib; do > + test -f "$lib" && cp "$lib" "$chroot_dir/lib/" 2>/dev/null || true > +done > + > +pwd > +# Copy chgrp binary to chroot > +cp ${srcdir}/src/chgrp "$chroot_dir/bin/chgrp" || framework_failure_ > + > +# Copy chgrp's required libraries > +ldd $chroot_dir/bin/chgrp | grep -o '/lib[^ ]*' | while read lib; do > + test -f "$lib" && cp "$lib" "$chroot_dir/lib/" 2>/dev/null || true > + test -f "$lib" && cp "$lib" "$chroot_dir/lib64/" 2>/dev/null || true > +done Note the chroot setup we use in https://github.com/coreutils/coreutils/blob/master/tests/nproc/nproc-qu ota.sh where we use `command -v ...` to identify the binary, and cp --parents to copy the dependencies. > + > +# Create minimal device nodes > +mknod "$chroot_dir/dev/null" c 1 3 2>/dev/null || true > +chmod 666 "$chroot_dir/dev/null" 2>/dev/null || true Why the || true here? > + > +# Test 1: --preserve-root should refuse recursive operation on / > +echo "=== Test 1: preserve-root on / ===" >&2 > +chroot "$chroot_dir" /bin/chgrp -R --preserve-root 1 / 2>out && fail=1 > +echo "=== Test 1 output ===" >&2 > +cat out >&2 > +echo "=== End Test 1 output ===" >&2 > +grep "it is dangerous to operate recursively.*'/'" out >/dev/null || fail=1 > + > +# Test 2: --no-preserve-root should allow the operation > +chroot "$chroot_dir" /bin/chgrp -R --no-preserve-root 1 / 2>out || true and here? thanks, Padraig ---
"0002-tests-chgrp-test-preserve-root-within-a-chroot-env.patch
Description: Binary data
