clojure.java.shell/sh hangs calling xdg-open

Description

Please note that not only is this important for clojure.java.shell and, by implication, clojure.java.browse, but this will also enable the ClojureScript browser REPL to correctly launch the browser on platforms where xdg-open is used. Currently the ClojureScript Quick Start instructions are, essentially, broken on many systems. Most first-time users of ClojureScript won't enjoy debugging something like this (or maybe I'm just more stubborn than most ).

On some platforms clojure.java.browse/browse-url calls clojure.java.shell/sh to execute a cmdline using xdg-open to launch the web browser. As a proper *Nix utility xdg-open passes along its open file descriptors to the new process. (I don't know if this is documented somewhere for xdg-open, but it's easy to demonstrate.)

clojure.java.shell/sh always reads both the STDOUT & STDERR streams from the executed process. When the purpose is to gather the output of the subprocess that, of course, is a good thing. However, when launching a browser with xdg-open, the streams aren't closed until the browser exits. For a caller to clojure.java.browse/browse-url the function seems to "never" return.

What we need is a variant of clojure.java.shell/sh (or an option to sh) that ignores the output streams & only returns the exit code. We're using the subprocess strictly for its side-effect. Somewhat analogous to using doseq instead of map, except that we do want the exit code of the subprocess.

To get the ClojureScript browser REPL working, I made a local copy of clojure.java.{browse,shell}, added a function (launch) to clojure.java.shell that ignores the I/O streams but returns the exit code, & modified browse-url to use launch. Much better! Both browser & REPL, just as promised in the Quick Start instructions.

Personal note: While I've just started experimenting with ClojureScript, I've been using Clojure for several years

Prescreened by: Alex Miller

Environment

Clojure 1.10.0
ClojureScript 1.10.516
Xubuntu 16.04

Activity

Show:
Andy Fingerhut
September 6, 2019, 9:24 PM
Edited

FYI, the behavior I describe in my previous comment also exists with google-chrome and Opera as the default browser on Ubuntu 18.04 Linux. It is not specific to Firefox.

The prescreened patch clj-2493-2.patch causes the behavior to be better when any of those 3 browsers are the default browser.

Andy Fingerhut
September 1, 2019, 9:04 PM

Out of curiosity on whether there was a change in xdg-open, I experimented with some older versions of Clojure and Linux, and it seems that ever since the change introduced in Clojure 1.6.0 to use xdg-open on Linux, this issue has existed.

I found that if you start Firefox, e.g., then start a Clojure REPL and eval a browse-url expression, the REPL prompt comes back quickly, as expected. Similarly if you start a Clojure REPL, then Firefox, then eval the browse-url expression.

The only case in which the eval of a browse-url expression does not quickly come back with a prompt is if Firefox was not yet started, and eval’ing browse-url caused it to start.

Still, that does seem like a situation worth improving.

Jonathan Johnston
March 22, 2019, 2:06 AM

Second patch of 21 March 2019 - Replaces first patch

  • Removed trailing whitespace

  • Added metadata in anticipation of acceptance

Alex Miller
March 22, 2019, 1:37 AM

There are two trailing whitespace errors when I apply the patch - lines 43 and 49 of the current patch. Would be good if you could tidy those up.

Regarding the metadata, either add the metadata with 1.11 and leave a note about it in the description OR leave it out but make a note in the description that the test will fail until it's added. Either way I can fix it up when we get to a point where it's being screened for a release.

Jonathan Johnston
March 22, 2019, 1:30 AM

I didn't think it was my place to include the {:added "1.11"}, especially since it hasn't been added at this point in time. Should I add the metadata & build a new patch - either an update or a replacement?

Your pinned fields
Click on the next to a field label to start pinning.

Assignee

Unassigned

Reporter

Jonathan Johnston

Approval

Vetted

Patch

Code

Priority

Major

Affects versions

Fix versions