edgecase / dieter

Asset pipeline ring middleware
134 stars 22 forks source link

lein-dieter-precompile #19

Closed pbiggar closed 12 years ago

pbiggar commented 12 years ago

I reckon this is good enough for a first version. It compiles all files in your :asset-root directory.

jxa commented 12 years ago

I'm having an issue. After including lein-dieter-precompile in my project.clj file and running lein deps successfully I get the following:

~/projects/pers/dieter $ lein dieter-precompile
That's not a task. Use "lein help" to list all tasks.
~/projects/pers/dieter $ lein help
Leiningen is a tool for working with Clojure projects.

Several tasks are available:
...
leiningen.dieter-precompile  Problem loading: Could not locate leiningen/dieter_precompile__init.class or leiningen/dieter_precompile.clj on classpath: 
jxa commented 12 years ago

I'm not sure why you have this in a subdirectory with its own project.clj file. Seems like it would be a good idea to ship it with dieter. Is it possible to create a lein plugin from within dieter's src directory?

pbiggar commented 12 years ago

@jxa Are you using lein 1 or lein 2? This works for me in lein 1.6.2 and lein 1.7.1. Did you make it a dev-dependency?

It didnt occur to me that they could be in the same project. I guess that does make sense. I'll change it.

jxa commented 12 years ago

I'm at lein 1.7.1. I thought it might be due to the jar being out of date so I tried linking it into a checkouts dir. Same error. The jar and the checkout both show up in lein classpath so I'm not sure what is going on.

pbiggar commented 12 years ago

Actually, thinking about it again, I think it does make sense to ship them separately - everyone else does (lein-ring, lein-midje, etc). Also, I can't think how I'd specify :eval-in-leiningen, and we'd have to make sure people put dieter in their dev-dependencies instead.

pbiggar commented 12 years ago

I believe the checkouts trick doesnt work for lein plugins, but I dont think that would explain it. Maybe install using maven and then do lein deps? Not really sure.

pbiggar commented 12 years ago

Do you have it as a dev-dependency? Can you post your project.clj's diff?

jxa commented 12 years ago

I see. So only dev-dependencies can be loaded as lein plugins. I was just thinking that since the lein plugin will need to load dieter anyways it would be simpler to only have the one dependency.

If there is an existing convention for having separate projects for the lein tasks then I suppose it makes a certain amount of sense to following that convention.

As for my lein issue: I updated to the latest in clojars. Now I'm able to call dieter-precompile. When I do lein help the task descriptions are funky:

dieter-precompilePrecompile dieter assets
leiningen.dieter-precompile-runtime  Problem loading: java.lang.IllegalArgumentException: Unable to resolve classname: :dynamic (fs.clj:27)
pbiggar commented 12 years ago

The lein-plugin doesnt load dieter. lein starts up a 2nd JVM for the project it is being called in, and dieter is loaded in that JVM. So a problem I've been fixing over the last few days is that lein1 runs in clojure 1.2, and my projects are clojure 1.3. Some projects, like dieter and fs, require 1.3. So we need to make sure that we dont load dieter in lein's JVM, and so we cant allow lein-dieter-precompile to have 1.3 dependencies either.

The "Problem loading" isn't a real problem because lein shouldn't be loading that anyway. I think putting that file in a different namespace should fix that, let me try.

pbiggar commented 12 years ago

Let me restate the problem, because I made a mess of it above. We can't allow clojure 1.3 to be loaded into a lein-plugin, beucase they run in clojure 1.2. So we need to make sure the plugin itself does not have transitive dependencies on clojure 1.3, so we can't use dieter in the plugin.

pbiggar commented 12 years ago

And that does indeed solve the problem.

pbiggar commented 12 years ago

@jxa How's this looking? Do you think you can commit in the current state?

jxa commented 12 years ago

@pbiggar This is looking pretty good. I just have 2 thoughts.

app.js-blah124124124124.dieter


dieter/
  lein-dieter-precompile/
    project.clj
    src/
  dieter-core/
    project.clj
    src/

I should have a few hours here & there this week where I can look at these, if you don't want to try to tackle them. The file extension piece might be a little hairy. Let's coordinate here so we're not both looking at the same piece. Please let me know if you start on it & I'll do the same.

pbiggar commented 12 years ago

The file extension isn't affected by this change at all. If you want to change that, there's no need to hold up this patch for it. Let's talk about that in #11.

Changing the location is fine with me. That'll only take a minute, so I can do it now.

pbiggar commented 12 years ago

Done. I left the README where it was, so that users still see it on the github page.

pbiggar commented 12 years ago

@jxa Are you waiting for anything from me for this pull request? I reckon it can go in as is.

pbiggar commented 12 years ago

@jxa would love to get this merged in. Do you have an ETA for that?

pbiggar commented 12 years ago

Hmmm, I dont understand why I cant merge this here. I'll try to push it directly instead.