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

Update sbt and coursier #1090

Merged
merged 3 commits into from
Apr 14, 2020
Merged

Update sbt and coursier #1090

merged 3 commits into from
Apr 14, 2020

Conversation

ckipp01
Copy link
Member

@ckipp01 ckipp01 commented Apr 13, 2020

This pr updates sbt from 1.2.8 to 1.3.9. Tests all seemed to pass locally as well as publishing locally. Since we were using an sbt version prior to 1.3.x, I'm assuming that's why we were explicitly using coursier-sbt. I therefore removed that dependency on the plugin.

Supercedes #1085

@ckipp01 ckipp01 changed the title update sbt and coursier Update sbt and coursier Apr 13, 2020
@ckipp01
Copy link
Member Author

ckipp01 commented Apr 13, 2020

Missed some relevant stuff. Looking into it.

@ckipp01 ckipp01 force-pushed the sbt branch 3 times, most recently from 4f17afc to 887cbea Compare April 13, 2020 17:50
@ckipp01
Copy link
Member Author

ckipp01 commented Apr 13, 2020

I'll need to spend some more time on this 🤔 ...

I continually get the following

error: error while loading Object, Missing dependency 'object scala.native in compiler mirror', required by /Users/ckipp/.sdkman/candidates/java/8.0.242-librca/jre/lib/rt.jar(java/lang/Object.class)

If found some information about how forking or other things can help, but nothing has seemed to work yet.

@@ -82,7 +82,7 @@ class ScalafixImplSuite extends FunSuite with DiffAssertions {
}

test("error") {
val cl = new URLClassLoader(Array())
val cl = new URLClassLoader(Array(), null)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh, I tried the fork := true up above like you did, but didn't catch this one down here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was also necessary to update baseDirectory.in(Test) := baseDirectory.in(ThisBuild).value. That should honestly be the default behavior in sbt, but changing the default would break a lot of builds

@olafurpg
Copy link
Contributor

Thank you for this contribution! It wasn't at all obvious what was causing those errors. The Scalafix test suite previously relied on both

  • the shape of this.getClass.getClassLoader to match exactly the layered classloader from sbt v1.2.x
  • the working directory to be the root of the build

The last commit enables forking and fixes the build to use a correct working directory as well as fixes a test that was written in a fragile way.

The Scalafix test suite uses `this.getClass.getClassLoader` to lookup
symbols from scala-library.jar. In sbt v1.3.x, the default classloader
layering strategy changed so that `this.getClass.getClassLoader` only
contains Scalafix-defined classes, with classes from dependency classes
coming from a parent classloader.

This commit makes the tests pass on sbt v1.3.x by enabling forking so
that `this.getClass.getClassLoader` is always a flat classloader
containing all classes.
Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 Thank you!

@olafurpg olafurpg merged commit 74f427c into scalacenter:master Apr 14, 2020
@ckipp01 ckipp01 deleted the sbt branch April 14, 2020 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants