[Libguestfs] [PATCH] generator: Share Common_utils code.

[Richard W.M. Jones]

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