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.

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

tools.nrepl cannot be specified effectively as a maven dependency. #1569

Closed
phillord opened this issue Jun 17, 2014 · 8 comments

Comments

Projects
None yet
3 participants
@phillord
Copy link

commented Jun 17, 2014

Lein appears to be injected tools.nrepl (and complete) into the dependency tree.

(defproject test-dep "0.1.0-SNAPSHOT"
  :description "FIXME: write description"
  :url "http://example.com/FIXME"
  :license {:name "Eclipse Public License"
            :url "http://www.eclipse.org/legal/epl-v10.html"}
  :dependencies [[org.clojure/clojure "1.6.0"]])

For instance gives:

 lein deps :tree
  [clojure-complete "0.2.3" :exclusions [[org.clojure/clojure]]]
  [org.clojure/clojure "1.6.0"]
  [org.clojure/tools.nrepl "0.2.3" :exclusions [[org.clojure/clojure]]]

Not a huge problem in and off itself, but it causes problems when a pom is produced and nrepl really is a dependency. So for example:

(defproject test-dep "0.1.0-SNAPSHOT"
  :description "FIXME: write description"
  :url "http://example.com/FIXME"
  :license {:name "Eclipse Public License"
            :url "http://www.eclipse.org/legal/epl-v10.html"}
  :dependencies [[org.clojure/clojure "1.6.0"]
                 [org.clojure/tools.nrepl "0.2.3"]])

gives a pom.xml with the following dependencies.

  <dependencies>
    <dependency>
      <groupId>org.clojure</groupId>
      <artifactId>clojure</artifactId>
      <version>1.6.0</version>
    </dependency>
    <dependency>
      <groupId>org.clojure</groupId>
      <artifactId>tools.nrepl</artifactId>
      <version>0.2.3</version>
    </dependency>
    <dependency>
      <groupId>org.clojure</groupId>
      <artifactId>tools.nrepl</artifactId>
      <version>0.2.3</version>
      <exclusions>
        <exclusion>
          <groupId>org.clojure</groupId>
          <artifactId>clojure</artifactId>
        </exclusion>
      </exclusions>
      <scope>test</scope>
    </dependency>
    <dependency>
      <groupId>clojure-complete</groupId>
      <artifactId>clojure-complete</artifactId>
      <version>0.2.3</version>
      <exclusions>
        <exclusion>
          <groupId>org.clojure</groupId>
          <artifactId>clojure</artifactId>
        </exclusion>
      </exclusions>
      <scope>test</scope>
    </dependency>
  </dependencies>

Unfortunately, the tools.nrepl dependency which appears first appears to be overwritten by the second occurrence and that is scoped to test. So no tools.nrepl dependency gets into the pom.xml -- anything using this artifact is then going to crash, unless it gets tools.nrepl from elsewhere.

@pw4ever

This comment has been minimized.

Copy link

commented Jul 9, 2014

I second this with #1593 (already closed).

For a hacky workaround in the upstream project (where we do "lein pom"), add the following to project.clj

{:profiles {:base {:dependencies ^:replace []}}}

Suggestion:

@technomancy

This comment has been minimized.

Copy link
Owner

commented Jul 9, 2014

@pw4ever The solution isn't to drop the test scope, it's to only apply the test scope if it appears in base but not elsewhere. Happy to take a patch for this.

@pw4ever

This comment has been minimized.

Copy link

commented Jul 9, 2014

@technomancy Marking :dev and :test dependencies as "test" scope is understandable. But could you clarify the logic of marking :base dependencies in test scope since it is not empty by default?

I believe a proper solution is to de-duplicate Maven coordinates before generating POM's xml mark-up. If so, what would you suggest as a proper de-duplication logic?

@technomancy

This comment has been minimized.

Copy link
Owner

commented Jul 9, 2014

The :base profile is semantically equivalent to :dev, except it's for things that are included with Leiningen by default rather than specified by the project.

Just adding a :when into the for call on line 328 that checks (:dependencies project) should be enough to fix this.

@pw4ever

This comment has been minimized.

Copy link

commented Jul 9, 2014

@technomancy Thanks for the clarification. That makes sense.

Just to be clear, you mean change the for on https://github.com/technomancy/leiningen/blob/2.4.2/src/leiningen/pom.clj#L328
to

(for [profile [:dev :test :base]
      dep (:dependencies (profile profiles))
      :when (not (contains? (->> (:dependencies project) 
                                 (map first) 
                                 set) 
                             dep)) ]
      (make-scope "test" dep))

to slightly abuse the fact that artifact/id is "first" of the dep vector?

I think the proper de-duplication is on artifact/id rather than the whole dep vector.

@technomancy

This comment has been minimized.

Copy link
Owner

commented Jul 9, 2014

Oh, hmmm... yeah I hadn't considered this, but technically the :extension and :classifier keys are also relevant to this. The dep-key function a few lines up is used for de-duplication similarly, but in this case we don't need :version or :scope, so maybe it makes sense to generalize it.

@pw4ever

This comment has been minimized.

Copy link

commented Jul 9, 2014

@technomancy I guess, in this case, explicitly specified dep in project should override implicit ones 'cause we do not want the final pom.xml to have 2 entries of the same dependency---my understanding is that the latter one will replace, rather than add to, the earlier ones in POM semantics.

@pw4ever

This comment has been minimized.

Copy link

commented Jul 9, 2014

Well, an alternative hack without touching project.clj (which may break, e.g., lein check) is

lein update-in :profiles:base empty -- update-in :profiles:base vec -- pom

Replace pom with install if install to local repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.