Can't load complete/core.clj in nonstandard classpath scenarios #108

Closed
jstoneham opened this Issue Apr 3, 2013 · 6 comments

2 participants

@jstoneham

I'm trying to wrap reply in another package so I can start it up with some dependencies and bindings already in place. That package is built with Maven.

I was trying to do "mvn compile exec:java -Dexec.mainClass=myClass", where myClass runs reply.main/launch-nrepl from Java. I get:
IllegalArgumentException No implementation of method: :make-reader of protocol: #'clojure.java.io/IOFactory found for class: nil
clojure.core/-cache-protocol-fn (core_deftype.clj:541)
clojure.java.io/fn--8551/G--8546--8558 (io.clj:73)
clojure.java.io/reader (io.clj:106)
clojure.core/apply (core.clj:619)
clojure.core/slurp (core.clj:6278)
reply.initialization/formify-file (initialization.clj:94)
reply.initialization/default-init-code (initialization.clj:144)
reply.initialization/construct-init-code (initialization.clj:166)
reply.eval-modes.nrepl/main/fn--3061 (nrepl.clj:203)
reply.eval-modes.nrepl/main (nrepl.clj:197)
reply.main/launch-nrepl/fn--3391 (main.clj:73)
clojure.core/with-redefs-fn (core.clj:6751)
Bye for now!

Reason for the error is that reply is loading complete/core.clj via ClassLoader/getSystemResource, so in this scenario it's not available via that classloader. We should probably load from something like the thread's current context classloader instead.

@trptcolin
Owner

Sure, I can't think of a reason not to use the thread's context classloader for this. Does a patch doing that fix the issue for you locally?

@trptcolin
Owner

Alternately: do you have a repo I can use on my end to reproduce & test it out? I'm pretty noobish when it comes to Maven, so any repro steps would be appreciated if you want to go that route.

@trptcolin trptcolin added a commit that closed this issue Apr 8, 2013
@trptcolin Use context classloader to get completions
Fall back gracefully if that fails too.

closes #108
b1251eb
@trptcolin trptcolin closed this in b1251eb Apr 8, 2013
@trptcolin
Owner

OK, using the context classloader is definitely fine locally for me. I haven't reproduced the failure you're seeing, but I've also made it so that at worst, it fails a bit more gracefully and just doesn't load completion, rather than bombing the rest of initialization.

Thanks for reporting this!

@jstoneham

Hey, sorry for going dark on you. I used to see all GitHub notifications via private Atom feed but it looks like that feed stopped working a couple months ago and I didn't notice.

What I was doing was:

package my.repl;

import clojure.lang.*;
import org.apache.commons.io.IOUtils;

import java.io.IOException;
import java.util.HashMap;
import java.util.Map;

public final class ClojureRepl {

    private static final Var readStringFn = RT.var("clojure.core", "read-string");

    private ClojureRepl() {
    }

    public static void main(final String[] args) throws IOException {
        initJLineLogging();
        launchNRepl();
    }

    private static void launchNRepl() throws IOException {
        RT.var("clojure.core", "require").invoke(Symbol.create("reply.main"));
        Map<Keyword, Object> argsMap = new HashMap<Keyword, Object>();
        argsMap.put(Keyword.intern("custom-init"), readInitRepl());
        RT.var("reply.main", "launch-nrepl").invoke(PersistentArrayMap.create(argsMap));
    }

    private static Object readInitRepl() throws IOException {
        String initReplClj = IOUtils.toString(ClojureRepl.class.getClassLoader().getResource("init-repl.clj"));
        return readString("(do " + initReplClj + ")");
    }

    private static void initJLineLogging() {
        String jlineLog = System.getenv("JLINE_LOGGING");
        if (jlineLog != null) {
            System.setProperty("jline.internal.Log." + jlineLog, "true");
        }
    }

    private static Object readString(String str) {
        return readStringFn.invoke(str);
    }
}

and init-repl.clj had some application-specific stuff to set some default bindings in the user namespace and connect up to an Accumulo database.

When I ran this with mvn exec:java like above, it failed.

I'll check current snapshot now.

@jstoneham

Yes, verified to work as of 3c84852. Thanks!

@trptcolin
Owner

Great, glad to hear it, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment