Skip to contentSkip to conversation

xfstests-bld: get-all improvements and adding keyctl

PATCH In review Eric Biggers 2 participants 9 messages May 25, 2017 – May 27, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover xfstests-bld: get-all improvements and adding keyctl

From: Eric Biggers <[email protected]>

Patches 1-6 make some improvements to how the xfstests-bld system handles external git repositories --- especially, making sure the build system recognizes changes made to the config file.

Patch 7 adds support for optionally including keyctl (used by the filesystem encryption tests) in the xfstests tarball built by xfstests-bld. As noted, this usually isn't needed, but it can be useful if the tarball is used "on its own", without being contained in a dedicated test appliance.

Eric Biggers (7):
get-all: use helper function to clone repositories
get-all: check out correct commits in already-cloned repositories
get-all: fail if optional repositories have been deconfigured
Makefile: always run get-all
do-all, get-all: run with 'set -e'
Makefile: check whether xfsprogs-dev exists before cleaning it
xfstests-bld: optionally build keyctl

 .gitignore                         |   1 +
 Documentation/building-xfstests.md |  36 ++++----
 Makefile                           |  24 ++---
 build-all                          |  28 +++++-
 config                             |   5 ++
 do-all                             |   2 +
 get-all                            | 173 +++++++++++++++++++++----------------
 7 files changed, 164 insertions(+), 105 deletions(-)
Signature · 1 line
2.13.0.219.gdb65acc882-goog
#1 lore raw

PATCH 1/7 get-all: use helper function to clone repositories

From: Eric Biggers <[email protected]>

Add a function which handles cloning a repository and optionally checking out a specific commit, then use it for all the repositories.

---
 get-all | 119 ++++++++++++++++++++++------------------------------------------
 1 file changed, 40 insertions(+), 79 deletions(-)
get-all +40 -79
index fc4dc2d..67a54b4 100755
@@ -6,88 +6,49 @@ else
 	. config
 fi
 
