:if-output-exists support incomplete in run-program

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

Bug Description

(Putting this here so it's documented. Fixing it will be non-trivial)

run-program has an option :if-output-exists which defaults to :supersede. If run-program calls %run-program through %use-run-program (soon to be called launch-program and %use-launch-program), this is essentially passed along unchanged.

If %system is used through %use-system instead, its value is discarded. Of the possible values :if-output-exists can take, :append is probably the one for which this is most surprising (you'll end up overwriting instead of appending to the file in question!). The fix would be to use '>>' instead of '>' here (this should work both in POSIX shell and CMD.EXE). If we also wanted to get things like :if-input-does-not-exist :create or :if-output-exists :error to work, though, the fix would take additional effort.

Please note that CLISP essentially does not support launch-program so that this is not easily worked around portably.

Revision history for this message
pipping (pipping) wrote :

I've thought a bit about this. One way to address this that came to mind would be to precede the actual call in %system by

  (with-open-file (dummy output :direction-output :if-file-exists if-output-exists))

and to additionally turn '> output' into '>> output'.

This should do the right thing for
 - :if-output-exists :error (because with-open-file will signal an error)
 - :if-output-exists :supersede (with-open-file will discard the old file and leave an empty one that we'll then append to)
 - :if-output-exists :append (with-open-file will signal an error if the file is missing; otherwise, it will append nothing on its own and we'll append to the existing file)

What I'm afraid this would handle properly I'm afraid is
 - :if-output-exists nil

(which raises the question: which values of :if-output-exists do we support anyway? :overwrite is currently treated as :supersede e.g. because the error signalling of :overwrite does not work reliably in %run-program/launch-program)

Revision history for this message
pipping (pipping) wrote :

The sentence

  "What I'm afraid this would handle properly I'm afraid is"

was supposed to read

  "What I'm afraid this would not handle properly is"

(damn you, launchpad, for not allowing me to edit my own comments!)

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

I admit I'm not too worried about implementations that only have %system and/or calls with :force-shell t being less featureful than others. I'm a bit more worried about run-program "promoting" (rather, demoting) calls with a command string to use %system by default instead of using launch-program whenever it can.

I would modify run-program so it always uses launch-program when available, even if given a string as a command.

As for doing things right painfully, one solution would be to redirect output to a temporary file, then manually append its contents to the destination file.

I'm not volunteering to actually write that.

Revision history for this message
pipping (pipping) wrote :

> I would modify run-program so it always uses launch-program when available, even if given a string as a command.

That would break things like

  (run-program "cmd1; cmd2")

or even

  (run-program "cmd1 | cmd2")

though, no?

I was also surprised how often one ends up calling %system inadvertently and would like to change that.

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

That would work just fine: launch-program would translate that to ("/bin/sh" "-c" "cmd1; cmd2") which would work, pretty much the same as %system does, except that Lisp rather than shell will handle redirections.

Revision history for this message
pipping (pipping) wrote :

Right, I got it backwards. Please disregard my previous comment.

Revision history for this message
pipping (pipping) wrote :

I'm wondering: Which type of error should be raised with

  :if-output-exists :error

or

  :if-input-does-not-exist :error

Revision history for this message
pipping (pipping) wrote :

I've now opened https://gitlab.common-lisp.net/asdf/asdf/merge_requests/35 to work on this issue.

I've answered the question to "Which type of error should be raised" with "whatever with-open-file would raise".

Revision history for this message
pipping (pipping) wrote :

Committed in 7267036000ed85d15f560ac9f57f2d3a6dbd47f7; thanks, Robert! This bug can now be closed.

pipping (pipping)
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