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.

Sign up for GitHub

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

manage all dependency settings with managed dependencies #77

Closed
wants to merge 3 commits into from
Closed

manage all dependency settings with managed dependencies #77

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8343007
first working version with tests
Oct 30, 2016
e51bdf1
Merge branch 'master' into manage-all-dep-options
Jan 13, 2017
a14b1e8
make merge-versions-from-managed-coords, conform-coord and unform-coo…
Jan 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 55 additions & 26 deletions src/main/clojure/cemerick/pomegranate/aether.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,17 +612,6 @@ kwarg to the repository kwarg.
(merge {:result result} m))))
coordinates))))


(defn- add-version-from-managed-coord
"Given an entry from a coordinates vector, and the corresponding entry from the
managed coordinates vector, update the version number in the coordinate with the
value from the managed coordinate."
[coord managed-coord]
(if-let [managed-version (second managed-coord)]
(vec (concat [(first coord) managed-version]
(nthrest coord 2)))
(throw (IllegalArgumentException. (str "Provided artifact is missing a version: " coord)))))

(defn- coordinates-match?
[[dep version & opts] [sdep sversion & sopts]]
(let [om (apply hash-map opts)
Expand All @@ -637,28 +626,68 @@ kwarg to the repository kwarg.
(= (:classifier om)
(:classifier som)))))

