weavejester / lein-ring

Ring plugin for Leiningen
Eclipse Public License 1.0
501 stars 100 forks source link

Remove need for AOT shim #130

Closed devn closed 9 years ago

devn commented 10 years ago

This pulls in a number of the changes hiredman made on his branch without deleting functions.

I don't think this is quite ready to merge, but wanted some feedback.

How should compile-servlet and compile-listener be guarded from running, if at all?

weavejester commented 10 years ago

When you do get it ready to merge, remember to remove any code that isn't directly to do with the feature being considered.

The config parameters could probably do with being named more explicitly and more consistently, e.g. lein-ring-ns-handler, lein-ring-var-handler, lein-ring-ns-init, lein-ring-var-init, etc.

The Java code could be split out into methods as well. Just because we're using Java doesn't mean we can't be a little neat in our code!

The compile-servlet and compile-listener functions are made obsolete by a servlet shim, as far as I can see.

devn commented 10 years ago

@weavejester I gave it a shot, but I'm coming up short. I'm getting java.lang.Exception: namespace 'foo.bar.baz' not found, compiling:(foo/bar/server.clj:1:1) when I go to deploy the war to Tomcat, and am not sure where to go from here. Thoughts?

weavejester commented 10 years ago

You'll need to provide some information on the options and project layout you're using.

devn commented 10 years ago

@weavejester Sure:

In the root of project.clj (not within a profile):

:ring {:handler foo.bar.server/handler
       :init foo.bar.server/init
       :port 12345
       :open-browser? false
       :nrepl {:start? true :port 22345}}

Hopefully that gives you something more to work from, but if not, let me know any specifics you're interested in knowing.

Thanks in advance for taking a look.

ETA: It's probably worth noting that lein ring server works, but the uberwar fails to start in Tomcat.

weavejester commented 10 years ago

Okay. Where's the foo.bar.baz the error message mentions?

devn commented 10 years ago

@weavejester foo.bar.baz is required inside of foo/bar/server.clj.

MichaelBlume commented 10 years ago

@devn I tried this on my project and I was able to get the war deployed and running -- can you tell me more about the trouble you're having?

What's here looks good, but it seems like if :destroy is ever going to work, we're going to need a separate java file that implements Listener

MichaelBlume commented 10 years ago

I'm concerned that the code for including the Servlet.class file is in uberwar.clj rather than war.clj -- shouldn't that be needed by war as well?

MichaelBlume commented 10 years ago

I've added some code to call the user's destroy fn here: https://github.com/MichaelBlume/lein-ring/tree/do-destroy

MichaelBlume commented 10 years ago

I've tried to address some of the issues in this pull in #136