run-program :output :interactive sometimes yields no output

Bug #1638870 reported by pipping
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
ASDF
Fix Released
Undecided
Unassigned

Bug Description

This has gone unnoticed primarily because I wouldn't know of a way to test this.

All of this is with ASDF 3.1.7.32. Consider e.g. this:

> (asdf:implementation-identifier)
"ecl-16.1.2-unknown-linux-x64"
> (prog1 t (uiop:run-program '("echo" "hello") :output :interactive))
hello

T
> (prog1 t (uiop:run-program '("echo" "hello") :output :interactive :force-shell t))

T
>

Or this (exactly the other way around):

: (asdf:implementation-identifier)
"abcl-1.4.0-fasl42-linux-x64"
: (prog1 t (uiop:run-program '("echo" "hello") :output :interactive))
T
: (prog1 t (uiop:run-program '("echo" "hello") :output :interactive :force-shell t))
hello
T
:

pipping (pipping)
summary: - :output :interactive sometimes yields no output
+ run-program :output :interactive sometimes yields no output
Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

Is it possible to rebind *TERMINAL-IO* as a way of testing this?

Revision history for this message
Robert P. Goldman (rpgoldman) wrote :

Another possibility, it seems to me, is to just stop supporting :interactive, since it doesn't work reliably. If a programmer wants interactive output, they just dump the output themselves. Interactive input is so wobbly that I'm happy to give that up. Again, if you want it, set up streams yourself, collect user input, and throw it at the program.

Revision history for this message
pipping (pipping) wrote :

Unfortunately, by far not all CL implementations allow a foreign stream to be used for output.

What we can currently guarantee is:
 - You can write to a file
 - You can write to a string (through a file; requires waiting)
 - You can request a stream via :stream (with launch-program only)

The :stream option is probably not very helpful here because then you'd have to take care of checking up on new output every now and then.

Revision history for this message
Faré (fahree) wrote :

I believe this feature is very useful on SBCL at least, and would be grateful that it shouldn't be removed from implementations that support it. I'm alright with a parameter-error being thrown on implementations known not to support it, however.

Revision history for this message
pipping (pipping) wrote :

I've looked a bit more at the handling of :interactive. I've so far found three issues.

 - An issue with ABCL. That was a bug in uiop/run-program (and not ABCL itself). Easily fixed, fortunately; please see also https://gitlab.common-lisp.net/asdf/asdf/merge_requests/46
 - An issue with ECL that I still need to look into
 - An inconsistency that affects every single CL implementation.

I think we can save interactive output; the worst possible outcome (with the ABCL fix in place) would be that it couldn't be supported on ECL (but I'm still optimistic that we can fix that in UIOP or ECL itself) which would still be better than dropping interactive output altogether. That said, let's talk about the inconsistent behaviour:

The inconsistency can be viewed as a regression, unfortunately, in the sense that it affects interfaces which are only now public with 3.1.7.x:

run-program contains

    (flet ((default (x xp output) (cond (xp x) ((eq output :interactive) :interactive))))
             [..]
             :input input (default input inputp output)
             :error-output (default error-output error-output-p output)
             [..])

The effect is:

  (run-program prog) behaves like (run-program prog :output nil :error-output nil)

but

  (run-program prog :output :interactive) behaves like (run-program prog :output :interactive :error-output :interactive)

In particular,

  (uiop:run-program "echo test >&2")

will print nothing but

  (uiop:run-program "echo test >&2" :output :interactive)

will print "test" because of the implied :error-output :interactive.

I find this to be surprising particularly because it's not documented anywhere (but then neither is what the default for :output or :error-output is). I was not aware of this behaviour, so that I did not copy it over to launch-program. Consequently, the inconsistent behaviour is that

  (launch-program prog) behaves like (run-program prog :output nil :error-output nil)

and

  (launch-program prog :output :interactive) behaves like (run-program prog :output :interactive :error-output nil)

But because none of the inconsistent behaviour I'm describing is documented everywhere, I believe the following two paths are approximately equally acceptable:

 - copy over run-program's behaviour to launch-program.
 - copy over launch-program's behaviour to run-program.

The advantage of the former is that run-program has been around for longer. The advantage of the latter is that it's less magical(*): The default for both :output and :error-output would be nil and if you change one, you change just that one and nothing else (I don't have a strong preference for either).

PS: If I understand that correctly, :output :interactive also sets :input :interactive? That's actually even weirder...

Revision history for this message
pipping (pipping) wrote :

A quick look at quicklisp shows that only a single project passes :output :interactive directly to run-program (I did not check for things like (let ((x :interactive)) (run-program :output x))) and that's

quicklisp/dists/quicklisp/software/eazy-gnuplot-20160825-git/src/package.lisp
185: (uiop:run-program *gnuplot-home*
186- :input in
187- :output :interactive
188- :error-output :interactive

which passes :error-output :interactive as well and explicitly overrides :input. That's a good indication that nobody's relying on the current run-program defaults.

Revision history for this message
Faré (fahree) wrote :

Some historic remarks:

* My original implementation of xcvb-driver:run-program/, that became uiop:run-program as part of ASDF 2.27 (initial pre-release of ASDF 3), only had a :output argument, and no :input or :error-output argument. That was all that was needed to replace asdf:run-shell-command with something that properly captures output. And it had been hell enough to write a fully portable run-program that way, especially considering how wildly implementations diverge in terms of run-program capabilities.

* Because there was no :input or :error-output argument, but I wanted an interactive mode (to allow for interactive debugging of the subprocess in case of failed compilation), :input and :error-output were then nil by default but :interactive if :error was :interactive.

* ASDF 3.0.3 introduced a new and improved run-program that can process :input and :error-output in addition to :output. I kept them the same for backward compatibility, but I may have failed to properly document the above default. Sorry.

Revision history for this message
Faré (fahree) wrote :

Since no one depends on the current defaulting, since a change in defaults and/or behavior is just a wrappping function away (such as inferior-shell:run/i), since no one seems to be relying on the old default, I think it's cleaner to incompatibly simplify away the previous default, that only had a historical backward-compatibility justification.

Revision history for this message
pipping (pipping) wrote :

I've now opened https://gitlab.common-lisp.net/asdf/asdf/merge_requests/47 with a possible fix that follows this route.

Revision history for this message
pipping (pipping) wrote :

I've now opened another MR for the ECL issue. To sum up, the three MRs that address everything observed in this bug are:

 - (ABCL) https://gitlab.common-lisp.net/asdf/asdf/merge_requests/46
 - (All implementations) https://gitlab.common-lisp.net/asdf/asdf/merge_requests/47
 - (ECL) https://gitlab.common-lisp.net/asdf/asdf/merge_requests/48

Revision history for this message
pipping (pipping) wrote :
Download full text (4.8 KiB)

For future reference, here's a script that one can use to check (unfortunately, only with one's own eyes) if an implementation behaves as expected.

SBCL e.g. does (with the change for consistency in place). Here's the output of the script:

====
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :OUTPUT :INTERACTIVE
stdout
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :OUTPUT :INTERACTIVE :FORCE-SHELL T
stdout
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stdout.sh") :OUTPUT :INTERACTIVE
stdout
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :OUTPUT :INTERACTIVE
stdout
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :OUTPUT :INTERACTIVE :FORCE-SHELL T
stdout
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM "./stdout.sh" :OUTPUT :INTERACTIVE
stdout

UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :OUTPUT NIL :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stdout.sh") :OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :OUTPUT NIL :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM "./stdout.sh" :OUTPUT NIL

UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :ERROR-OUTPUT :INTERACTIVE
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :ERROR-OUTPUT :INTERACTIVE :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stdout.sh") :ERROR-OUTPUT :INTERACTIVE
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :ERROR-OUTPUT :INTERACTIVE
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :ERROR-OUTPUT :INTERACTIVE :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM "./stdout.sh" :ERROR-OUTPUT :INTERACTIVE

UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :ERROR-OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :ERROR-OUTPUT NIL :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stdout.sh") :ERROR-OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :ERROR-OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :ERROR-OUTPUT NIL :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM "./stdout.sh" :ERROR-OUTPUT NIL

UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stderr.sh") :OUTPUT :INTERACTIVE
stderr
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stderr.sh") :OUTPUT :INTERACTIVE :FORCE-SHELL T
stderr
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stderr.sh") :OUTPUT :INTERACTIVE
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stderr.sh" :OUTPUT :INTERACTIVE
stderr
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stderr.sh" :OUTPUT :INTERACTIVE :FORCE-SHELL T
stderr
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM "./stderr.sh" :OUTPUT :INTERACTIVE

UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stderr.sh") :OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stderr.sh") :OUTPUT NIL :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stderr.sh") :OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stderr.sh" :OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stderr.sh" :OUTPUT NIL :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM "./stderr.sh" :OUTPUT NIL

UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stderr.sh") :ERROR-OUTPUT :INTERACTIVE
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stderr.sh") :ERROR-OUTPUT :INTERACTIVE :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stderr.sh") :ERROR-OUTPUT :INTERACTIVE
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stderr.sh" :ERROR-OUTPUT :INTERACTIVE
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stderr.sh" :ERROR-OUTPUT :INTERACTIVE :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNC...

Read more...

Revision history for this message
pipping (pipping) wrote :
Download full text (7.0 KiB)

The output I included with my previous comment was unfortunately the mistake of a copy&paste issue.

This is the inconsistent output you would get with the current master branch:

====
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :OUTPUT :INTERACTIVE
stdout
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :OUTPUT :INTERACTIVE :FORCE-SHELL T
stdout
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stdout.sh") :OUTPUT :INTERACTIVE
stdout
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :OUTPUT :INTERACTIVE
stdout
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :OUTPUT :INTERACTIVE :FORCE-SHELL T
stdout
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM "./stdout.sh" :OUTPUT :INTERACTIVE
stdout

UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :OUTPUT NIL :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stdout.sh") :OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :OUTPUT NIL :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM "./stdout.sh" :OUTPUT NIL

UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :ERROR-OUTPUT :INTERACTIVE
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :ERROR-OUTPUT :INTERACTIVE :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stdout.sh") :ERROR-OUTPUT :INTERACTIVE
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :ERROR-OUTPUT :INTERACTIVE
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :ERROR-OUTPUT :INTERACTIVE :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM "./stdout.sh" :ERROR-OUTPUT :INTERACTIVE

UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :ERROR-OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stdout.sh") :ERROR-OUTPUT NIL :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stdout.sh") :ERROR-OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :ERROR-OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stdout.sh" :ERROR-OUTPUT NIL :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM "./stdout.sh" :ERROR-OUTPUT NIL

UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stderr.sh") :OUTPUT :INTERACTIVE
stderr
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stderr.sh") :OUTPUT :INTERACTIVE :FORCE-SHELL T
stderr
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stderr.sh") :OUTPUT :INTERACTIVE
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stderr.sh" :OUTPUT :INTERACTIVE
stderr
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stderr.sh" :OUTPUT :INTERACTIVE :FORCE-SHELL T
stderr
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM "./stderr.sh" :OUTPUT :INTERACTIVE

UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stderr.sh") :OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stderr.sh") :OUTPUT NIL :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stderr.sh") :OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stderr.sh" :OUTPUT NIL
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stderr.sh" :OUTPUT NIL :FORCE-SHELL T
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM "./stderr.sh" :OUTPUT NIL

UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stderr.sh") :ERROR-OUTPUT :INTERACTIVE
stderr
UIOP/RUN-PROGRAM:RUN-PROGRAM ("./stderr.sh") :ERROR-OUTPUT :INTERACTIVE :FORCE-SHELL T
stderr
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM ("./stderr.sh") :ERROR-OUTPUT :INTERACTIVE
stderr
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stderr.sh" :ERROR-OUTPUT :INTERACTIVE
stderr
UIOP/RUN-PROGRAM:RUN-PROGRAM "./stderr.sh" :ERROR-OUTPUT :INTERACTIVE :FORCE-SHELL T
stderr
UIOP/RUN-PROGRAM:LAUNCH-PROGRAM "./stderr.sh" :ER...

Read more...

Revision history for this message
Faré (fahree) wrote :

I admit I don't know how to interpret this output. Can you summarize or explain?

Revision history for this message
pipping (pipping) wrote :

I shouldn't have used launchpad for this. Its lack of formatting makes it really difficult to present such things in a readable fashion. Here it goes again.

If you run the script https://gitlab.common-lisp.net/snippets/19 you'll get the following output with sbcl:

on the master branch: https://gitlab.common-lisp.net/snippets/16
on the feature/consistent-program-defaults branch: https://gitlab.common-lisp.net/snippets/18

What this test does is: It runs two trivial bash scripts (one that writes '1> stdout' to stdout, and one that writes '2> stderr' to stderr) in different ways: Through (1) launch-program and run-program (2) without :force-shell and (3) with :force-shell. Once by passing a string and once by passing a list (which gives you 6 combinations) and then it also tests different combinations of :output/:error-output :interactive/nil/nothing(=the default) (which gives you another 6 combinations), hence the 6 blocks of 6 lines that each tell you what they're running.

Most of these lines are not followed by anything but some are followed by '1> stdout' or '2> stderr' because the corresponding output was interactive(*). The two snippets I mentioned earlier thus show: With the consistency change in place we get something on stdout if-and-only-if we pass :output :interactive and on stderr if-and-only-if we pass :error-output :interactive, in all 6 combinations (launch-program/run-program/etc.). Prior to the change, you can see the launch-program/run-program inconsistency.

(*) if you do the same with CCL, you get a bit of a mess because the output to stdout and stderr is randomly mixed

Revision history for this message
pipping (pipping) wrote :

> and then it also tests different combinations of :output/:error-output :interactive/nil/nothing(=the default) (which gives you another 6 combinations), hence the 6 blocks of 6 lines that each tell you what they're running.

Correction: It's actually :output/:error-output and :interactive/nil, which gives you 4 combinations, times 2 for the stdout/stderr shell script, thus 8 blocks of 6 lines.

Revision history for this message
Faré (fahree) wrote :

Somehow my mind is not at all into this right now, and I'm afraid I won't be able to give much feedback, unless you have a specific question — but I trust your judgement.

Also, note that you CAN use run-program and lisp-invocation to write tests, if you so desire. It's contrived enough that I wouldn't demand it, and probably wouldn't do it unless in even deeper procrastination mode than now.

Revision history for this message
pipping (pipping) wrote :

This is fixed now. The bug can be closed.

Revision history for this message
Faré (fahree) wrote :

Is it fixed in master or in an unmerged branch?

Do you lack access rights to mark the bug as "fix committed" or "in progress" or anything?

Revision history for this message
pipping (pipping) wrote :

This was fixed with

https://gitlab.common-lisp.net/asdf/asdf/merge_requests/46
https://gitlab.common-lisp.net/asdf/asdf/merge_requests/47
https://gitlab.common-lisp.net/asdf/asdf/merge_requests/48

all three of which have been merged into master.

It seems I never noticed I have the rights to close bugs because I expected the functionality to be exposed in a different way than the way launchpad does it.

Changed in asdf:
status: New → Fix Committed
pipping (pipping)
Changed in asdf:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.