[PATCH 0/3] erofs-utils: tests: cover the canonical xattr order

[PATCH 0/3] erofs-utils: tests: cover the canonical xattr order

[Martin Pitt]

A test for b260119b8edb ("erofs-utils: mkfs: emit an inode's xattrs in a
canonical order"), plus two portability fixes that I ran into along
the way. Based on experimental-tests (a3b00c4).

The test stages the same tree twice with the same attributes set in
opposite order and compares the two images, so it needs neither root nor a
mount. It is red without b260119b8edb and green with it. That commit is
currently only in experimental, so the test fails as long as
experimental-tests is run without it.

Martin Pitt (3):
  erofs-utils: tests: fix broken loop in POSIX shells
  erofs-utils: tests: look up programs with command -v
  erofs-utils: tests: check that the xattr layout is order-independent

 tests/Makefile.am   |  5 ++-
 tests/common/rc     |  8 ++---
 tests/erofs/021     |  2 +-
 tests/erofs/028     |  6 ++--
 tests/erofs/032     | 77 +++++++++++++++++++++++++++++++++++++++++++++
 tests/erofs/032.out |  2 ++
 6 files changed, 91 insertions(+), 9 deletions(-)
 create mode 100755 tests/erofs/032
 create mode 100644 tests/erofs/032.out

-- 
2.55.0

[PATCH 1/3] erofs-utils: tests: fix broken loop in POSIX shells

[Martin Pitt]

`{1..n}` is a bashism. A POSIX shell iterates once over that literal
string, so the test silently sets a single attribute instead of three and
nothing crosses the block boundary any more. Sadly, `seq` is also not
POSIX (although ubiquitous).

Spell out the loop.

Signed-off-by: Martin Pitt <martin@amutable.com>
---
 tests/erofs/021 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/erofs/021 b/tests/erofs/021
index d36aa56..b77a8d7 100755
--- a/tests/erofs/021
+++ b/tests/erofs/021
@@ -39,7 +39,7 @@ mkdir -p $localdir
 # cross the block boundary; besides set three xattrs to ensure at least
 # one xattr name crosses the block boundary
 touch $localdir/file1
-for i in {1..3}; do
+for i in 1 2 3; do
 	setfattr -n user.p$(_random 249) -v $(_random 512) $localdir/file1 \
 		|| _notrun "no space for xattrs"
 done
-- 
2.55.0

[PATCH 2/3] erofs-utils: tests: look up programs with command -v

[Martin Pitt]

`which` stopped being installed by default in recent distros, and it is
not needed at all. Use the POSIX `command -v` instead.

Drop a doubled "unless" from the POSIX compliant note.

Signed-off-by: Martin Pitt <martin@amutable.com>
---
 tests/Makefile.am | 2 +-
 tests/common/rc   | 8 ++++----
 tests/erofs/028   | 6 +++---
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2f109b8..c0291ac 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -137,7 +137,7 @@ TESTS += erofs/031
 
 # NOTE: When adding a new test case or updating an existing one, please write
 # POSIX-compliant shell scripts and avoid using Linux-specific features unless
-# unless the test case is expected to run only on Linux platforms.
+# the test case is expected to run only on Linux platforms.
 
 EXTRA_DIST = common/rc erofs
 
diff --git a/tests/common/rc b/tests/common/rc
index 78e704d..aa7ef99 100644
--- a/tests/common/rc
+++ b/tests/common/rc
@@ -48,10 +48,10 @@ _fatal()
     exit 1
 }
 
-export MOUNT_PROG="$(which mount)"
+export MOUNT_PROG="$(command -v mount)"
 [ "$MOUNT_PROG" = "" ] && _fatal "mount not found"
 
-export UMOUNT_PROG="$(which umount)"
+export UMOUNT_PROG="$(command -v umount)"
 [ "$UMOUNT_PROG" = "" ] && _fatal "umount not found"
 [ "$MKFS_EROFS_PROG" = "" ] && _fatal "mkfs.erofs not found"
 
@@ -94,7 +94,7 @@ _require_root()
 
 _require_xattr()
 {
-	which setfattr >/dev/null 2>&1 ||
+	command -v setfattr >/dev/null 2>&1 ||
 		_notrun "attr isn't installed, skipped."
 }
 
