Re: [Libguestfs] [PATCH] generator: Share Common_utils code.
On Fri, Dec 09, 2016 at 10:20:36AM +0100, Pino Toscano wrote:
On Thursday, 8 December 2016 10:36:45 CET Richard W.M. Jones wrote:
> For a very long time we have maintained two sets of utility functions,
> in mllib/common_utils.ml and generator/utils.ml. This changes things
> so that the same set of utility functions can be shared with both
> directories.
>
> It's not possible to use common_utils.ml directly in the generator
> because it provides several functions that use modules outside the
> OCaml stdlib. Therefore we add some lightweight post-processing which
> extracts the functions using only the stdlib:
>
> (*<stdlib>*)
> ...
> (*</stdlib>*)
One idea here: instead of using a custom <stdlib> markup and sed code
for it, what about using the standard C preprocessor for this?
I.e. have comments like:
(*
#ifdef STDLIB
*)
...
(*
#endif
*)
and using cpp -DSTDLIB ... to output that.
OK good idea, I'll do that instead.
> diff --git a/generator/utils.ml b/generator/utils.ml
> index 3e81433..ba5e045 100644
> --- a/generator/utils.ml
> +++ b/generator/utils.ml
> @@ -23,6 +23,8 @@
> * makes this a bit harder than it should be.
> *)
>
> +open Common_utils
> +
> open Unix
> open Printf
>
> @@ -119,85 +121,6 @@ let rstructs_used_by functions =
>
> let failwithf fs = ksprintf failwith fs
>
> -let unique = let i = ref 0 in fun () -> incr i; !i
This seems to be used only in generator, so I'd leave it here for now
(reduces the changes in this patch, and it can always be moved later
on when needed).
[...]
Ditto.
Since these were generically useful functions I felt we might as well
have them in the common utils code. Maybe we'll use them in future.
I don't know if this makes binaries bigger -- possibly it does by a
small amount.
> -let spaces n = String.make n ' '
Oh there's a copy of this in mllib/getopt.ml, so could you please
include that in this patch?
OK.
> diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
> index 78618f5..e1d1ab8 100644
> --- a/mllib/common_utils.ml
> +++ b/mllib/common_utils.ml
> @@ -16,7 +16,13 @@
> * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> *)
>
> +(* The parts between <stdlib>..</stdlib> are copied into the
> + * generator/common_utils.ml file. These parts must ONLY use
> + * functions from the OCaml stdlib.
> + *)
> +(*<stdlib>*)
> open Printf
> +(*</stdlib>*)
Theoretically this could stay in both version, couldn't it? This way
there are less changes between the mllib version and the generation one.
Not sure I understand.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW