[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [MirageOS-devel] command line arguments for unikernels

Thanks very much for the detailed comments. I am responding inline
below. I will also be updating the document and the code soon to
reflect the new suggestions.

On Thu, Dec 4, 2014 at 4:12 PM, Daniel BÃnzli
<daniel.buenzli@xxxxxxxxxxxx> wrote:
> Le jeudi, 4 dÃcembre 2014 Ã 16:02, Thomas Gazagnaire a Ãcrit :
>> I am not very fond of the default references, not sure why you want someone 
>> to change them globally.
> Same reaction, changing defaults by side effects is a recipe for crazy 
> debugging sessions. Always assume users will abuse what you put in their 
> hands; so minimize the exposition of (unecessary) side-effects.

Agreed.  This is going away.

> Other things:
> 1) You need to tell us something about some notion of identity for the 
> parameters. How is it defined
> is it the name ? What happens if I declare two parameters with the same name 
> [1] ?

Right now they are defined by their name, and it is a fatal error if
you use two different parameters with the same name.  But I like the
idea of
emitting a warning and renaming the parameter if there is a clash.

> 2) In the manpage I would list the parameters under a different section (e.g. 

Good idea.  I had forgotten about this feature of `Cmdliner`.

> 3) I would prefix or suffix the command line option name with a constant, 
> this ensures that unikernel developers will never clash with options of the 
> mirage tool. So for a parameters named "address" the option names should be  
> `--address-param` or `--param-address`

Again, I think it is an excellent idea.

> 4) Is the no [def] with [dyn] unspecified option runtime error a feature ? If 
> not it's easy to ensure statically you always have a value to use. Are you 
> using cmdliner's required optional arguments for that (it seems a legitimate 
> use for once, as it would be difficult to make positional arguments in a 
> compositional manner) ?

I was told of at least one use-case where it would not make sense to
require a default value for an argument that is meant to be set at
run-time, so I would like to support this as well.

And yes, right now I am using Cmdliner's required optional arguments
for this feature.

> Best,
> Daniel
> P.S. Rather than configuration Param I would use configuration Key which is 
> shorter and a full word, but naming is subjective.

Agreed, sounds and looks better as well.


Best wishes,

> [1]  That's:
> https://github.com/samoht/assemblage/blob/757b36c6b880b3a86b6177277e2a66555bfb7af2/lib/assemblage.mli#L1091-L1096
> what assemblage says on the topic. Basically we have uid internally so we 
> don't discriminate on the name. However that warning in the documentation is 
> over pessimistic. In fact if I get two configuration keys with the same name 
> I emit a warning and generate an alternate name for the option on the command 
> line (e.g. --dupname~1-key). See here:
> https://github.com/samoht/assemblage/blob/757b36c6b880b3a86b6177277e2a66555bfb7af2/lib-driver/assemblage_driver.ml#L42-L53
>  but if you have two configuration with the same name,  I think that in 
> assemblage this is not a real problem (unless you load more than one project)

MirageOS-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.