Re: [PATCH] generic/683: _require_chmod

On Thu, Aug 20, 2026 at 09:57:56AM -0500, Eric Sandeen wrote:
> On 8/20/26 9:37 AM, Zorro Lang wrote:
> > On Wed, Aug 19, 2026 at 03:32:01PM -0500, Eric Sandeen wrote:
> >> generic/683 uses chmod but does not _require_chmod, so filesystems
> >> like exfat fail.
> >>
> >> Signed-off-by: Eric Sandeen <[email protected]>
> >> ---
> > 
> > Hi Eric,
> > 
> > So great to see you ! This patch makes sense to me. Actually, g/683 belongs
> > to the same group/patchset of SUID test cases as g/684 through g/688, and all
> > of them use chmod. If we need to add _require_chmod to g/683, I would suggest
> > adding it to all of them.
> 
> Ok, fair enough - I should have looked more closely, and when running exfat
> all of those tests properly did a _notrun due to /other/ constraints, but
> we probably should explicitly mark each requirement even if they are
> stacked.

Maybe some of the other _require_ checks helped those tests to _notrun on exfat.

> 
> I wonder if some sort of checkpatch or linter for xfstests might be
> helpful, it's pretty simple to grep a test for chmod and ensure that
> it has _require_chmod for example.

Sure, I just tried this:

  #!/bin/bash

  TESTS_DIR="${1:-tests}"

  find "$TESTS_DIR" -type f -regex '.*/[0-9]+$' | sort | while read -r test_file; do
        if grep -qE '^[[:space:]]*_require_chmod\b' "$test_file"; then
                continue
        fi

        if grep -v '^[[:space:]]*#' "$test_file" | grep -qE '\b(chmod|\$CHMOD_PROG)\b'; then
                echo "[Miss _require_chmod] $test_file"
                grep -v '^[[:space:]]*#' "$test_file" | grep -nE '\b(chmod|\$CHMOD_PROG)\b' | sed 's/^/    Line /'
        fi
  done

Not sure if we should blindly add _require_chmod to every single case we found.

>
> ... or I wonder if it would be runtime-prohibitive to just build that
> into the check script; if the script contains "chmod," run _require_chmod
> automatically... similar for all the other trivial _require tests.
> 
> But maybe that's trying to be too clever. :)

xfstests contains thousands of test cases, so this approach would introduce
substantial redundant overhead and frustrate a lot of developers :)

Thanks,
Zorro

> 
> -Eric
> 
> > Thanks,
> > Zorro
> > 
> >>
> >> diff --git a/tests/generic/683 b/tests/generic/683
> >> index 883905da..226d2758 100755
> >> --- a/tests/generic/683
> >> +++ b/tests/generic/683
> >> @@ -24,6 +24,7 @@ _cleanup()
> >>  # Modify as appropriate.
> >>  _require_user
> >>  _require_test
> >> +_require_chmod
> >>  verb=falloc
> >>  _require_xfs_io_command $verb
> >>  _require_congruent_file_oplen $TEST_DIR 65536
> >>
> 
>