From 5bd091a2bf997cb556a3216930976c7152e29fff Mon Sep 17 00:00:00 2001 From: Chris Price Date: Fri, 16 Sep 2016 15:07:40 -0700 Subject: [PATCH] Don't require `nil` for version in managed deps - fixes #2195 Prior to this commit, if you wanted to use modifiers such as `:exclusions` or `:classifier` for a dependency whose version you were managing with `:managed-dependencies`, you would need to explicitly pass a `nil` as the version string in the dependency tuple. This commit adds some logic to coerce the vectors before they are processed, so that if the version string is simply omitted instead of being set to `nil`, the `nil` will be implicitly inserted and things will continue to work as before. This provides a slightly nicer and more intuitive UX for the managed-dependencies feature. --- doc/MANAGED_DEPS.md | 27 +++++++++++++-- .../src/leiningen/core/classpath.clj | 34 ++++++++++++++++++- leiningen-core/src/leiningen/core/project.clj | 2 +- test/leiningen/test/pom.clj | 20 ++++++----- .../managed-deps-snapshot/project.clj | 4 +++ test_projects/managed-deps/project.clj | 4 +++ 6 files changed, 78 insertions(+), 13 deletions(-) diff --git a/doc/MANAGED_DEPS.md b/doc/MANAGED_DEPS.md index c480c22d..885b73e7 100644 --- a/doc/MANAGED_DEPS.md +++ b/doc/MANAGED_DEPS.md @@ -55,7 +55,19 @@ section across multiple projects. ## A note on modifiers (`:exclusions`, `:classifier`, etc.) -The managed dependencies support in leiningen *does* work with modifiers such as `:exclusions` and `:classifier`. However, at present, because of the way that lein and pomegranate process the args in the dependencies vector, you will need to explicitly specify `nil` as the value for the version arg in order to achieve this: +The managed dependencies support in leiningen *does* work with modifiers such as +`:exclusions` and `:classifier`. There are two legal syntaxes; you can explicitly +specify a `nil` for the version string, or you can simply omit the version string: + +```clj +(defproject superfun/happyslide "1.0.0-SNAPSHOT" + :description "A Clojure project with managed dependencies" + :min-lein-version "2.7.0" + :managed-dependencies [[clj-time "0.12.0"]] + :dependencies [[clj-time :exclusions [foo]]]) +``` + +or ```clj (defproject superfun/happyslide "1.0.0-SNAPSHOT" @@ -65,7 +77,18 @@ The managed dependencies support in leiningen *does* work with modifiers such as :dependencies [[clj-time nil :exclusions [foo]]]) ``` -Issue #2195 covers the possibility of doing some future work to allow omission of the `nil` in the example above. +Note that `:classifier` is actually a part of the maven coordinates for an +artifact, so for `:classifier` artifacts you will need to specify the `:classifier` +value in both the `:managed-dependencies` and the normal `:dependencies` section: + + +```clj +(defproject superfun/happyslide "1.0.0-SNAPSHOT" + :description "A Clojure project with managed dependencies" + :min-lein-version "2.7.0" + :managed-dependencies [[commons-math "1.2" :classifier "sources"]] + :dependencies [[commons-math :classifier "sources"]]) +``` ## Lein "parent" projects diff --git a/leiningen-core/src/leiningen/core/classpath.clj b/leiningen-core/src/leiningen/core/classpath.clj index 9ef7e640..a23be32c 100644 --- a/leiningen-core/src/leiningen/core/classpath.clj +++ b/leiningen-core/src/leiningen/core/classpath.clj @@ -531,6 +531,36 @@ :managed-dependencies)] (apply resolve-managed-dependencies dependencies-key managed-dependencies-key project rest))) +(defn normalize-dep-vector + "Normalize the vector for a single dependency, to ensure it is compatible with + the format expected by pomegranate. The main purpose of this function is to + to detect the case where the version string for a dependency has been omitted, + due to the use of `:managed-dependencies`, and to inject a `nil` into the + vector in the place where the version string should be." + [dep] + (if dep + (let [id (first dep) + sec (second dep) + version (if-not (keyword? sec) sec) + opts (if (keyword? sec) + (nthrest dep 1) + (nthrest dep 2))] + ;; it's important to preserve the metadata, because it is used for + ;; profile merging, etc. + (with-meta + (concat [id version] opts) + (meta dep))))) + +(defn normalize-dep-vectors + "Normalize the vectors for the `:dependencies` section of the project. This + ensures that they are compatible with the format expected by pomegranate. + The main purpose of this function is to to detect the case where the version + string for a dependency has been omitted, due to the use of `:managed-dependencies`, + and to inject a `nil` into the vector in the place where the version string + should be." + [deps] + (map normalize-dep-vector deps)) + (defn merge-versions-from-managed-coords [deps managed-deps] ;; NOTE: there is a new function in the 0.3.1 release of pomegranate that @@ -538,7 +568,9 @@ ;; via the symbol dereference for now, but this can be changed to a ;; regular function call once https://github.com/cemerick/pomegranate/pull/74 ;; is merged. - (#'aether/merge-versions-from-managed-coords deps managed-deps)) + (#'aether/merge-versions-from-managed-coords + (normalize-dep-vectors deps) + managed-deps)) (defn managed-dependency-hierarchy "Returns a graph of the project's dependencies. diff --git a/leiningen-core/src/leiningen/core/project.clj b/leiningen-core/src/leiningen/core/project.clj index 7e6b7739..6a6ee9b9 100644 --- a/leiningen-core/src/leiningen/core/project.clj +++ b/leiningen-core/src/leiningen/core/project.clj @@ -74,7 +74,7 @@ "Transform a dependency vector into a map that is easier to combine with meta-merge. This allows a profile to override specific dependency options." [dep] - (if-let [[id version & {:as opts}] dep] + (if-let [[id version & {:as opts}] (classpath/normalize-dep-vector dep)] (-> opts (merge (artifact-map id)) (assoc :version version) diff --git a/test/leiningen/test/pom.clj b/test/leiningen/test/pom.clj index ecd6994d..952305ad 100644 --- a/test/leiningen/test/pom.clj +++ b/test/leiningen/test/pom.clj @@ -360,28 +360,30 @@ (let [xml (xml/parse-str (make-pom proj))] (testing "normal dependencies are written to pom properly" - (is (= ["org.clojure" "rome" "ring" "ring" "commons-codec" "commons-math" - "org.clojure" "org.clojure"] + (is (= ["org.clojure" "rome" "ring" "ring" "ring" "commons-codec" + "commons-math" "org.apache.commons" "org.clojure" "org.clojure"] (map #(first-in % [:dependency :groupId]) (deep-content xml [:project :dependencies])))) - (is (= ["clojure" "rome" "ring" "ring-codec" "commons-codec" "commons-math" + (is (= ["clojure" "rome" "ring" "ring-codec" "ring-headers" + "commons-codec" "commons-math" "commons-csv" "tools.emitter.jvm" "tools.namespace"] (map #(first-in % [:dependency :artifactId]) (deep-content xml [:project :dependencies])))) - (is (= [nil nil nil nil "1.6" nil "0.1.0-beta5" "0.3.0-alpha3"] + (is (= [nil nil nil nil nil "1.6" nil nil "0.1.0-beta5" "0.3.0-alpha3"] (map #(first-in % [:dependency :version]) (deep-content xml [:project :dependencies]))))) (testing "managed dependencies are written to pom properly" - (is (= ["org.clojure" "rome" "ring" "ring" "commons-math" "ring" "org.clojure"] + (is (= ["org.clojure" "rome" "ring" "ring" "ring" "commons-math" + "org.apache.commons" "ring" "org.clojure"] (map #(first-in % [:dependency :groupId]) (deep-content xml [:project :dependencyManagement :dependencies])))) - (is (= ["clojure" "rome" "ring" "ring-codec" "commons-math" "ring-defaults" - "tools.reader"] + (is (= ["clojure" "rome" "ring" "ring-codec" "ring-headers" + "commons-math" "commons-csv" "ring-defaults" "tools.reader"] (map #(first-in % [:dependency :artifactId]) (deep-content xml [:project :dependencyManagement :dependencies])))) - (is (= ["1.3.0" "0.9" "1.0.0" "1.0.1" "1.2" "0.2.1" "1.0.0-beta3"] + (is (= ["1.3.0" "0.9" "1.0.0" "1.0.1" "0.2.0" "1.2" "1.4" "0.2.1" "1.0.0-beta3"] (map #(first-in % [:dependency :version]) (deep-content xml [:project :dependencyManagement :dependencies])))) - (is (= [nil nil nil nil "sources" nil nil] + (is (= [nil nil nil nil nil "sources" "sources" nil nil] (map #(first-in % [:dependency :classifier]) (deep-content xml [:project :dependencyManagement :dependencies])))))))) \ No newline at end of file diff --git a/test_projects/managed-deps-snapshot/project.clj b/test_projects/managed-deps-snapshot/project.clj index df1deee4..a75826f5 100644 --- a/test_projects/managed-deps-snapshot/project.clj +++ b/test_projects/managed-deps-snapshot/project.clj @@ -7,7 +7,9 @@ [rome ~(str "0." "9")] [ring/ring "1.0.0"] [ring/ring-codec "1.0.1"] + [ring/ring-headers "0.2.0"] [commons-math/commons-math "1.2" :classifier "sources"] + [org.apache.commons/commons-csv "1.4" :classifier "sources"] [ring/ring-defaults "0.2.1"] [org.clojure/tools.reader "1.0.0-beta3"]] @@ -15,8 +17,10 @@ [rome/rome nil] [ring] [ring/ring-codec nil :exclusions [commons-codec]] + [ring/ring-headers :exclusions [ring/ring-core]] [commons-codec "1.6"] [commons-math nil :classifier "sources"] + [org.apache.commons/commons-csv :classifier "sources"] [org.clojure/tools.emitter.jvm "0.1.0-beta5"] ; depends on tools.reader 0.8.5 [org.clojure/tools.namespace "0.3.0-alpha3"] ; depends on tools.reader 0.10.0 ]) diff --git a/test_projects/managed-deps/project.clj b/test_projects/managed-deps/project.clj index 065259cc..ceecae9d 100644 --- a/test_projects/managed-deps/project.clj +++ b/test_projects/managed-deps/project.clj @@ -7,7 +7,9 @@ [rome ~(str "0." "9")] [ring/ring "1.0.0"] [ring/ring-codec "1.0.1"] + [ring/ring-headers "0.2.0"] [commons-math/commons-math "1.2" :classifier "sources"] + [org.apache.commons/commons-csv "1.4" :classifier "sources"] [ring/ring-defaults "0.2.1"] [org.clojure/tools.reader "1.0.0-beta3"]] @@ -15,8 +17,10 @@ [rome/rome nil] [ring] [ring/ring-codec nil :exclusions [commons-codec]] + [ring/ring-headers :exclusions [ring/ring-core]] [commons-codec "1.6"] [commons-math nil :classifier "sources"] + [org.apache.commons/commons-csv :classifier "sources"] [org.clojure/tools.emitter.jvm "0.1.0-beta5"] ; depends on tools.reader 0.8.5 [org.clojure/tools.namespace "0.3.0-alpha3"] ; depends on tools.reader 0.10.0 ])