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

Document order-dependent semantics of :pedantic #1337

Closed
ghadishayban opened this issue Oct 2, 2013 · 11 comments
Closed

Document order-dependent semantics of :pedantic #1337

ghadishayban opened this issue Oct 2, 2013 · 11 comments
Labels
Milestone

Comments

@ghadishayban
Copy link
Contributor

Background facts
Clojurescript 0.0-1909 requires tools.reader, as does clj-http 0.7.7.

clojurescript 0.0-1909 requires tools.reader 0.7.8
clj-http 0.7.7 requires tools.reader 0.7.7

Issue Part 1
lein deps :tree inside a project with clojurescript and clj-http versions above will not give a warning.
lein deps :tree shows tools.reader 0.7.8 underneath the clojurescript subtree, while the clj-http subtree is missing a transitive dep to tools.reader

Issue Part 2
However, lein classpath will show tools.reader 0.7.7 (the one needed for clj-http) being loaded, which breaks Clojurescript.

@cemerick
Copy link
Collaborator

cemerick commented Oct 2, 2013

Each dependency is only ever shown once within a tree, so the "missing" transitive dep is expected.

But, I can't repro: given the noted deps, I see tools.reader 0.7.7 under clj-http, matching what's in the classpath; the workaround for that is known (exclude tools.reader from clj-http).

@technomancy
Copy link
Owner

I can't reproduce the mismatch you're describing between deps :tree and classpath either. Can you include a minimal repro case project.clj file? What version of Leiningen are you using?

However, I can confirm that if you put the clojurescript dependency ahead of clj-http, you don't get the warning you should, which is definitely a bug.

@ghadishayban
Copy link
Contributor Author

Lein 2.3.2
Yes you're right I think order does matter.

No warning:

  :dependencies [[org.clojure/clojure "1.5.1"]
                 [org.clojure/clojurescript "0.0-1909"]
                 [clj-http "0.7.7"]]

Move clj-http to the front and you get a warning.

@xeqi
Copy link
Collaborator

xeqi commented Oct 3, 2013

Given

$ lein version
Leiningen 2.3.2 on Java 1.7.0_03 OpenJDK 64-Bit Server VM

Using

:dependencies [[org.clojure/clojure "1.5.1"]
               [org.clojure/clojurescript "0.0-1909"]
               [clj-http "0.7.7"]]

I get

$ lein deps :tree
 [clj-http "0.7.7"]
   [cheshire "5.2.0"]
     [com.fasterxml.jackson.core/jackson-core "2.2.1"]
     [com.fasterxml.jackson.dataformat/jackson-dataformat-smile "2.2.1"]
     [tigris "0.1.1"]
   [commons-codec "1.8"]
   [commons-io "2.4"]
   [crouton "0.1.1" :exclusions [[org.clojure/clojure]]]
     [org.jsoup/jsoup "1.7.1"]
   [org.apache.httpcomponents/httpclient "4.3"]
     [commons-logging "1.1.3"]
   [org.apache.httpcomponents/httpcore "4.3"]
   [org.apache.httpcomponents/httpmime "4.3"]
   [slingshot "0.10.3" :exclusions [[org.clojure/clojure]]]
 [clojure-complete "0.2.3" :exclusions [[org.clojure/clojure]]]
 [org.clojure/clojure "1.5.1"]
 [org.clojure/clojurescript "0.0-1909"]
   [com.google.javascript/closure-compiler "v20130603"]
     [args4j "2.0.16"]
     [com.google.code.findbugs/jsr305 "1.3.9"]
     [com.google.guava/guava "14.0.1"]
     [com.google.protobuf/protobuf-java "2.4.1"]
     [org.json/json "20090211"]
   [org.clojure/data.json "0.2.3"]
   [org.clojure/google-closure-library "0.0-20130212-95c19e7f0f5f"]
     [org.clojure/google-closure-library-third-party "0.0-20130212-95c19e7f0f5f"]
   [org.clojure/tools.reader "0.7.8"]
   [org.mozilla/rhino "1.7R4"]
 [org.clojure/tools.nrepl "0.2.3" :exclusions [[org.clojure/clojure]]]

and

$ lein classpath | egrep -o 'tools.reader-[0-9]+.[0-9]+.[0-9]+.jar'
tools.reader-0.7.8.jar

This is the latest of the two versions specified by the transitive dependencies. No warning is given by lein deps :tree. The version shown by both commands is the same.


Using

:dependencies [[org.clojure/clojure "1.5.1"]                                                                                                        
               [clj-http "0.7.7"]
               [org.clojure/clojurescript "0.0-1909"]]

I get

$ lein deps :tree
WARNING!!! possible confusing dependencies found:
[clj-http "0.7.7"] -> [org.clojure/tools.reader "0.7.7"]
 overrides
[org.clojure/clojurescript "0.0-1909"] -> [org.clojure/tools.reader "0.7.8"]

 [clj-http "0.7.7"]
   [cheshire "5.2.0"]
     [com.fasterxml.jackson.core/jackson-core "2.2.1"]
     [com.fasterxml.jackson.dataformat/jackson-dataformat-smile "2.2.1"]
     [tigris "0.1.1"]
   [commons-codec "1.8"]
   [commons-io "2.4"]
   [crouton "0.1.1" :exclusions [[org.clojure/clojure]]]
     [org.jsoup/jsoup "1.7.1"]
   [org.apache.httpcomponents/httpclient "4.3"]
     [commons-logging "1.1.3"]
   [org.apache.httpcomponents/httpcore "4.3"]
   [org.apache.httpcomponents/httpmime "4.3"]
   [org.clojure/tools.reader "0.7.7" :exclusions [[org.clojure/clojure]]]
   [slingshot "0.10.3" :exclusions [[org.clojure/clojure]]]
 [clojure-complete "0.2.3" :exclusions [[org.clojure/clojure]]]
 [org.clojure/clojure "1.5.1"]
 [org.clojure/clojurescript "0.0-1909"]
   [com.google.javascript/closure-compiler "v20130603"]
     [args4j "2.0.16"]
     [com.google.code.findbugs/jsr305 "1.3.9"]
     [com.google.guava/guava "14.0.1"]
     [com.google.protobuf/protobuf-java "2.4.1"]
     [org.json/json "20090211"]
   [org.clojure/data.json "0.2.3"]
   [org.clojure/google-closure-library "0.0-20130212-95c19e7f0f5f"]
     [org.clojure/google-closure-library-third-party "0.0-20130212-95c19e7f0f5f"]
   [org.mozilla/rhino "1.7R4"]
 [org.clojure/tools.nrepl "0.2.3" :exclusions [[org.clojure/clojure]]]

and

$ lein classpath | egrep -o 'tools.reader-[0-9]+.[0-9]+.[0-9]+.jar'
tools.reader-0.7.7.jar

This is the earlier of the two versions specified by the transitive dependencies. A warning is given by lein deps :tree. The version shown by both commands is the same.


It appears that the correct behavior occurs in both cases. @technomancy what am I missing that you think is a bug?

@technomancy
Copy link
Owner

@xeqi ah, I thought that the warning was supposed to be triggered for any conflict, but what you're saying is the warning is "hey, the newest one didn't win here, which is probably not what you want". I guess I misunderstood the original intent.

@ghadishayban
Copy link
Contributor Author

Bug or documentation change? I was also under the impression that it would trigger on any conflict.

@technomancy
Copy link
Owner

Yeah, I'm not sure how I feel about the current behaviour; intentional or not. I know on some level it's impossible to avoid order-dependent semantics, but it seems pretty unexpected here.

What's the motivation behind silently allowing conflicts of lower version numbers which come later?

@xeqi
Copy link
Collaborator

xeqi commented Oct 8, 2013

