orthecreedence / wookie

Asynchronous HTTP server in common lisp
http://wookie.lyonbros.com/
MIT License
189 stars 19 forks source link

Quicklisp symbols #82

Closed mtstickney closed 6 years ago

mtstickney commented 6 years ago

This PR is a preliminary change to fix issues using quicklisp symbols in the plugin code. I'm afraid I haven't tested it, since I'm really not familiar with wookie's plugin system; if you have suggestions or would like to see a different approach, please let me know.

The problem is that your solution of using macros to avoid errors when quicklisp isn't present is only half-correct. The problem is this: you load the system with quicklisp available, so the macros emit ql:quickload & co; asdf caches the resulting fasl file; the system now errors whenever it is loaded from an image without quicklisp (e.g. buildapp with a manifest file) because asdf is loading the cached fasl with quicklisp symbols in it, but the quicklisp package doesn't exist.

This branch does a few things:

  1. Converts the macros to local functions
  2. Defers symbol lookup to runtime (using find-symbol, and not intern, because it doesn't have unpleasant side-effects)
  3. Tries to warn or error if the quicklisp package was available, but the expected symbols were missing.

There are some other changes that could be made as well:

  1. The system name could be reported from asdf errors with asdf/missing-component:missing-requires
  2. The compiler says the *log-output* variable is not actually a special variable (I'm guessing this was meant to be vom:*log-stream*?)

Personally, I think wookie should get out of the business of loading code itself. It's a lot of extra complication (there are at least 5 systems for loading lisp code that I know of), and it doesn't seem like it's asking a lot of users to add a system to their dependencies lists and (maybe) insert a one-line (wookie:register-plugin 'boogle) line. Still, it's your project; I mostly just want to fix the embedded-symbol issue.

orthecreedence commented 6 years ago

Personally, I think wookie should get out of the business of loading code itself.

Oh man, you have no idea how much I wish I'd realized that up front, and how much I wished I'd known about middleware before building the stupid plugin system.

Regardless, Wookie is pretty much in stasis for the time being, so it will probably remain how it is (unless somebody wants to rewrite the plugin system).

Thanks for digging in and fixing this.