The Wayback Machine - https://web.archive.org/web/20200913022251/https://github.com/boot-clj/boot/issues/672
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better warning when there is a classpath conflict for org.clojure/clojure #672

Open
martinklepsch opened this issue Dec 22, 2017 · 9 comments
Open

Comments

@martinklepsch
Copy link
Member

@martinklepsch martinklepsch commented Dec 22, 2017

Classpath conflict: org.clojure/clojure version 1.8.0 already loaded, NOT loading version 1.3.0

If you specify a Clojure version in boot.properties and don't specify the same Clojure version in your :dependencies you will get a warning like the one above.

Not sure if this is worth it but we could provide some more context for the specific case of Clojure:

Classpath conflict: org.clojure/clojure version 1.8.0 already loaded, NOT loading version 1.3.0
This is commonly caused by:
- not specifying a Clojure version in your :dependencies
- specifying a different version of Clojure than in boot.properties

What do you think?

@mprokopov
Copy link

@mprokopov mprokopov commented Jan 16, 2018

An another workaround is to make exclusions in dependencies, as I've described here https://stackoverflow.com/questions/43018876/classpath-conflict-org-clojure-clojure-version-1-7-0-already-loaded-not-loadin/48282148#48282148 But it's better to resolve this warning by inclusion of proper version of Clojure.

And in case you forget to include Clojure in app dependency list, you may end up with the bug, when uber jar is compiled successfully, but application can not find its entry point when launched. I spend almost a day figuring out this bug. Hope it helps someone.

@martinklepsch
Copy link
Member Author

@martinklepsch martinklepsch commented Jan 16, 2018

@mprokopov Would you be interested in providing a PR for this? 🙂

@mprokopov
Copy link

@mprokopov mprokopov commented Jan 18, 2018

@martinklepsch I think this is good idea to provide some hints in warning message to resolve this issue for an end user, but this warning could easily be other than org.clojure/clojure.

Inclusion of dependency in top dependency tree seems to be proper resolution for any library with such warning. Is it reasonable to advice user to include dependency in a such way?

Maybe it's just worth to be noted in README/FAQ for boot.

@martinklepsch
Copy link
Member Author

@martinklepsch martinklepsch commented Jan 18, 2018

I think org.clojure/clojure is a special case here because you need to specify it in two places: build.boot and boot.properties. This does not apply to libraries where this warning usually only appears if you call set-env! multiple times and dependencies get resolved differently.

While having multiple set-env! calls is usually something you want to avoid it also is handy in a REPL/exploratory context.

Also note that the code checks for Clojure specifically and then warns "NOT loading" whereas with regular libraries it warns "ALSO loading:

(defn- report-version-conflicts
"Warn, when the version of a dependency changes. Call this with the
result of find-version-conflicts as arguments"
[coll]
(let [clj-name (symbol (boot.App/getClojureName))]
(doseq [[name new-coord old-coord] coll]
(let [op (if (= name clj-name) "NOT" "ALSO")]
(-> "Classpath conflict: %s version %s already loaded, %s loading version %s\n"
(util/warn name old-coord op new-coord))))))

Besides a warning we could also just add the version of Clojure to the list of :dependencies but this would most likely be a breaking change.

So — following suggestion:

  1. We create a PR for this issue that takes care of warning about the special case of org.clojure/clojure
  2. We open an additional issue to figure out what to do with the general case of multiple set-env! calls. I think the warning and a link to a document with some context around why it should only be done for exploratory stuff could be a good way forward but this is up for discussion and should be treated separately from this issue which focuses on the Clojure dependency specifically.

Does that sound good to you?

@martinklepsch
Copy link
Member Author

@martinklepsch martinklepsch commented Jan 24, 2018

@mprokopov hey again! Just wanted to check what you think about my suggestion and if you’d be interested to help with this? 🙂

@mprokopov
Copy link

@mprokopov mprokopov commented Jan 24, 2018

Hi again!

I think it's better just to make this warning a bit better with explanation you suggested:

Classpath conflict: org.clojure/clojure version 1.8.0 already loaded, NOT loading version 1.3.0
This is commonly caused by:
- not specifying a Clojure version in your :dependencies
- specifying a different version of Clojure than in boot.properties

But it would be not so good, if this full warning message will be displayed several times.
For example I forgot to include top Clojure dependency, and ring, ring-jetty required Clojure 1.6.3, and pandeiro/boot-http Clojure 1.7.0, so I've got 3 warnings about it.

How do you think @martinklepsch, is it enough just to update boot wiki FAQ with your suggestion?

@martinklepsch
Copy link
Member Author

@martinklepsch martinklepsch commented Jan 24, 2018

For example I forgot to include top Clojure dependency, and ring, ring-jetty required Clojure 1.6.3, and pandeiro/boot-http Clojure 1.7.0, so I've got 3 warnings about it.

The warning will only get printed three times if you call set-env! with each of these dependencies individually. This arguably happens less often but it could still become annoying. Not sure what to do — does this change your opinion?

@arichiardi
Copy link
Contributor

@arichiardi arichiardi commented Jan 24, 2018

I would also mention that org.clojure can be put in :exclusions automatically by using the -x option to boot.

@flyboarder
Copy link
Contributor

@flyboarder flyboarder commented Dec 18, 2018

I agree with @martinklepsch , shadow-cljs is patching their dependencies and warning the user that setting the clojure version has no effect and to remove it. I think this is the correct way to reason about it. This prevents the user from creating a conflict to begin with.

We could do the same, patch the version in the dependencies with whats in boot.properties and notify the user, again breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.