[Monotone-devel] Updated Issue 132 - mtn_automate() does not allow the execution of the `remote' command (monotone)

[code]

Hello,

The following issue has been updated:

132 - mtn_automate() does not allow the execution of the `remote'  command
Project: monotone
Status: Accepted
Reported by: Thomas Keller
URL: https://code.monotone.ca/p/monotone/issues/132/
Labels:
 Type:Incorrect Behavior
 Priority:Medium

Comments (last first):

# By Timothy Brownawell, Jan 18, 2011:

Calling remote/push/pull/sync via remote or remote_stdio will block the 
server's event loop (well, everything really blocks the event loop for its 
duration. But network commands tend to be slower.).

Calling *stdio inside *stdio doesn't make much sense because you have to 
provide all the input up front (or maybe it's still useful just as a batching 
mechanism).

Calling stdio inside either stdio or remote_stdio will screw with 
sanity::set_out_of_band_handler, because that isn't reentrant (it would need to 
be replace/restore instead of set/reset). Calling remote_stdio doesn't do this, 
because the handler is on the target server.

I think all we really *need* to prevent is stdio inside of either stdio or 
remote_stdio, but the get_remote_automate_permitted documentation should note 
that network commands and stdio might stall the event loop longer than other 
things.

# By Thomas Keller, Jan 17, 2011:

I obviously forgot the "remote_stdio in remote_stdio" pair which should also be 
prevented.

I also tested out remote in remote_stdio and this worked just well. remote in 
stdio worked as well, but leaves some out-of-band gunk that breaks stdio. I'll 
try to fix that and afterwards enable remote in stdio and remote_stdio (and of 
course I'll write test for that so it does not break again).

 Status: Accepted
 Owner: tommyd

# By Thomas Keller, Jan 12, 2011:

Steps to reproduce the problem:
-------------------------------

Write a small hook / user command which calls mtn_automate("remote", 
"--remote-stdio-host=mtn://code.monotone.ca/monotone", "branches")

Expected result:
----------------

See the list of branches from the remote monotone server.


Actual results:
---------------

automate.cc:2448: detected network error, 'E(acmd->can_run_from_stdio())' 
violated

Output of `mtn version --full`:
-------------------------------

0.99.1 and probably earlier. This was likely introduced when the stdio setup 
code was refactored. A quick fix might be to add a setter for stdio_ok in 
cmd_automate.cc (set_can_run_from_stdio) and call this setter later in the same 
file in LUAEXT(mtn_automate) conditionally when the command "remote" should be 
executed.

Maybe it would also be a good idea to rethink the stdio_ok flag, i.e. why 
shouldn't it be allowed to call "remote" also via stdio? Things we'd like to 
block because of otherwise mixed up communication streams are

* calling stdio in remote_stdio
* calling stdio in stdio
* calling remote_stdio in stdio
* calling remote inside remote_stdio (never tried that, it could 
_theoretically_ work, but it also might break horribly)

So the first four items are blocked nicely with the existing stdio_ok, maybe we 
should just find out whether it would hurt us a lot to switch 
CMD_AUTOMATE_NO_STDIO(remote) to CMD_AUTOMATE(remote).



--
Issue: https://code.monotone.ca/p/monotone/issues/132/