Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add binary xapi_gzip for testing Xapi_compression #4685

Merged
merged 1 commit into from
Apr 20, 2022

Conversation

lindig
Copy link
Contributor

@lindig lindig commented Apr 14, 2022

The compression library Xapi_compression spawns processes to compress
and uncompress streams. To facilitate easier testing, add a small
stand-alone filter that compresses or uncompresses its standard input.
This filter requires the presence of forkexecd to work - so typically a
Citrix Hypervisor installation. We don't install this binary but it is
available for developers.

Signed-off-by: Christian Lindig christian.lindig@citrix.com

gunzip Unix.stdin Unix.stdout

let decompress =
let doc = "Decomppress." in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: pp


module C = Cmdliner

module Gzip = Xapi_compression.Make (struct let executable = "/bin/gzip" end)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a gzip module that does this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, but I wanted to avoid the dependency given that it depends on this opam package - that would be circular.

let doc = "Test compression and uncompression" in
C.Term.(const compress $ decompress, info "xapi-gzip" ~doc ~man:help)

let () = if !Sys.interactive then () else C.Term.(exit @@ eval main)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting way of making the main module testable from utop, I'll keep this in mind


(** copy bytes from file descriptor [src] to [dst] *)
let copy ~src ~dst =
let size = 1024 in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@psafont psafont left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding an empty xapi.gzip.mli will make the CI happy and highlight any unused bindings.

The Ci is also unhappy about missing labels:

File "ocaml/libs/xapi-compression/xapi_gzip.ml", line 44, characters 50-54:
44 | let gzip ~src ~dst = Gzip.compress dst (fun fd -> copy src fd)
                                                       ^^^^
Error (warning 6 [labels-omitted]): labels src, dst were omitted in the application of this function.
File "ocaml/libs/xapi-compression/xapi_gzip.ml", line 46, characters 62-66:
46 | let gunzip ~src ~dst = Gzip.decompress_passive src (fun fd -> copy fd dst)
                                                                   ^^^^
Error (warning 6 [labels-omitted]): labels src, dst were omitted in the application of this function.
File "ocaml/libs/xapi-compression/xapi_gzip.ml", line 51, characters 6-10:
51 |       gzip Unix.stdin Unix.stdout
           ^^^^
Error (warning 6 [labels-omitted]): labels src, dst were omitted in the application of this function.
File "ocaml/libs/xapi-compression/xapi_gzip.ml", line 53, characters 6-12:
53 |       gunzip Unix.stdin Unix.stdout
           ^^^^^^
Error (warning 6 [labels-omitted]): labels src, dst were omitted in the application of this function.

The compression library Xapi_compression spawns processes to compress
and uncompress streams. To facilitate easier testing, add a small
stand-alone filter that compresses or uncompresses its standard input.
This filter requires the presence of forkexecd to work - so typically a
Citrix Hypervisor installation. We don't install this binary but it is
available for developers.

Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
@lindig lindig merged commit 441cc4e into xapi-project:master Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants