Scalacheck version update #2312

barambani · 2018-07-04T09:12:58Z

No description provided.

codecov-io · 2018-07-04T09:55:36Z

Codecov Report

Merging #2312 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2312   +/-   ##
=======================================
  Coverage   95.03%   95.03%           
=======================================
  Files         338      338           
  Lines        5878     5878           
  Branches      210      210           
=======================================
  Hits         5586     5586           
  Misses        292      292

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b77c388...b906493. Read the comment docs.

kailuowang

This breaks the binary compatibly of cats-laws. We were hoping to have a cross build solution. Not sure how that looks now, cc @BennyHill

barambani · 2018-07-04T11:56:10Z

My problem is in fact that I cannot use 1.14.0 in my projects with cats-laws 1.1.0. I'm forced to 1.13.5 as it fails with

[error] java.lang.NoSuchMethodError: org.scalacheck.Cogen$.apply(Lorg/scalacheck/Cogen;Lorg/scalacheck/Cogen;)Lorg/scalacheck/Cogen;
[error] 	at cats.laws.discipline.arbitrary$.<init>(Arbitrary.scala:21)
[error] 	at cats.laws.discipline.arbitrary$.<clinit>(Arbitrary.scala)
[error] 	at cats.laws.discipline.ApplicativeErrorTests$$anon$2.props(ApplicativeErrorTests.scala:49)
[error] 	at org.typelevel.discipline.Laws$RuleSet.collectParentProps(Laws.scala:80)
[error] 	at org.typelevel.discipline.Laws$RuleSet.$anonfun$collectParentProps$1(Laws.scala:80)
[error] 	at scala.collection.TraversableLike.$anonfun$flatMap$1(TraversableLike.scala:241)
[error] 	at scala.collection.immutable.List.foreach(List.scala:389)
[error] 	at scala.collection.TraversableLike.flatMap(TraversableLike.scala:241)
[error] 	at scala.collection.TraversableLike.flatMap$(TraversableLike.scala:238)
[error] 	at scala.collection.immutable.List.flatMap(List.scala:352)
[error] 	at org.typelevel.discipline.Laws$RuleSet.collectParentProps(Laws.scala:80)
[error] 	at org.typelevel.discipline.Laws$RuleSet.$anonfun$all$1(Laws.scala:83)
[error] 	at org.typelevel.discipline.AllProperties.<init>(Laws.scala:128)
[error] 	at org.typelevel.discipline.Laws$RuleSet.all(Laws.scala:83)
[error] 	at org.typelevel.discipline.Laws$RuleSet.all$(Laws.scala:83)
[error] 	at cats.laws.discipline.MonadErrorTests$$anon$2.all(MonadErrorTests.scala:36)
[error] 	at org.typelevel.discipline.scalatest.Discipline.checkAll(Discipline.scala:12)
[error] 	at org.typelevel.discipline.scalatest.Discipline.checkAll$(Discipline.scala:11)

may be you know if I have a workaround ?

johnynek · 2018-07-04T15:07:29Z

This is why binary versioning is pretty tough. If we bump this for you, everyone using 1.13 is hosed. We could imagine expanding a matrix of compatibility.

Side note, I bet this is source compatible. One day, maybe we will have source builds that can deal with this.

Cc @non here is an example where maybe we could have minimized the changes in 1.14 to make it more compatible. We need to not view binary compatibility as binary and instead what is the probability a project is broken by a change. We still want to minimize that probability even when it is not zero.

barambani · 2018-07-04T15:17:02Z

I understand perfectly, thanks. Also I can stay on 1.13.5 for now as I don't see a way around this.
EDIT ~~At this point I will close the PR if that's ok~~. Thanks.

rossabaker · 2018-07-04T17:28:05Z

I would still appreciate a cross-build with this. Without cats-laws, scalacheck-1.14 might as well not exist for me.

kailuowang · 2018-07-05T16:15:14Z

