Support managed dependencies in projects

[bbbates]

Using doo in a project that makes use of managed-dependencies (introduced in leiningen ~2.7) results in a failure when running any doo command:

Provided artifact is missing a version: [org.clojure/clojure nil]

Sample project:

(defproject doo-fail-sample "1.0.0-SNAPSHOT"
 :managed-dependencies [[org.clojure/clojure "1.8.0]
                 [org.clojure/clojurescript "1.9.854"]]
  :dependencies [[org.clojure/clojure]
                 [org.clojure/clojurescript]]

  :profiles {:dev {:plugins [[lein-cljsbuild "1.1.7"]
                             [lein-doo "0.1.7"]]

                   :doo {:build "test"}

                   :cljsbuild {:builds {:test {:source-paths ["src" "test"]
                                               :compiler     {:main          my.doo-runner
                                                              :asset-path    "/js/out"
                                                              :output-to     "target/test.js"
                                                              :output-dir    "target/cljstest/public/js/out"
                                                              :optimizations :whitespace
                                                              :pretty-print  true}}}}}})

Running (for example):

lein doo phantom

Easiest way to replicate is to remove the fix in plugin/src/leiningen/doo.clj and run the provided test.

[MatthewDarling]

I opened #145 today, which unintentionally duplicates this.

I checked out lein-figwheel and lein-cljsbuild, which seem to be the source of the affected function, and I don't think doo really needs the select-keys. I think this would be sufficient:

(defn make-subproject [project]
  (with-meta
    (assoc project :local-repo-classpath true)
    (meta project)))

Your test passes with this version, as well. But it's a more drastic change and it would warrant more extensive testing.

[MatthewDarling]

@bbbates Actually, the way you've written this test, it will fail on a system which doesn't have a pre-existing copy of Clojure 1.8.0.

You can reproduce it by doing these steps:

$ rm -rf ~/.m2/repository/org/clojure/clojure/1.8.0/
$ cd plugin
$ lein test

You'll get an error like this:

lein test test.lein-doo.config
Could not find artifact org.clojure:clojure:jar:1.8.0
This could be due to a typo in :dependencies or network issues.
If you are behind a proxy, try setting the 'http_proxy' environment variable.

lein test :only test.lein-doo.config/run-local-project

ERROR in (run-local-project) (core.clj:4617)
Uncaught exception, not in assertion.
expected: nil
  actual: clojure.lang.ExceptionInfo: Could not resolve dependencies
... more stack trace ...

Using Clojure 1.7.0 in the test project works, since doo currently requires Clojure 1.7.0. But it would mean having to keep the two values in sync over time. Instead of doing that, I think it's best to add a test profile which requires the same Clojure version as the one in the test. This way the dependency is explicit.

Here's what that looks like.

[bbbates]

Hmm..ta - I'll take a look.

[bbbates]

@MatthewDarling - Yep, you're right about the missing dependency issue. 🤔
I guess the easiest solution is to change the dependency in the test from "1.8.0" -> "1.7.0" to match the doo dependency graph, and then logically this should not fail again (until doo upgrades to a latter clojure version, of course).

RE your proposal for a fix - I'm happy to use your solution - have you had a chance to test it out in more depth? I've been side-tracked a bit, so haven't gotten time yet, myself.

[MatthewDarling]

"my solution" as in using the :test profile? lein test for the plugin worked in our CI when done that way. And I've been using that version of doo to run our tests. But I don't know if that really counts as "in depth".

I like that having the :test profile makes it explicit that there's a test which relies on a specific dependency being available. But it's too bad it adds another piece of synchronization - updating the main org.clojure/clojure dependency plus the one in :test.

Thinking about that a little more... There's a significantly hackier approach that would remove that synchronization: detecting some existing dependency and then using the version of that. For instance:

user> (-> (System/getProperty "java.class.path")
          (clojure.string/split #":")
          (->> (filter #(re-find #"org.clojure/clojure" %))
               first
               (re-find #"(?:clojure-)(.+)(?:.jar)")
               second))
;; => "1.8.0"

Then you'd just stick that version in the map used by the test.

But, well, this is kind of a personal preference thing. I'd say leave the test as-is (since it passes currently) and wait for an official maintainer judgement.

[miikka]

@MatthewDarling, do you know if :local-repo-classpath actually does anything? It seems to me that it's Leiningen 1.x era feature. I'm not sure if lein-doo even works with Leiningen 1.x, so maybe make-subproject could be removed altogether.

For the record: I'm inclined to merge this PR as it is.

[miikka]

Right, you've reached the same conclusion as me regarding :local-repo-classpath in #145!

[MatthewDarling]

Yeah, I couldn't find any evidence of :local-repo-classpath doing anything in Lein 2.x. But I'm inclined not to mess with stuff I don't 100% understand :)

I think removing make-subproject would make doo more future-proof of new Leiningen features, and simplify the code too. I'll open a PR to remove it; let's see if the CI tests pass.