@@ -309,7 +309,7 @@ _do_unmount()
 {
 	local mnt_path=$1
 
-	if [ "$FSTYP" = "erofsfuse" -a "$(which fusermount > /dev/null && echo ok)" = "ok" ]; then
+	if [ "$FSTYP" = "erofsfuse" ] && command -v fusermount > /dev/null 2>&1; then
 		fusermount -u $mnt_path
 	else
 		$UMOUNT_PROG $mnt_path
diff --git a/tests/erofs/028 b/tests/erofs/028
index bf31c3f..05d9a5e 100755
--- a/tests/erofs/028
+++ b/tests/erofs/028
@@ -42,9 +42,9 @@ check_ishare()
 _require_erofs
 _require_erofs_inode_sharing
 _require_erofs_compression "-zlz4"
-which strace > /dev/null 2>&1 || _notrun "strace is not found"
-which fincore > /dev/null 2>&1 || _notrun "fincore is not found"
-which fadvise > /dev/null 2>&1 || _notrun "fadvise is not found"
+command -v strace > /dev/null 2>&1 || _notrun "strace is not found"
+command -v fincore > /dev/null 2>&1 || _notrun "fincore is not found"
+command -v fadvise > /dev/null 2>&1 || _notrun "fadvise is not found"
 
 # remove previous $seqres.full before test
 rm -f $seqres.full
-- 
2.55.0

[PATCH 3/3] erofs-utils: tests: check that the xattr layout is order-independent

[Martin Pitt]

b260119b8edb ("erofs-utils: mkfs: emit an inode's xattrs in a canonical
order") made the layout independent of the order listxattr(2) reports an
inode's attributes in. Cover it: stage the same tree twice with the same
attributes set in opposite order, and compare the two images. The names
differ in length and share prefixes, so a subtly wrong comparison cannot
order them correctly by accident.

Signed-off-by: Martin Pitt <martin@amutable.com>
---
 tests/Makefile.am   |  3 ++
 tests/erofs/032     | 77 +++++++++++++++++++++++++++++++++++++++++++++
 tests/erofs/032.out |  2 ++
 3 files changed, 82 insertions(+)
 create mode 100755 tests/erofs/032
 create mode 100644 tests/erofs/032.out

diff --git a/tests/Makefile.am b/tests/Makefile.am
index c0291ac..363b93d 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -132,6 +132,9 @@ TESTS += erofs/030
 # 031 - test chunk-based mapping with shared chunks across inodes
 TESTS += erofs/031
 
+# 032 - check that the xattr layout does not depend on listxattr(2) order
+TESTS += erofs/032
+
 # NEW TEST CASE HERE
 # TESTS += erofs/999
 
diff --git a/tests/erofs/032 b/tests/erofs/032
new file mode 100755
index 0000000..063ed2c
--- /dev/null
+++ b/tests/erofs/032
@@ -0,0 +1,77 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Check that the xattr layout does not depend on listxattr(2) order
+#
+# listxattr(2) makes no promise about the order it reports, and filesystems
+# disagree: tmpfs reports the order the attributes were set in, while ext4 and
+# btrfs report their own on-disk order. The same set of attributes must end up
+# in the same layout however mkfs.erofs is handed them, otherwise images are
+# not reproducible. The names also differ in length and share prefixes, which
+# exercises the length tiebreak.
+#
+# On a filesystem that reports its own order rather than the insertion order
+# this check cannot fail, so it needs $tmp on a filesystem that keeps insertion
+# order or randomizes it, like tmpfs, which /tmp normally is.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$(echo $0 | awk '{print $((NF-1))"/"$NF}' FS="/")
+
+# get standard environment, filters and checks
+. "${srcdir}/common/rc"
+
+cleanup()
+{
+	cd /
+	rm -rf $tmp.*
+}
+
+_require_xattr
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+echo "QA output created by $seq"
+
+attrs="user.aa=1 user.mm=22 user.mm_long=333 user.zz=4444
+user.a_long_attribute_name=55555 user.growfs=666666 user.roothash=7777777
+user.verity=88888888 user.gpt_label=999999999 user.gpt_type_uuid=aaaaaaaaaa"
+
+localdir=$tmp/$seq
+rm -rf $localdir
+
+# same tree and same attributes twice, set in opposite order
+for pass in forward reverse; do
+	tree=$localdir/$pass
+	mkdir -p $tree/dir
+	echo payload > $tree/dir/file
+
+	list=$attrs
+	if [ $pass = reverse ]; then
+		list=
+		for attr in $attrs; do
+			list="$attr $list"
+		done
+	fi
+	for attr in $list; do
+		setfattr -n "${attr%%=*}" -v "${attr#*=}" $tree ||
+			_fail "failed to set ${attr%%=*}"
+	done
+
+	# pin the timestamps and the filesystem UUID, so the attribute order
+	# is the only thing that can differ
+	$MKFS_EROFS_PROG -T1739577600 -U 5230d7cf-f2ce-43ed-9ae2-39e7e2fe48ca \
+		$localdir/$pass.img $tree >> $seqres.full 2>&1 ||
+		_fail "failed to mkfs"
+done
+
+$FSCK_EROFS_PROG --extract $localdir/forward.img >> $seqres.full 2>&1 ||
+	_fail "fsck failed"
+
+cmp -s $localdir/forward.img $localdir/reverse.img ||
+	_fail "the image depends on the order listxattr(2) reported"
+
+echo Silence is golden
+status=0
+exit 0
diff --git a/tests/erofs/032.out b/tests/erofs/032.out
new file mode 100644
index 0000000..34e059f
--- /dev/null
+++ b/tests/erofs/032.out
@@ -0,0 +1,2 @@
+QA output created by 032
+Silence is golden
-- 
2.55.0