Skip to content
This repository has been archived by the owner on Jul 2, 2022. It is now read-only.

Remove project dependency on leiningen #47

Closed
danielcompton opened this issue Nov 7, 2014 · 14 comments
Closed

Remove project dependency on leiningen #47

danielcompton opened this issue Nov 7, 2014 · 14 comments

Comments

@danielcompton
Copy link

Chestnut has a dependency on Leiningen which depends on Maven. This means we pull in a ton of unnecessary dependencies into our uberjar including Plexus and Lucene. It seems to be only used in dev.clj. Does it make sense to:

  1. Put the Leiningen dependency (and dev.clj) into a dev profile or
  2. Remove it entirely and just shell out our calls to lein/-main?
@danielcompton danielcompton changed the title Dependency on leiningen Remove project dependency on leiningen Nov 7, 2014
@pjagielski
Copy link

+1

I can't integrate https://github.com/ddellacosta/friend-oauth2 because of conflicting clj-http dependency with that provided by leiningen.

@plexus
Copy link
Owner

plexus commented Nov 17, 2014

The dependency on Leiningen is to invoke figwheel from inside the same process. There should be a better way to do that, I just didn't manage to figure that out. It seems figwheel relies quite heavily on the project context provided by leiningen, but maybe there's a way to invoke it directly without having to pull in leiningen? ping @bhauman

The dev profile approach seems fine as well. I would accept a PR for that.

@danielcompton
Copy link
Author

I realised that the dev profile won't really solve the problem for @pjagielski as his dependencies will still conflict. I'll have a look at putting together a PR.

@bhauman
Copy link

bhauman commented Nov 17, 2014

There is a separate Figwheel library that handles the serving of changed file notifications. You would have to handle the watching and the building of cljs and then call the figwheel API to handle notifying the browser.

There would be need to duplicate a small portion of cljsbuild functionality.

@plexus
Copy link
Owner

plexus commented Dec 7, 2014

After some consideration I've figured out what I think is a clean way to do this. Building an uberjar will no longer include leiningen, figwheel, weasel. This is available on 0.7.0-SNAPSHOT. I would really appreciate if people could try this out and report their findings.

@plexus plexus closed this as completed Dec 7, 2014
@dgellow
Copy link

dgellow commented Dec 8, 2014

This comment from @pjagielski is still valid.

Steps to reproduce

  • lein new chestnut test-chestnut-snapshot --snapshot
  • add [com.cemerick/friend "0.2.0"] and [friend-oauth2 "0.1.2"] as deps in project.clj
  • launch a REPL and require friend-oauth2 two times (first and second require don't throw the same exception)
test-snapshut-chestnut.server=> (require '[friend-oauth2.workflow :as w])
CompilerException java.lang.ExceptionInInitializerError, compiling:(friend_oauth2/workflow.clj:1:1) 

test-snapshut-chestnut.server=> (require '[friend-oauth2.workflow :as w])
CompilerException java.lang.NoClassDefFoundError: Could not initialize class clj_http.client__init, compiling:(friend_oauth2/workflow.clj:1:1)

@plexus
Copy link
Owner

plexus commented Dec 8, 2014

Leiningen depends on clj-http 0.9.2, friend-oauth2 depends on 0.7.7, so they have incompatible dependencies. That's not strictly an issue with Chestnut, although I understand it's a nuisance.

I see three ways to solve this

  1. (preferred) File an issue with friend-oauth2 to update their dependency. This would be the most straightforward, and will prevent friend-oauth2 from clashing with other libs that depend on a more recent clj-http in the future.
  2. Shell out to leiningen to invoke figwheel, instead of calling leiningen's main from clojure. Not sure if that would lead to other problems or not, and I don't have the time now to explore that path. I blew my Chestnut time budget on cleaning up speclj support and reorganizing the dev/prod environment split. If someone can come up with a clean and well tested patch for that then I wouldn't mind going that route though.
  3. only depend on the library figwheel, not lein-figwheel. This means we would have to take care of clojurescript compilation ourselves, which I don't find very appealing because it would mean adding more boilerplate to every generated Chestnut app.

@plexus plexus reopened this Dec 8, 2014
@dgellow
Copy link

dgellow commented Dec 8, 2014

@plexus Thank you for taking the time to write a clear response.
I will follow the 1st way.

@pjagielski
Copy link

I think that 2) is still important, it's not good when your production code depends on the build tool.

@plexus
Copy link
Owner

plexus commented Dec 8, 2014

It doesn't, that's what's been addressed in 0.7.0-SNAPSHOT. Chestnut projects do not directly depend on Leiningen, the dependency is only included in development mode. It will not be included in production mode or when building an uberjar.

@plexus
Copy link
Owner

plexus commented Dec 9, 2014

For reference: corresponding issue on the friend-oauth2 side clojusc/friend-oauth2#34

@ddellacosta
Copy link

I'm happy to bump the clj-http version and if push comes to shove I'll add the exclusion, but isn't the problem here not in friend-oauth2, but in friend?

ddellacosta added a commit to clojusc/friend-oauth2 that referenced this issue Dec 13, 2014
@ddellacosta
Copy link

Nevermind, I can't bump clj-http without that exclusion (duh)--anyways, released a new version: 0.1.3.

Regardless, seems like it would be worth bringing this up in the friend issue I linked to above to give them a nudge--presumably this will be a problem with any project using friend and clj-http.

@plexus
Copy link
Owner

plexus commented Dec 26, 2014

The two issues raised in this thread have been resolved

  • leiningen is not included in the uberjar, so a production system based on chestnut does not require leiningen
  • there is no longer a dependency conflict with friend-oauth2

If there are still concerns about the relationship between Chestnut and Leiningen, please start a new thread.

@plexus plexus closed this as completed Dec 26, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants