Welcome! Please see the About page for a little more info on how this works.

+4 votes
in Clojure by
closed by

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

closed with the note: Released in 1.11.0-alpha4

7 Answers

0 votes
by

Comment made by: jdjohnston

clj-2493.patch Attached 21 March 2019

0 votes
by

Comment made by: jafingerhut

I have tested (clojure.java.browse/browse-url "https://github.com") in a clj REPL on a local machine of mine in Clojure 1.10.0, and it does indeed not give another REPL prompt, unless the browser is closed. I made the last changes to this function to add xdg-open capability in 2012, and I am pretty sure it did not do this then, but perhaps the behavior of xdg-open has changed since then. The Clojure source for the files containing clojure.java.browse/browse-url and clojure.java.shell/sh have not changed since 2012.

Here is the system I have good test results from:

Ubuntu 16.04.6 Desktop Linux with latest updates as of 2019-Mar-21
OpenJDK 1.8.0_191 installed from openjdk-8-jre-headless and openjdk-8-jdk-headless Ubuntu packages
latest Clojure master as of 2019-Mar-21 plus clj-2493.patch

0 votes
by
_Comment made by: jafingerhut_

These changes do cause a test to fail when building Clojure, I think because the new launch function has no {:added "1.11"} metadata after the doc string.
0 votes
by
_Comment made by: jdjohnston_

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?
0 votes
by

Comment made by: alexmiller

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.

0 votes
by

Comment made by: jdjohnston

Second patch of 21 March 2019 - Replaces first patch
- Removed trailing whitespace
- Added metadata in anticipation of acceptance :)

0 votes
by
Reference: https://clojure.atlassian.net/browse/CLJ-2493 (reported by jdjohnston)
by
I also had this exact same problem on debian stretch, openjdk version 1.8.0_222, with clojure installed via the linux-install-1.10.1.469.sh script.

While following the "quick start" ( https://clojurescript.org/guides/quick-start ) guide for clojurescript, the clj command would start up the browser, and display the "Hello World!" text, but would never show a REPL prompt.  I tried hitting Enter mutiple times, but that did not work.  In my case, closing the browser would return an error and clojure would exit.

I tried changing the default browser (tried Firefox and Chromium) and the result was the same.

After cloning the clojure repository, applying this patch, and building/running with the patched clojure, I was finally able to get the browser window to open, text displayed on the REPL, and the REPL prompt.  Victory at last! :-D

All of that to say, thanks for the patch, and it worked for me also.
by
Thanks for the feedback - this is in the queue for consideration in Clojure 1.11.
by
Looking at 1.11.0-alpha4,

The new clojure.java.shell/launch may seem to work for xdg-open'ing a web browser, but only because xdg-open and shrinkwrapped browsers usually write little or nothing to their standard streams.

According to Javadoc (https://docs.oracle.com/en/java/javase/12/docs/api/java.base/java/lang/Process.html), "Because some native platforms only provide limited buffer size for standard input and output streams, failure to promptly write the input stream or read the output stream of the process may cause the process to block, or even deadlock."

For example,

    (require '[clojure.java.shell :as shell])

    ;; exec with 'sh' - does not hang
    (time (:exit (shell/sh "bash" "-c" "sleep 1; z=0; while [ $z -lt 25000 ] ; do echo $z; z=$((z+1)) ; done")))

    ;; exec with 'launch' - hangs
    (shell/launch "bash" "-c" "sleep 1; z=0; while [ $z -lt 25000 ] ; do echo $z; z=$((z+1)) ; done")
       
One way around the problem, in general (I don't know how it works with xdg-open and browsers in particular) is to use ProcessBuilder, and `.redirectError` and .redirectOutput to java.lang.ProcessBuilder$Redirect/DISCARD before calling .start.
...