Libguestfs - Mar 2017 - Re: [PATCH 4/5] dib: require a Python interpreter

Hi,

this series start to implement some of the changes needed to support
d-i-b 2.0; normal VM distro builds seem to work correctly, ramdisk
builds are still broken and require more efforts.

Thanks,

Pino Toscano (5):
  dib: implement get_image_element_array stuff
  dib: export IMAGE_BLOCK_DEVICE_WITHOUT_PART
  dib: extract get_required_tool out of require_tool
  dib: require a Python interpreter
  dib: implement IMAGE_ELEMENT_YAML

 dib/cmdline.ml   | 21 ++++++++++++++++++++-
 dib/cmdline.mli  |  1 +
 dib/dib.ml       | 44 ++++++++++++++++++++++++++++++++++++++++----
 dib/utils.ml     |  7 +++++--
 dib/virt-dib.pod | 11 +++++++++++
 5 files changed, 77 insertions(+), 7 deletions(-)

-- 
2.9.3

Output the get_image_element_array function like d-i-b does, together
with the eval stuff needed to make it work.

get_image_element_array is basically a bash array, with the keys are the
used elements, and the values are the paths of each element; it is a new
addition in d-i-b 2.0.

This is used so far only for the extra-data.d phase.
---
 dib/dib.ml | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/dib/dib.ml b/dib/dib.ml
index 2e4da4b..0d128e3 100644
--- a/dib/dib.ml
+++ b/dib/dib.ml
@@ -74,9 +74,15 @@ let envvars_string l  
 let prepare_external ~envvars ~dib_args ~dib_vars ~out_name ~root_label
   ~rootfs_uuid ~image_cache ~arch ~network ~debug ~fs_type ~checksum
-  destdir libdir fakebindir all_elements element_paths +  destdir libdir
fakebindir loaded_elements all_elements element_paths    let network_string = if
network then "" else "1" in
   let checksum_string = if checksum then "1" else "" in
+  let elements_paths_array +    List.map (
+      fun e ->
+        sprintf "[%s]=%s" e (quote (Hashtbl.find loaded_elements
e).directory)
+    ) (StringSet.elements all_elements) in
+  let elements_paths_array = String.concat " " elements_paths_array
in
 
   let run_extra = sprintf "\
 #!/bin/bash
@@ -91,6 +97,8 @@ shift
 script=$1
 shift
 
+VIRT_DIB_OURPATH=$(dirname $(realpath $0))
+
 # user variables
 %s
 
@@ -116,6 +124,9 @@ export DIB_DEBUG_TRACE=%d
 export FS_TYPE=%s
 export DIB_CHECKSUM=%s
 
+elinfo_out=$(<${VIRT_DIB_OURPATH}/elinfo_out)
+eval \"$elinfo_out\"
+
 ENVIRONMENT_D_DIR=$target_dir/../environment.d
 
 if [ -d $ENVIRONMENT_D_DIR ] ; then
@@ -148,7 +159,15 @@ $target_dir/$script
     debug
     fs_type
     checksum_string in
-  write_script (destdir // "run-part-extra.sh") run_extra
+  write_script (destdir // "run-part-extra.sh") run_extra;
+  let elinfo_out = sprintf "\
+function get_image_element_array {
+  echo \"%s\"
+};
+export -f get_image_element_array;
+"
+    elements_paths_array in
+  write_script (destdir // "elinfo_out") elinfo_out
 
 let prepare_aux ~envvars ~dib_args ~dib_vars ~log_file ~out_name ~rootfs_uuid
   ~arch ~network ~root_label ~install_type ~debug ~extra_packages ~fs_type
@@ -636,7 +655,7 @@ let main ()                     ~checksum:cmdline.checksum
                    tmpdir cmdline.basepath
                    (auxtmpdir // "fake-bin")
-                   all_elements cmdline.element_paths;
+                   loaded_elements all_elements cmdline.element_paths;
 
   let run_hook ~blockdev ~sysroot ?(new_wd = "") (g :
Guestfs.guestfs) hook      try
-- 
2.9.3

New environment variable exported by d-i-b 2.0.
---
 dib/dib.ml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dib/dib.ml b/dib/dib.ml
index 0d128e3..1df9aff 100644
--- a/dib/dib.ml
+++ b/dib/dib.ml
@@ -217,6 +217,7 @@ export TMP_HOOKS_PATH=$mysysroot/tmp/in_target.aux/hooks
 export DIB_ARGS=\"%s\"
 export
DIB_MANIFEST_SAVE_DIR=\"$mysysroot/tmp/in_target.aux/out/${IMAGE_NAME}.d\"
 export IMAGE_BLOCK_DEVICE=$blockdev
+export IMAGE_BLOCK_DEVICE_WITHOUT_PART=$(echo ${IMAGE_BLOCK_DEVICE} | sed -e
\"s|^\\(.*loop[0-9]*\\)p[0-9]*$|\\1|g\")
 export IMAGE_ELEMENT=\"%s\"
 export DIB_ENV=%s
 export DIB_DEBUG_TRACE=%d
-- 
2.9.3

Extract from require_tool an helper function to search for the tool, and
return the found path (instead of ignoring it).
---
 dib/utils.ml | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/dib/utils.ml b/dib/utils.ml
index da5e738..967754d 100644
--- a/dib/utils.ml
+++ b/dib/utils.ml
@@ -90,11 +90,14 @@ let digit_prefix_compare a b  let do_mkdir dir    mkdir_p
dir 0o755
 
-let require_tool tool -  try ignore (which tool)
+let get_required_tool tool +  try which tool
   with Executable_not_found tool ->
     error (f_"%s needed but not found") tool
 
+let require_tool tool +  ignore (get_required_tool tool)
+
 let do_cp src destdir    let cmd = [ "cp"; "-t"; destdir;
"-a"; src ] in
   if run_command cmd <> 0 then exit 1
-- 
2.9.3

d-i-b 2.0 is written in Python, and thus it passes the information about
the interpreter used for it (sys.executable) to the scripts that need
it; this mechanism replaces the old discovery of what is the default
Python interpreter in the system.

Since we are not Python-based, look for 'python' and use it as default
interpreter, with the --python command line option to set a different
one.

The only place where it is used so far are extra-data.d scripts; in case
it will be used also in other out-of-chroot phases, a different solution
will be needed.
---
 dib/cmdline.ml   | 21 ++++++++++++++++++++-
 dib/cmdline.mli  |  1 +
 dib/dib.ml       | 10 +++++++++-
 dib/virt-dib.pod | 11 +++++++++++
 4 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/dib/cmdline.ml b/dib/cmdline.ml
index 875f617..2cc1722 100644
--- a/dib/cmdline.ml
+++ b/dib/cmdline.ml
@@ -54,6 +54,7 @@ type cmdline = {
   arch : string;
   envvars : string list;
   checksum : bool;
+  python : string option;
 }
 
 let parse_cmdline () @@ -151,6 +152,9 @@ read the man page virt-dib(1).
 
   let checksum = ref false in
 
+  let python = ref None in
+  let set_python arg = python := Some arg in
+
   let argspec = [
     [ S 'p'; L"element-path" ],           Getopt.String
("path", append_element_path),  s_"Add new a elements
location";
     [ L"exclude-element" ], Getopt.String ("element",
append_excluded_element),
@@ -167,6 +171,7 @@ read the man page virt-dib(1).
     [ L"extra-packages" ], Getopt.String ("pkg,...",
append_extra_packages),
       s_"Add extra packages to install";
     [ L"checksum" ],   Getopt.Set checksum,          s_"Generate
MD5 and SHA256 checksum files";
+    [ L"python" ],     Getopt.String ("python",
set_python),         s_"Set Python interpreter";
 
     [ L"ramdisk" ],    Getopt.Set is_ramdisk,        "Switch to
a ramdisk build";
     [ L"ramdisk-element" ], Getopt.Set_string ("name",
ramdisk_element), s_"Main element for building ramdisks";
@@ -223,6 +228,7 @@ read the man page virt-dib(1).
   let machine_readable = !machine_readable in
   let extra_packages = List.rev !extra_packages in
   let checksum = !checksum in
+  let python = !python in
 
   (* No elements and machine-readable mode?  Print some facts. *)
   if elements = [] && machine_readable then (
@@ -246,6 +252,19 @@ read the man page virt-dib(1).
   if elements = [] then
     error (f_"at least one distribution root element must be
specified");
 
+  let python +    match python with
+    | Some exe ->
+      let p +        if Filename.is_relative exe then
+          get_required_tool exe
+        else (
+          Unix.access exe [Unix.X_OK];
+          exe
+        ) in
+      Some p
+    | None -> None in
+
   { debug = debug; basepath = basepath; elements = elements;
     excluded_elements = excluded_elements; element_paths = element_paths;
     excluded_scripts = excluded_scripts; use_base = use_base; drive = drive;
@@ -256,5 +275,5 @@ read the man page virt-dib(1).
     extra_packages = extra_packages; memsize = memsize; network = network;
     smp = smp; delete_on_failure = delete_on_failure;
     formats = formats; arch = arch; envvars = envvars;
-    checksum = checksum;
+    checksum = checksum; python = python;
   }
diff --git a/dib/cmdline.mli b/dib/cmdline.mli
index acfce5a..5c82efd 100644
--- a/dib/cmdline.mli
+++ b/dib/cmdline.mli
@@ -46,6 +46,7 @@ type cmdline = {
   arch : string;
   envvars : string list;
   checksum : bool;
+  python : string option;
 }
 
 val parse_cmdline : unit -> cmdline
diff --git a/dib/dib.ml b/dib/dib.ml
index 1df9aff..ab5481a 100644
--- a/dib/dib.ml
+++ b/dib/dib.ml
@@ -74,6 +74,7 @@ let envvars_string l  
 let prepare_external ~envvars ~dib_args ~dib_vars ~out_name ~root_label
   ~rootfs_uuid ~image_cache ~arch ~network ~debug ~fs_type ~checksum
+  ~python
   destdir libdir fakebindir loaded_elements all_elements element_paths    let
network_string = if network then "" else "1" in
   let checksum_string = if checksum then "1" else "" in
@@ -123,6 +124,7 @@ export TMP_DIR=\"${TMPDIR}\"
 export DIB_DEBUG_TRACE=%d
 export FS_TYPE=%s
 export DIB_CHECKSUM=%s
+export DIB_PYTHON_EXEC=%s
 
 elinfo_out=$(<${VIRT_DIB_OURPATH}/elinfo_out)
 eval \"$elinfo_out\"
@@ -158,7 +160,8 @@ $target_dir/$script
     (quote dib_vars)
     debug
     fs_type
-    checksum_string in
+    checksum_string
+    python in
   write_script (destdir // "run-part-extra.sh") run_extra;
   let elinfo_out = sprintf "\
 function get_image_element_array {
@@ -524,6 +527,10 @@ let main ()      error (f_"the specified base path is
not the diskimage-builder library");
 
   (* Check for required tools. *)
+  let python +    match cmdline.python with
+    | None -> get_required_tool "python"
+    | Some exe -> exe in
   require_tool "uuidgen";
   Output_format.check_formats_prerequisites cmdline.formats;
   if cmdline.checksum then
@@ -654,6 +661,7 @@ let main ()                     ~network:cmdline.network
~debug
                    ~fs_type:cmdline.fs_type
                    ~checksum:cmdline.checksum
+                   ~python
                    tmpdir cmdline.basepath
                    (auxtmpdir // "fake-bin")
                    loaded_elements all_elements cmdline.element_paths;
diff --git a/dib/virt-dib.pod b/dib/virt-dib.pod
index be7550a..5c1423e 100644
--- a/dib/virt-dib.pod
+++ b/dib/virt-dib.pod
@@ -354,6 +354,17 @@ to debug failures to run scripts.
 The default is to delete the output files if virt-dib fails (or,
 for example, some script that it runs fails).
 
+=item B<--python> PYTHON
+
+Specify a different Python interpreter to use.  Parts of
+C<diskimage-builder> are implemented in Python, and thus an
+interpreter is needed.
+
+C<PYTHON> can either be an executable filename (e.g. F<python2>,
+which is then searched in C<$PATH>), or a full path (e.g.
+F</usr/bin/python2>).  If not specified, the default value is
+F<python>.
+
 =item B<-q>
 
 =item B<--quiet>
-- 
2.9.3

Export a new environment variable for extra-data.d scripts, that
contains a YAML representation of the elements in use.

This is a new addition in d-i-b 2.0.
---
 dib/dib.ml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/dib/dib.ml b/dib/dib.ml
index ab5481a..9ccf9ba 100644
--- a/dib/dib.ml
+++ b/dib/dib.ml
@@ -78,6 +78,12 @@ let prepare_external ~envvars ~dib_args ~dib_vars ~out_name
~root_label
   destdir libdir fakebindir loaded_elements all_elements element_paths    let
network_string = if network then "" else "1" in
   let checksum_string = if checksum then "1" else "" in
+  let elements_paths_yaml +    List.map (
+      fun e ->
+        sprintf "%s: %s" e (quote (Hashtbl.find loaded_elements
e).directory)
+    ) (StringSet.elements all_elements) in
+  let elements_paths_yaml = String.concat ", " elements_paths_yaml in
   let elements_paths_array      List.map (
       fun e ->
@@ -164,11 +170,13 @@ $target_dir/$script
     python in
   write_script (destdir // "run-part-extra.sh") run_extra;
   let elinfo_out = sprintf "\
+export IMAGE_ELEMENT_YAML=\"{%s}\"
 function get_image_element_array {
   echo \"%s\"
 };
 export -f get_image_element_array;
 "
+    elements_paths_yaml
     elements_paths_array in
   write_script (destdir // "elinfo_out") elinfo_out
 
-- 
2.9.3

On Wed, Mar 22, 2017 at 11:19:49AM +0100, Pino Toscano
wrote:
>    if elements = [] && machine_readable then (
> @@ -246,6 +252,19 @@ read the man page virt-dib(1).
>    if elements = [] then
>      error (f_"at least one distribution root element must be
specified");
>  
> +  let python > +    match python with
> +    | Some exe ->
> +      let p > +        if Filename.is_relative exe then
IIUC what you mean here is "the 'exe' filename doesn't contain
any slashes"?  That isn't what is written above.
> +          get_required_tool exe
> +        else (
> +          Unix.access exe [Unix.X_OK];
> +          exe
> +        ) in
> +      Some p
> +    | None -> None in
Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

On Wed, Mar 22, 2017 at 11:19:45AM +0100, Pino Toscano
wrote:
> Hi,
> 
> this series start to implement some of the changes needed to support
> d-i-b 2.0; normal VM distro builds seem to work correctly, ramdisk
> builds are still broken and require more efforts.
This seems fine, modulo a minor comment that I made about
patch 4, so ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top