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

Eviction warning is unclear #2699

Closed
chris-martin opened this issue Aug 17, 2016 · 35 comments
Closed

Eviction warning is unclear #2699

chris-martin opened this issue Aug 17, 2016 · 35 comments
Assignees

Comments

@chris-martin
Copy link

Example output from a compile using SBT 0.13.7:

[warn] There may be incompatibilities among your library dependencies.
[warn] Here are some of the libraries that were evicted:
[warn]  * com.google.code.findbugs:jsr305:2.0.1 -> 3.0.0
[warn] Run 'evicted' to see detailed eviction warnings

I don't understand what 2.0.1 -> 3.0.0 signifies. Which version was evicted, 2.0.1 or 3.0.0? Which version was used? What does the arrow mean?

@eed3si9n
Copy link
Member

Ivy normally picks the latest. In this case, 2.0.1 got evicted in favor of 3.0.0.

@chris-martin
Copy link
Author

I take it you disagree that the warning is insufficiently explanatory?

@eed3si9n
Copy link
Member

eed3si9n commented Aug 17, 2016

I'm open to alternative wording, but I personally think it's clear enough once you know what it means. Given that the user might see the message multiple times a day, this sort of messages need to err on the shorter/insufficient side.

@chris-martin
Copy link
Author

Replacing the arrow with your English explanation adds 22 characters:

[warn] There may be incompatibilities among your library dependencies.
[warn] Here are some of the libraries that were evicted:
[warn]  * com.google.code.findbugs:jsr305: 2.0.1 got evicted in favor of 3.0.0
[warn] Run 'evicted' to see detailed eviction warnings

@chris-martin
Copy link
Author

once you know what it means

That's the tricky part. Is there a path to finding out what it means?

@eed3si9n
Copy link
Member

If someone googles "sbt eviction warning" the first hit would be the documentation http://www.scala-sbt.org/0.13/docs/Library-Management.html#Eviction+warning and the second is this blog post Improved dependency management with sbt 0.13.7.

If someone knows what "eviction" means, I think current arrow is clear enough. Also note there might be multiple (potentially many) entries

[warn] There may be incompatibilities among your library dependencies.
[warn] Here are some of the libraries that were evicted:
[warn]  * com.google.code.findbugs:jsr305:2.0.1 -> 3.0.0
[warn]  * bar.foo:x:2.0.1 -> 3.0.0
[warn]  * io.commons:io.commons:2.0.1 -> 3.0.0
[warn]  * com.typesafe.akka:akka-actor_2.10:2.1.4 -> 2.3.7
[warn] Run 'evicted' to see detailed eviction warnings

Turning it into English I'm not sure would make it better if someone doesn't know what "eviction" means. Also it might make it harder to read.

[warn] There may be incompatibilities among your library dependencies.
[warn] Here are some of the libraries that were evicted:
[warn]  * com.google.code.findbugs:jsr305:2.0.1 got evicted in favor of 3.0.0
[warn]  * bar.foo:x:2.0.1 got evicted in favor of 3.0.0
[warn]  * io.commons:io.commons:2.0.1 got evicted in favor of 3.0.0
[warn]  * com.typesafe.akka:akka-actor_2.10:2.1.4 got evicted in favor of 2.3.7
[warn] Run 'evicted' to see detailed eviction warnings

@christopher-prince
Copy link

I have to smile at "it's clear enough once you know what it means". It suggests it's meaning is't clear from the notation itself.

@eed3si9n
Copy link
Member

Fair enough. I'll reopen this case to solicit more suggestions.

@eed3si9n
Copy link
Member

Perhaps we can adopt some vertical alignment that seems to be in vogue:

[warn] There may be incompatibilities among your library dependencies.
[warn] Here are some of the libraries that were evicted:
[warn]  * com.google.code.findbugs:jsr305:2.0.1               became 3.0.0
[warn]  * bar.foo:x:2.0.1                                     became 3.0.0
[warn]  * io.commons:io.commons:2.0.1                         became 3.0.0
[warn]  * com.typesafe.akka:akka-actor_2.10:2.1.4             became 2.3.7
[warn] Run 'evicted' to see detailed eviction warnings

