Libguestfs - Mar 2019 - Re: [PATCH 2/3] v2v: add Var_expander

On Mon, Feb 25, 2019 at 05:22:51PM +0100, Pino Toscano wrote:
[...]

After being burned a few times with custom parsing (hello, guestfish)
I'm not a big fan.

Is there not an existing C or OCaml library/facility we could use
here?  It's a shame we can't use Perl Template Toolkit because it
would be ideal here.

There are all kinds of questions that aren't answered such as: Should
variables be replaced recursively?  How do you escape %{..} if you
don't want it to be replaced?  Should we allow loops or similar
constructs?  Existing template systems solve these kinds of problems
already.

Anyway ...
> +let var_re = PCRE.compile "%{([^}]+)}"
Are we planning to allow a completely free choice for variable names,
or could we limit this regexp to only matching ASCII alphanumeric +
underscore?
> +let check_variable var > +  String.iter (
> +    function
> +    | '0'..'9'
> +    | 'a'..'z'
> +    | 'A'..'Z'
> +    | '_'
> +    | '-' -> ()
> +    | _ -> raise (Invalid_variable var)
> +  ) var
... and then this function would presumably go away.

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

On Monday, 25 March 2019 15:55:46 CET Richard W.M. Jones
wrote:
> On Mon, Feb 25, 2019 at 05:22:51PM +0100, Pino Toscano wrote:
> [...]
> 
> After being burned a few times with custom parsing (hello, guestfish)
> I'm not a big fan.
I can perfectly understand that, however ...
> Is there not an existing C or OCaml library/facility we could use
> here?  It's a shame we can't use Perl Template Toolkit because it
> would be ideal here.
... sadly I did not find anything simple convering the use case that
this Var_expander module covers.  The closest thing I found was the
usage of ${..}/$(..) variables in dune (the OCaml build system), with
the following differences:
- manually tokenizes the string
- allows anything as variable name, splitting it in two if a ':' is
  found
- ${..}/$(..) instead of %{..}
> There are all kinds of questions that aren't answered such as: Should
> variables be replaced recursively?
> [...]
> Should we allow loops or similar constructs?
I don't think they are needed.
> How do you escape %{..} if you don't want it to be replaced?
Good question... dune does not seem to allow that; maybe we can allow
a trailing '$'/'\' as escape character.
> Existing template systems solve these kinds of problems already.
Yes, however they are much more complex, and with a number of features
like conditionals, filters, loops, etc.  Considering this is needed so
far for the file paths of disks in -o json, IMHO plain variables are
enough.
> > +let var_re = PCRE.compile "%{([^}]+)}"
> 
> Are we planning to allow a completely free choice for variable names,
> or could we limit this regexp to only matching ASCII alphanumeric +
> underscore?
This regex allows anything as variables for two reasons:

1) we do not miss any variable-like pattern (so we do not silently
   skip any now, while handling it in the future in case we accept
   more characters)
> > +let check_variable var > > +  String.iter (
> > +    function
> > +    | '0'..'9'
> > +    | 'a'..'z'
> > +    | 'A'..'Z'
> > +    | '_'
> > +    | '-' -> ()
> > +    | _ -> raise (Invalid_variable var)
> > +  ) var
> 
> ... and then this function would presumably go away.
2) we can check that a variable has only allowed characters, and report
   that to the user

-- 
Pino Toscano

On Fri, Mar 29, 2019 at 01:38:55PM +0100, Pino Toscano
wrote:
> On Monday, 25 March 2019 15:55:46 CET Richard W.M. Jones wrote:
> > On Mon, Feb 25, 2019 at 05:22:51PM +0100, Pino Toscano wrote:
> > [...]
> > 
> > After being burned a few times with custom parsing (hello, guestfish)
> > I'm not a big fan.
> 
> I can perfectly understand that, however ...
> 
> > Is there not an existing C or OCaml library/facility we could use
> > here?  It's a shame we can't use Perl Template Toolkit because
it
> > would be ideal here.
> 
> ... sadly I did not find anything simple convering the use case that
> this Var_expander module covers.  The closest thing I found was the
> usage of ${..}/$(..) variables in dune (the OCaml build system), with
> the following differences:
> - manually tokenizes the string
> - allows anything as variable name, splitting it in two if a ':' is
>   found
> - ${..}/$(..) instead of %{..}
> 
> > There are all kinds of questions that aren't answered such as:
Should
> > variables be replaced recursively?
> > [...]
> > Should we allow loops or similar constructs?
> 
> I don't think they are needed.
> 
> > How do you escape %{..} if you don't want it to be replaced?
> 
> Good question... dune does not seem to allow that; maybe we can allow
> a trailing '$'/'\' as escape character.
RPM uses double %%
> > Existing template systems solve these kinds of problems already.
> 
> Yes, however they are much more complex, and with a number of features
> like conditionals, filters, loops, etc.  Considering this is needed so
> far for the file paths of disks in -o json, IMHO plain variables are
> enough.
> 
> > > +let var_re = PCRE.compile "%{([^}]+)}"
> > 
> > Are we planning to allow a completely free choice for variable names,
> > or could we limit this regexp to only matching ASCII alphanumeric +
> > underscore?
> 
> This regex allows anything as variables for two reasons:
> 
> 1) we do not miss any variable-like pattern (so we do not silently
>    skip any now, while handling it in the future in case we accept
>    more characters)
So I think you're saying that if we see (for example) this input:

  { foo: "%{" }

we want %{ to be flagged as an error instead of being ignored?  I
guess could argue this one both ways, but yours is a reasonable
explanation.
> > > +let check_variable var > > > +  String.iter (
> > > +    function
> > > +    | '0'..'9'
> > > +    | 'a'..'z'
> > > +    | 'A'..'Z'
> > > +    | '_'
> > > +    | '-' -> ()
> > > +    | _ -> raise (Invalid_variable var)
> > > +  ) var
> > 
> > ... and then this function would presumably go away.
> 
> 2) we can check that a variable has only allowed characters, and report
>    that to the user
Anyway that's for investigating.  If there's no alternative I guess
we'll need to stick to hand-parsing.

Rich.


-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/