monotone

Issue 132: mtn_automate() does not allow the execution of the `remote' command

Reported 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).

Comment 1 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

Comment 2 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.

Created: 8 years 7 months ago by Thomas Keller

Updated: 8 years 6 months ago

Status: Accepted

Owner: Thomas Keller

Followed by: 1 person

Labels:
Type:Incorrect Behavior
Priority:Medium

Quick Links:     www.monotone.ca    -     Downloads    -     Documentation    -     Wiki    -     Code Forge    -     Build Status