@NeQuissimus
Copy link

I like the alignment, but maybe "was replaced by"?

@jvican
Copy link
Member

jvican commented May 11, 2017

@christopher-prince If you want to take a shot, you can change the wording of that warning here: https://github.com/sbt/librarymanagement/blob/1.0/librarymanagement/src/main/scala/sbt/librarymanagement/EvictionWarning.scala.

Make sure you also change the tests here: https://github.com/sbt/librarymanagement/blob/1.0/librarymanagement/src/test/scala/EvictionWarningSpec.scala.

In my opinion, I would like to see something like:

[warn] There may be incompatibilities among your library dependencies.
[warn] The conflict strategy picked up the following revisions:
[warn]    -> com.google.code.findbugs:jsr305:3.0.0 instead of 2.0.1
[warn]    -> bar.foo:x:3.0.0 instead of 2.0.1
[warn]    -> io.commons:io.commons:3.0.0 instead of 3.0.0
[warn]    -> com.typesafe.akka:akka-actor_2.10:2.3.7 instead of 2.1.4
[warn] Run 'evicted' to see the detailed eviction warnings.

Note that we first list the chosen revision, not the evicted one. There are also some stylistic changes. I don't have strong opinions on the vertical alignment, but given that it's not common in sbt, we should not use it (or if we do, it should become optional).

The arrows will be coloured with the same warning color.

@chris-martin
Copy link
Author

chris-martin commented May 11, 2017

I really think the word "eviction" is the problem. It just doesn't make much sense from a user's perspective. There were multiple requested versions of the same library, SBT has to pick one, so is picks the latest one. That's not what eviction means to me at all.

@jvican
Copy link
Member

jvican commented May 11, 2017

I agree the term is confusing. My rewording does not use the word eviction, it only uses it at the end to let users know what command to use to check all the logs.

@eed3si9n
Copy link
Member

For the record when you run evicted you get more detailed report:

project> evicted
[info] Resolving org.fusesource.jansi#jansi;1.4 ...
[info] Done updating.
[warn] There may be incompatibilities among your library dependencies.
[warn] Here are some of the libraries that were evicted:
[warn] 	* com.typesafe.sbt:sbt-site:0.8.1 -> 0.8.2 (caller: com.typesafe.sbt:sbt-ghpages:0.5.4 (sbtVersion=0.13, scalaVersion=2.10), default:project:0.1-SNAPSHOT (sbtVersion=0.13, scalaVersion=2.10))
[warn] 	* io.netty:netty:3.6.4.Final -> 3.9.2.Final (caller: com.ning:async-http-client:1.8.10, net.databinder:unfiltered-netty_2.10:0.7.0)
[warn] 	* net.databinder:unfiltered-util_2.10:0.6.8 -> 0.7.0 (caller: net.databinder:unfiltered-jetty_2.10:0.6.8, net.databinder:unfiltered-netty-server_2.10:0.7.0, net.databinder:unfiltered_2.10:0.7.0)
[info] Here are other libraries that were evicted:
[info] 	* org.codehaus.plexus:plexus-utils:3.0.17 -> 3.0.20 (caller: org.eclipse.sisu:org.eclipse.sisu.plexus:0.3.0, org.apache.maven:maven-model:3.3.3, org.apache.maven:maven-artifact:3.3.3)
[info] 	* org.slf4j:slf4j-api:(1.7.2, 1.7.5) -> 1.7.7 (caller: com.ning:async-http-client:1.8.10, org.asciidoctor:asciidoctorj:1.5.2, org.eclipse.jgit:org.eclipse.jgit:3.7.0.201502260915-r, org.slf4j:slf4j-nop:1.7.7)

@eed3si9n
Copy link
Member

I really think the word "eviction" is a problem. It just doesn't make much sense from a user's perspective.

I accept the possibility that I could be blind to "eviction" being a jargon or I could be misusing the term. In Ivy's conflict resolution, a version or an entire module is said to be "evicted" when it loses the conflict resolution - http://ant.apache.org/ivy/history/2.3.0/ivyfile/conflicts.html