What's the motivation behind silently allowing conflicts of lower version numbers which come later?

The assumptions the warnings are based of is:
Given requirements A and B on a version of a dependency
Given Y is the version chosen by requirement A
Given Z is the version chosen by requirement B
Given B is the requirement used by dependency ordering

  1. When A is a top level requirement and Y not= Z; Then warn
  2. When Y > Z; Then warn (expect the later version)

Condition 2 is what we are referring to in this issue. The ordering of the top level dependencies affects which version of org.clojure/tools.reader is chosen (becomes Z). When the lower version is chosen condition 2 is violated, and a warning is presented. When the higher version is chosen, condition 2 is satisfied and no warning happens.

Lets consider some requirements condition 2 could be:

Do not care about Y and Z.

This case can be discarded, as it does not address the original motivation for the warnings: cemerick/friend#15 (which has been followed by cemerick/friend#57 and cemerick/friend#61).

When Y not= Z; Then error

This case will cause an error anytime two versions are possible. This is the most pedantic of the options, and the most annoying. This would require always excluding org.clojure/clojure from ring or any other library that declares backwards compatibility.

When Y < Z; Then error

This case will cause an error if the later of the two version is chosen. This would break whatever is relying on the functionality present in Y that is not in Z. Since Z is the later version, it is possible some functionality was removed, such as on a major version change on a sem ver project. However, is likely that a major change like that will cause breakage when either choice is made, and expecting the earlier version seems backwards, so we discard this option

When Y > Z; Then error

This case will cause an error if the later of the two version is chosen. This would break whatever is relying on the functionality present in Z that is not in Y. Since Z is the later version, it is likely that new functionality was added. Also, since Z was the result of a requirement, there is a decent chance the new functionality is needed.


Thus, the options are to warn on any conflict, or warn if not the latest. The verbosity required by warning on every conflict is overwhelming when using common libraries, such as clojure, and the apache projects. Therefore warn if not latest was chosen.

I've talked to @cemerick about the overall concept several times, and we have come to the same conclusion on the expect latest behavior. I'd be interested in getting his reasoning here as well, as it maybe articulated in a different/better way then mine.

@hypirion
Copy link
Collaborator

hypirion commented Oct 8, 2013

@xeqi, I can see the rationale for the warnings, what I think we wonder about is why said situation happens in the first place. I'm not sure why the ordering of the top level dependencies affect which version we pick on transitive dependencies, or why that would be a sane choice.

Is this a result of how Aether handle conflict resolutions?

@xeqi
Copy link
Collaborator

xeqi commented Oct 9, 2013

Is this a result of how Aether handle conflict resolutions?

Yep.

I'm not sure why the ordering of the top level dependencies affect which version we pick on other dependencies

Imagine you have a dependency graph like:

[A "1"] -> [[B "1"]]
[B "1"] -> []
[B "2"] -> []
[D "1"] -> [[B "2"]]

and then you have a dependency list of [[A "1"] [D "1"]]. What version of B do you get? Why?

.
.
.
.
.
.
.
.

Maven/Aether has decided to use a breadth first search, which makes ordering matter. In particular it uses a NearestVersionConflictResolver. For [[A "1"] [D "1"]] it would chose [B "1"]. For [[D "1"] [A "1"]] it would chose [B "2"].

or why that would be a sane choice.

Given you have to make a choice between 2 versions, basing it off of ordering uses the only user provided information available to make a deterministic choice.

@technomancy
Copy link
Owner

Thanks for the explanation @xeqi. I agree that warning on all mismatches is too noisy to be useful in something commonly used like lein deps :tree, but I think there are still some cases where it would be helpful to flag all mismatches for debugging. The docs aren't clear on the topic and seem to imply that any mismatch would trigger a warning, so that should definitely be cleared up.

hypirion added a commit that referenced this issue Nov 9, 2013
hypirion added a commit that referenced this issue Nov 10, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants