<< Back to previous view

[TNS-20] tracker's unload order sometimes incorrect Created: 23/Aug/14  Updated: 24/Aug/14

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File tns-20-v1.patch    

 Description   

The attached patch contains a new test namespace clojure.tools.namespace.load-unload-order-test that demonstrates the incorrect unload order, if you leave out the proposed fix in file track.clj



 Comments   
Comment by Andy Fingerhut [ 23/Aug/14 3:02 PM ]

Patch tns-20-v1.patch contains a test that demonstrates the bug, and contains a proposed fix. Please scrutinize the proposed fix carefully, as I haven't yet convinced myself 100% that it is the right fix.

Also adds one more dependency check in dependency_test.clj that I discovered while reviewing those tests, to model my dependency checking tests on.

Comment by Andy Fingerhut [ 24/Aug/14 4:52 PM ]

I've got some code that checks that the load and unload orders in a tools.namespace tracker are consistent with its dependencies. It is similar to the checks done now in dependencies_test.clj, except it assumes that the dependencies in the tracker are correct and verifies the load and unload orders against those.

If you would be interested in a patch that added these checks to tools.namespace, perhaps along with a debug/consistency-check flag that when true causes these checks to be run every time a tracker is updated, let me know.





[TNS-6] Attempt to reload deleted file Created: 14/Dec/12  Updated: 14/Dec/12

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I can't identify the exact circumstances, but I have seen events where a source code file has been deleted but clojure.tools.namespace.repl/refresh still tries to reload it. Because the file doesn't exist, there's an exception when you try to load it, so you're stuck.






[TNS-5] Allow any valid .clj* source file to be parsed/analysed Created: 01/Nov/12  Updated: 13/Dec/12

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Max Penet Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: enhancement

Attachments: Text File e3cd6d1fa6e0c900bc1086e4a93bbc9cb343a820.patch    

 Description   

This broadens the allowed file types to anything ending with #"\.clj.?$", meaning this would work for clj, cljs, cljc, cljx and possibly other Clojure implementations with their own extension in the future.

This allows libraries such as codox (and possibly autodoc) to work with ClojureScript and others implementations without any modification.

Note: My CA is on the way, I sent it a week ago, I am not sure if it arrived yet (it was sent from Switzerland with normal mail, with an ETA of 1 week).



 Comments   
Comment by Max Penet [ 02/Nov/12 6:51 AM ]

