Merge pull request technomancy#2207 from cprice404/2195-dont-require-…

…nil-for-version-in-managed-deps

Don't require `nil` for version in managed deps - fixes technomancy#2195

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
 

leiningen-core/src/leiningen/core/classpath.clj

@@ -531,14 +531,46 @@
                                    :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
+       (into [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
   ;;  is needed here, but was accidentally marked as private.  Calling it
   ;;  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.

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)

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]))))))))

test_projects/managed-deps-snapshot/project.clj

@@ -7,16 +7,20 @@
                  [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"]]
 
   :dependencies [[org.clojure/clojure]
                  [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
                  ])

test_projects/managed-deps/project.clj

@@ -7,16 +7,20 @@
                  [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"]]
 
   :dependencies [[org.clojure/clojure]
                  [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
                  ])