jcrossley3 / lein-modules

An alternative to Maven multi-module projects in Leiningen
Eclipse Public License 1.0
83 stars 17 forks source link

Expected behavior of versionization when child doesn't use "_" in version #36

Open ejschoen opened 7 years ago

ejschoen commented 7 years ago

Jim, Thanks for providing lein-modules. It solves a lot of maintenance issues for us. However, I am having trouble understanding the expected behavior of versionization when a child project specifies an explicit version for a library and there is a matching version in the parent's version map.

Case in point: We use Clojure 1.6 for a variety of reasons in most projects, but use 1.8 in a single project. The parent project specifies org.clojure/clojure "1.6.0" and most subprojects specify [org.clojure/clojure "_"] in their dependency vectors. However, the one project that needs Clojure 1.8 specifies [org.clojure/clojure "1.8.0"]. Still, at lein time, versionization replaces the 1.8.0 with 1.6.0. Looking at the lein-modules source, I see why. However, the README explanation lead me to think that a version in a child dependency vector other than "_" shouldn't be replaced.

Am I missing something?

Thanks, Eric

jcrossley3 commented 7 years ago

Hi Eric. I'm glad lein-modules is useful to you.

There is really nothing significant about the underscore at all. From the README: "Whatever you set the version to in your dependency vector will be overwritten if a version is found in :versions."

The only reason I originally recommended using "_" as the version is because lein expected something to be there. Recent versions of lein (since the introduction of :managed-dependencies) do not require a version.

If you need sibling projects to have different versions, you'll need to make sure the dep identifier can't be found in their parents' :versions map. For at least one of the siblings, anyway.

How might I change the README to communicate that better?

ejschoen commented 7 years ago

I guess you could reword the explanation in the example to say that "Any value, even a legal version, that you provide will be overwritten..."

Or could I possibly convince you to change expand-version to be this, which would preserve child project versions when they basically match semver conventions, but replace anything else?

(defn expand-version
  "Recursively search for a version string in the vmap using the first
  non-nil result of trying the following keys, in order: the
  fully-qualified id field of the dependency vector, its version
  field, just the artifact id, and then finally just the group id. If
  none of those are found, just return the version."
  [d vmap]
  (when-let [[id ver & opts] d]
    (if (re-matches #"[\d.]+(-.*)?" (str ver))
      (apply vector id ver opts)
      (let [result (apply vector id
                          (or
                           (recursive-get id vmap)
                           (recursive-get ver vmap)
                           (recursive-get (-> id artifact-map :artifact-id symbol) vmap)
                           (recursive-get (-> id artifact-map :group-id symbol) vmap)
                           ver)
                          opts)]
        result))))
ejschoen commented 7 years ago

Or actually, to avoid breaking anyone depending upon current behavior, something like this:

(defn expand-version
  "Recursively search for a version string in the vmap using the first
  non-nil result of trying the following keys, in order: the
  fully-qualified id field of the dependency vector, its version
  field, just the artifact id, and then finally just the group id. If
  none of those are found, just return the version."
  [d vmap]
  (when-let [[id ver & opts] d]
    (if (and (re-matches #"[\d.]+(-.*)?" (str ver))
             (:keep-version (meta d)))
      (apply vector id ver opts)
      (let [result (apply vector id
                          (or
                           (recursive-get id vmap)
                           (recursive-get ver vmap)
                           (recursive-get (-> id artifact-map :artifact-id symbol) vmap)
                           (recursive-get (-> id artifact-map :group-id symbol) vmap)
                           ver)
                          opts)]
        result))))

Then in a child project, I could do:

:dependencies [^:keep-version [org.clojure/clojure "1.8.0"]...]

I know newer versions of leiningen don't require a version, but we're on an older version ourselves...

jcrossley3 commented 7 years ago

I'm hesitant to introduce a new metadata option to be documented, and I can see where over-writing a real version in a child project could lead to confusion, so I'm partial to your first idea. Let me think on it.