[Libguestfs] [PATCH v4 5/9] builder: add Index_parser.write_entry function
Pino Toscano
ptoscano at redhat.com
Thu Mar 9 16:16:55 UTC 2017
On Tuesday, 7 March 2017 15:27:01 CET Cédric Bosdonnat wrote:
> Add a function to properly write virt-builder source index entries.
> Note that this function is very similar to Index.print_entry that is
> meant for debugging purposes.
> ---
> .gitignore | 1 +
> builder/Makefile.am | 36 +++++++++++++++-
> builder/index.mli | 3 ++
> builder/index_parser.ml | 52 +++++++++++++++++++++++
> builder/index_parser.mli | 6 +++
> builder/index_parser_tests.ml | 95 +++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 191 insertions(+), 2 deletions(-)
> create mode 100644 builder/index_parser_tests.ml
>
> diff --git a/.gitignore b/.gitignore
> index e3b6d7b1c..06b16a49a 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -104,6 +104,7 @@ Makefile.in
> /builder/virt-index-validate
> /builder/virt-index-validate.1
> /builder/*.xz
> +/builder/index_parser_tests
> /builder/yajl_tests
> /cat/stamp-virt-*.pod
> /cat/virt-cat
> diff --git a/builder/Makefile.am b/builder/Makefile.am
> index 218f64b4c..bf4ccb7d7 100644
> --- a/builder/Makefile.am
> +++ b/builder/Makefile.am
> @@ -217,13 +217,36 @@ yajl_tests_BOBJECTS = \
> yajl_tests.cmo
> yajl_tests_XOBJECTS = $(yajl_tests_BOBJECTS:.cmo=.cmx)
>
> +index_parser_tests_SOURCES = \
> + index-scan.c \
> + index-struct.c \
> + index-parser-c.c \
> + index-parse.c
> +index_parser_tests_CPPFLAGS = $(virt_builder_CPPFLAGS)
> +index_parser_tests_BOBJECTS = \
> + utils.cmo \
> + cache.cmo \
> + downloader.cmo \
> + sigchecker.cmo \
> + index.cmo \
> + ini_reader.cmo \
> + index_parser.cmo \
> + index_parser_tests.cmo
> +index_parser_tests_XOBJECTS = $(index_parser_tests_BOBJECTS:.cmo=.cmx)
> +
> # Can't call the following as <test>_OBJECTS because automake gets confused.
> if HAVE_OCAMLOPT
> yajl_tests_THEOBJECTS = $(yajl_tests_XOBJECTS)
> yajl_tests.cmx: OCAMLPACKAGES += $(OCAMLPACKAGES_TESTS)
> +
> +index_parser_tests_THEOBJECTS = $(index_parser_tests_XOBJECTS)
> +index_parser_tests.cmx: OCAMLPACKAGES += $(OCAMLPACKAGES_TESTS)
> else
> yajl_tests_THEOBJECTS = $(yajl_tests_BOBJECTS)
> yajl_tests.cmo: OCAMLPACKAGES += $(OCAMLPACKAGES_TESTS)
> +
> +index_parser_tests_THEOBJECTS = $(index_parser_tests_BOBJECTS)
> +index_parser_tests.cmo: OCAMLPACKAGES += $(OCAMLPACKAGES_TESTS)
> endif
>
> yajl_tests_DEPENDENCIES = \
> @@ -236,6 +259,15 @@ yajl_tests_LINK = \
> $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLPACKAGES) $(OCAMLPACKAGES_TESTS) $(OCAMLLINKFLAGS) \
> $(yajl_tests_THEOBJECTS) -o $@
>
> +index_parser_tests_DEPENDENCIES = \
> + $(index_parser_tests_THEOBJECTS) \
> + ../mllib/mllib.$(MLARCHIVE) \
> + $(top_srcdir)/ocaml-link.sh
> +index_parser_tests_LINK = \
> + $(top_srcdir)/ocaml-link.sh -cclib '$(OCAMLCLIBS)' -- \
> + $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLPACKAGES) $(OCAMLPACKAGES_TESTS) $(OCAMLLINKFLAGS) \
> + $(index_parser_tests_THEOBJECTS) -o $@
> +
> TESTS = \
> test-docs.sh \
> test-virt-builder-list.sh \
> @@ -249,8 +281,8 @@ if ENABLE_APPLIANCE
> TESTS += test-virt-builder.sh
> endif ENABLE_APPLIANCE
> if HAVE_OCAML_PKG_OUNIT
> -check_PROGRAMS += yajl_tests
> -TESTS += yajl_tests
> +check_PROGRAMS += yajl_tests index_parser_tests
> +TESTS += yajl_tests index_parser_tests
> endif
>
> check-valgrind:
> diff --git a/builder/index.mli b/builder/index.mli
> index ff5ec4a35..6202d636e 100644
> --- a/builder/index.mli
> +++ b/builder/index.mli
> @@ -39,3 +39,6 @@ and entry = {
> }
>
> val print_entry : out_channel -> (string * entry) -> unit
> +(** Debugging helper function dumping an index entry to a stream.
> + To write entries for non-debugging purpose, use the
> + [Index_parser.write_entry] function. *)
> diff --git a/builder/index_parser.ml b/builder/index_parser.ml
> index a3cae7d1a..9b3daed24 100644
> --- a/builder/index_parser.ml
> +++ b/builder/index_parser.ml
> @@ -226,3 +226,55 @@ let get_index ~downloader ~sigchecker
> in
>
> get_index ()
> +
> +let write_entry chan (name, { Index.printable_name = printable_name;
> + file_uri = file_uri;
> + arch = arch;
> + osinfo = osinfo;
> + signature_uri = signature_uri;
> + checksums = checksums;
> + revision = revision;
> + format = format;
> + size = size;
> + compressed_size = compressed_size;
> + expand = expand;
> + lvexpand = lvexpand;
> + notes = notes;
> + aliases = aliases;
> + hidden = hidden }) =
> + let fp fs = fprintf chan fs in
> + fp "[%s]\n" name;
> + may (fp "name=%s\n") printable_name;
> + may (fp "osinfo=%s\n") osinfo;
> + fp "file=%s\n" file_uri;
> + fp "arch=%s\n" arch;
> + may (fp "sig=%s\n") signature_uri;
> + (match checksums with
> + | None -> ()
> + | Some checksums ->
> + List.iter (
> + fun c ->
> + fp "checksum[%s]=%s\n"
> + (Checksums.string_of_csum_t c) (Checksums.string_of_csum c)
> + ) checksums
> + );
> + fp "revision=%s\n" (string_of_revision revision);
> + may (fp "format=%s\n") format;
> + fp "size=%Ld\n" size;
> + may (fp "compressed_size=%Ld\n") compressed_size;
> + may (fp "expand=%s\n") expand;
> + may (fp "lvexpand=%s\n") lvexpand;
> + List.iter (
> + fun (lang, notes) ->
> + let format_notes notes =
> + String.trim (Str.global_replace (Str.regexp "^\\([^ ]\\)" ) " \\1" notes) in
Hmm this is still expensive, and partially incorrect (which your tests
shows too): if the index entry was:
Notes=SomeDistro
This is the version of
SomeDistro 1.0
then it will be parsed as
"SomeDistro\n\nThis is the version of\n SomeDistro 1.0"
but then the above format_notes will produce:
Notes=SomeDistro
This is the version of
SomeDistro 1.0
(note one missing space at the beginning of the last line.)
Also, the trim will remove extra spaces at the beginning or end of
descriptions, which were kept when parsing.
Also, I'd put this helper function outside the loop, since it does not
depend on anything of the loop itself (it already takes the string as
parameter).
> + match lang with
> + | "" -> fp "notes=%s\n" (format_notes notes)
> + | lang -> fp "notes[%s]=%s\n" lang (format_notes notes)
> + ) notes;
> + (match aliases with
> + | None -> ()
> + | Some l -> fp "aliases=%s\n" (String.concat " " l)
> + );
> + if hidden then fp "hidden=true\n";
> + fp "\n"
> diff --git a/builder/index_parser.mli b/builder/index_parser.mli
> index b8d8ddf3d..7c1c423ad 100644
> --- a/builder/index_parser.mli
> +++ b/builder/index_parser.mli
> @@ -17,3 +17,9 @@
> *)
>
> val get_index : downloader:Downloader.t -> sigchecker:Sigchecker.t -> Sources.source -> Index.index
> +(** [get_index download sigchecker source] will parse the source index file
> + into an index entry list. *)
> +
> +val write_entry : out_channel -> (string * Index.entry) -> unit
> +(** [write_entry chan entry] writes the index entry to the chan output
> + stream.*)
> diff --git a/builder/index_parser_tests.ml b/builder/index_parser_tests.ml
> new file mode 100644
> index 000000000..e254fbf84
> --- /dev/null
> +++ b/builder/index_parser_tests.ml
> @@ -0,0 +1,95 @@
> +(* builder
> + * Copyright (C) 2017 SUSE 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.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + *)
> +
> +(* This file tests the Index_parser module. *)
> +
> +open OUnit2
> +open Printf
> +open Unix_utils
> +open Common_utils
> +
> +let tmpdir = Mkdtemp.temp_dir "guestfs-tests." "";;
> +rmdir_on_exit tmpdir
> +
> +let dummy_sigchecker = Sigchecker.create ~gpg:"gpg"
> + ~check_signature:false
> + ~gpgkey:Utils.No_Key
> + ~tmpdir
> +
> +(* Utils. *)
> +let assert_equal_string = assert_equal ~printer:(fun x -> sprintf "\"%s\"" x)
> +
> +let test_open_out () =
> + open_out (tmpdir // "out")
> +
> +let test_read_out chan =
> + close_out chan;
> + read_whole_file (tmpdir // "out")
These two functions are somehow odd -- I'd have:
- one that takes a list of pairs (name * entry), and writes it to a file
- one that reads a file from the temporary directory
i.e. something like:
val write_entries : string -> (string * entry) list -> unit
val read_file : string -> string
so the code below can be...
> +let test_write_complete ctx =
> + let entry =
> + ("test-id", { Index.printable_name = Some "test_name";
> + osinfo = Some "osinfo_data";
> + file_uri = "test/path";
> + arch = "test_arch";
> + signature_uri = None;
> + checksums = Some [Checksums.SHA256 "256checksum"; Checksums.SHA512 "512checksum"];
> + revision = Utils.Rev_int 42;
> + format = Some "qcow2";
> + size = Int64 .of_int 123456;
Extra space right after Int64.
> + compressed_size = Some (Int64.of_int 12345);
> + expand = Some "/dev/sda1";
> + lvexpand = Some "/some/lv";
> + notes = [ ("", "Notes split\non several lines\n with starting space") ];
> + hidden = false;
> + aliases = Some ["alias1"; "alias2"];
> + sigchecker = dummy_sigchecker;
> + proxy = Curl.UnsetProxy }) in
> +
> + let chan = test_open_out () in
> + Index_parser.write_entry chan entry;
> + let actual = test_read_out chan in
write_entries "out" [entry];
let actual = read_file "out" in
> + let expected = "[test-id]
> +name=test_name
> +osinfo=osinfo_data
> +file=test/path
> +arch=test_arch
> +checksum[sha256]=256checksum
> +checksum[sha512]=512checksum
> +revision=42
> +format=qcow2
> +size=123456
> +compressed_size=12345
> +expand=/dev/sda1
> +lvexpand=/some/lv
> +notes=Notes split
> + on several lines
> + with starting space
> +aliases=alias1 alias2
> +
> +" in
> + assert_equal_string expected actual
An addendum here could be to parse the file again, and compare the
newly parsed entry with what was written in the file.
Thanks,
--
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20170309/a34b5699/attachment.sig>
More information about the Libguestfs
mailing list