-if test ! -d xfsprogs-dev
-then
-	if test -z "$XFSPROGS_GIT"; then
-		echo "XFSPROGS_GIT not set; check your config file!"
-		exit 1
-        fi
-	if ! git clone $XFSPROGS_GIT xfsprogs-dev; then
-		echo "Failed to get xfsprogs-dev from $XFSPROGS_GIT"
-                exit 1
-	fi
-	if test -n "$XFSPROGS_COMMIT"; then
-		cd xfsprogs-dev
-		git branch xfstests-bld $XFSPROGS_COMMIT
-		git checkout xfstests-bld
-		cd ..
-	fi
-fi
-if test ! -d xfstests-dev
-then
-	if test -z "$XFSTESTS_GIT"; then
-		echo "XFSTESTS_GIT not set; check your config file!"
-		exit 1
-        fi
-	if ! git clone $XFSTESTS_GIT xfstests-dev; then
-		echo "Failed to get xfstests-dev from $XFSTESTS_GIT"
-                exit 1
-	fi
-	if test -n "$XFSTESTS_COMMIT"; then
-		cd xfstests-dev
-		git branch xfstests-bld $XFSTESTS_COMMIT
-		git checkout xfstests-bld
-		cd ..
-	fi
-fi
-if test ! -d fio
-then
-	if test -z "$FIO_GIT"; then
-		echo "FIO_GIT not set; check your config file!"
-		exit 1
-        fi
-	if ! git clone $FIO_GIT fio; then
-		echo "Failed to get fio from $FIO_GIT"
-                exit 1
-	fi
-	if test -n "$FIO_COMMIT"; then
-		cd fio
-		git branch xfstests-bld $FIO_COMMIT
-		git checkout xfstests-bld
-		cd ..
-	fi
-fi
-if test ! -d quota
-then
-	if test -z "$QUOTA_GIT"; then
-		echo "QUOTA_GIT not set; check your config file!"
-		exit 1
-        fi
-	if ! git clone $QUOTA_GIT quota; then
-		echo "Failed to get quota from $QUOTA_GIT"
-                exit 1
-	fi
-	if test -n "$QUOTA_COMMIT"; then
-		cd quota
-		git branch xfstests-bld $QUOTA_COMMIT
-		git checkout xfstests-bld
-		cd ..
-	fi
-fi
+setup_repo()
+{
+    local repo_name="$1"
+    local repo_url_variable="$2"
+    local repo_url="${!2}"
+    local commit_variable="$3"
+    local commit="${!3}"
+    local required="$4"
 
-if test ! -d stress-ng -a -n "$STRESS_NG_GIT"
-then
-	if ! git clone $STRESS_NG_GIT stress-ng; then
-		echo "Failed to get stress-ng from $STRESS_NG_GIT"
-                exit 1
+    # Clone the repository if needed.
+    if [ ! -d "$repo_name" ]; then
+	if [ -z "$repo_url" ]; then
+	    if ! $required; then
+		return
+	    fi
+	    echo 1>&2 "$repo_url_variable not set; check your config file!"
+	    exit 1
 	fi
-	if test -n "$STRESS_NG_COMMIT"; then
-		cd stress-ng
-		git branch xfstests-bld $STRESS_NG_COMMIT
-		git checkout xfstests-bld
-		cd ..
+
+	echo
+	if ! git clone "$repo_url" "$repo_name"; then
+	    echo 1>&2 "Failed to clone $repo_name from $repo_url"
+	    exit 1
 	fi
-fi
+
+	# If a specific commit was specified, check it out.
+	if [ -n "$commit" ]; then
+	    ( cd "$repo_name";
+	      git branch xfstests-bld "$commit";
+	      git checkout xfstests-bld;
+	    )
+	fi
+    fi
+}
+
+# required repositories
+setup_repo fio			FIO_GIT		FIO_COMMIT		true
+setup_repo quota		QUOTA_GIT	QUOTA_COMMIT		true
+setup_repo xfsprogs-dev		XFSPROGS_GIT	XFSPROGS_COMMIT		true
+setup_repo xfstests-dev		XFSTESTS_GIT	XFSTESTS_COMMIT		true
+
+# optional repositories
+setup_repo stress-ng		STRESS_NG_GIT	STRESS_NG_COMMIT	false
 
 # Make sure acl doesn't try regenerate these files because of the
 # vagrancies of the timestamps when they were checked out
Signature · 1 line
2.13.0.219.gdb65acc882-goog
#2 lore raw

PATCH 2/7 get-all: check out correct commits in already-cloned repositories

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Update the get-all script to check out the requested commit in each git repository, even if the repository has already been cloned. Previously, the repositories would become outdated if the config file was changed.

It was also possible for local changes (even uncommitted changes) to be included in the build --- maybe intentionally, maybe unintentionally. But there is a clear way to express the difference now: just comment out ${REPO_NAME}_COMMIT in config.custom if it's desired to manage a repository manually and do the builds from whatever happens to be in the working tree rather than from a specific commit.

---
 Documentation/building-xfstests.md | 32 +++++++++++---------
 get-all                            | 62 +++++++++++++++++++++++++++++++++-----
 2 files changed, 72 insertions(+), 22 deletions(-)
Documentation/building-xfstests.md +17 -15
index 3697838..4ec3042 100644
@@ -14,21 +14,23 @@ The xfstests-bld package depends on a number of external git trees:
 * fio
 * quota
 
