[Libguestfs] [supermin PATCH 3/4] Extend modes with list of outputs

[Richard W.M. Jones]

On Fri, Apr 03, 2020 at 12:20:57PM +0200, Pino Toscano wrote:
> Add a function for each mode to return the list of potential outputs, so
> that the existance/timestamp checks done for --if-newer can take those
> into accounts.
> 
> At the moment both modes return no outputs, so there is no behaviour
> change.

Patches 1, 2 and 4 are fine.

I have a minor problem with this patch:

> diff --git a/src/mode_prepare.ml b/src/mode_prepare.ml
> index 70f9dd4..e0ad1a8 100644
> --- a/src/mode_prepare.ml
> +++ b/src/mode_prepare.ml
> @@ -21,7 +21,7 @@ open Printf
>  open Package_handler
>  open Utils
>  
> -let prepare debug (copy_kernel, format, host_cpu,
> +let rec prepare debug (copy_kernel, format, host_cpu,
>               packager_config, tmpdir, use_installed, size,
>               include_packagelist)
>      inputs outputdir =
> @@ -175,3 +175,10 @@ let prepare debug (copy_kernel, format, host_cpu,
>      (* No config files to copy, so do not create base.tar.gz. *)
>      if debug >= 1 then printf "supermin: not creating base.tar.gz\n%!";
>    )
> +
> +and get_outputs
> +    (copy_kernel, format, host_cpu,
> +     packager_config, tmpdir, use_installed, size,
> +     include_packagelist)
> +    inputs =
> +  []
> diff --git a/src/mode_prepare.mli b/src/mode_prepare.mli
> index e2d677a..be45730 100644
> --- a/src/mode_prepare.mli
> +++ b/src/mode_prepare.mli
> @@ -21,3 +21,7 @@
>  val prepare : int -> (bool * Types.format * string * string option * string * bool * int64 option * bool) -> string list -> string -> unit
>  (** [prepare debug (args...) inputs outputdir] performs the
>      [supermin --prepare] 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 80c48e6..4091f1d 100644
> --- a/src/supermin.ml
> +++ b/src/supermin.ml
> @@ -236,10 +236,15 @@ appliance automatically.
>     *)
>    if if_newer then (
>      try
> -      let odate = (lstat outputdir).st_mtime in
> +      let mode_outputs =
> +        match mode with
> +        | Prepare -> Mode_prepare.get_outputs args inputs
> +        | Build -> Mode_build.get_outputs args inputs in

We actually document that --if-newer can only be used in build mode.
It sort of makes no sense in prepare mode.  So shouldn't this just
give an error (either here, or earlier) if we're in prepare mode?

Rich.

> +      let mode_outputs = List.map ((//) outputdir) mode_outputs in
> +      let odates = List.map (fun d -> (lstat d).st_mtime) (outputdir :: mode_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
> -- 
> 2.25.1
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html