There were multiple requested versions of the same library, SBT has to pick one, so is picks the latest one. That's not what eviction means to me at all.

sbt gets request for the direct dependencies. Eviction often happens with transitive dependencies, like versions of commons-io. To make the matter complicated, it's not always the latest one since Ivy tries to emulate Maven's nearest-wins semantics.

Also note that sbt displays the warning when it suspects API breakage has occurred based on SemVer (or its Scala variation). If not eviction how should we generally call this phenomenon?

@chris-martin
Copy link
Author

I guess here's a thing we should ask: Is this something that only happens in the JVM ecosystem, or do build tools in other languages also need to do eviction? And if they do, do they call it that, or describe it in some other way?

@chris-martin
Copy link
Author

In that example, what's the difference between "some of the libraries" and "other libraries"?

project> evicted
...
[warn] Here are some of the libraries that were evicted:
...
[info] Here are other libraries that were evicted:
...

Why is the list split into two groups, and what determines which eviction goes in which list?

@chris-martin
Copy link
Author

Is this the gist of the information we're trying to display?

project> dependencyConflicts

Package                                                  Depends on                           Version         But we're using this version instead
-------------------------------------------------------  -----------------------------------  --------------  ------------------------------------
com.typesafe.sbt:sbt-ghpages:0.5.4                       com.typesafe.sbt:sbt-site            0.8.1           0.8.2
com.ning:async-http-client:1.8.10                        io.netty:netty                       3.6.4.Final     3.9.2.Final
net.databinder:unfiltered-jetty_2.10:0.6.8               net.databinder:unfiltered-util_2.10  0.6.8           0.7.0
net.databinder:unfiltered-netty-server_2.10:0.7.0        net.databinder:unfiltered-util_2.10  0.6.8           0.7.0
net.databinder:unfiltered_2.10:0.7.0                     net.databinder:unfiltered-util_2.10  0.6.8           0.7.0
org.eclipse.sisu:org.eclipse.sisu.plexus:0.3.0           org.codehaus.plexus:plexus-utils     3.0.17          3.0.20
org.apache.maven:maven-model:3.3.3                       org.codehaus.plexus:plexus-utils     3.0.17          3.0.20
org.apache.maven:maven-artifact:3.3.3                    org.codehaus.plexus:plexus-utils     3.0.17          3.0.20
com.ning:async-http-client:1.8.10                        org.slf4j:slf4j-api                  (1.7.2, 1.7.5)  1.7.7
org.asciidoctor:asciidoctorj:1.5.2                       org.slf4j:slf4j-api                  (1.7.2, 1.7.5)  1.7.7
org.eclipse.jgit:org.eclipse.jgit:3.7.0.201502260915-r   org.slf4j:slf4j-api                  (1.7.2, 1.7.5)  1.7.7
org.slf4j:slf4j-nop:1.7.7                                org.slf4j:slf4j-api                  (1.7.2, 1.7.5)  1.7.7

@clhodapp
Copy link

clhodapp commented May 12, 2017

What about:

project> dependencyConflicts
Due to the way the JVM works, when more than one version of the same library is present at
runtime, undefined behavior will occur. To avoid this, sbt applies a conflict resolution strategy when
your project depends on more than one version of a library. The default strategy is to use the latest
version, but this can be overridden.

Here is a list of all the dependency conflicts for your project and what sbt did to resolve each:

Package                                                  Depends on                           Version         But we're using this version instead
-------------------------------------------------------  -----------------------------------  --------------  ------------------------------------
com.typesafe.sbt:sbt-ghpages:0.5.4                       com.typesafe.sbt:sbt-site            0.8.1           0.8.2
com.ning:async-http-client:1.8.10                        io.netty:netty                       3.6.4.Final     3.9.2.Final
net.databinder:unfiltered-jetty_2.10:0.6.8               net.databinder:unfiltered-util_2.10  0.6.8           0.7.0
net.databinder:unfiltered-netty-server_2.10:0.7.0        net.databinder:unfiltered-util_2.10  0.6.8           0.7.0
net.databinder:unfiltered_2.10:0.7.0                     net.databinder:unfiltered-util_2.10  0.6.8           0.7.0
org.eclipse.sisu:org.eclipse.sisu.plexus:0.3.0           org.codehaus.plexus:plexus-utils     3.0.17          3.0.20
org.apache.maven:maven-model:3.3.3                       org.codehaus.plexus:plexus-utils     3.0.17          3.0.20
org.apache.maven:maven-artifact:3.3.3                    org.codehaus.plexus:plexus-utils     3.0.17          3.0.20
com.ning:async-http-client:1.8.10                        org.slf4j:slf4j-api                  (1.7.2, 1.7.5)  1.7.7
org.asciidoctor:asciidoctorj:1.5.2                       org.slf4j:slf4j-api                  (1.7.2, 1.7.5)  1.7.7
org.eclipse.jgit:org.eclipse.jgit:3.7.0.201502260915-r   org.slf4j:slf4j-api                  (1.7.2, 1.7.5)  1.7.7
org.slf4j:slf4j-nop:1.7.7                                org.slf4j:slf4j-api         

@eed3si9n
Copy link
Member

Why is the list split into two groups, and what determines which eviction goes in which list?

sbt first guesses if the result of the version conflict would result to a potential runtime error by comparing the version numbers for each modules. Using Semantic versioning (major.minor.patch) for Java modules, and a variation commonly used in Scala (epoch.major.minor) for Scala modules.

For example:

[warn]  * com.google.code.findbugs:jsr305:2.0.1 -> 3.0.0

According to Semantic versioning, change in the first segment from 2 to 3 could implies potential lack of backward compatibility.

[info] 	* org.slf4j:slf4j-api:(1.7.2, 1.7.5) -> 1.7.7

Above on the other hand would be backward compatible if the library were following Semantic versioning.

@eed3si9n
Copy link
Member

The warning level gets displayed automatically after running update (which is called on compile then cached). There's currently a bug on that but that's another story. Running evicted shows both the API-breakage suspects and non-suspects.

@eed3si9n
Copy link
Member