-The location of these files are specified in the top-level config
-file, but you can copy the config file to config.custom and then make
-changes if desired.
-
-The first time you run "make", the scripts will automatically fetch
-these git trees from the locations specified in the top-level config
-file.  You can also manually run the "get-all" script which will
-actually do the dirty deed.
-
-There may be updates in some or any of these git trees for these
-subcomponents.  You can use "git pull" or "git fetch" as necessary to
-update them.  (Please take care before updating the fio repository;
-some updates to the fio tree have caused test regressions in the past,
-so it may be preferable to let things be as far as the fio repo is
-concerned.)
+The first time you run "make", the build system will clone these
+repositories by running ./get-all.  Their remote URLs are set in the
+top-level "config" file.  If you wish to make changes, copy "config"
+to "config.custom" and make changes there.
+
+The config file can also specify the commit to use for each
+repository.  If a commit is specified, the build system will check it
+out after cloning the repository.  The commit will also be checked out
+each time a new build is done, in case the config file was changed to
+specify a different commit.  Note that this will override any local
+changes.  If, on the other hand, no commit is specified, then the
+repository will simply start out at the latest "master", and you will
+be free to make local changes or update it with "git pull" as desired.
+
+(Please take care before updating the fio repository; some updates to
+the fio tree have caused test regressions in the past, so it may be
+preferable to let things be as far as the fio repo is concerned.)
 
 ## Installing the necessary packages to build xfstests
 
get-all +55 -7

75 lines not on this page · raw message

Signature · 1 line
2.13.0.219.gdb65acc882-goog
#3 lore raw

PATCH 3/7 get-all: fail if optional repositories have been deconfigured

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

If an optional repository is enabled and then later disabled, require that the corresponding directory manually be removed; otherwise, it would still be included in the build.

---
 get-all | 9 +++++++++
 1 file changed, 9 insertions(+)
get-all +9 -0
index a0f0d05..fa27229 100755
@@ -81,6 +81,15 @@ setup_repo()
 	fi
     fi
 
+    if [ -z "$repo_url" ] && ! $required; then
+	cat 1>&2 <<EOF
+ERROR: $repo_url_variable has been removed from the config file,
+but the $repo_name directory still exists.  Remove it if you don't
+want it to be built.
+EOF
+	exit 1
+    fi
+
     # If a specific commit was specified, check it out.
     if [ -n "$commit" ]; then
 	( cd "$repo_name";
Signature · 1 line
2.13.0.219.gdb65acc882-goog
#4 lore raw

PATCH 4/7 Makefile: always run get-all

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Run get-all as the first step of 'make all'. We were previously running get-all only if required repositories were missing. But we may also need to clone optional repositories or check out different commits in already-cloned repositories.

---
 Makefile | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
Makefile +12 -9
index f5e2245..6be3752 100644
@@ -2,29 +2,30 @@
 # A simple makefile for xfstests-bld
 #
 
+REPOS =		fio \
+		quota \
+		stress-ng \
+		xfsprogs-dev \
+		xfstests-dev
+
 SUBDIRS =	acl \
 		android-compat \
 		attr \
 		dbench \
 		e2fsprogs-libs \
-		fio \
-		quota \
 		libaio \
 		misc \
 		popt \
-		stress-ng \
-		xfsprogs-dev \
-		xfstests-dev
+		$(REPOS)
 
 SCRIPTS =	android-xfstests.sh \
 		gce-xfstests.sh \
 		kvm-xfstests.sh
 
-all: xfsprogs-dev xfstests-dev fio quota $(SCRIPTS)
-	./build-all
 
-xfsprogs-dev xfstests-dev fio quota:
+all: $(SCRIPTS)
 	./get-all
+	./build-all
 
 $(SCRIPTS): %.sh: kvm-xfstests/%.in
 	sed -e "s;@DIR@;$$(pwd);" < $< > $@
@@ -43,7 +44,9 @@ kvm-xfstests/util/zerofree: kvm-xfstests/util/zerofree.c
 	cc -static -o $@ $< -lext2fs -lcom_err -lpthread
 
 realclean: clean
-	rm -rf xfsprogs-dev xfstests-dev fio quota *.ver
+	rm -rf $(REPOS) *.ver
 
 tarball:
 	./gen-tarball
+
+.PHONY: all clean realclean tarball
Signature · 1 line
2.13.0.219.gdb65acc882-goog
#5 lore raw

PATCH 5/7 do-all, get-all: run with 'set -e'

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

If one of the steps fails, we should not continue on.

---
 do-all  | 2 ++
 get-all | 2 ++
 2 files changed, 4 insertions(+)
do-all +2 -0
index d4daad0..d6b8169 100755
@@ -1,5 +1,7 @@
 #!/bin/bash
 
+set -e
+
 if test -f config.custom ; then
 	. config.custom
 else
get-all +2 -0
index fa27229..bb1cd4c 100755
@@ -1,5 +1,7 @@
 #!/bin/bash
 
+set -e
+
 if test -f config.custom ; then
 	. config.custom
 else
Signature · 1 line
2.13.0.219.gdb65acc882-goog
#6 lore raw

PATCH 6/7 Makefile: check whether xfsprogs-dev exists before cleaning it

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

If running 'make clean' (or do-all) before the repositories have been cloned, don't try to clean a nonexistent xfsprogs-dev directory.

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Makefile +1 -1
index 6be3752..ced32b2 100644
@@ -36,7 +36,7 @@ clean:
 	do \
 		if test -f $$i/Makefile ; then make -C $$i clean ; fi ; \
 	done
-	make -C xfsprogs-dev realclean
+	if test -d xfsprogs-dev; then make -C xfsprogs-dev realclean; fi
 	rm -rf bld xfstests
 	rm -f kvm-xfstests/util/zerofree $(SCRIPTS)
 
Signature · 1 line
2.13.0.219.gdb65acc882-goog
#7 lore raw

PATCH 7/7 xfstests-bld: optionally build keyctl

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

Add support for optionally including the keyctl program in the xfstests tarball built by xfstests-bld. keyctl is used by the filesystem encryption tests. Although keyctl is already included in the test appliances, including it in the xfstests tarball can be useful in cases where the tarball is being used on its own, without being contained in a dedicated test appliance.

---
 .gitignore                         |  1 +
 Documentation/building-xfstests.md |  4 ++++
 Makefile                           |  1 +
 build-all                          | 28 ++++++++++++++++++++++++----
 config                             |  5 +++++
 get-all                            |  1 +
 6 files changed, 36 insertions(+), 4 deletions(-)
.gitignore +1 -0
index db604f8..6831ef0 100644
@@ -62,6 +62,7 @@ MAKELOG
 /e2fsprogs-libs/lib/blkid/blkid.h
 /e2fsprogs-libs/lib/uuid/uuid.h
 /fio/
+/keyutils/
 /popt/.deps/
 /popt/Doxyfile
 /popt/Makefile
Documentation/building-xfstests.md +4 -0
index 4ec3042..fc1c783 100644
@@ -32,6 +32,10 @@ be free to make local changes or update it with "git pull" as desired.
 the fio tree have caused test regressions in the past, so it may be
 preferable to let things be as far as the fio repo is concerned.)
 
+The build also supports some optional repositories which are only
+included when their URLs are uncommented in the config file; see the
+config file for a full list.
+
 ## Installing the necessary packages to build xfstests
 
 In order to build xfstests, a number of prerequisite packages are
Makefile +1 -0
index ced32b2..9d505f3 100644
@@ -3,6 +3,7 @@
 #
 
 REPOS =		fio \
+		keyutils \
 		quota \
 		stress-ng \
 		xfsprogs-dev \
build-all +24 -4
index b41c51a..c3392ad 100755
@@ -24,6 +24,7 @@ case "$CROSS_COMPILE" in
 	SKIP_QUOTA=yes
 	SKIP_FIO=yes
 	SKIP_DBENCH=yes
+	SKIP_KEYUTILS=yes
 	;;
 esac
 
@@ -43,13 +44,13 @@ function set_skip_all () {
     SKIP_XFSTESTS=yes
     SKIP_QUOTA=yes
     SKIP_KVM_UTILS=yes
+    SKIP_KEYUTILS=yes
     SKIP_STRESS_NG=yes
 }
 
