<< Back to previous view

[CLASSPATH-4] clojure.java.classpath/classpath gives bogus %20 paths when there are spaces in the pathnames Created: 12/Dec/12  Updated: 18/Jan/13  Resolved: 18/Jan/13

Status: Closed
Project: java.classpath
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Tim McCormack Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-CLASSPATH-4-Use-io-as-file-instead-of-treating-URL-a.patch    

 Description   

Reproduce: In a directory called "foo bar", create this project file:

(defproject test-proj "1.0.0-SNAPSHOT"
  :dependencies [[org.clojure/clojure "1.4.0"]
                 [org.clojure/java.classpath "0.2.0"]])

In the REPL:

(require '[clojure.java.classpath :as cp])
(cp/classpath)

Expected results:

(... #<File /home/timmc/foo bar/src> ...)

Actual results:

(... #<File /home/timmc/foo%20bar/src> ...)



 Comments   
Comment by Tim McCormack [ 12/Dec/12 11:02 PM ]

This seems to be the result of how loader-classpath handles URL -> File conversion:

#(java.io.File. (.getPath ^java.net.URL %))
Comment by Tim McCormack [ 13/Dec/12 5:39 PM ]

I think using clojure.java.io/as-file would be sufficient to handle this conversion properly.

Comment by Tim McCormack [ 11/Jan/13 3:10 PM ]

Attached patch uses clojure.java.io/as-file to handle URL -> File conversion.

Comment by Stuart Sierra [ 18/Jan/13 8:58 AM ]

applied





[CLASSPATH-1] clojure.java.classpath/system-classpath is incorrect when running in a classloader other than the application classloader Created: 13/Sep/11  Updated: 08/Jan/14  Resolved: 27/Jan/12

Status: Closed
Project: java.classpath
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Justin Balthrop Assignee: Stuart Sierra
Resolution: Duplicate Votes: 0
Labels: None

Attachments: File system-classpath.diff    
Patch: Code and Test

 Description   

Because system-classpath uses java.class.path, it doesn't work when running in a classloader other than the application classloader.

This happens when running in any container that uses a custom classloader (e.g. Jetty, Cake, TomCat).

The fix is to use (.getURLs (.getClassLoader clojure.lang.RT)) instead of (System/getProperty "java.class.path"). A patch is attached.



 Comments   
Comment by Justin Balthrop [ 13/Sep/11 8:56 PM ]

Hugo provides a more general solution in CLASSPATH-2.

Comment by Stuart Sierra [ 27/Jan/12 9:44 AM ]

Resolved by CLASSPATH-2





[CLASSPATH-2] Reported classpath should include classpath from parent classloaders, but not from the system classloader Created: 13/Sep/11  Updated: 08/Jan/14  Resolved: 15/Sep/11

Status: Closed
Project: java.classpath
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Hugo Duncan Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: File recursive-classpath.diff     File recursive-classpath.diff     File recursive-classpath.diff    
Patch: Code

 Description   

Since a classloader delegates to its parent if it fails to provide a class, the classpath reported by java.classpath should include the classpath of parent classloaders.

At present the reported classpath contains both system and baseloader classpaths, which I believe is incorrect, since the system classloader is not necessarily a parent of the baseloader.



 Comments   
Comment by Justin Balthrop [ 13/Sep/11 8:55 PM ]

If this patch is accepted, it will fix the issue described in http://dev.clojure.org/jira/browse/CLASSPATH-1. Hugo and I talked and his fix is more general than mine, so you can close CLASSPATH-1.

Comment by Stuart Sierra [ 14/Sep/11 9:48 AM ]

Patch does not compile.

Comment by Hugo Duncan [ 14/Sep/11 11:26 AM ]

Oops, I ran the tests, but failed to notice there weren't any...

Updated patch, with a unit test.

Comment by Hugo Duncan [ 14/Sep/11 11:27 AM ]

Updated patch with unit test

Comment by Stuart Sierra [ 15/Sep/11 9:08 AM ]

Patch is good. Not sure about the test. Finding "clojure-.*jar" in the classpath is subject to the build environment. I think all we can reasonably expect is that the classpath is a seq of java.io.Files.

Comment by Hugo Duncan [ 15/Sep/11 9:39 AM ]

The test are normally run from the project's own pom, and even if run from an external project, the classpath will include a clojure jar in the huge majority of cases. I suppose the tests could be guarded with a java property set in the project's pom.

If the tests for the clojure jar are dropped, a test for a non-empty sequence should at least be added.

Comment by Stuart Sierra [ 15/Sep/11 10:52 AM ]

Agreed: the seq should be non-empty and contain Files.

Comment by Hugo Duncan [ 15/Sep/11 11:18 AM ]

Patch with unit test for classpath, testing for non-empty sequence of java.io.File objects as return value.

Comment by Stuart Sierra [ 15/Sep/11 12:53 PM ]

Patch applied. Will release new version soon.





[CLASSPATH-5] Make extensible to any ClassLoader, not just URLClassLoader derivatives Created: 07/Jan/14  Updated: 10/Jan/14  Resolved: 08/Jan/14

Status: Closed
Project: java.classpath
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Jim Crossley Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: File classpath-5.diff    
Patch: Code

 Description   

Currently, only URLClassLoaders are supported, but with the introduction of a protocol for obtaining a sequence of URL's corresponding to the segments of a ClassLoader's classpath, we make it possible to use this library in JVM's that don't necessarily use a URLClassLoader, e.g. JBoss.



 Comments   
Comment by Jim Crossley [ 07/Jan/14 3:58 PM ]

Attached patch file: classpath-5.diff

Comment by Stuart Sierra [ 07/Jan/14 4:14 PM ]

The goal is reasonable, but I don't understand this patch: it's extending the Classpath protocol to java.lang.ClassLoader and then trying to call a method which is not defined in java.lang.ClassLoader

Also, I don't understand the description: JBoss is not a JVM.

Comment by Jim Crossley [ 07/Jan/14 4:23 PM ]

Sorry, that patch was a mistake. Corrected one attached (same name)

Comment by Jim Crossley [ 07/Jan/14 4:26 PM ]

Regarding JBoss as JVM, I meant that JBoss, as well as other containers, is a single JVM process, with multiple ClassLoaders active, each responsible for an isolated, deployed app. And none of them are URLClassLoader instances, but they all are capable of returning a list of their classpath segments as URLs.

Comment by Jim Crossley [ 07/Jan/14 4:35 PM ]

Rebased the patch to make it more clear

Comment by Stuart Sierra [ 07/Jan/14 5:01 PM ]

Modified to avoid changes to existing public functions. Committed to master at 0b3401cc

Will build as version 0.2.2-SNAPSHOT, available via Sonatype repo.

Please test and confirm that this satisfies your use case.

Comment by Jim Crossley [ 07/Jan/14 5:34 PM ]

That looks good, Stuart, thanks! I have two followup questions:

1) Are you open to using this in tools.namespace?
2) Do you think it's worth obviating dynapath with java.classpath?

This jira gets you most of the way to #2 already. Lacking is the ability to invoke .addURL to expand a classloader's classpath at runtime. If you think it's a good idea, I'll add a jira and take a whack at it.

Comment by Stuart Sierra [ 07/Jan/14 7:53 PM ]

1. Yes, but I would prefer to keep it optional.

2. Not at this time.

Comment by Stuart Sierra [ 10/Jan/14 11:26 AM ]

in release 0.2.2





[CLASSPATH-3] Eliminate one use of reflection in java.classpath Created: 28/Oct/12  Updated: 08/Jan/14  Resolved: 28/Oct/12

Status: Closed
Project: java.classpath
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File classpath-3-eliminate-reflection-v1.txt    

 Description   

There is one use of reflection in fn classpath that is easy to eliminate with a type hint.



 Comments   
Comment by Andy Fingerhut [ 28/Oct/12 6:19 PM ]

classpath-3-eliminate-reflection-v1.txt dated Oct 28 2012 eliminates the one use of reflection that currently exists in java.classpath, by adding a type hint.

Comment by Stuart Sierra [ 28/Oct/12 8:09 PM ]

Patch applied. Thanks!





Generated at Mon Apr 21 07:47:28 CDT 2014 using JIRA 4.4#649-r158309.