Here's @paulp's suggestion that I got a few hours ago on Twitter (https://twitter.com/contrarivariant/status/862827568382214148):

Evictions differing in patch version:

com.chuusai:shapeless_2.11:{ 2.3.2, 2.3.1, 2.3.0 }
  eu.timepit:refined_2.11:0.8.0                               depends on 2.3.2
  org.quasar-analytics:quasar-foundation-internal_2.11:17.0.2 depends on 2.3.1
  org.tpolecat:doobie-core_2.11:0.3.0                         depends on 2.3.0
  org.parboiled:parboiled_2.11:2.1.3                          depends on 2.3.0

Evictions differing in minor version:

org.scala-lang.modules:scala-java8-compat_2.11:{ 0.8.0, 0.7.0 }
  org.http4s:http4s-core_2.11:0.15.9a                depends on 0.8.0
  com.github.julien-truffaut:monocle-core_2.11:1.4.0 depends on 0.7.0

@eed3si9n
Copy link
Member

eed3si9n commented May 12, 2017

I guess here's a thing we should ask: Is this something that only happens in the JVM ecosystem, or do build tools in other languages also need to do eviction? And if they do, do they call it that, or describe it in some other way?

I Googled a bit, but don't have a clear answer here.

side note

The general issue of dependency conflict (diamond problem, conflict resolution, dependency hell etc) is an universal problem.

JVM tools like sbt / Ivy let user over-specify the module dependency to a version number (due to culture or lack of good index?). Strict interpretation of the dependency quickly fails, so the tools implemented lenient (automatic) conflict resolution algorithms. Using Ivy, we interpret com.example:foo:2.0.1 to be an advisory that you want version >= 2.0.1. This is now under-specification because this would admit backward incompatible modules. Eviction warning (#1200) is my attempt at swinging the pendulum back to more stricter side.

More complete solution might be:

  • Allow modules to declare version semantics.
  • Easier version range notation like that used by npm such as 1.x and ^1.2.3 - https://docs.npmjs.com/misc/semver
  • Dependency manager that is able to resolve version ranges efficiently.
  • Fail resolution when the constraints are not satisfied, and let the user override them.

Interestingly the users of the tools that uses the above range-and-strict combo seem to be asking for lenient resolution anyway (e.g. rubygems/bundler#4552) so the grass might always look greener on the other side.

@omervk
Copy link

omervk commented May 12, 2017

As an aside, I would love to be able to see which module was the source of the newer dependency, too. It takes way too long to figure out a->b->c(1.0) got evicted in favor of d->e->f->c(1.1).

@chris-martin
Copy link
Author

I would love to be able to see which module was the source of the newer dependency

That may be more than an aside here; I think it would help explain what's going on.

@jvican
Copy link
Member

jvican commented May 12, 2017

Allow modules to declare version semantics.

@eed3si9n Yes, please, this is utterly important. We can add this information to a module ID.

Easier version range notation like that used by npm such as 1.x and ^1.2.3 - https://docs.npmjs.com/misc/semver

I think this is a bad idea. We should be using ordinary semantic versioning. JS and other programming languages need that solution because the community does not respect semantic versioning. Allowing this notation will complicate the current situation, not make it easier, and it's not necessary once you specify the semantic versioning that a module uses. We don't need more ways to express dependencies.

Dependency manager that is able to resolve version ranges efficiently.

Dependency lock file will help here.

@clhodapp I don't like your proposed message, it's utterly verbose. People that don't know about semantic versioning and dependency conflicts should look it up. Also, this:

Due to the way the JVM works, when more than one version of the same library is present at
runtime, undefined behavior will occur. 

is not accurate. There is no way you can have two versions of the same library in the same classpath. The problem comes when a library depends on another library whose version has been changed, and whose JVM classes have suffered binary changes (therefore changing their ids and causing all sort of linkage issues).

Remember that, currently, this message is shown every time there is an eviction between minor bumps of a given version, even though it may be safe. The issue is that the whole Scala community is divided between two different versioning schemes: the one from Java (the normal semantic versioning) and the "Scala" one (epoch.major.minor). Sbt assumes the latter one (which we should change). I know that @sjrd has had several issues in the ScalaJS ecosystem because ScalaJS has not ever broken binary compatibility in 0.x.x series.

The major problem when it comes to evicted warnings is that there's not a general way to reason about version numbers for Scala modules. We should try to change that, and stick with only one way of versioning artifacts. I will open a thread in Scala Contributors about this.

@paulp
Copy link
Contributor

paulp commented May 12, 2017

@eed3si9n There are always going to be constituencies for everything from "it should resolve everything magically" to "any conflict should be an error." What everyone can agree on is that if it threatens not to work, the reason should be made clear. This is addressable in a general way, so I'd say it makes sense to focus on that.

Given the soft semantics of artifact versioning and the absence of any direct analysis of the true differences between versions, the rest is at best solvable by letting people choose their preferred poison. I think it would be a disaster to introduce version ranges. As observed by @jvican, the core issue is that versions don't have any consistent meaning. The meaningfulness of a version range is not the sum of the versions it contains - it's the minimum, or worse.

@clhodapp
Copy link

@jvican
I disagree with the bulk of your last comment, partially on factual grounds and partially just based on opinion.

First, the factual stuff:

currently, this message is shown every time there is an eviction between minor bumps of a given version, even though it may be safe.

This isn't true: @chris-martin I were pasting examples of what might be shown for a theoretical dependencyConflicts task or command, which would replace evicted (to be less jargon-y).

There is no way you can have two versions of the same library in the same classpath

This is absolutely untrue. The classpath is just list of places for the system classloader to look for classfiles, including directories and jar files. The default system classloader has absolutely no notion of whether two jars constitute "two versions of the same library" or not. It just goes down the list, searching each location until it finds (a version of) the desired class. Side-note: If you know exactly what order things will be in on the classpath, it is actually not non-deterministic to have two versions of the same library present. It's just that it's generally not particularly useful (one will generally mostly shadow the other and render most unshadowed classes unlinkable). Also, sbt does not make knowable classpath ordering a goal. Therefore, it is both useless and non-deterministic in practice to allow multiple versions of the same library onto the classpath when using sbt.

Now the more opinion-oriented stuff.

People that don't know about semantic versioning and dependency conflicts should look it up.

I feel that this is basically saying "If users are not already familiar with this other world-theory, they are inadequate. Seeing this warning (which makes no reference to the world-theory in question) should prompt them to learn all about it and solve their inadequacy". I think that's counter-productive. I believe that people who use a tool for a while should be naturally pushed toward actually understanding the potentially-dangerous tradeoffs that the tool is making for them and that the tool should do its best to help them gain that understanding. At the very least, I believe that this information should be available under e.g. sbt help dependencyConflicts, and that the user should be directed to run that comand for more info when they view the conflict report.

We should be using ordinary semantic versioning

While I agree that version ranges are a bad idea, I don't think that trying to get the entire community to shift to semantic versioning (and assuming that they just have after some cutover date in the future) is a very good solution. Instead, what if we added some sort of MiMa-verified "binary compatibility manifest" to the set of files that may be produced when an artifact was published? The details would have to be worked out, but sbt could pull down one or more compatibility manifests during the dependency resolution process and use them to decide what warnings / errors to display to the user? This way we wouldn't have to enforce any particular versioning scheme on everybody, the community could gradually transition to a more robust system, and binary compatibility breaks would be much more manageable for well-behaved projects. (I actually have other ideas for less-well-behaved projects, like the ecosystem around Finagle, but I'll save those for another context, as implementing them wouldn't really affect sbt).

@paulp
Copy link
Contributor

paulp commented May 15, 2017

Instead, what if we added some sort of MiMa-verified "binary compatibility manifest" to the set of files that may be produced when an artifact was published?

I resisted suggesting any of the many, many technical measures which are available here because I figured the most likely result of doing so would be slowing things down.

But yes there are many, many technical measures available. And there's no reason to wait years (or forever) until most jars have incorporated binary compatibility manifests. All the bytecode is sitting there, or could be. Just look at it. Sbt could run mima! But I have a strong feeling it's a bad idea to bring any of this up.

@clhodapp
Copy link

I have a feeling that doing as much resolve-time MiMa-ing as would be required to never show spurious warnings would have too high a cost. It's possible that it would at least be ok to do a comprehensive bytecode-level check when specifically requested, though.

Point taken about the potential for discussions of technical solutions to derail things.. the rabbit hole goes pretty deep. And of course two versions can always be binary compatible and still have incompatible semantics. It's just that I think that technical solutions are the general way toward a robust future for this stuff, especially in a typed, complied language like this.

@jvican
Copy link
Member

jvican commented May 15, 2017

This isn't true: @chris-martin I were pasting examples of what might be shown for a theoretical dependencyConflicts task or command, which would replace evicted (to be less jargon-y).

No, this is true. Currently, update will show any eviction warning produced by the UpdateReport. Have a look at the implementation https://github.com/sbt/sbt/blob/1.0.x/main/src/main/scala/sbt/internal/LibraryManagement.scala#L52-L56.

This is absolutely untrue.

What you describe is what happens when there is only one user of the library and yes, there is shadowing among jar entries. I misrepresented what I wanted to say.

Let's start again:

Due to the way the JVM works, when more than one version of the same library is present at runtime, undefined behavior will occur.

You have two options:

  1. Different jars of the same library are binary compatible.
  2. Different jars of the same library are binary incompatible.

In the first one, there is no issue, shadowing of jar entries will make sure the first jar entry has always more priority than the second jar entry.

In the second one, though, there is a big issue. If there are more than two users of one library, there will be a clash. Imagine user 1 links Foo from jar 1.2.0 and that will link all its transitive dependencies from its constant pool. Now, user 2 linksBar (which is defined in 2.0.0) and depends on a binary incompatible Foo. When the JVM tries to load Foo in the same namespace as the previous Foo, it will fail.

I don't think this behaviour is undefined. If you're gonna tell the users that message above, why don't tell them the full story?

I feel that this is basically saying "If users are not already familiar with this other world-theory, they are inadequate. Seeing this warning (which makes no reference to the world-theory in question) should prompt them to learn all about it and solve their inadequacy".

I'm all in for changing the message as you can see from my previous message. But I disagree with you on which message to show. Let me explain why.

There are different levels of knowledge, if a kid wants to learn how to solve an equation, he better understands algebra first. Educating is good. But only in the right scenario. Your comprehensive message should be shown per users' request: by typing help evicted, for instance. But it should not be shown every time I do update and sbt resolves dependencies, because a significant percentage of the users hacking on Scala are familiar with the inner workings of the JVM.

While I agree that version ranges are a bad idea, I don't think that trying to get the entire community to shift to semantic versioning (and assuming that they just have after some cutover date in the future) is a very good solution. Instead, what if we added some sort of MiMa-verified "binary compatibility manifest" to the set of files that may be produced when an artifact was published? The details would have to be worked out, but sbt could pull down one or more compatibility manifests during the dependency resolution process and use them to decide what warnings / errors to display to the user?

I think this is a great idea, but it has the same problem as semantic versioning: adoption. If you're gonna take the time to create such a solution, why not bother enforcing a simpler policy such as semantic versioning?

I have a proposal together with @sjrd to make a semantic version scheme for Scala that covers both binary and source compatibility. I'll make it public soon in Scala Contributors.

However, your proposal has two more weaknesses:

  • MiMa is far from perfect and there are binary changes that won't be caught. Versioning jars is something that requires human intelligence (for now) and cannot be automated. I think that semantic versioning is a good way to reason about binary incompatible changes. In fact, it only describes that. People that want to express more than that in a version number are doing it wrong.
  • You cannot know whether two jars are binary incompatible from the version number. This is bad to diagnose dependency problems.

All this wouldn't happen if we agree on some standards, and we enforce them across the Scala community. Those standards would be accompanied by the use of MiMa and human API design, and would result in a semantic version scheme to rule them all.

In any case, as this is still a pipe dream and I'm sure not all projects will be on board, we better allow what Eugene said: let every module define how its version number should be interpreted for eviction.

Finally, my last comment: all this happens because people don't take binary and source compatibility seriously. That has to change. Otherwise, we'll continue in the same situation forever.

@paulp
Copy link
Contributor

paulp commented May 17, 2017

MiMa is far from perfect and there are binary changes that won't be caught.

@jvican Can you be much more specific about what you mean by far from perfect? MiMa is probably the most successful scala project of which I am aware. What are the binary changes which will not be caught?

@jvican
Copy link
Member

jvican commented May 22, 2017

@paulp What I meant is that we cannot unconditionally rely on MiMa to version jars. There has to be "human reasoning" about every change to the codebase, assumption that brings me to favor semantic versioning as I propose it in my last comment. I like to use MiMa to double-check the implications of some changes, rather than to decide whether I should bump a major or a minor number of a jar revision.

There have recently been several bugs/false negatives in MiMa (less than half a year ago):

  1. mima failed to detect a removed inner object of an object lightbend/mima#127
  2. Adding method to value class breaks binary compatibility but is not reported lightbend/mima#135
  3. False negative: change in default parameter type lightbend/mima#136
  4. False negatives when MiMa finds the wrong previous artefact lightbend/mima#124

These bugs tell me to be careful when using MiMa, since I take bincompat seriously and cannot prove the absence of more of them. These are (normal) misbehaviours of a complex tool trying to solve a challenging task, so I don't think MiMa is to blame. In fact, I'm surprised there are so few bugs for the inherent level of complexity MiMa is coping with. I think it's an excellent tool -- I agree with your statement.

To sum up, my only point is that:

MiMa-verified "binary compatibility manifest" to the set of files that may be produced when an artifact was published

is not enough.

eed3si9n added a commit to eed3si9n/sbt that referenced this issue May 23, 2017
Fixes sbt#2699

Before:

    [warn] There may be incompatibilities among your library dependencies.
    [warn] Here are some of the libraries that were evicted:
    [warn]  * com.google.code.findbugs:jsr305:2.0.1 -> 3.0.0
    [warn] Run 'evicted' to see detailed eviction warnings

After:

    [warn] Multiple versions of the library dependencies were found; some suspected to be binary incompatible:
    [warn]
    [warn] 	* com.typesafe.akka:akka-actor_2.12:2.5.0 is selected over 2.4.17
    [warn] 	    +- de.heikoseeberger:akka-log4j_2.12:1.4.0            (depends on 2.5.0)
    [warn] 	    +- com.typesafe.akka:akka-parsing_2.12:10.0.6         (depends on 2.4.17)
    [warn] 	    +- com.typesafe.akka:akka-stream_2.12:2.4.17 ()       (depends on 2.4.17)
    [warn]
@eed3si9n eed3si9n self-assigned this May 23, 2017
@eed3si9n
Copy link
Member

Sent a PR #3204. The sample output looks like this:

[warn] Multiple versions of the library dependencies were found; some suspected to be binary incompatible:
[warn]
[warn] 	* com.typesafe.akka:akka-actor_2.12:2.5.0 is selected over 2.4.17
[warn] 	    +- de.heikoseeberger:akka-log4j_2.12:1.4.0            (depends on 2.5.0)
[warn] 	    +- com.typesafe.akka:akka-parsing_2.12:10.0.6         (depends on 2.4.17)
[warn] 	    +- com.typesafe.akka:akka-stream_2.12:2.4.17 ()       (depends on 2.4.17)
[warn]

eed3si9n added a commit to eed3si9n/sbt that referenced this issue May 23, 2017
Fixes sbt#2699

Before:

    [warn] There may be incompatibilities among your library dependencies.
    [warn] Here are some of the libraries that were evicted:
    [warn]  * com.google.code.findbugs:jsr305:2.0.1 -> 3.0.0
    [warn] Run 'evicted' to see detailed eviction warnings

After:

    [warn] Found version conflict(s) in library dependencies; some are suspected to be binary incompatible:
    [warn]
    [warn] 	* com.typesafe.akka:akka-actor_2.12:2.5.0 is selected over 2.4.17
    [warn] 	    +- de.heikoseeberger:akka-log4j_2.12:1.4.0            (depends on 2.5.0)
    [warn] 	    +- com.typesafe.akka:akka-parsing_2.12:10.0.6         (depends on 2.4.17)
    [warn] 	    +- com.typesafe.akka:akka-stream_2.12:2.4.17 ()       (depends on 2.4.17)
    [warn]
    [warn] Run 'evicted' to see detailed eviction warnings
@eed3si9n
Copy link
Member

@omervk's suggestion is branched out to #3210

eed3si9n added a commit to eed3si9n/librarymanagement that referenced this issue May 27, 2017
Fixes sbt/sbt#2699

Before:

    [warn] There may be incompatibilities among your library dependencies.
    [warn] Here are some of the libraries that were evicted:
    [warn]  * com.google.code.findbugs:jsr305:2.0.1 -> 3.0.0
    [warn] Run 'evicted' to see detailed eviction warnings

After:

    [warn] Found version conflict(s) in library dependencies; some are suspected to be binary incompatible:
    [warn]
    [warn] 	* com.typesafe.akka:akka-actor_2.12:2.5.0 is selected over 2.4.17
    [warn] 	    +- de.heikoseeberger:akka-log4j_2.12:1.4.0            (depends on 2.5.0)
    [warn] 	    +- com.typesafe.akka:akka-parsing_2.12:10.0.6         (depends on 2.4.17)
    [warn] 	    +- com.typesafe.akka:akka-stream_2.12:2.4.17 ()       (depends on 2.4.17)
    [warn]
    [warn] Run 'evicted' to see detailed eviction warnings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants