Support cross-compiling build with 2.13.0-M5 #759

erikerlandson · 2019-02-03T15:27:23Z

PR for #742

erikerlandson · 2019-02-03T16:26:05Z

2.13 build is failing on breaking changes to collections. This appears several places:

pre 2.13: def make[T]()(implicit arg0: ClassTag[T]): ArrayBuilder[T]
2.13: def make[T](implicit arg0: ClassTag[T]): ArrayBuilder[T]

erikerlandson · 2019-02-03T19:59:56Z

note to self, use https://github.com/scala/scala-collection-compat ?

codecov-io · 2019-02-10T21:05:16Z

Codecov Report

Merging #759 into master will decrease coverage by 3.67%.
The diff coverage is 56.57%.

@@            Coverage Diff             @@
##           master     #759      +/-   ##
==========================================
- Coverage   52.36%   48.68%   -3.68%     
==========================================
  Files         196      212      +16     
  Lines       11885    12817     +932     
  Branches     1613     1088     -525     
==========================================
+ Hits         6223     6240      +17     
- Misses       5662     6577     +915

Impacted Files	Coverage Δ
...c/main/scala/spire/algebra/NormedVectorSpace.scala	`60% <ø> (ø)`	⬆️
core/src/main/scala/spire/random/Generator.scala	`40.08% <ø> (ø)`	⬆️
...re/src/main/scala/spire/optional/vectorOrder.scala	`60% <ø> (ø)`	⬆️
core/src/main/scala/spire/math/ScalaWrappers.scala	`3.33% <0%> (+0.1%)`	⬆️
examples/src/main/scala/spire/example/kmeans.scala	`0% <0%> (ø)`	⬆️
...amples/src/main/scala/spire/example/DataSets.scala	`0% <0%> (ø)`	⬆️
core/src/main/scala/spire/random/Random.scala	`0% <0%> (ø)`	⬆️
...cala/spire/math/extras/interval/IntervalTrie.scala	`0% <0%> (ø)`	⬆️
laws/src/main/scala/spire/laws/package.scala	`50% <0%> (ø)`	⬆️
.../src/main/scala/spire/example/simplification.scala	`0% <0%> (ø)`	⬆️
... and 40 more

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 6b8e6a0...aaa4715. Read the comment docs.

erikerlandson · 2019-02-13T00:57:44Z

commit above cross compiles for main src, but test code compile fails

larsrh · 2019-02-13T18:22:08Z

I don't have a strong opinion, but I'd prefer if you could use scala-collection-compat.

erikerlandson · 2019-02-13T19:16:16Z

@larsrh I am using it. OTOH I might not be using it in the best possible way, since I'm figuring out both the spire code and compat somewhat by trial and error.

larsrh · 2019-02-17T13:10:04Z

I am using it.

Sorry, I must've missed that. Unfortunately I currently don't have the bandwidth to aid in the process.

denisrosset · 2019-02-19T07:28:20Z

I've had a look at your ongoing work, thank you so much! It can be quite painful to move a beast of a codebase like Spire.

Some of the things you've been modifying are quite obscure, such as the partial algebras (groupoids and the like). Happy to cross check your progress.

I suggest the following: let's not try to upgrade everything at the same time. A source of breakage is the immtuable Seq in 2.13; let's keep it for later.

Instead, we should stay on 2.11/2.12 and upgrade the collection syntax (factories, etc...), merge that. Then, in a second step, address the immutable Seq.

