ASDF/BACKWARD-INTERFACE:RUN-SHELL-COMMAND no longer returns exit status, at least on ACL

[Robert P. Goldman]

On ACL 8.2, when I run this function, which is supposed to provide backward compatibility to "classic" ASDF:RUN-SHELL-COMMAND, it returns NIL, rather than the exit status of ZERO on a successful run:

CL-USER(18): (ASDF/BACKWARD-INTERFACE:RUN-SHELL-COMMAND (FORMAT NIL "cd \"~a\" ; ./build_all" *))
; $ cd "/Users/rpg/stratus/stratus-trunk/code/externals/fast-downward/src/" ; ./build_all
make: Nothing to be done for `default'.
make: Nothing to be done for `default'.
make: Nothing to be done for `default'.
make: Nothing to be done for `default'.
make: `validate' is up to date.
NIL

This breaks legacy code that calls ZEROP on the return value of this function.

According to the docstring on UIOP:RUN-PROGRAM, the error is that :ignore-error-status is passed T instead of NIL.

If this passes the test suite, I will push such a patch.

[Robert P. Goldman (rpgoldman)]

Oh no! IGNORE-ERROR-STATUS is used for *two* purposes. If you set it to T, then not only do we not get the numerical error returned, but you also ignore subprocess errors. But if you set it to NIL, then we may get the exit status, but we also raise an error if the exit status is not 0.

What's needed is an option to return the error status without raising an error on non-zero.

I suppose one could do this with IGNORE-ERRORS. But that seems really evil.

[Robert P. Goldman (rpgoldman)]

OK, my bad. The REAL problem is that :OUTPUT here is set to *VERBOSE-OUT*. That means that it's not NIL or INTERACTIVE, which means that the error status is not returned.

I hate to go messing around with things I don't understand, but I think *VERBOSE-OUT* must be removed here, and replaced with :INTERACTIVE. We *must* get back that exit status, and I don't know enough to go rummaging around in UIOP:RUN-PROGRAM to fix how it treats its arguments. I will see if that change passes the test and commit, if so.

[Faré (fahree)]

Isn't the issue that there are conflicting demands on run-shell-command to *both* kind-of capture the output in a clumsy way with *verbose-out* (with a ; command as an extra first line) and yet return the command result? These are kind of incompatible demands on uiop:run-program. Maybe by having :ignore-error-status nil *but* explicitly handler-case'ing the error status, we can make it work?

[Robert P. Goldman (rpgoldman)]

Faré wrote:
> Isn't the issue that there are conflicting demands on run-shell-command
> to *both* kind-of capture the output in a clumsy way with *verbose-out*
> (with a ; command as an extra first line) and yet return the command
> result? These are kind of incompatible demands on uiop:run-program.
> Maybe by having :ignore-error-status nil *but* explicitly handler-
> case'ing the error status, we can make it work?
>

I'll see, thanks. I thought it was simple, but I need to add some more test cases to the test suite, and identify precisely which cases are causing the problem. I've gone off half-cocked twice now, I'm going to make sure I get it right the third time!

[Robert P. Goldman (rpgoldman)]

OK, now I have a better understanding. It looks like in some earlier version of ASDF:RUN-SHELL-COMMAND, when I bound *VERBOSE-OUT* to T, I still got the exit status. But with the backward compatibility interface, I get only the output returned, and not the exit status.

This seems to be because that is what UIOP:RUN-PROGRAM does. I don't claim to understand this very well -- is there some reason why it must have only a single return value, instead of returning both the exit status, and the output (run through a slurper)?

I had this because I had an ASDF system that reached out into "C world" and ran a complex make script. I wanted the output echoed to the screen in case the make failed.

I don't know how to check to see if the build ran successfully with ASDF/BACKWARD-INTERFACE:RUN-SHELL-COMMAND. Unfortunately, RUN-SHELL-COMMAND quashes errors, so one avenue for compatibility (relying on the subprocess error) is closed to me. Is there a #+UIOP I could use to detect the presence/absence of RUN-PROGRAM as a work-around?

I don't have perfect control over the environments that all the users of the system will have, so it's hard to jump from ASDF:RUN-SHELL-COMMAND to UIOP:RUN-PROGRAM, even though the former is deprecated.

Looking back at 2.35, returning the output when *VERBOSE-OUT* is T instead of dumping it to the value of *VERBOSE-OUT* is a change in the API. In that older version, the output would be dumped to *VERBOSE-OUT*, and the exit status would still be returned.

Is there some way to get RUN-PROGRAM to give both the output and the exit status, so that we can recreate what RUN-SHELL-COMMAND used to do?

[Robert P. Goldman (rpgoldman)]

OK, so the work-around is to say #+asdf3 with UIOP:RUN-PROGRAM and #-asdf3 with ASDF:RUN-SHELL-COMMAND. This isn't ideal, since I think there may be versions that don't have ASDF3 in *FEATURES* where ASDF:RUN-SHELL-COMMAND isn't backward compatible. But it's at least a partial work-around.

[Faré (fahree)]

Can't you "just" :depends-on (#-asdf3 :uiop) and always use asdf-driver:run-program ? Or deliver your application with asdf 3.0.1...

[Faré (fahree)]

OK, I pushed some change as 3.0.1.2, please tell me if they help with backward compatibility.

That said, I encourage you to use to new interface, and import uiop and/or asdf-driver in old settings that don't provide asdf 3. Quicklisp, for instance, has uiop 2.32.5 but not asdf 3.

[Robert P. Goldman (rpgoldman)]

Faré wrote:
> Can't you "just" :depends-on (#-asdf3 :uiop) and always use asdf-driver
> :run-program ? Or deliver your application with asdf 3.0.1...
>

Let me file this as a separate ticket. The problem, it seems to me, is
that the :asdf3 feature was added *before* ASDF-DRIVER was renamed to
UIOP. That's really a separate issue.

[Faré (fahree)]

Well, the compatibility name asdf-driver remains, so you can
  :depends-on (#-asdf3 :asdf-driver)
and use the asdf-driver package.

Or you might rely on the fact (if it is one) that there are few copies of asdf 2.27 to 2.31 at large, and that people using asdf 3 will be using a recent version.

[Faré (fahree)]

This has been fixed in 3.0.1.2.

[Faré (fahree)]

Was released in 3.0.2