ring-clojure / ring

Clojure HTTP server abstraction
MIT License
3.77k stars 520 forks source link

Add type hint for server-connector #381

Closed danielcompton closed 5 years ago

danielcompton commented 5 years ago

Because:

Without this patch, the native image will build, but fail with:

Exception in thread "main" java.lang.ClassNotFoundException: org.eclipse.jetty.server.ServerConnector
    at com.oracle.svm.core.hub.ClassForNameSupport.forName(ClassForNameSupport.java:60)
    at java.lang.Class.forName(DynamicHub.java:1174)
    at clojure.lang.RT.classForName(RT.java:2207)
    at clojure.lang.RT.classForName(RT.java:2216)
    at ring.adapter.jetty$server_connector.invokeStatic(jetty.clj:44)
    at ring.adapter.jetty$server_connector.doInvoke(jetty.clj:44)
    at clojure.lang.RestFn.invoke(RestFn.java:423)
    at ring.adapter.jetty$http_connector.invokeStatic(jetty.clj:57)
    at ring.adapter.jetty$http_connector.invoke(jetty.clj:55)
    at ring.adapter.jetty$create_server.invokeStatic(jetty.clj:122)
    at ring.adapter.jetty$create_server.invoke(jetty.clj:119)
    at ring.adapter.jetty$run_jetty.invokeStatic(jetty.clj:164)
    at ring.adapter.jetty$run_jetty.invoke(jetty.clj:127)
    at simple.main$_main.invokeStatic(main.clj:15)
    at simple.main$_main.invoke(main.clj:13)
    at clojure.lang.AFn.applyToHelper(AFn.java:152)
    at clojure.lang.AFn.applyTo(AFn.java:144)
    at simple.main.main(Unknown Source)

cc: @BrunoBonacci, with this patch I was able to run https://github.com/BrunoBonacci/graalvm-clojure/tree/master/ring-jetty.

BrunoBonacci commented 5 years ago

Great work, It would be nice to be able to compile to native binaries microservices with GraalVM. The native compilation offers a major step up in startup times. Looking forward to this PR to get merged. 👏

weavejester commented 5 years ago

That's interesting; I've never seen a type hint like that before. Can you remove the "Because" and "-" from the commit message? i.e.

Add type hint for server-connector

When building a Graal native image, Graal isn't able to resolve which
method to call. Adding the varargs type hint fixes this.

Once that's done I'll merge it in. I should be able to cut a release soon, I'm just waiting on one more commit.

danielcompton commented 5 years ago

Thanks, done. There's reference to this kind of hint at https://groups.google.com/d/msg/clojure/TFLUw8GSAbY/xgTR74fr8IgJ