Libguestfs - Apr 2020 - [supermin PATCH v2 4/4] build: check for outputs in --if-newer check (RHBZ#1813809)

This is an attempt to make supermin check for the existing results of an
output when checking whether the appliance must be rebuilt using
--if-newer.

At the moment it is implemented only for the ext2 output format of the
build mode.

Changes from v1:
- drop empty stub for the prepare mode
- add patch to ignore --if-newer on modes different than build
- squash patch with stub for the build mode and patch with the build
  mode files together

Pino Toscano (4):
  build: factor ext2 filenames
  Tighten Unix_error check for missing outputdir
  Act on --if-newer only for build mode
  build: check for outputs in --if-newer check (RHBZ#1813809)

 src/mode_build.ml           | 22 ++++++++++++--
 src/mode_build.mli          |  4 +++
 src/supermin.ml             | 10 ++++---
 tests/Makefile.am           |  3 +-
 tests/test-if-newer-ext2.sh | 57 +++++++++++++++++++++++++++++++++++++
 5 files changed, 88 insertions(+), 8 deletions(-)
 create mode 100755 tests/test-if-newer-ext2.sh

-- 
2.25.1

Factor them in own variables to avoid repeating them in following
commits.

This is just refactoring, no behaviour changes.
---
 src/mode_build.ml | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/mode_build.ml b/src/mode_build.ml
index 8a06012..d54a3cd 100644
--- a/src/mode_build.ml
+++ b/src/mode_build.ml
@@ -59,6 +59,10 @@ and string_of_file_content = function
   | Excludefiles -> "excludefiles"
   | Empty -> "empty"
 
+let kernel_filename = "kernel"
+and appliance_filename = "root"
+and initrd_filename = "initrd"
+
 let rec build debug
     (copy_kernel, format, host_cpu,
      packager_config, tmpdir, use_installed, size,
@@ -223,9 +227,9 @@ let rec build debug
     Format_chroot.build_chroot debug files outputdir packagelist_file
 
   | Ext2 ->
-    let kernel = outputdir // "kernel"
-    and appliance = outputdir // "root"
-    and initrd = outputdir // "initrd" in
+    let kernel = outputdir // kernel_filename
+    and appliance = outputdir // appliance_filename
+    and initrd = outputdir // initrd_filename in
     let kernel_version, modpath        Format_ext2_kernel.build_kernel debug
host_cpu copy_kernel kernel in
     Format_ext2.build_ext2 debug basedir files modpath kernel_version
-- 
2.25.1

When getting the timestamp of the output directory for the --if-newer
checks, ignore ENOENT instead of any Unix_error: the lack of it is OK,
while any other stat error is most likekly an actual problem.
---
 src/supermin.ml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/supermin.ml b/src/supermin.ml
index 7c7135b3..80c48e6 100644
--- a/src/supermin.ml
+++ b/src/supermin.ml
@@ -245,7 +245,7 @@ appliance automatically.
         exit 0
       )
     with
-      Unix_error _ -> () (* just continue *)
+      Unix_error (ENOENT, _, _) -> () (* just continue *)
   );
 
   (* Create the output directory nearly atomically. *)
-- 
2.25.1

This option is specific for the build mode, so do not use it when the
mode is a different one.
---
 src/supermin.ml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/supermin.ml b/src/supermin.ml
index 80c48e6..6e93b83 100644
--- a/src/supermin.ml
+++ b/src/supermin.ml
@@ -234,7 +234,7 @@ appliance automatically.
    * does not exist, or if the dates of either input files or package
    * database is newer, then we rebuild.  Else we can just exit.
    *)
