Re: [Monotone-devel] New commands (for mtn, in lua)

[Nathaniel Smith]

On Tue, Sep 04, 2007 at 09:52:06PM +1000, William Uther wrote:
> On 04/09/2007, at 6:45 PM, Nathaniel Smith wrote:
> >On Thu, Jul 05, 2007 at 03:09:04PM -0700, William Uther wrote:
> >>  register_command(command, abstract, description, lua_function) :
> >>Adds a new command to the "user" group of commands.  When the command
> >>is called, the associated lua function is executed.  That lua will
> >>normally use mtn_automate() calls to do its work.
> >>
> >>I also added automate versions of the following normal commands:
> >>push, pull, sync, merge, update and commit.  These are currently not
> >>very 'automate' friendly in that they still output everything to
> >>standard out.  They work for this purpose though.
> >
> >The requirement for landing a new automate command in mainline is that
> >the semantics (what it does) and interface (how you tell it what to do
> >and how it tells you what its done) must be fully documented and
> >tested.  Also, we should be willing to keep those semantics and
> >interface fixed going forward.  Having automate access to the
> >functionality you mention would indeed be nice; you can decide what
> >the easiest way to get commands that expose that functionality and
> >meet the bar for automate inclusion is.
> 
> The documentation is not great.  I have already merged with trunk.
> 
> Would you like me to disapprove?

I tried to look at the documentation in trunk, but I can't find it at
all.  Is it even documented in monotone.texi?

The --help docstrings are okay for just docstrings, but definitely not as
actual documention.  There is Massive Magic in update, commit, merge
at least -- for instance proper documentation, by automate standards,
would have to say something about how update determines what to update
to (it is *not* "the head of the current branch"), how commit decides
the what the commit message should be (this is like 20 lines of
logic), and so on.  The netsync commands probably aren't so bad, but
even then there's all the stuff with the default sync target being
magically read and sometimes magically set.

Basically, the problem is that there's no-where near enough
information here to write programs that talk to these commands and
get reliable results.  The only programs you can write are the ones
who actually want the interface specification to be "do whatever you'd
do if the user made some request directly in the user-level
interface", i.e., programs that are just thin proxies between the user
and mtn's normal interface.

AFAICT there are also no tests for the new automate commands
whatsoever?  (I may have missed something.)

Having no tests is unacceptable in general; having no tests and having
no docs in the manual are both automatic reject-this-patch checkboxes
for automate commands in particular; these particular commands seem to
have some major problems being automate-friendly as well.  Sorry to be
so negative :-/.


There is nothing wrong with writing programs that are thin wrappers
around mtn's normal user interface, though, just we can't make the
same kind of guarantees for those commands stability, so they need to
be set apart from the rest of automate. Would it make people happier
if we added an automate command 'mtn automate user <foo>' which does
exactly the same thing as 'mtn <foo>' except with output redirection
and it can be used over automate stdio?

> The netsync operations still output their ticker to stdio.  You can  
> turn it off with
> the normal --ticker option.  There isn't much other interface there.
>
> The other commands, update and commit, do have defined interfaces.   
> I'm not sure that
> they're the best interfaces yet, and they are only defined in

only defined in...?

> >>I've also added contrib/extra-commands.lua which has the following:
> >
> >In principle I have no real problem with making monotone more
> >user-extendable; it can be a great way to prototype new features, see
> >how useful they are in the wild and whether they're worth folding into
> >core, etc.
> 
> That was my main aim.

Cool.

> >By the time we're actually be *shipping* these in our own source
> >tarball, though, maybe we should be considering whether we're just
> >adding these features as extensions because we don't have the communal
> >will to make the program itself better.  (See also
> >http://ometer.com/free-software-ui.html)
> 
> How can you see how useful something is in the wild without shipping it?

By 'shipping' I mean in particular 'in the official release tarball'.
Not that contrib/ is particularlly official, even if it is part of the
official tarball.  I'm not really objecting to the particular case,
just raising a warning flag for us to keep in mind...

> I also think the monotone interface needs work.  I don't want to get  
> into
> a big argument about it though.  I'd rather propose solutions.
> 
> However, I don't claim to be able to get it right first time either.   
> I want
> an easy way to try a bunch of commands and see which work for me over  
> time.
> It is an easy way to prototype.

Cool.

> My current set of commands is here:
> 
> http://www.cse.unsw.edu.au/~willu/monotone-patches/mtnrc
> 
> Note that I'm using alias_command for more than just blame and  
> praise.  It also
> allows you to name your commands properly and add short aliases just  
> like a C++
> command.

Yes, if you can define new commands, you should definitely also be
able to alias them.

> Also note that I have error checking in there now.  I just haven't  
> had time to
> commit this yet.

What's the interface like for getting errors out of mtn_automate()?

> >It looks like in both of the examples, you actually don't want  
> >automate
> >at all -- you're not doing anything programmatic against mtn's data
> >model, you're just driving the user interface.
> 
> Not entirely true.  The remote_commit command gathers the number of  
> heads
> and outputs a warning if there is more than one.  It asks the user to
> merge and check that the merged revision is ok before pushing.

Okay, so you're doing some things programmatic, and some things
driving the user interface.  These are still different things :-).

> >If what you want is to
> >just run user commands, maybe it would be better to be running user
> >commands than to first move them over to automate (in a sort of
> >half-done way) and then use them via automate?
> 
> There had been requests on the mailing list for a more complete  
> automate set.
> I was also planning to add "add" and "drop", etc, but I haven't had  
> time.

Those would be very cool indeed, though again the user commands have
lots of weird and magic stuff in them, so we want to be careful to add
things with reliable semantics.

> >Another approach to consider might be to have lua commands that were
> >run with very little mtn infrastructure baked in around them -- like
> >add a way to use mtn as just a lua interpreter (one that is always
> >available when mtn is), that has no additional fanciness except maybe
> >scripts are told where the mtn binary is and what command line options
> >they might want to pass.  Then scripts could just shell out to mtn
> >like any other app, and they could use the command line options thing
> >to pass on -d and friends correctly.  This would make writing such
> >scripts somewhat more complicated -- you'd need the same shelling-out
> >support code that other frontends to mtn need -- but that's code that
> >only needs to be written once and then we have everyone working on
> >improving one interface, instead of splitting development effort
> >across two.
> 
> That is possible.  I didn't consider that.  Would you prefer that
> solution?  I don't think it is better than what is there currently.

The advantages would presumably be:
  -- somewhat less maintainence work, maybe
  -- you could call both automate and user commands with impunity.
The latter is really the reason I was thinking of -- as you may have
gathered, the tension between UI-level and engine-level interfaces
seems pretty central to all of this stuff we're talking about to me
:-).

> The bottom line here: I've become pretty busy lately.  I don't have time
> to fine-tune this right now.  If there are parts of this that are a  
> problem
> merged into trunk, then I apologise - feel free to revert them.

I guess for me there are two parts here: the new automate commands,
and the extension command stuff.  The former part I discussed above.
The latter I don't have any major objection too, so long as it's
documented and tested (it is documented and tested, right?  I'm having
a lot of trouble figuring out your changes, because somehow you seem
to have not sent the .lua_cmds branch certs to monotone.ca?  Anyway,
experience has shown that documentation and tests get written before
merging or they don't get written at all, so we try to be anal when we
can).

Could you revert the parts that don't meet this?  Like I said, I'm
having trouble tracking down what happened.

-- Nathaniel

-- 
Damn the Solar System.  Bad light; planets too distant; pestered with
comets; feeble contrivance; could make a better one myself.
  -- Lord Jeffrey