(defn- find-managed-coord
"Given an entry from a coordinates vector, and a managed coordinates vector, find
the entry in the managed coordinates vector (if any) that matches the coordinate."
[coord managed-coords]
(first (filter #(coordinates-match? coord %) managed-coords)))

(defn- add-version-from-managed-coords-if-missing
"Given a managed coordinates vector and an entry from a coordinates vector, check
to see if the coordinate specifies a version string, and if not, update it with
the version string from the managed coordinates (if it is defined)."
[managed-coords coord]
(if (nil? (second coord))
(add-version-from-managed-coord coord (find-managed-coord coord managed-coords))
coord))
(defn- coord-map-key
"Creates the unique key for the given coordinate map. This is
essentially a representation of the full maven coordinate except
the version."
[{:keys [project extension classifier]}]
[project extension classifier])

(defn- canonical-id [id]
(apply symbol (distinct ((juxt group name) id))))

(defn- conform-coord
"Returns a map describing the coordinate. The full project ID is
under the :project key and the version, if specified and not nil,
under the :version key. All other specified options appear with
the keys and values specified (including nil values)."
[[project & opts]]
(if project
(let [has-version? (odd? (count opts))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hypirion I'd be curious to hear if you had any thoughts on whether this is a reasonably future-proof way to check whether a version is specified. It seems like leiningen is already somewhat committed to this approach so it's probably fine?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more accurate way to do this would be to fully parse the dependency vector. Doing this by hand is a lot of work for little gain. I think the above works for all valid dependency vectors with/without versions, but if you can think of cases where it doesn't I'm happy to change this.

As an aside, I have (alpha) spec definitions for the dependency vectors. I didn't think of adding them here as they would require Clojure 1.9+. However, I can put them in if they are useful, although I'd probably need help in creating the conditional build/test with Clojure 1.9.

(ns deps.spec
  (:require [clojure.spec :as s]))

(s/def ::extension string?)
(s/def ::classifier string?)
(s/def ::scope #{"compile" "test" "provided"})
(s/def ::optional boolean?)

(s/def ::exclusion
  (s/& (s/cat :project symbol?
              :options (s/keys* :opt-un [::extension ::classifier]))
       (s/conformer (fn [{:keys [project options]}] (merge options {:project project}))
                    (fn [{:keys [project] :as m}] (let [m (dissoc m :project)]
                                                    {:project project
                                                     :options m})))))

(s/def ::exclusions (s/coll-of ::exclusion :min-count 1))

(s/def ::dep
  (s/& (s/cat :project symbol?
              :version string?
              :options (s/keys* :opt-un [::extension ::classifier ::scope ::optional ::exclusions]))
       (s/conformer (fn [{:keys [project version options]}]
                      (merge options {:project project, :version version}))
                    (fn [{:keys [project version] :as m}]
                      (let [m (dissoc m :project :version)]
                        {:project project
                         :version version
                         :options m})))))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cprice404: Yeah, this should be fine. If this changes in tools depending on pomegranate, then they can just normalize the input arguments.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@loomis nice... you might be interested in technomancy/leiningen#2223, which introduces some spec stuff for the full lein project map (and does so in a way that doesn't require Clojure 1.9 AFAIK). It might be nice to try to consolidate the parts of the spec that are common - e.g. dependency vectors... and maybe pomegranate is a reasonable place to do that. (Not suggesting it would need to be done as part of this PR or anything, just food for thought :) )

version (if has-version? (first opts))
opts (if has-version? (rest opts) opts)
opts-map (apply hash-map opts)]
(-> (if (and has-version? version) {:version version} {})
(assoc :project (canonical-id project))
(merge opts-map)))))

(defn- strip-defaults [dep-map]
(remove #(or (some #{:project :version} %)
(= [:scope "compile"] %)
(= [:extension "jar"] %)
(= [:optional false] %)) dep-map))

(defn- unform-coord
"Returns the canonical, vector form of a dependency that was
specified as a map."
[{:keys [project version] :as dep-map}]
(-> (if version [project version] [project])
(cons (strip-defaults dep-map))
((partial mapcat identity))
(vec)))

(defn- managed-coords-map [managed-coords]
(->> managed-coords
(map conform-coord)
(map (juxt coord-map-key identity))
(into {})))

(defn- merge-managed-coord [managed-coords-m coord]
(let [coord-map (conform-coord coord)
coord-key (coord-map-key coord-map)
managed-coord (get managed-coords-m coord-key)
merged-map (merge managed-coord coord-map)]
(if (:version merged-map)
(unform-coord (into {} (remove (comp nil? second) merged-map)))
(throw (IllegalArgumentException. (str "Provided artifact is missing a version: " coord))))))

(defn- merge-versions-from-managed-coords
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be awesome if you could make this fn public while you're in here; the fact that it is private was an oversight on my part, and led to this nastiness: https://github.com/technomancy/leiningen/blob/master/leiningen-core/src/leiningen/core/classpath.clj#L571

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made merge-versions-from-managed-coords public and added a simple test. Edge cases are tested in the tests for the other functions, so didn't try to be exhaustive with this one.

I've made conform-coord and unform-coord public as well. These seem to be utilities that crop up every time someone's having to manipulate dependencies. If you don't think these are generally useful, I can make them private again.

"Given a vector of coordinates (e.g. [[group/name <\"version\"> & settings] ..])
where the version field is optional or can be nil, and a vector of managed coordinates,
returns an updated vector of coordinates with version numbers merged in from the
managed-coordinates vector as necessary."
[coordinates managed-coordinates]
(vec (map (partial add-version-from-managed-coords-if-missing managed-coordinates)
(vec (map (partial merge-managed-coord (managed-coords-map managed-coordinates))
coordinates)))

(defn- coords->Dependencies
Expand Down
64 changes: 64 additions & 0 deletions src/test/clojure/cemerick/pomegranate/aether_test.clj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,70 @@
['tester "0.1.0-20120403.012847-1"] nil}
(aether/dependency-hierarchy coords deps)))))

(deftest check-canonical-id
(let [f @#'aether/canonical-id]
(are [expect input] (= expect (f input)))
'foo 'foo
'foo 'foo/foo
'foo/bar 'foo/bar))

(deftest check-conform-coord
(let [f @#'aether/conform-coord]
(are [expect input] (= expect (f input))
nil nil
{:project 'foo} '[foo]
{:project 'foo/bar} '[foo/bar]
{:project 'foo} '[foo nil]
{:project 'foo, :version "1.2.3"} '[foo "1.2.3"]
{:project 'foo, :version "1.2.3", :scope "test"} '[foo "1.2.3" :scope "test"]
{:project 'foo, :scope "test"} '[foo :scope "test"]
{:project 'foo, :scope "test"} '[foo nil :scope "test"]
{:project 'foo, :scope nil} '[foo nil :scope nil]
{:project 'foo, :scope "test", :optional true} '[foo :scope "test" :optional true]
{:project 'foo, :version "1.2.3" :exclusions '[[bar]]} '[foo "1.2.3" :exclusions [[bar]]])))

(deftest check-unform-coord
(let [f @#'aether/unform-coord]
(are [expect input] (= expect (f input))
'[foo] {:project 'foo}
'[foo/bar] {:project 'foo/bar}
'[foo "1.2.3"] {:project 'foo, :version "1.2.3"}
'[foo "1.2.3" :scope "test"] {:project 'foo, :version "1.2.3", :scope "test"}
'[foo :scope "test"] {:project 'foo, :scope "test"}
'[foo :scope nil] {:project 'foo, :scope nil}

'[foo :scope "test"] {:project 'foo, :scope "test"}
'[foo] {:project 'foo, :scope "compile"}
'[foo :optional true] {:project 'foo, :optional true}
'[foo] {:project 'foo, :optional false}
'[foo :extension "zip"] {:project 'foo, :extension "zip"}
'[foo] {:project 'foo, :extension "jar"}

'[foo "1.2.3" :exclusions [[bar]]] {:project 'foo, :version "1.2.3" :exclusions '[[bar]]})))

(deftest check-merge-managed-coord
(let [f @#'aether/merge-managed-coord
managed-coords-map @#'aether/managed-coords-map
managed-coords-m (managed-coords-map
'[[demo "1.0.0"]
[demo-test "2.0.0" :scope "test"]
[demo-compile "3.0.0" :scope "compile"]
[demo-excl "4.0.0" :exclusions [[demo] [demo-test]]]])]
(is (thrown-with-msg? IllegalArgumentException #"Provided artifact is missing a version: "
(f nil nil)))
(is (thrown-with-msg? IllegalArgumentException #"Provided artifact is missing a version: \[demo-unk\]"
(f nil '[demo-unk])))
(is (= '[demo "1.0.0"] (f nil '[demo/demo "1.0.0"])))
(is (= '[demo "1.0.0"] (f managed-coords-m '[demo])))
(is (= '[demo "1.0.0"] (f managed-coords-m '[demo nil])))
(is (= '[demo-test "2.0.0" :scope "test"] (f managed-coords-m '[demo-test])))
(is (= '[demo-test "2.0.0" :scope "provided"] (f managed-coords-m '[demo-test :scope "provided"])))
(is (= '[demo-test "2.0.0"] (f managed-coords-m '[demo-test :scope "compile"])))
(is (= '[demo-test "2.0.0"] (f managed-coords-m '[demo-test :scope nil])))
(is (= '[demo-compile "3.0.0"] (f managed-coords-m '[demo-compile])))
(is (= '[demo-excl "4.0.0" :exclusions [[demo] [demo-test]]] (f managed-coords-m '[demo-excl])))
(is (= '[demo-excl "4.0.0"] (f managed-coords-m '[demo-excl :exclusions nil])))))

(comment
"tests needed for:
repository authentication
Expand Down