Re: [Monotone-devel] review of nvm.automate_out_of_band

[Timothy Brownawell]

Thomas Keller wrote:
> Am 22.11.09 20:26, schrieb Thomas Keller:
>>> ...hm, how much do we care about keeping stdio output compatible? Right
>>> now the error indicator distinguishes between normal output and error
>>> messages and doubles as the return code, and the more/last indicator is
>>> just an EOF marker. So the most compatible thing might be to leave the
>>> more/last indicator alone, and indicate alternate streams by setting the
>>> 'error' field to something silly, like 128 and over (and then just set
>>> the actual exit code for the last (empty) packet). But then if frontends
>>> actually error out on unknown interface_version's this doesn't really
>>> matter.
>> I don't think its actually feasible to have multiple meanings of the
>> error indicator within the output of a single call (after all, the
>> program either executes successfully or fails with an specific code). If
>> we don't care about breaking stdio, we could catch two flies: Make the
>> 'l' stream really the last thing every command issues (without any more
>> payload) and encode the return code of the command as payload of this
>> stream. This would then also mean that we could remove the individual
>> error indicator from all other streams, i.e.
>>
>>      l5:headse
>>
>> would lead to
>>
>>      0:m:41:503c40cda521bbba7dd971110d50b265681979cb
>>      0:l:1:0
>>
>> instead of
>>
>>      0:0:l:41:503c40cda521bbba7dd971110d50b265681979cb
> 
> This has been implemented in 6848a33ed9bbffebbfa5654648af1d5ce36b041a -
> of course this breaks BC with earlier monotone versions, but its only a
> small change in everyone's stdio parser implementation (we have three or
> four, including my own one for guitone, Thomas Moschny's for TracMTN,
> ViewMTN's [which might be actually the same since its Python as well]
> and the one from mtn-browse, which is Perl, iirc) and it makes the whole
> thing much clearer and straight-forward (actually I think we should have
> done this right from the start).
> 
> What do you think?

I like it and it really does make more sense, the only issue is if
interface_version is being checked through stdio instead of as a
standalone command. Probably stdio/remote_stdio should start by
outputting <stdio-version> <whitespace> in case we eventually change the
stdio format again (not interface_version, something specific to stdio.
interface_version changes when even some command you don't care at all
about changes formats. we also need a "command_version command ..." that
can list versions (including "does not exist") of specific automate
commands, which is entirely unrelated to this discussion except that
what it would print for "command_version stdio" is what I'm thinking
stdio should start with.). That way if we change the format again,
frontends won't have to call interface_version separately and will be
able to give better error messages ("please upgrade") than just failing
to parse the output when they don't call it separately.