9:24 a.m.
On Tuesday, 12 September 2017 09:03:14 CEST Cédric Bosdonnat wrote:
virt-builder-repository allows users to easily create or update
a virt-builder source repository out of disk images. The tool can
be run in either interactive or automated mode.
---
Since this patch was done, Rich introduced Unicode quotes, so
- '%s'/'...' -> ‘%s’/‘...’
- don't/etc'etera -> don’t/etc’etera
and so on -- mostly in error/etc messages, and documentation
This looks almost like the last version I reviewed; will give a test
run on Monday.
A minor note on the commit message: I'd use
New tool: virt-builder-repository
which is what was used in the past for other tools (so it makes it
sligtly easier to search for that).
@@ -101,8 +137,7 @@ virt_builder_CPPFLAGS = \
-I$(shell $(OCAMLC) -where) \
-I$(top_srcdir)/gnulib/lib \
-I$(top_srcdir)/common/utils \
- -I$(top_srcdir)/lib \
- -I$(top_srcdir)/fish
+ -I$(top_srcdir)/lib
Extra change, can go in directly if split as own commit.
+virt_builder_repository_SOURCES = $(REPOSITORY_SOURCES_C)
+virt_builder_repository_CPPFLAGS = \
+ -I. \
+ -I$(top_builddir) \
+ -I$(top_srcdir)/gnulib/lib -I$(top_builddir)/gnulib/lib \
+ -I$(shell $(OCAMLC) -where) \
+ -I$(top_srcdir)/gnulib/lib \
+ -I$(top_srcdir)/lib \
+ -I$(top_srcdir)/fish
guestfish is needed here?
+module StringSet = Set.Make(String)
This is now provided by mlstdutils.
+let increment_revision = function
+ | Utils.Rev_int n -> Utils.Rev_int (n + 1)
+ | Utils.Rev_string s ->
+ if Str.string_match (Str.regexp "^\\(.*[-._]\\)\\([0-9]+\\)$") s 0 then
There's a PCRE OCaml module now, so maybe it could be a better choice
here (and make the regexp less ugly).
+let get_mime_type filepath =
+ let file_cmd = "file --mime-type " ^ (quote filepath) in
+ let output = List.hd (external_command file_cmd) in
+ let _, mime = String.split ":" output in
+ String.trim mime
- passing --brief removes the filename from the output, and thus the
need to strip it (which will not work in case the file has a colon in
the name)
- List.hd will raise an exception if the list is empty (so the command
produced no output for some reason) -- you might need to check it,
for example like I did in dib/utils.ml:var_from_lines
+ printf "%s%s%s" message default_str list_str;
I think it might be a good idea to flush stdout before reading the
answer from the user -- just append "%!" at the end of the format
string.
+ if x == "" then
You just need '=', as '==' is stronger than that (compares also object
equality).
+ (id, { Index.printable_name = printable_name;
+ osinfo = osinfo;
+ file_uri = Filename.basename out_path;
+ arch = arch;
+ signature_uri = None;
+ checksums = Some [checksum];
+ revision = revision;
+ format = Some format;
+ size = size;
+ compressed_size = Some compressed_size;
+ expand = expand;
+ lvexpand = lvexpand;
+ notes = notes;
+ hidden = hidden;
+ aliases = aliases;
+ sigchecker = sigchecker;
+ proxy = Curl.UnsetProxy })
Most probably it won't change (although I remember a chat on IRC about
different proxy values leading to failures in the unit test added in
patch #4), but better use one (e.g. Curl.SystemProxy) thoroughly.
+ (* If debugging, echo the command line arguments. *)
+ debug "command line: %s\n" (String.concat " " (Array.to_list
Sys.argv));
debug already prints the newline at the end of the message. (Also in
other occurrences.)
+ (* Remove the processed image files *)
+ if not cmdline.no_compression then
+ List.iter (
+ fun filename -> Sys.remove (cmdline.repo // filename)
+ ) images
Indented twice?
+The file value needs to match the image name extended with the
".xz"
C<.xz>.
+suffix if the C<--no-compression> parameter is not provided or
the
Options with I<>.
+To remove an image from the repository, just remove the
corresponding
+compressed image file before running virt-builder-repository.
I'd remove "compressed" here, since both options (compressed, and not)
are supported.
Thanks,
--
Pino Toscano