(I haven't checked, though, whether spire-collection-compat renders Seq immutable)

erikerlandson · 2019-02-19T18:05:43Z

Thanks @denisrosset!

I have been fooling around with getting test code to compile for 2.11 (which is likely to also work for 2.12 when it does).

You can see that the bulk of the breakage (no surprise) has been around the removal of TraversableLike, and CanBuildFrom -> Factory. I got somewhat rat-holed drilling into why the testing code is failing to resolve the new implicit signatures, although I may have just needed include scala.collection.compat._ in some additional places.

One thing I'd love to find is a write-up of what cross-compiling code is "supposed" to look like. "If you are doing everything right, your code should look analogous to these examples..."

Based on source diving, I think it's supposed to be "use Iterable and friends as the replacement for Traversable, and use Factory as the replacement CBF" but until I can compile the testing my confidence isn't very high 😁

denisrosset · 2019-02-19T21:58:59Z

You know what? After you get part of this done, let's write a post for the Typelevel blog. That would be greatly valuable information.

(And you know more than me about the transition at this point!)

Very happy to have Iterable as a replacement for Traversable, and IterableOnce (which is new in compat/2.13) as a replacement for TraversableOnce.

larsrh · 2019-02-24T11:20:54Z

#765 is merged now which contained the bumped dependency part of this PR.

erikerlandson · 2019-02-24T17:56:31Z

just a note on current state. the analog of SeqLike in 2.13 is SeqOps which has two interesting consequences.

spire also defines a thing called SeqOps, which is more of an annoyance than anything else but is obviously a bit confusing.
In the spirit of cross-compiling but also attempting to put a foot forward into the future, I defined a spire.scalacompat.SeqOps so that pre-2.13, it looks like trait SeqOps[A, CC[_], C] extends SeqLike[A, CC[A]]. This was promising but it caused some re-definitions of things like SA to CC[A] which "worked" until it started to break @sp for some reason, I am assuming something to do with @sp being unable to resolve the entire sequence type CC[A] with these new signatures.

Next experiment will be defining something like spire.scalacompat.MySeqLike[A, Repr] that somehow can resolve to the 2.13 SeqOps. scala 2.13 itself defines this (deprecated) as SeqOps[A, Seq, Repr] - the Seq here seems unappealing but maybe it's not as bad as I think.

The general theme with this has been that spire leverages pre-2.13 constructs (SeqLike, Traversable, @specialized, etc) in such a highly coupled way that cross-compiling to 2.13 is tricky

erikerlandson · 2019-02-24T23:30:44Z

Another thing I'm going to try is replacing signatures of the form SA <: SeqLike[A, SA] with SA <: SeqOps[A, Seq, SA], possibly that will avoid the cases where I broke @sp

denisrosset · 2019-03-01T06:37:15Z

@sp is notably fickle.

And even when it compiles, it sometimes still maps to nonspecialized code, so buyer beware.

For these collection enrichment operations, why not fork the pre-2.13 and 2.13 code? and maintain both versions?

Also, let's rename SeqOps and IndexedSeqOps to RichSeqOps and RichIndexedSeqOps in a separate PR, with a comment explaining the deviation to the rule XXX + Ops.

erikerlandson · 2019-03-01T14:31:46Z

I started over with a fresh branch (not pushed yet) that is somewhat cleaner. The Factory and SeqOps stuff I have compiles and tests for pre-2.13, but is snagging for 2.13 on some of the SeqOps[A, Seq, SA] on account of binding CC[_] elsewhere, which it of course cannot prove is <: Seq[_]. I have other ways to square this circle, but it was worth a try.

erikerlandson · 2019-03-05T21:55:51Z

Looks like tut problems with 2.13 are a known issue: tpolecat/tut#246

erikerlandson · 2019-03-06T15:27:38Z

A summary of quirks unresolved on this PR:

tut is broken on 2.13, so doc generation is currently disabled for 2.13 here.
SeqLike and friends are defined via shim for cross-compiling. A more future-oriented solution would be to migrate signatures to SeqOps. This is do-able but it has impacts on how the signatures are currently defined that I would not want to introduce unilaterally. Additionally those changes might break @specialized.
TraversableOnce is still in use. This is deprecated in favor of Iterable/IterableOnce, however the monoid definitions inherited from algebra & cats are also still using Traversable and so for now that forces the issue.
A small number of unit tests do not compile, and a couple more seem to induce infinite looping. .pow and .nroot methods appear to be involved in all of these, so there's a good lead on where to start on that diagnosis. Currently I just have these commented out.
the simplification.scala example is also currently just commented out, due to cross compile incompatabilities around the pre 2.13 newBuilder method.

erikerlandson · 2019-03-07T21:39:08Z

OK, the infinite looping is a BigDecimal regression: scala/bug#11152

larsrh · 2019-03-07T21:41:37Z

Thanks for digging into this! Truly heroic ☺

Ichoran · 2019-03-07T22:08:17Z

I'll fix it really soon! (It's already fixed, but I'll get a proper test into the PR.) Or I won't, if it's not a range thing, and you will have to set a sane precision with MathContext to get reasonable behaviors.

erikerlandson · 2019-03-07T22:15:52Z

@Ichoran thank you! If I set a MathContext now, will that also work on 2.13.0-M5 ?

erikerlandson · 2019-03-08T19:13:33Z

The Sized compile errors are a new import conflict with sized method from scalacheck, renaming it seems to fix it.

[info] /home/eje/git/spire/tests/src/test/scala/spire/math/RealCheck.scala:111:12: <sized: error> is not a valid implicit value for org.scalacheck.Arbitrary[spire.math.ArbitrarySupport.Sized[Int,spire.math.ArbitrarySupport.Ordinal._1,spire.math.ArbitrarySupport.Ordinal._10]] because:
[info] reference to sized is ambiguous;
[info] it is imported twice in the same scope by
[info] import ArbitrarySupport._
[info] and import Gen._
[info]     forAll { (x0: Real, k: Sized[Int, _1, _10]) =>

erikerlandson · 2019-03-08T21:14:23Z

picking up some additional inf-looping, which I am pretty sure has to do with Rational and its use of BigDecimal

erikerlandson · 2019-03-10T00:03:06Z

Now not so sure that scala/bug#11152 is the problem here, but definitely seems to have something to do with constructing BigDecimal instances.

erikerlandson · 2019-03-10T17:18:28Z

I have now resolved the remaining compile errors. There are two remaining unit tests that are inf-looping for scala 2.13. I have these turned off at run-time when running 2.13, and they will now continue to run pre 2.13.

@denisrosset, @larsrh, I am ready to propose this for review and hopefully merging. I believe the remaining loose ends are best addressed by filing dedicated issues:

Use of deprecated Traversable[Once], here and in algebra and cats
Tracking down inf-looping in unit tests w.r.t. 2.13
eventual SeqLike -> SeqOps
re-enabling tut when it works for 2.13

denisrosset · 2019-03-16T09:03:46Z

I had a look, fine for me; if you can open issues to track the following:

Use of deprecated Traversable[Once], here and in algebra and cats (in Spire and in cats/algebra if it is not present)
Tracking down inf-looping in unit tests w.r.t. 2.13 (we'll probably revisit this through the 2.13 milestones)
Re-enabling tut when it works for 2.13
I'd be happy to merge. Thanks a lot for this chunk of work!
For the eventual SeqLike -> SeqOps, I'm not too worried. The main use cases of Seq are in examples, and in the vector space stuff which is not principled.

larsrh

LGTM!

build.sbt

compat/src/main/scala-2.13/package.scala

…g unit tests

larsrh · 2019-03-16T17:19:10Z

This is epic! Thank you so much, @erikerlandson 🎉

erikerlandson · 2019-03-16T17:20:38Z

@larsrh, my pleasure! Can we publish a "-M5" package build for this?

larsrh · 2019-03-16T22:23:38Z

I'm travelling at the moment, so it'll have to wait for a few days. But I can equip @denisrosset with the publishing rights.

SethTisue · 2019-03-17T11:01:05Z

build.sbt

@@ -543,8 +548,9 @@ lazy val warnUnusedImport = Seq(
    CrossVersion.partialVersion(scalaVersion.value) match {
      case Some((2, 10)) =>
        Seq()
-      case Some((2, n)) if n >= 11 =>
+      case Some((2, n)) if ((n >= 11) && (n <= 12)) =>


just a silly style thing, but you can case Some((2, 11 | 12)). I sometimes forget that | is usable other than at the top level of the match.

I had no idea!

There's also

import sbt.librarymanagement.{ SemanticSelector, VersionNumber } VersionNumber(scalaVersion.value).matchesSemVer(SemanticSelector("2.11.x || 2.12.x"))

…t_eff sql package lint + minor efficiencies

erikerlandson mentioned this pull request Feb 10, 2019

[2.13.0-M4] missing classes on scala.collection.mutable scala/bug#10987

Closed

18 tasks

erikerlandson changed the title ~~update deps to support build with 2.13.0-M5~~ Support cross-compiling build with 2.13.0-M5 Feb 24, 2019

erikerlandson added 7 commits February 27, 2019 16:16

compat code for ScalaOrderingWrapper and ArrayBuilder.make

95f41a8

add scala-2.13 and scala-pre-2.13 as conditional source dirs

e94e694

add scala-collection-compat dependency

85913cd

guard -Ywarn-unused-import flag

53c18c2

cross compile ArrayBuilder.make calls

258346d

insert ScalaOrderingWrapperCompat

862f28b

CanBuildFrom -> Factory

91112b8

erikerlandson added 6 commits March 2, 2019 10:59

Traversable -> Iterable

89d3246

add toMap call to fix MapView returns

d3563d1

fix some 2.13 compiles issues in Dist

63f84b4

code tweaks for 2.13 compat

6ff2288

compat layer for SeqLike

c4217c3

change imports to compat layer for SeqLike

b58eb69

turn off docs/tut on 2.13

314878d

erikerlandson mentioned this pull request Mar 7, 2019

'(BigDecimal(1) to BigDecimal(s"1.${"0" * 32}1") by BigDecimal(s"0.${"0" * 33}1")).toList' cause infinite loop in Scala 2.13.0-M5 scala/bug#11152

Closed

erikerlandson added 2 commits March 8, 2019 12:30

rename implicits to avoid import conflicts

f95d0c7

uncomment unit tests using Sized that are now compiling with 2.13

c506939

erikerlandson added 2 commits March 9, 2019 07:33

add preScala2p13 method for runtime disable of a couple unit tests

e9c06b8

turn off inf-looping tests at runtime for 2.13 only

37d8e9c

erikerlandson added 2 commits March 10, 2019 09:43

IterableLikeCompat, for squaring the newBuilder method circle

3db7fb7

simplification example complies using IterableLikeCompat shim

69f3e40

larsrh reviewed Mar 16, 2019

View reviewed changes

build.sbt Outdated Show resolved Hide resolved

compat/src/main/scala-2.13/package.scala Show resolved Hide resolved

This was referenced Mar 16, 2019

A couple unit tests are infinite-looping when running against scala 2.13 #778

Open

parallel Sequence is being shimmed for 2.13 cross-compatability #780

Closed

remove logBuffered and parallelExecution settings I used for debuggin…

aaa4715

…g unit tests

larsrh approved these changes Mar 16, 2019

View reviewed changes

larsrh merged commit 3446d41 into typelevel:master Mar 16, 2019

SethTisue reviewed Mar 17, 2019

View reviewed changes

JoeWrightss pushed a commit to JoeWrightss/spire that referenced this pull request Apr 13, 2019

Merge pull request typelevel#759 from amoore877/T2556563_database_lin…

953eb8b

…t_eff sql package lint + minor efficiencies

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support cross-compiling build with 2.13.0-M5 #759

Support cross-compiling build with 2.13.0-M5 #759

erikerlandson commented Feb 3, 2019 •

edited

Loading

erikerlandson commented Feb 3, 2019 •

edited

Loading

erikerlandson commented Feb 3, 2019

codecov-io commented Feb 10, 2019 •

edited

Loading

erikerlandson commented Feb 13, 2019

larsrh commented Feb 13, 2019

erikerlandson commented Feb 13, 2019

larsrh commented Feb 17, 2019

denisrosset commented Feb 19, 2019

erikerlandson commented Feb 19, 2019

denisrosset commented Feb 19, 2019

larsrh commented Feb 24, 2019

erikerlandson commented Feb 24, 2019

erikerlandson commented Feb 24, 2019

denisrosset commented Mar 1, 2019

erikerlandson commented Mar 1, 2019 •

edited

Loading

erikerlandson commented Mar 5, 2019

erikerlandson commented Mar 6, 2019

erikerlandson commented Mar 7, 2019

larsrh commented Mar 7, 2019

Ichoran commented Mar 7, 2019

erikerlandson commented Mar 7, 2019

erikerlandson commented Mar 8, 2019

erikerlandson commented Mar 8, 2019

erikerlandson commented Mar 10, 2019

erikerlandson commented Mar 10, 2019

denisrosset commented Mar 16, 2019

larsrh left a comment

larsrh commented Mar 16, 2019

erikerlandson commented Mar 16, 2019 •

edited by larsrh

Loading

larsrh commented Mar 16, 2019

SethTisue Mar 17, 2019

erikerlandson Mar 17, 2019

dwijnand Mar 18, 2019

Support cross-compiling build with 2.13.0-M5 #759

Support cross-compiling build with 2.13.0-M5 #759

Conversation

erikerlandson commented Feb 3, 2019 • edited Loading

erikerlandson commented Feb 3, 2019 • edited Loading

erikerlandson commented Feb 3, 2019

codecov-io commented Feb 10, 2019 • edited Loading

Codecov Report

erikerlandson commented Feb 13, 2019

larsrh commented Feb 13, 2019

erikerlandson commented Feb 13, 2019

larsrh commented Feb 17, 2019

denisrosset commented Feb 19, 2019

erikerlandson commented Feb 19, 2019

denisrosset commented Feb 19, 2019

larsrh commented Feb 24, 2019

erikerlandson commented Feb 24, 2019

erikerlandson commented Feb 24, 2019

denisrosset commented Mar 1, 2019

erikerlandson commented Mar 1, 2019 • edited Loading

erikerlandson commented Mar 5, 2019

erikerlandson commented Mar 6, 2019

erikerlandson commented Mar 7, 2019

larsrh commented Mar 7, 2019

Ichoran commented Mar 7, 2019

erikerlandson commented Mar 7, 2019

erikerlandson commented Mar 8, 2019

erikerlandson commented Mar 8, 2019

erikerlandson commented Mar 10, 2019

erikerlandson commented Mar 10, 2019

denisrosset commented Mar 16, 2019

larsrh left a comment

Choose a reason for hiding this comment

larsrh commented Mar 16, 2019

erikerlandson commented Mar 16, 2019 • edited by larsrh Loading

larsrh commented Mar 16, 2019

SethTisue Mar 17, 2019

Choose a reason for hiding this comment

erikerlandson Mar 17, 2019

Choose a reason for hiding this comment

dwijnand Mar 18, 2019

Choose a reason for hiding this comment

erikerlandson commented Feb 3, 2019 •

edited

Loading

erikerlandson commented Feb 3, 2019 •

edited

Loading

codecov-io commented Feb 10, 2019 •

edited

Loading

erikerlandson commented Mar 1, 2019 •

edited

Loading

erikerlandson commented Mar 16, 2019 •

edited by larsrh

Loading