CA received it seems (I am listed on http://clojure.org/contributing ).

Comment by Stuart Sierra [ 02/Nov/12 3:23 PM ]

I'm not sure about this. If you're only using c.t.n.find in isolation, it's fine. But if you're using code-reloading and c.t.n.repl, it could incorrectly try to reload .cljs files in JVM Clojure.

We really need [Feature Expressions]http://dev.clojure.org/display/design/Feature+Expressions or something like it to get away from multiple file extensions.

Until then, I think it has to be optional. I don't know how best to achieve this. The APIs in c.t.n.repl and c.t.n.dir are not amenable to extension. I'll think about it.

Comment by Max Penet [ 06/Nov/12 6:11 PM ]

True, I didn't realize that.

Maybe using a dynamic var to hold the regex pattern (or a predicate?) could be a reasonable solution in the meantime, this would allow to rebind it in the case of codox & similar libs.
I don't really "like" to use dynamic vars (or regexes!), but in this case it might make sense.
Not to mention it would also allow more flexibility on projects where you don't want to have your clj codoxed, but only your cljs for instance.

Comment by Max Penet [ 24/Nov/12 7:47 AM ]

Any thoughts on my last comment/edit? I don't think there is a single doc lib that works with cljs at the moment, it is a bit painful to be honest.

Comment by Stuart Sierra [ 24/Nov/12 4:25 PM ]

Yes, I think a dynamic var would be OK. However, I would like to know of a real use case, not just a potential one.

Comment by Max Penet [ 13/Dec/12 2:40 AM ]

The idea was to be able to use codox on cljs files, I tried locally but there are other problems with this approach to be able to get to cljs vars metadata. So in the end I think you were right, maybe it's better to wait for feature expressions for this.





[TNS-22] Function ns-file-name makes incorrect call to clojure.string/replace when File/separator is "\\" (e.g. Windows) Created: 24/Aug/14  Updated: 24/Aug/14

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File tns-22-v1.patch    
Patch: Code

 Description   

On Windows, where File/separator is a string consisting of a single backslash character:

user=> (require '[clojure.string :as str])
nil

;; This works as expected, because str/replace with second arg being a string handles arbitrary strings as the third arg
user=> (str/replace "foo.bar" "." File/separator)
"foo\\bar"

;; This gives an exception, because str/replace with second arg being a regex treats backslash characters in third arg specially.
user=> (str/replace "foo.bar" #"\." File/separator)
StringIndexOutOfBoundsException String index out of range: 1  java.lang.String.charAt (String.java:658)

;; This works as expected, but is more complex than the first alternative above
user=> (str/replace "foo.bar" #"\." (str/re-quote-replacement File/separator))
"foo\\bar"

I'd recommend using str/replace calls like the first one above when the things you want to replace are constant strings.



 Comments   
Comment by Andy Fingerhut [ 24/Aug/14 7:01 PM ]

Patch tns-22-v1.patch changes the only 2 calls to clojure.string/replace I found in tools.namespace that could have their second arg be a constant string rather than a regex.





[TNS-21] tools.namespace ignores ns form dependencies inside vectors Created: 23/Aug/14  Updated: 24/Aug/14

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Cross reference: TLOG-13

For example, tools.logging has this ns form (metadata omitted):

(ns clojure.tools.logging
  [:use
   [clojure.string :only [trim-newline]]
   [clojure.pprint :only [code-dispatch pprint with-pprint-dispatch]]]
  [:require [clojure.tools.logging.impl :as impl]])

tools.namespace ignores these dependencies, only processing those that are in list subforms of the ns form.

Not sure if the best way to handle this is to update tools.logging or tools.namespace. tools.logging seems pretty unusual in using vectors like this, but the Clojure compiler seems to accept it just fine.



 Comments   
Comment by Stuart Sierra [ 24/Aug/14 1:29 PM ]

The ns macro docstring is quite clear that references inside it should be lists. I've seen a lot of weird ns forms, and this is the first time I've seen vectors used this way.

I would say I'm surprised that vectors work here, except nothing weird about the ns macro surprises me any more.





[TNS-7] Stack overflow on refresh after circular dependency detected Created: 17/Apr/13  Updated: 17/Apr/13

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 1
Labels: None
Environment:

tools.namespace 0.2.3



 Description   
user> (refresh)
Exception Circular dependency between com.example.foo and com.example.bar  clojure.tools.namespace.dependency.MapDependencyGraph (dependency.clj:74)
user> (refresh)
StackOverflowError   clojure.lang.PersistentHashMap$BitmapIndexedNode.index (PersistentHashMap.java:576)
user> (clojure.repl/pst)
StackOverflowError 
	clojure.lang.PersistentHashMap$BitmapIndexedNode.index (PersistentHashMap.java:576)
	clojure.lang.PersistentHashMap$BitmapIndexedNode.find (PersistentHashMap.java:676)
	clojure.lang.PersistentHashMap$ArrayNode.find (PersistentHashMap.java:396)
	clojure.lang.PersistentHashMap.valAt (PersistentHashMap.java:152)
	clojure.lang.PersistentHashMap.valAt (PersistentHashMap.java:156)
	clojure.lang.RT.get (RT.java:645)
	clojure.tools.namespace.dependency/transitive (dependency.clj:52)
	clojure.tools.namespace.dependency/transitive/fn--7043 (dependency.clj:51)
	clojure.core.protocols/fn--6022 (protocols.clj:79)
	clojure.core.protocols/fn--5979/G--5974--5992 (protocols.clj:13)
	clojure.core/reduce (core.clj:6177)
	clojure.tools.namespace.dependency/transitive (dependency.clj:52)
nil





Generated at Wed Aug 27 13:59:34 CDT 2014 using JIRA 4.4#649-r158309.