technomancy / leiningen

Moved to Codeberg; this is a convenience mirror
https://codeberg.org/leiningen/leiningen
Other
7.29k stars 1.61k forks source link

Global :exclusions in profile do not apply to root :dependencies #2811

Closed jpmonettas closed 7 months ago

jpmonettas commented 8 months ago

Currently adding a global :exclusions in a profile doesn't remove dependencies added directly in the main :dependencies vector.

This is useful when you want your personal profile to remove a dependency for development without having to comment out dependencies in your root vector.

For a concrete example, for using ClojureStorm with FlowStorm you need to be able to swap the official Clojure compiler by the ClojureStorm dev compiler.

The way to do this is to configure your project.clj like this :

(defproject my.project "1.0.0"
  :dependencies [
                ;; [org.clojure/clojure "1.11.1"]
                ]
  :profiles {:dev {:dependencies [[com.github.flow-storm/clojure "1.11.1-17"]]
                   :exclusions [org.clojure/clojure]}})

where you exclude org.clojure/clojure to include com.github.flow-storm/clojure which provides a compiler replacement under a different group. The problem is this doesn't work unless you comment out the org.clojure/clojure dependency.

To Reproduce Steps to reproduce the behavior:

  1. Create an empty folder with the project.clj file described above
  2. Run lein repl. The lein welcome message should say Clojure 1.11.1-17
  3. Uncomment the org.clojure/clojure dependency while keeping the exclusion
  4. Run lein repl. The lein welcome message now says Clojure 1.11.1, which means the exlcusion did not apply

This is independent of lein, OS, and jvm version

technomancy commented 8 months ago

OK, sounds reasonable. I would take a patch for this.

jpmonettas commented 8 months ago

@technomancy The problem is related to how [org.clojure/clojure "1.11.1" :exclusions [[org.clojure/clojure]]] is handled.

For building the java -classpath ... command for starting the JVM process lein is currently callingleiningen.core.classpath/get-classpath with a project that contains :dependencies [... [org.clojure/clojure "1.11.1" :exclusions [[org.clojure/clojure]]]] which in turn calls cemerick.pomegranate.aether/dependency-files which doesn't take into account exclusions (it just returns the jar file on the coordinates meta).

So my question is, should something like [org.clojure/clojure "1.11.1" :exclusions [[org.clojure/clojure]]] be there in the first place? Should it be discarded as early as possible (like by leiningen.core.project/init-project), or we want to keep that and discard it before building a classpath?

technomancy commented 8 months ago

So my question is, should something like [org.clojure/clojure "1.11.1" :exclusions [[org.clojure/clojure]]] be there in the first place? Should it be discarded as early as possible (like by leiningen.core.project/init-project), or we want to keep that and discard it before building a classpath?

Yes, great question. The details of how the project is initialized are subtle, but I think the best place to handle this would be around init-project, or to be more specific, the add-global-exclusions function which is called by init-profiles. Here we are are already looking at top-level :exclusions and splicing them into the individual :dependencies vectors.

I think this would be the ideal point to filter out "self-contradictory" dependency entries like [org.clojure/clojure "1.11.1" :exclusions [[org.clojure/clojure]].

Does that make sense?

jpmonettas commented 8 months ago

@technomancy Sure make sense. add-global-exclusions could do a (->> keep (into []) instead of a mapv and get rid of the "self-contradictory" ones there, or if we want to keep the names cleaner (add-* shouldn't be removing dependencies) we could add a extra step in init-profiles pipe, right after add-global-exclusions, something like remove-self-excluded-deps to do the filtering, which I think it is clenner.

technomancy commented 8 months ago

Sure; another option could be to rename add-global-exclusions to add-and-apply-global-exclusions so it would be clearer that it's actively excluding things. But it's up to you; either way works.

technomancy commented 7 months ago

Forgot to close this; it's now implemented. Thanks again.