-if test ! -d stress-ng
-then
-    SKIP_STRESS_NG=yes
-fi
+# Optional components
+[ -d keyutils ] || SKIP_KEYUTILS=yes
+[ -d stress-ng ] || SKIP_STRESS_NG=yes
 
 
 while [ "$1" != "" ]; do
@@ -88,6 +89,10 @@ while [ "$1" != "" ]; do
 	    set_skip_all
 	    unset SKIP_FIO
 	    ;;
+	--keyutils-only)
+	    set_skip_all
+	    unset SKIP_KEYUTILS
+	    ;;
 	--quota-only)
 	    set_skip_all
 	    unset SKIP_QUOTA
@@ -182,6 +187,21 @@ if test -z "$SKIP_AIO" ; then
     rm $DESTDIR/lib/libaio.so*
 fi
 
+if test -z "$SKIP_KEYUTILS" ; then
+    build_start "keyutils"
+    (cd keyutils ;
+     ver=$(git describe --always --dirty); echo "keyutils	$ver ($(git log -1 --pretty=%cD))" > ../keyutils.ver ;
+
+     # For now we only care about keyctl, not libkeyutils.  Therefore, specify
+     # NO_SOLIB=1 so that libkeyutils is only built as a static library.  Then
+     # keyctl will be statically linked to it, and we won't have to install
+     # libkeyutils.so.
+     make $J NO_SOLIB=1 CFLAGS="$LCF" LDFLAGS="$EXEC_LDFLAGS" keyctl ;
+
+     install -D -m 0755 keyctl $DESTDIR/bin/keyctl ;
+    )
+fi
+
 if test -z "$SKIP_STRESS_NG" ; then
     build_start "stress-ng"
     mkdir -p $DESTDIR/lib
config +5 -0
index 0703829..5b21803 100644
@@ -7,6 +7,11 @@ XFSPROGS_GIT=git://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git
 FIO_GIT=git://git.kernel.dk/fio.git
 QUOTA_GIT=git://git.kernel.org/pub/scm/utils/quota/quota-tools.git
 
+# Optional repositories, uncomment only if needed
+#
+# KEYUTILS_GIT=git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/keyutils.git
+# STRESS_NG_GIT=https://github.com/ColinIanKing/stress-ng
+
 FIO_COMMIT=fio-2.19
 QUOTA_COMMIT=e0b6335
 XFSPROGS_COMMIT=v4.11.0-rc1
get-all +1 -0
index bb1cd4c..45f6c57 100755
@@ -107,6 +107,7 @@ setup_repo xfsprogs-dev		XFSPROGS_GIT	XFSPROGS_COMMIT		true
 setup_repo xfstests-dev		XFSTESTS_GIT	XFSTESTS_COMMIT		true
 
 # optional repositories
+setup_repo keyutils		KEYUTILS_GIT	KEYUTILS_COMMIT		false
 setup_repo stress-ng		STRESS_NG_GIT	STRESS_NG_COMMIT	false
 
 # Make sure acl doesn't try regenerate these files because of the
Signature · 1 line
2.13.0.219.gdb65acc882-goog
#8 lore raw
In reply to Eric Biggers
Quoted 12 lines · Eric Biggers

From: Eric Biggers <[email protected]>

Patches 1-6 make some improvements to how the xfstests-bld system handles external git repositories --- especially, making sure the build system recognizes changes made to the config file.

Patch 7 adds support for optionally including keyctl (used by the filesystem encryption tests) in the xfstests tarball built by xfstests-bld. As noted, this usually isn't needed, but it can be useful if the tarball is used "on its own", without being contained in a dedicated test appliance.

Quoted 8 lines

Eric Biggers (7):
get-all: use helper function to clone repositories
get-all: check out correct commits in already-cloned repositories
get-all: fail if optional repositories have been deconfigured
Makefile: always run get-all
do-all, get-all: run with 'set -e'
Makefile: check whether xfsprogs-dev exists before cleaning it
xfstests-bld: optionally build keyctl

Thanks, I've applied this whole series to my tree.

- Ted