[PATCH 0/7] xfstests-bld: get-all improvements and adding keyctl

[PATCH 0/7] xfstests-bld: get-all improvements and adding keyctl

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

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(-)

-- 
2.13.0.219.gdb65acc882-goog

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

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

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

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 get-all | 119 ++++++++++++++++++++++------------------------------------------
 1 file changed, 40 insertions(+), 79 deletions(-)

diff --git a/get-all b/get-all
index fc4dc2d..67a54b4 100755
--- a/get-all
+++ b/get-all
@@ -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
-- 
2.13.0.219.gdb65acc882-goog

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

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

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.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 Documentation/building-xfstests.md | 32 +++++++++++---------
 get-all                            | 62 +++++++++++++++++++++++++++++++++-----
 2 files changed, 72 insertions(+), 22 deletions(-)

diff --git a/Documentation/building-xfstests.md b/Documentation/building-xfstests.md
index 3697838..4ec3042 100644
--- a/Documentation/building-xfstests.md
+++ b/Documentation/building-xfstests.md
@@ -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
 
diff --git a/get-all b/get-all
index 67a54b4..a0f0d05 100755
--- a/get-all
+++ b/get-all
@@ -6,6 +6,55 @@ else
 	. config
 fi
 
+have_commit()
+{
+    git rev-parse --verify --quiet "$1^{commit}" >/dev/null
+}
+
+checkout_commit()
+{
+    local repo_name="$1"
+    local repo_url="$2"
+    local commit_variable="$3"
+    local commit="${!3}"
+    local old_url
+
+    # Make sure there are no uncommitted changes.
+    if ! git diff-index --quiet HEAD --; then
+	cat 1>&2 <<EOF
+ERROR: $repo_name has uncommitted changes.
+
+If you want to build from the (dirty) working tree of $repo_name,
+remove $commit_variable from config.custom.
+EOF
+	exit 1
+    fi
+
+    # If we don't have the needed commit, try fetching from the remote.
+    if ! have_commit "$commit"; then
+	old_url="$(git remote get-url origin)"
+	if [ "$old_url" != "$repo_url" ]; then
+	    echo "$repo_name URL changed to $repo_url (previously was $old_url)"
+	    git remote set-url origin "$repo_url"
+	fi
+	if ! git fetch origin; then
+	    echo 1>&2 "ERROR: unable to fetch from $repo_url"
+	    exit 1
+	fi
+	if ! have_commit "$commit"; then
+	    echo 1>&2 "ERROR: commit $commit does not exist in $repo_url"
+	    exit 1
+	fi
+    fi
+
+    # Check out the commit.
+    if [ "$(git rev-parse HEAD)" != "$(git rev-parse "$commit^{commit}")" ]
+    then
+	echo "Checking out $repo_name $commit (previously was $(git describe --always HEAD))"
+    fi
+    git checkout --quiet "$commit"
+}
+
 setup_repo()
 {
     local repo_name="$1"
@@ -30,14 +79,13 @@ setup_repo()
 	    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
+    # If a specific commit was specified, check it out.
+    if [ -n "$commit" ]; then
+	( cd "$repo_name";
+	  checkout_commit "$repo_name" "$repo_url" "$commit_variable";
+	)
     fi
 }
 
-- 
2.13.0.219.gdb65acc882-goog

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

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

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.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 get-all | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/get-all b/get-all
index a0f0d05..fa27229 100755
--- a/get-all
+++ b/get-all
@@ -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";
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 4/7] Makefile: always run get-all

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

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.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 Makefile | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index f5e2245..6be3752 100644
--- a/Makefile
+++ b/Makefile
@@ -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
-- 
2.13.0.219.gdb65acc882-goog

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

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

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

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 do-all  | 2 ++
 get-all | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/do-all b/do-all
index d4daad0..d6b8169 100755
--- a/do-all
+++ b/do-all
@@ -1,5 +1,7 @@
 #!/bin/bash
 
+set -e
+
 if test -f config.custom ; then
 	. config.custom
 else
diff --git a/get-all b/get-all
index fa27229..bb1cd4c 100755
--- a/get-all
+++ b/get-all
@@ -1,5 +1,7 @@
 #!/bin/bash
 
+set -e
+
 if test -f config.custom ; then
 	. config.custom
 else
-- 
2.13.0.219.gdb65acc882-goog

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

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

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

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 6be3752..ced32b2 100644
--- a/Makefile
+++ b/Makefile
@@ -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)
 
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 7/7] xfstests-bld: optionally build keyctl

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

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.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 .gitignore                         |  1 +
 Documentation/building-xfstests.md |  4 ++++
 Makefile                           |  1 +
 build-all                          | 28 ++++++++++++++++++++++++----
 config                             |  5 +++++
 get-all                            |  1 +
 6 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/.gitignore b/.gitignore
index db604f8..6831ef0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -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
diff --git a/Documentation/building-xfstests.md b/Documentation/building-xfstests.md
index 4ec3042..fc1c783 100644
--- a/Documentation/building-xfstests.md
+++ b/Documentation/building-xfstests.md
@@ -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
diff --git a/Makefile b/Makefile
index ced32b2..9d505f3 100644
--- a/Makefile
+++ b/Makefile
@@ -3,6 +3,7 @@
 #
 
 REPOS =		fio \
+		keyutils \
 		quota \
 		stress-ng \
 		xfsprogs-dev \
diff --git a/build-all b/build-all
index b41c51a..c3392ad 100755
--- a/build-all
+++ b/build-all
@@ -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
diff --git a/config b/config
index 0703829..5b21803 100644
--- a/config
+++ b/config
@@ -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
diff --git a/get-all b/get-all
index bb1cd4c..45f6c57 100755
--- a/get-all
+++ b/get-all
@@ -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
-- 
2.13.0.219.gdb65acc882-goog

Re: [PATCH 0/7] xfstests-bld: get-all improvements and adding keyctl

[Theodore Ts'o]

On Thu, May 25, 2017 at 12:25:42PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> 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

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

	     	     	  	       - Ted