<< Back to previous view

[CLJ-1085] clojure.main/repl unconditionally refers REPL utilities into *ns* Created: 10/Oct/12  Updated: 20/Oct/12  Resolved: 20/Oct/12

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.4, Release 1.5
Fix Version/s: Release 1.5

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Completed Votes: 3
Labels: None

Attachments: File CLJ-1085.diff     File CLJ-1085-refactor.diff    
Patch: Code
Approval: Ok

 Description   

A number of vars from clojure.repl, clojure.java.javadoc, and clojure.pprint are unconditionally referred into *ns* by clojure.main/repl. This is fine when it is being used e.g. as the primary driver of a terminal-bound Clojure REPL, but other usages can end up bringing those utility vars into namespaces other than 'user. This can cause problems if clojure.main/repl is used to drive a REPL within namespaces that already have referred or interned vars with the same names as those utility vars, e.g.:

$ java -jar ~/.m2/repository/org/clojure/clojure/1.5.0-alpha6/clojure-1.5.0-alpha6.jar
Clojure 1.5.0-alpha6
user=> (ns foo)
nil
foo=> (defn pp [] "hi!")
#'foo/pp
foo=> (pp)
"hi!"
foo=> (clojure.main/repl)
foo=> (pp)
nil
nil
foo=> (defn pp [] "whoops")
CompilerException java.lang.IllegalStateException: pp already refers to: #'clojure.pprint/pp in namespace: foo, compiling:(NO_SOURCE_PATH:7:1)

Worse, nREPL uses clojure.main/repl (in large part to maximize the consistency of REPL behaviour across different Clojure versions), where each user expression is evaluated through a separate clojure.main/repl invocation. This leads to the same problems as above, but for every nREPL user, session, and expression (reported @ NREPL-31).

A simple fix for this is to perform these refers only if *ns* is 'user (which, AFAICT, was the only intended effect of CLJ-310, CLJ-454, and https://github.com/clojure/clojure/commit/04764db, the changes that added these automatic implicit refers to clojure.main/repl).

 Comments   
Comment by Chas Emerick [ 10/Oct/12 1:36 PM ]

Patch attached to only refer in the utility vars if in the user namespace.

Comment by Steve Miner [ 10/Oct/12 2:03 PM ]

It would probably be better to test with ns-name (as opposed to comparing strings):

(= 'user (ns-name ns))

Comment by Kevin Downey [ 10/Oct/12 2:18 PM ]

I don't follow how it is required to call `clojure.main/repl` for every input

Comment by Chas Emerick [ 10/Oct/12 2:19 PM ]

Gah, of course. :-/ Patch updated.

Comment by Chas Emerick [ 10/Oct/12 2:32 PM ]

@Kevin: It's not required, but I found it far more straightforward to not try to pretend that the underlying transport was stream-based when it's actually message-based. It also means that sessions can be very lightweight: unless code is being evaluated within a session, it is not occupying a thread, and takes up only as much space as its map of thread-local bindings.

Comment by Chas Emerick [ 15/Oct/12 5:10 AM ]

Based on discussion on clojure-dev, I have attached an alternative patch ({{CLJ-1085-refactor.diff}}), which:

  • breaks out the libspecs specifying the implicit refers into their own var (so that they can be consistently applied by other REPL implementations)
  • moves the default require of the libspecs to be invoked only when REPL is started from clojure.main
Comment by Stuart Halloway [ 19/Oct/12 2:12 PM ]

Screened and liked the second "refactor" patch.

Generated at Thu May 23 22:01:49 CDT 2013 using JIRA 4.4#649-r158309.