-  if if_newer then (
+  if mode = Build && if_newer then (
     try
       let odate = (lstat outputdir).st_mtime in
       let idates = List.map (fun d -> (lstat d).st_mtime) inputs in
-- 
2.25.1

In case we need to check whether the appliance must be rebuilt, check
also the timestamp of the outputs of the mode, i.e. the kernel, initrd,
and root files. This way, when either of these files does not exist or
is older than the package manager DB we can rebuild the appliance.

Add a simple test to verify this behaviour.
---
 src/mode_build.ml           | 12 ++++++++
 src/mode_build.mli          |  4 +++
 src/supermin.ml             |  6 ++--
 tests/Makefile.am           |  3 +-
 tests/test-if-newer-ext2.sh | 57 +++++++++++++++++++++++++++++++++++++
 5 files changed, 79 insertions(+), 3 deletions(-)
 create mode 100755 tests/test-if-newer-ext2.sh

diff --git a/src/mode_build.ml b/src/mode_build.ml
index d54a3cd..ed47366 100644
--- a/src/mode_build.ml
+++ b/src/mode_build.ml
@@ -462,3 +462,15 @@ and munge files    let files = loop files in
 
   files
+
+and get_outputs
+    (copy_kernel, format, host_cpu,
+     packager_config, tmpdir, use_installed, size,
+     include_packagelist)
+    inputs +  match format with
+  | Chroot ->
+    (* The content for chroot depends on the packages. *)
+    []
+  | Ext2 ->
+    [kernel_filename; appliance_filename; initrd_filename]
diff --git a/src/mode_build.mli b/src/mode_build.mli
index 0f8b956..4fba2ab 100644
--- a/src/mode_build.mli
+++ b/src/mode_build.mli
@@ -21,3 +21,7 @@
 val build : int -> (bool * Types.format * string * string option * string *
bool * int64 option * bool) -> string list -> string -> unit
 (** [build debug (args...) inputs outputdir] performs the
     [supermin --build] subcommand. *)
+
+val get_outputs : (bool * Types.format * string * string option * string * bool
* int64 option * bool) -> string list -> string list
+(** [get_outputs (args...) inputs] gets the potential outputs for the
+    appliance. *)
diff --git a/src/supermin.ml b/src/supermin.ml
index 6e93b83..e923111 100644
--- a/src/supermin.ml
+++ b/src/supermin.ml
@@ -236,10 +236,12 @@ appliance automatically.
    *)
   if mode = Build && if_newer then (
     try
-      let odate = (lstat outputdir).st_mtime in
+      let outputs = Mode_build.get_outputs args inputs in
+      let outputs = List.map ((//) outputdir) outputs in
+      let odates = List.map (fun d -> (lstat d).st_mtime) (outputdir ::
outputs) in
       let idates = List.map (fun d -> (lstat d).st_mtime) inputs in
       let pdate = (get_package_handler ()).ph_get_package_database_mtime () in
-      if List.for_all (fun idate -> idate < odate) (pdate :: idates) then
(
+      if List.for_all (fun idate -> List.for_all (fun odate -> idate <
odate) odates) (pdate :: idates) then (
         if debug >= 1 then
           printf "supermin: if-newer: output does not need
rebuilding\n%!";
         exit 0
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 42d1b82..070f6d9 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -28,7 +28,8 @@ TESTS = \
 	test-execstack.sh \
 	test-build-bash.sh \
 	test-binaries-exist.sh \
-	test-harder.sh
+	test-harder.sh \
+	test-if-newer-ext2.sh
 
 if NETWORK_TESTS
 TESTS += \
diff --git a/tests/test-if-newer-ext2.sh b/tests/test-if-newer-ext2.sh
new file mode 100755
index 0000000..4f49bda
--- /dev/null
+++ b/tests/test-if-newer-ext2.sh
@@ -0,0 +1,57 @@
+#!/bin/bash -
+# supermin
+# (C) Copyright 2009-2020 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+set -e
+
+# XXX Hack for Arch.
+if [ -f /etc/arch-release ]; then
+    export SUPERMIN_KERNEL=/boot/vmlinuz-linux
+fi
+
+tmpdir=`mktemp -d`
+
+d1=$tmpdir/d1
+d2=$tmpdir/d2
+
+# We assume 'bash' is a package everywhere.
+../src/supermin -v --prepare --use-installed bash -o $d1
+
+run_supermin ()
+{
+  ../src/supermin -v --build -f ext2 --if-newer $d1 -o $d2
+}
+
+# Build the appliance the first time, which will work.
+run_supermin
+
+# No changes, hence nothing to do.
+run_supermin | grep 'if-newer: output does not need rebuilding'
+
+# Try removing any of the files, and check that supermin will detect that.
+ext2_files="kernel initrd root"
+for ext2_file in $ext2_files
+do
+  rm $d2/$ext2_file
+  run_supermin
+  for ext2_file in $ext2_files
+  do
+    test -e $d2/$ext2_file
+  done
+done
+
+rm -rf $tmpdir ||:
-- 
2.25.1

Sorry I missed this series - all looks good now, ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org