Cross building on Scalacheck would require a cross build on discipline as well (which is already on ScalaCheck 1.14).
According to this poll [https://twitter.com/kailuowang/status/993853527998025728]
4% of the users would feel pain if we break the binary compatibility of cats-laws.
I feel it might be easier and (also better in the long run) to help the whole community move to Scala 1.14.

For now I am going to schedule it for 1.2

kailuowang · 2018-07-05T16:47:15Z

The cross-build solution suggested seems involves Sbt 1.2 (just released RC1) and sbt-projectmatrix . both very young and I am not sure if they support scalajs yet. My proposal is that we release 1.2 with ScalaCheck 1.14 for now and, if we really need to, we can find a way to release a cats-laws and cats-testkit with 1.2.0-ScalaCheck1.13

Update: alleycats-laws also need to be cross build

djspiewak · 2018-07-05T16:57:56Z

@kailuowang Should be possible to do this without SBT 1.2 or a separate plugin, unless I'm missing something. djspiewak/smock does something actually even a bit more intricate than what you're trying to accomplish here: https://github.com/djspiewak/smock/blob/master/build.sbt#L17-L31

rossabaker · 2018-07-05T17:06:43Z

Another irritant is that this matrix is only applicable to three modules. It can still be done, but I think the stack o' hacks is one deeper than smock's.

djspiewak · 2018-07-05T17:44:19Z

@rossabaker I agree the stack-o-hacks is a little deeper (especially since it involves discipline, which is a separate repo), but the approach seems relatively general to me.

rossabaker · 2018-07-05T19:13:31Z

The biggest worry I see in dropping 1.13 is that scalatest is not there yet. specs2 and minitest have already switched to 1.14. Imposing 1.14 becomes more tenable if we have all the major test libraries on board.

I tried to analyze downstream projects, but I don't know how to filter test-scoped from runtime dependencies. The latter set of projects would need to rebuild for their own consumers. This includes cats-effect, circe, and http4s. It would be nice if these projects all followed cats' lead, whether that's a crossbuild or a simple upgrade.

kailuowang · 2018-07-05T19:41:30Z

I spent the last 2 hours trying to write a cross build hack in cats. The fact that we only have 3 modules that need to be cross build+ we cross build scalajs + we use sbt-release makes it more complex than my limited sbt skills can handle.
As pointed out by @rossabaker above, if cats go cross build on ScalaCheck, downstream projects also have to setup cross build. Now I am even less sure if we should try pursue this through some sbt hack.

I suspect that Scalatest's integration with ScalaCheck doesn't suffer a binary compatibility issue. At least discipline was able to update to ScalaCheck 1.14 without problems. The BC issue reported by @barambani is on Cogen, which seems to have no mentions in ScalaCheck

djspiewak · 2018-07-05T20:04:27Z

we use sbt-release makes it more complex

I forgot about sbt-release. I hate sbt-release. A lot. That definitely puts things a bit beyond the pale.

As pointed out by @rossabaker above, if cats go cross build on ScalaCheck, downstream projects also have to setup cross build. Now I am even less sure if we should try pursue this through some sbt hack.

My proposal, if we do pursue an SBT hack, would be to only apply it to the 2.13 build. Downstream projects have no expectations at that point. For the most part, they're only going to be affected if they depend on cats-laws, and especially if they pass along cats-laws as a dependency. Broadly, this means cats-effect is going to get the worst of it, but most other projects shouldn't care. Endpoint downstream projects, which don't cross-build across Scala versions, shouldn't be affected at all.

rossabaker · 2018-07-05T20:19:25Z

I suspect cats-effect-laws has few consumers, though its TestContext is handy even if not checking instances. http4s-testing is of narrow interest, though it could be a lot more compelling if we'd pour more effort into arbitrary requests for fuzzing. I suspect circe-testing is the most depended upon project that depends on cats-laws. I have used this in several endpoint projects.

kailuowang · 2018-07-05T20:47:53Z

@djspiewak Are you suggesting we cross build ScalaCheck on Scala 2.13? or are you suggesting we have 2.13 build on ScalaCheck 1.14 and the rest 2.10-2.12 on ScalaCheck 1.13?

djspiewak · 2018-07-05T21:39:37Z

@kailuowang At least for the time being, yes. The concept basically being that we can push a 2.13 build by depending on 1.14, but the rest of all the things can still depend on 1.13 and retain compatibility. Obviously this would be a temporary situation.

djspiewak · 2018-07-05T22:39:37Z

BTW, this should be relatively achievable by doing something like:

libraryDependencies += "org.scalacheck" %% "scalacheck" % {
  CrossVersion.partialVersion(scalaVersion.value) match {
    case Some((2, major)) if major < 13 => "1.13.5"
    case Some((2, major)) if major >= 13 => "1.14.0"
  }
} % Test

More or less. (note: isolating this to a custom setting key will be more elegant and allow reuse) The differences between the two frameworks in terms of source compatibility (should be negligible) then can be factored out using the normal cross-building mechanisms (e.g. src/main/scala-2.13/).

travisbrown · 2018-07-06T09:49:09Z

@rossabaker I'm not too worried about bumping the ScalaCheck version in circe-testing to 1.14, given that we have no guarantees of compatibility between minor versions before 1.0, and I almost certainly wouldn't cross-build for 1.13 / 1.14 even if cats-laws did (unless someone paying me wanted it or someone opened a PR and promised to maintain it forever).

ghost · 2018-07-06T14:20:32Z

We were hoping to have a cross build solution.

involves Sbt 1.2 (just released RC1) and sbt-projectmatrix

Actually the newer crossProject that we don't use yet.

And I'm actually camping at the moment, typing away under canvas ⛺ But reading the above does suggest a better/proper cross versioning build is worth the effort including for related projects such as discipline,etc that, IMHO, should remain separate.

But the fact remains that the (ideal) required infra projects are not there yet - sbt, crossproject, etc. But I believe it would be quite straightforward to mimic the output, but I suggest gitter would be better for me to air my thoughts for now

ghost · 2018-07-06T14:39:06Z

chat at https://gitter.im/typelevel/sbt-catalysts?at=5b3f7b0c3d8f71623d61ebb4

djspiewak · 2018-07-06T15:51:37Z

But the fact remains that the (ideal) required infra projects are not there yet - sbt, crossproject, etc.

@BennyHill @kailuowang If you could describe your ideal infrastructural solution to this problem, what would it be? I don't mind whipping up SBT plugins which solve plumbing projects like this (actually, I quite enjoy that sort of thing). The only reason I haven't is I don't have any immediate inspiration for what a good solution would look like.

ghost · 2018-07-06T16:24:27Z

what a good solution would look like.

Is perhaps the root problem 😜 And awesome offer, @djspiewak , btw 👍

My (our?) current thoughts I've summarized as per gitter link^^, but the end result would be that cats 1.2 would still scalacheck 1.3 and be fully bin compat, but we would have another lib with a classifier of say "BC1", in the same way that classifiers are added for different target architectures and/or environments - and that would use SC 1.4 and any other different library versions.

kailuowang · 2018-07-06T16:35:14Z

@djspiewak I believe sbt-projectmatrix (cc @eed3si9n) is trying to provide a unified solution for cross- building. For now, we need something that can work with sbt-crossproject. I can imagine the usage would be something like

lazy val laws = crossProject(JSPlatform, JVMPlatform).in(file("."))
  .settings(
    commenSettings,
    moduleName := "cats-laws",
     libraryDependencies += 
        "org.scalacheck" %%% "scalacheck" % "1.14.0"
  )

lazy val lawsJVM = laws.jvm 
lazy val lawsJS = laws.js  

val lawsSC1_13 = laws.settings(   
     libraryDependencies += 
       "org.scalacheck" %%% "scalacheck" % "1.13.5",
     versionSuffix := "ScalaCheck1.13"   
  )

lazy val lawsSC1_13JVM = lawsSC1_13.jvm 
lazy val lawsSC1_13JS = lawsSC1_13.js

Obviously We'll also need to add lawsSC1_13JVM and lawsSC1_13JS to root modules.

This should result in testing and publishing additional cats-laws versions with ScalaCheck 1.13.5 and a version suffix ScalaCheck1.13 , e.g. cats-laws 1.2.0-ScalaCheck1.13

If we use this setup the only magic the plugin needs to provide is probably just appending the versionSuffix, if set, to the release version.

Update: I forgot to mention, that at least for now, integration with sbt-release would be great for Cats. (maybe a task to append the suffix? thus can be run as a release step command?)

djspiewak · 2018-07-06T16:49:30Z

@BennyHill @kailuowang I'll give it some thought. I will warn you though that sbt-release integration is probably relatively low on my motivational TODO list. This is for two reasons:

Fundamentally, I disagree with the existence of sbt-release. It strongly encourages a lot of anti-patterns (like -SNAPSHOT releases) and doesn't achieve anything that cannot be done in a more structured way with sbt-git and aliases
sbt-release does some WEIRD MOJO with the internal SBT state monad, which makes it quite difficult to extend in non-trivial ways.

It's doable, just a massive pain. :-)

Anyway, I'll look into it and see if I can come up with something.

johnynek · 2018-07-06T17:00:12Z

To me, the ideal solution is for the community to work towards source builds (like Rust and Haskell have).

It’s not a quick fix, but if we had source builds we would detect these issues at compile time, and avoid many of them due to how much easier source compatibility is than binary for scala.

kailuowang · 2018-07-06T17:03:06Z

@djspiewak I am open to a replacement of sbt-release in Cats. The main benefit of sbt-release is that the release process enabled by it is scripted and well known. As long as the replacement is equally straightforward for a new maintainer to do a Cats release, I am all for it.

djspiewak · 2018-07-06T17:08:39Z

@johnynek I'm still not convinced that source dependencies are the promised land. They just have a very different set of tradeoffs, and they haven't been in wide use since the days of gnu autoconf, so the warts aren't as familiar or as widely understood. I've spent enough time with Gentoo and similar to be extremely leery of source dependencies, though obviously a Linux distribution is a slightly bigger problem than a PL ecosystem.

At any rate, we've had this discussion in parts before. :-) And it's more future-speculative-ish.

@kailuowang This is what I use on my projects, which in turn enables this versioning outcome. (basically semver with stable snapshots and better major/minor bump determination)

It wouldn't be hard to split this away from sbt-spiewak and make it a bit more configurable. I've been meaning to extend it with support for matrix releases (e.g. for smock). Basing things around addCommandAlias does have some downsides, but it carries considerably fewer gotchas than the sbt-release strategy of explicitly fiddling with SBT's internal state.

I agree that scripting, repeatability, and accessibility are 100% essential components of any release process.

kailuowang · 2018-07-06T17:25:33Z

It wouldn't be hard to split this away from sbt-spiewak and make it a bit more configurable. I've been meaning to extend it with support for matrix releases

@djspiewak That would be amazing. As a side note, in regards to mima check, we check against all previous minor versions with patch ver 0, i.e. 1.0.0, 1.1.0 etc, and all previous bug-fix version under the same minor, code here.

ghost · 2018-07-06T17:55:37Z

@kailuowang / anyone

Your source example above looks good, 👍 , but I wonder if that should be for a cats 2.0 release when we break BC? So for now, 1.2, we would have instead :

lazy val laws = crossProject(JSPlatform, JVMPlatform).in(file("."))
  .settings(
    commenSettings,
    moduleName := "cats-laws",
     libraryDependencies += 
        "org.scalacheck" %%% "scalacheck" % "1.13.5"
  )

lazy val lawsJVM = laws.jvm 
lazy val lawsJS = laws.js  

val lawsSC1_14 = laws.settings(   
     libraryDependencies += 
       "org.scalacheck" %%% "scalacheck" % "1.14.0",
     versionSuffix := "ScalaCheck1.14"   
  )

lazy val lawsSC1_14JVM = lawsSC1_14.jvm 
lazy val lawsSC1_14JS = lawsSC1_14.js

So for now , breaking BC is opt-in, but for a major release, old library versions are opt-in.

And before I disappear, I kinda agree with @djspiewak re source builds - they're cool, but I don't think they will solve all the problems that folk want 😄

kailuowang · 2018-07-06T17:58:45Z

@BennyHill agree, we should make default not binary breaking.

djspiewak · 2018-07-12T18:06:44Z

Quick update: I haven't forgotten about this! I spent a bit of time playing around with how I wanted this to work, ran into a major blocker, and found a nice solution to it last night in a random revelation.

cats-laws 1.x depends on scalacheck 1.13.5, which is binary-incompatible with the newer 1.14.0 - causing NoSuchMethodErrors in law tests for MonadError See typelevel/cats#2312

kailuowang · 2018-12-18T17:22:06Z

An update: I tried several approaches for cross releasing for scalacheck 1.14, they either don't work or introduce a heck amount of complexity to the build. I ran out of steam for this. Considering the fact that other libs need to do the same thing, I start to believe that cost wise, it's probably just easier to have the whole ecosystem upgrade to scalacheck 1.14 at one go - i.e. cats upgrades it in 1.6 and projects follows.

johnynek · 2018-12-18T18:06:48Z

PS could we upstream our concerns about better binary compatibility checking (cc @non ) or maybe introduce typeclasses to abstract out scalacheck and allow users to work with hedgehog, scalacheck, etc... via a module.

barambani · 2019-03-07T00:53:36Z

This is made obsolete by #2734 and can be closed when that's merged.

Scalacheck version update

b906493

kailuowang reviewed Jul 4, 2018

View reviewed changes

kailuowang added the Triage needed label Jul 5, 2018

kailuowang added this to the 1.2 milestone Jul 5, 2018

ghost mentioned this pull request Aug 22, 2018

Add support for maven classifier sbt/sbt-release#235

Open

SethTisue mentioned this pull request Sep 14, 2018

unfreeze blaze, add boopickle, fix http4s scala/community-build#788

Merged

This was referenced Sep 25, 2018

Upgrade scalacheck to 1.14.0 #2449

Closed

Downgrade specs2 to 4.1.0 http4s/http4s.g8#85

Merged

kailuowang mentioned this pull request Oct 2, 2018

Update discipline to 0.10.0 typelevel/cats-mtl#78

Closed

kailuowang closed this Mar 7, 2019

kailuowang modified the milestones: 1.2, Excluded from release notes Mar 7, 2019

barambani deleted the scalacheck-update branch March 7, 2019 18:04

Scalacheck version update #2312

Scalacheck version update #2312

Conversation

barambani commented Jul 4, 2018

codecov-io commented Jul 4, 2018 • edited Loading

Codecov Report

kailuowang left a comment • edited Loading

Choose a reason for hiding this comment

barambani commented Jul 4, 2018

johnynek commented Jul 4, 2018

barambani commented Jul 4, 2018 • edited Loading

rossabaker commented Jul 4, 2018

kailuowang commented Jul 5, 2018

kailuowang commented Jul 5, 2018 • edited Loading

djspiewak commented Jul 5, 2018

rossabaker commented Jul 5, 2018

djspiewak commented Jul 5, 2018

rossabaker commented Jul 5, 2018

kailuowang commented Jul 5, 2018 • edited Loading

djspiewak commented Jul 5, 2018

rossabaker commented Jul 5, 2018

kailuowang commented Jul 5, 2018

djspiewak commented Jul 5, 2018

djspiewak commented Jul 5, 2018 • edited Loading

travisbrown commented Jul 6, 2018

ghost commented Jul 6, 2018

ghost commented Jul 6, 2018

djspiewak commented Jul 6, 2018

ghost commented Jul 6, 2018

kailuowang commented Jul 6, 2018 • edited Loading

djspiewak commented Jul 6, 2018

johnynek commented Jul 6, 2018

kailuowang commented Jul 6, 2018

djspiewak commented Jul 6, 2018 • edited Loading

kailuowang commented Jul 6, 2018

ghost commented Jul 6, 2018

kailuowang commented Jul 6, 2018

djspiewak commented Jul 12, 2018

kailuowang commented Dec 18, 2018

johnynek commented Dec 18, 2018

barambani commented Mar 7, 2019 • edited Loading

codecov-io commented Jul 4, 2018 •

edited

Loading

kailuowang left a comment •

edited

Loading

barambani commented Jul 4, 2018 •

edited

Loading

kailuowang commented Jul 5, 2018 •

edited

Loading

kailuowang commented Jul 5, 2018 •

edited

Loading

djspiewak commented Jul 5, 2018 •

edited

Loading

kailuowang commented Jul 6, 2018 •

edited

Loading

djspiewak commented Jul 6, 2018 •

edited

Loading

barambani commented Mar 7, 2019 •

edited

Loading