scala / community-build

Scala 2 community build — a corpus of open-source repos built against Scala nightlies
Apache License 2.0
136 stars 60 forks source link

add more scalafix projects #1673

Closed SethTisue closed 11 months ago

SethTisue commented 1 year ago

as per https://github.com/scalacenter/scalafix/issues/1699 (previous history: #1602), -release:8 should allow code that references sun.misc.Unsafe to compile. I'll need to dig into why it isn't working in the dbuild context

I'm also having a weird Scala version thing where the project extracts with version=2.13.11 but there's an error message (that I don't have handy) when the nightly version number is used

bjaglin commented 1 year ago

as per https://github.com/scalacenter/scalafix/issues/1699 (previous history: https://github.com/scala/community-build/pull/1602), -release:8 should allow code that references sun.misc.Unsafe to compile. I'll need to dig into why it isn't working in the dbuild context

Actually, the current behavior of -release:8 does prevent usage of sun.misc.Unsafe, but sbt's rt.jar effectively cancels this behavior (as discussed in https://github.com/sbt/sbt/issues/6817). So it might just be that the dbuild context is stricter?

bjaglin commented 1 year ago

I'm also having a weird Scala version thing where the project extracts with version=2.13.11 but there's an error message (that I don't have handy) when the nightly version number is used

Let me know if/how I can help. I will try to get dbuild running locally this week, understanding how the scala/scalameta versions get injected specifically, evaluating their impact through projects using scalafix-testkit .

I realized that I never followed up on your previous request, apologies for that. Now that I have a bit more free time to invest on OSS, I definitely want to help on that front, to continue detecting regressions like https://github.com/scalacenter/scalafix/pull/1556 :muscle:

SethTisue commented 1 year ago

@bjaglin I'm very glad you're interested in helping — but note that I have so much experience troubleshooting dbuild issues that the most efficient approach is probably for me to be the next one to dig, and then let you know if I have questions

SethTisue commented 1 year ago

you are right — it's -release:8 that prevents it from compiling. on JDK 17 this succeeds:

scala-cli compile -S 2.13.11 \
  --dep org.scalameta::scalameta:4.7.8 \
  scalafix/scalafix-reflect/src/main/scala/scalafix/internal/reflect/ClasspathOps.scala

but if I add -O -release:8 it fails with

[error] not found: object sun
[error] import sun.misc.Unsafe
[error]        ^^^

this is the opposite of what I assumed.

SethTisue commented 1 year ago

okay, the sun.misc.Unsafe thing is sorted, it just needed:

+    "removeScalacOptions -release 8"
SethTisue commented 1 year ago

as for the thing where it doesn't work with a 2.13.12 nightly but does work with version=2.13.11 ./run scalafix, the problem is reproducible outside of dbuild by simply starting sbt in the scalafix repo and typing ++2.13.12-bin-1a2373b!:

% sbt
[info] welcome to sbt 1.8.3 (Eclipse Adoptium Java 17.0.7)
...
sbt:scalafix> ++2.13.12-bin-1a2373b!
[info] Forcing Scala version to 2.13.12-bin-1a2373b on all projects.
[info] Reapplying settings...
java.lang.RuntimeException: project matching List(PlatformAxis(jvm,JVM,jvm)) and 2.13.12-bin-1a2373b was not found
    at scala.sys.package$.error(package.scala:30)
    at sbt.internal.ProjectMatrix$ProjectMatrixDef$AxisBaseProjectFinder.$anonfun$apply$1(ProjectMatrix.scala:466)
    at scala.Option.getOrElse(Option.scala:189)
    at sbt.internal.ProjectMatrix$ProjectMatrixDef$AxisBaseProjectFinder.apply(ProjectMatrix.scala:466)
    at ScalafixBuild$autoImport$.$anonfun$resolve$1(ScalafixBuild.scala:134)
    at sbt.internal.util.EvaluateSettings$MixedNode.evaluate0(INode.scala:228)
    at sbt.internal.util.EvaluateSettings$INode.evaluate(INode.scala:170)
    at sbt.internal.util.EvaluateSettings.$anonfun$submitEvaluate$1(INode.scala:87)
    at sbt.internal.util.EvaluateSettings.sbt$internal$util$EvaluateSettings$$run0(INode.scala:99)
    at sbt.internal.util.EvaluateSettings$$anon$3.run(INode.scala:94)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
    at java.base/java.lang.Thread.run(Thread.java:833)

it looks like this has to do with some custom code in the Scalafix build; at ScalafixBuild.scala:134 we see

        val project = matrix.jvm(sv)
        Def.setting((project / key).value)                                                          

@bjaglin if you want to try this yourself outside of dbuild and make it possible to do ++2.13.12-bin-1a2373b!, note that you would probably need to publish Scalameta locally for that specific Scala version in order to actually compile or run anything. but perhaps it's not necessary for you to get that deep into it; do you see a way to at least allow the build to load?

SethTisue commented 1 year ago

oh, just getting the build to load is easy; editing project/Dependencies.scala as follows does the trick:

-  val scala213 = "2.13.11"
+  val scala213 = "2.13.12-bin-1a2373b"

seems to do the trick

I'm trying out a change of the form

-  val scala213 = "2.13.11"
+  val scala213 = sys.props.getOrElse("scala.nightly", "2.13.11")

since I can control system properties from dbuild

SethTisue commented 1 year ago

I merged #1674 which gains us some good ground that I'm pleased with.

There's two areas in which further progress might be possible:

One: of the five repos which use scalafix-testkit, #1674 was only able to enable testing in one. The other four seem to be on too-old Scalafix versions. It might be nice to get those repos on a newer version, to increase the overall amount of testing Scalafix gets in the community build. (Or maybe you or me attempting that would be an out-of-scope fishing expedition...)

Two: you'll see in #1674 that I'm still excluding some tests, namely "ScalafixSuite.scala" || "ScalafixImplSuite.scala" || "ScalafixArgumentsSuite.scala". I might be able to make them work in the dbuild context with some digging. But I don't have time right now.

SethTisue commented 1 year ago

Note that in #1674 I forked Scalafix in order to include this change: https://github.com/scalacommunitybuild/scalafix/commit/df9636ff0df76752bdf486a39d234e7c98c1f13d

@bjaglin would you take the change if I PRed it? Or can you think of a better way to make it work with Scala nightlies?

SethTisue commented 12 months ago

@bjaglin ping

SethTisue commented 12 months ago

@bjaglin I added this to https://github.com/scala/community-build/pull/1697 and it worked in a local test, so we're good, thanks!

leaving the ticket open since enabling more subprojects remains an open possibility

SethTisue commented 11 months ago

I think I was confused in my previous comment. We have:

  extra.projects: ["*2_13"]
  extra.exclude: ["docs2_13"]

so that's good, and yes we are still excluding some tests but we normally track that kind of with comments in the .conf file, rather than keeping tickets open. so, closing!