Richard W.M. Jones
2011-Oct-18 13:20 UTC
[Libguestfs] [PATCH febootstrap] Some cleanups for Debian and Ubuntu
I just tried to get libguestfs to compile on Ubuntu 11.10 using the latest febootstrap, and the following patches were necessary for me. They are all just reasonable code cleanups *except* for patch 5/5 which is a gross hack for something I don't understand about how Ubuntu 11.10 multiarch support works. Rich.
Richard W.M. Jones
2011-Oct-18 13:20 UTC
[Libguestfs] [PATCH 1/5] Don't pass use_installed to every package handler function.
From: "Richard W.M. Jones" <rjones at redhat.com> use_installed is a global variable (defined in febootstrap_cmdline.mli) so there's not much point in passing it around to every function that needs it. This commit removes the optional argument in favour of just using the global variable in each package handler. However we still need a place where we can bail if the --use-installed flag is used for package handlers which don't support this yet. Thus add a ph_init function is called after the right package handler has been detected but before it is used. This is a convenient place to put the --use-installed checking and any other initialization that is required. --- src/febootstrap.ml | 4 ++-- src/febootstrap_debian.ml | 8 ++++++-- src/febootstrap_package_handlers.ml | 14 ++++++++------ src/febootstrap_package_handlers.mli | 13 ++++++++++--- src/febootstrap_pacman.ml | 15 +++++++-------- src/febootstrap_yum_rpm.ml | 15 +++++++-------- 6 files changed, 40 insertions(+), 29 deletions(-) diff --git a/src/febootstrap.ml b/src/febootstrap.ml index 3afb1bb..7e48206 100644 --- a/src/febootstrap.ml +++ b/src/febootstrap.ml @@ -67,7 +67,7 @@ let () List.flatten ( List.map ( fun pkg -> - let files = ph.ph_list_files ~use_installed pkg in + let files = ph.ph_list_files pkg in List.map (fun (filename, ft) -> filename, ft, pkg) files ) packages ) in @@ -320,7 +320,7 @@ let () * original file from the package. *) else if config then ( - let outfile = ph.ph_get_file_from_package ~use_installed pkg path in + let outfile = ph.ph_get_file_from_package pkg path in (* Note that the output config file might not be a regular file. *) let statbuf = lstat outfile in diff --git a/src/febootstrap_debian.ml b/src/febootstrap_debian.ml index e7fcffd..e4b42af 100644 --- a/src/febootstrap_debian.ml +++ b/src/febootstrap_debian.ml @@ -45,6 +45,9 @@ let debian_detect () file_exists "/etc/debian_version" && Config.aptitude <> "no" && Config.apt_cache <> "no" && Config.dpkg <> "no" +let debian_init () + () + let rec debian_resolve_dependencies_and_download names let cmd sprintf "%s depends --recurse -i %s | grep -v '^[<[:space:]]'" @@ -191,14 +194,14 @@ let debian_list_files_installed pkg ) lines in files -let debian_list_files ?(use_installed=false) pkg +let debian_list_files pkg if use_installed && List.exists ((=) pkg) (get_installed_pkgs ()) then debian_list_files_installed pkg else debian_list_files_downloaded pkg (* Easy because we already unpacked the archive above. *) -let debian_get_file_from_package ?(use_installed=false) pkg file +let debian_get_file_from_package pkg file if use_installed && List.exists (fun p -> p = pkg) (get_installed_pkgs ()) then file @@ -208,6 +211,7 @@ let debian_get_file_from_package ?(use_installed=false) pkg file let () let ph = { ph_detect = debian_detect; + ph_init = debian_init; ph_resolve_dependencies_and_download debian_resolve_dependencies_and_download; ph_list_files = debian_list_files; diff --git a/src/febootstrap_package_handlers.ml b/src/febootstrap_package_handlers.ml index f627d2f..0d5cc72 100644 --- a/src/febootstrap_package_handlers.ml +++ b/src/febootstrap_package_handlers.ml @@ -24,9 +24,10 @@ open Febootstrap_cmdline type package_handler = { ph_detect : unit -> bool; + ph_init : unit -> unit; ph_resolve_dependencies_and_download : string list -> string list; - ph_list_files : ?use_installed:bool -> string -> (string * file_type) list; - ph_get_file_from_package : ?use_installed:bool -> string -> string -> string + ph_list_files : string -> (string * file_type) list; + ph_get_file_from_package : string -> string -> string } and file_type = { ft_dir : bool; @@ -46,14 +47,15 @@ let register_package_handler name ph let handler = ref None -let check_system () +let rec check_system () try handler := Some ( List.find ( fun (_, ph) -> ph.ph_detect () ) !handlers - ) + ); + (get_package_handler ()).ph_init () with Not_found -> eprintf "\ febootstrap: could not detect package manager used by this system or distro. @@ -65,14 +67,14 @@ then it may be that the package detection code is not working. "; exit 1 -let rec get_package_handler () +and get_package_handler () match !handler with | Some (_, ph) -> ph | None -> check_system (); get_package_handler () -let rec get_package_handler_name () +and get_package_handler_name () match !handler with | Some (name, _) -> name | None -> diff --git a/src/febootstrap_package_handlers.mli b/src/febootstrap_package_handlers.mli index ebf0386..88916ad 100644 --- a/src/febootstrap_package_handlers.mli +++ b/src/febootstrap_package_handlers.mli @@ -20,7 +20,14 @@ type package_handler = { ph_detect : unit -> bool; - (** Detect if the current system uses this package manager. *) + (** Detect if the current system uses this package manager. This is + called in turn on each package handler, until one returns [true]. *) + + ph_init : unit -> unit; + (** After a package handler is selected, this function is called + which can optionally do any initialization that is required. + This is only called on the package handler if it has returned + [true] from {!ph_detect}. *) ph_resolve_dependencies_and_download : string list -> string list; (** [ph_resolve_dependencies_and_download pkgs] @@ -31,11 +38,11 @@ type package_handler = { Note this should also process the [excludes] list. *) - ph_list_files : ?use_installed:bool -> string -> (string * file_type) list; + ph_list_files : string -> (string * file_type) list; (** [ph_list_files pkg] lists the files and file metadata in the package called [pkg] (a package file). *) - ph_get_file_from_package : ?use_installed:bool -> string -> string -> string; + ph_get_file_from_package : string -> string -> string; (** [ph_get_file_from_package pkg file] extracts the single named file [file] from [pkg]. The path of the extracted file is returned. *) diff --git a/src/febootstrap_pacman.ml b/src/febootstrap_pacman.ml index bd12f69..7fbb72b 100644 --- a/src/febootstrap_pacman.ml +++ b/src/febootstrap_pacman.ml @@ -32,6 +32,10 @@ let pacman_detect () file_exists "/etc/arch-release" && Config.pacman <> "no" +let pacman_init () + if use_installed then + failwith "pacman driver doesn't support --use-installed" + let pacman_resolve_dependencies_and_download names let cmd sprintf "(for p in %s; do pactree -u $p; done) | awk '{print $1}' | sort -u" @@ -71,10 +75,7 @@ let pacman_resolve_dependencies_and_download names List.sort compare pkgs -let pacman_list_files ?(use_installed=false) pkg - if use_installed then - failwith "pacman driver doesn't support --use-installed"; - +let pacman_list_files pkg debug "unpacking %s ..." pkg; (* We actually need to extract the file in order to get the @@ -119,15 +120,13 @@ let pacman_list_files ?(use_installed=false) pkg files (* Easy because we already unpacked the archive above. *) -let pacman_get_file_from_package ?(use_installed=false) pkg file - if use_installed then - failwith "pacman driver doesn't support --use-installed"; - +let pacman_get_file_from_package pkg file tmpdir // pkg ^ ".d" // file let () let ph = { ph_detect = pacman_detect; + ph_init = pacman_init; ph_resolve_dependencies_and_download pacman_resolve_dependencies_and_download; ph_list_files = pacman_list_files; diff --git a/src/febootstrap_yum_rpm.ml b/src/febootstrap_yum_rpm.ml index 815c5ba..bfda11e 100644 --- a/src/febootstrap_yum_rpm.ml +++ b/src/febootstrap_yum_rpm.ml @@ -32,6 +32,10 @@ let yum_rpm_detect () (file_exists "/etc/redhat-release" || file_exists "/etc/fedora-release") && Config.yum <> "no" && Config.rpm <> "no" +let yum_rpm_init () + if use_installed then + failwith "yum_rpm driver doesn't support --use-installed" + let yum_rpm_resolve_dependencies_and_download names (* Liberate this data from python. *) let tmpfile = tmpdir // "names.tmp" in @@ -172,10 +176,7 @@ if verbose: sprintf "%s/%s-%s-%s.%s.rpm" tmpdir name version release arch ) pkgs -let rec yum_rpm_list_files ?(use_installed=false) pkg - if use_installed then - failwith "yum_rpm driver doesn't support --use-installed"; - +let rec yum_rpm_list_files pkg (* Run rpm -qlp with some extra magic. *) let cmd sprintf "rpm -q --qf '[%%{FILENAMES} %%{FILEFLAGS:fflags} %%{FILEMODES} %%{FILESIZES}\\n]' -p %s" @@ -231,10 +232,7 @@ let rec yum_rpm_list_files ?(use_installed=false) pkg files -let yum_rpm_get_file_from_package ?(use_installed=false) pkg file - if use_installed then - failwith "yum_rpm driver doesn't support --use-installed"; - +let yum_rpm_get_file_from_package pkg file debug "extracting %s from %s ..." file (Filename.basename pkg); let outfile = tmpdir // file in @@ -247,6 +245,7 @@ let yum_rpm_get_file_from_package ?(use_installed=false) pkg file let () let ph = { ph_detect = yum_rpm_detect; + ph_init = yum_rpm_init; ph_resolve_dependencies_and_download yum_rpm_resolve_dependencies_and_download; ph_list_files = yum_rpm_list_files; -- 1.7.6
Richard W.M. Jones
2011-Oct-18 13:21 UTC
[Libguestfs] [PATCH 2/5] debian: Get installed package list in the ph_init handler.
From: "Richard W.M. Jones" <rjones at redhat.com> Use the Debian ph_init handler to get the list of installed packages. --- src/febootstrap_debian.ml | 23 +++++++++-------------- 1 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/febootstrap_debian.ml b/src/febootstrap_debian.ml index e4b42af..85c9b7e 100644 --- a/src/febootstrap_debian.ml +++ b/src/febootstrap_debian.ml @@ -28,25 +28,20 @@ open Febootstrap_cmdline (* Create a temporary directory for use by all the functions in this file. *) let tmpdir = tmpdir () -let get_installed_pkgs - let pkgs = ref None in - let rec f () - match !pkgs with - | None -> - pkgs :- Some (run_command_get_lines - "dpkg-query --show --showformat='${Package}\\n'"); - f () - | Some pkgs -> pkgs - in - f - let debian_detect () file_exists "/etc/debian_version" && Config.aptitude <> "no" && Config.apt_cache <> "no" && Config.dpkg <> "no" +let installed_pkgs = ref [] + let debian_init () - () + installed_pkgs :+ run_command_get_lines "dpkg-query --show --showformat='${Package}\\n'" + +let get_installed_pkgs () + match !installed_pkgs with + | [] -> assert false + | pkgs -> pkgs let rec debian_resolve_dependencies_and_download names let cmd -- 1.7.6
Richard W.M. Jones
2011-Oct-18 13:21 UTC
[Libguestfs] [PATCH 3/5] debian: Fix Debian package handler when --use-installed not given.
From: "Richard W.M. Jones" <rjones at redhat.com> If there is no --use-installed option, then it didn't download all the packages (only the ones not installed). But this failed later when it tried to unpack the packages that hadn't been downloaded. Thus download all the packages if !use_installed. --- src/febootstrap_debian.ml | 10 +++++++--- 1 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/febootstrap_debian.ml b/src/febootstrap_debian.ml index 85c9b7e..0169756 100644 --- a/src/febootstrap_debian.ml +++ b/src/febootstrap_debian.ml @@ -62,9 +62,13 @@ let rec debian_resolve_dependencies_and_download names not (List.exists (fun re -> Str.string_match re name 0) excludes) ) pkgs in - let present_pkgs, download_pkgs = List.partition ( - fun pkg -> List.exists ((=) pkg) (get_installed_pkgs ()) - ) pkgs in + let present_pkgs, download_pkgs + if not use_installed then + [], pkgs + else + List.partition ( + fun pkg -> List.exists ((=) pkg) (get_installed_pkgs ()) + ) pkgs in debug "wanted packages (present / download): %s / %s\n" (String.concat " " present_pkgs) -- 1.7.6
Richard W.M. Jones
2011-Oct-18 13:21 UTC
[Libguestfs] [PATCH 4/5] debian: Clean up a debug message.
From: "Richard W.M. Jones" <rjones at redhat.com> --- src/febootstrap_debian.ml | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/febootstrap_debian.ml b/src/febootstrap_debian.ml index 0169756..ace04d3 100644 --- a/src/febootstrap_debian.ml +++ b/src/febootstrap_debian.ml @@ -70,9 +70,8 @@ let rec debian_resolve_dependencies_and_download names fun pkg -> List.exists ((=) pkg) (get_installed_pkgs ()) ) pkgs in - debug "wanted packages (present / download): %s / %s\n" - (String.concat " " present_pkgs) - (String.concat " " download_pkgs); + debug "packages already present: %s" (String.concat " " present_pkgs); + debug "wanted packages to download: %s" (String.concat " " download_pkgs); (* Download the packages. *) if (List.length download_pkgs > 0) -- 1.7.6
Richard W.M. Jones
2011-Oct-18 13:21 UTC
[Libguestfs] [PATCH 5/5] ubuntu: Remove '*:i386' (multiarch?) packages.
From: "Richard W.M. Jones" <rjones at redhat.com> This is a hack. Unclear if this is really needed or what it does, but it fixes Ubuntu 11.10 builds for me. --- src/febootstrap_debian.ml | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/febootstrap_debian.ml b/src/febootstrap_debian.ml index ace04d3..58e1b15 100644 --- a/src/febootstrap_debian.ml +++ b/src/febootstrap_debian.ml @@ -45,7 +45,7 @@ let get_installed_pkgs () let rec debian_resolve_dependencies_and_download names let cmd - sprintf "%s depends --recurse -i %s | grep -v '^[<[:space:]]'" + sprintf "%s depends --recurse -i %s | grep -v '^[<[:space:]]' | grep -v :i386" Config.apt_cache (String.concat " " (List.map Filename.quote names)) in let pkgs = run_command_get_lines cmd in -- 1.7.6