wix-incubator / accord

Accord: A sane validation library for Scala
http://wix.github.io/accord/
Other
529 stars 55 forks source link

GenBCode issue with compiling #84

Closed dispalt closed 7 years ago

dispalt commented 7 years ago

I am running into an issue when compiling my project with GenBCode backend, it appears related to the use of the toolbox in the MacroHelper class.

[error] missing or invalid dependency detected while loading class file 'MacroHelper.class'.
[error] Could not access term tools in package scala,
[error] because it (or its dependencies) are missing. Check your build definition for
[error] missing or conflicting dependencies. (Re-run with `-Ylog-classpath` to see the problematic classpath.)
[error] A full rebuild may help if 'MacroHelper.class' was compiled against an incompatible version of scala.
[error] missing or invalid dependency detected while loading class file 'MacroHelper.class'.
[error] Could not access term nsc in value scala.tools,
[error] because it (or its dependencies) are missing. Check your build definition for
[error] missing or conflicting dependencies. (Re-run with `-Ylog-classpath` to see the problematic classpath.)
[error] A full rebuild may help if 'MacroHelper.class' was compiled against an incompatible version of scala.tools.
[error] missing or invalid dependency detected while loading class file 'MacroHelper.class'.
[error] Could not access term ast in value scala.nsc,
[error] because it (or its dependencies) are missing. Check your build definition for
[error] missing or conflicting dependencies. (Re-run with `-Ylog-classpath` to see the problematic classpath.)
[error] A full rebuild may help if 'MacroHelper.class' was compiled against an incompatible version of scala.nsc.

Any ideas?

holograph commented 7 years ago

Interesting, I didn't know the need backend had any fundamental differences. I'll need to look into this. Can you share your compilation settings?

On Thu, 10 Nov 2016 at 18:54 Dan Di Spaltro notifications@github.com wrote:

I am running into an issue when compiling my project with GenBCode backend, it appears related to the use of the toolbox in the MacroHelper class.

[error] missing or invalid dependency detected while loading class file 'MacroHelper.class'. [error] Could not access term tools in package scala, [error] because it (or its dependencies) are missing. Check your build definition for [error] missing or conflicting dependencies. (Re-run with -Ylog-classpath to see the problematic classpath.) [error] A full rebuild may help if 'MacroHelper.class' was compiled against an incompatible version of scala. [error] missing or invalid dependency detected while loading class file 'MacroHelper.class'. [error] Could not access term nsc in value scala.tools, [error] because it (or its dependencies) are missing. Check your build definition for [error] missing or conflicting dependencies. (Re-run with -Ylog-classpath to see the problematic classpath.) [error] A full rebuild may help if 'MacroHelper.class' was compiled against an incompatible version of scala.tools. [error] missing or invalid dependency detected while loading class file 'MacroHelper.class'. [error] Could not access term ast in value scala.nsc, [error] because it (or its dependencies) are missing. Check your build definition for [error] missing or conflicting dependencies. (Re-run with -Ylog-classpath to see the problematic classpath.) [error] A full rebuild may help if 'MacroHelper.class' was compiled against an incompatible version of scala.nsc.

Any ideas?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/wix/accord/issues/84, or mute the thread https://github.com/notifications/unsubscribe-auth/ABEC4AcM73JXPbdNWC0B8-aoM90oAys1ks5q80xcgaJpZM4Ku5AF .

dispalt commented 7 years ago

Yeah I was surprised too, I am guessing it's related to this https://github.com/wix/accord/blob/master/core/src/main/scala-2.11/com/wix/accord/transform/MacroHelper.scala#L65

scalacOptions ++= Seq(
        "-unchecked",
        "-deprecation",
        "-language:_",
        "-target:jvm-1.8",
        "-encoding",
        "UTF-8",
        "-feature",
        "-language:existentials",
        "-language:higherKinds",
        "-language:implicitConversions",

        "-Yinline-warnings",
        "-Yno-adapted-args",
        "-Ywarn-dead-code",
        "-Ywarn-numeric-widen",
        "-Ywarn-value-discard",

        "-Ybackend:GenBCode"
      ),

The last one being the addition that resulted in that error.

holograph commented 7 years ago

I've verified that Accord itself works successfully with this flag; I suspect compiling code with GenBCode doesn't support depending on non-GenBCode'd libraries where macros are concerned, but I'll do some more digging first.

dispalt commented 7 years ago

I just did that, compiled accord with the flag and recompiled the dependent project to no luck. I am honestly not sure what's going on, but considering that GenBCode is a backend I am sure its conditionally presenting classes and maybe that structure changed or something, I am not sure. The main reason why this is useful is its the default backend to 2.12

holograph commented 7 years ago

2.12 is compiled and packaged separately with the default compiler (and its associated backend), so I don't expect any issues with 2.12. That being said, I'll try and reproduce the problem with 2.11 and update here.

holograph commented 7 years ago

Sorry, I haven't been able to replicate this with the Accord example project with your options. I'm not sure why you're running into this issue, but if you're willing to provide access to your codebase (or have a screen sharing session) I'd love to help you try and figure it out...

dispalt commented 7 years ago

Yeah I was being lazy, Ill work up a small public example, thanks!

jasongilanfarr commented 7 years ago

I recently tried to upgrade http://github.com/mesosphere/marathon to 0.6 and we also use GenBCode and ran into the same issue. As soon as I fixed the last compile error (you can see that if you look for https://github.com/wix/accord/issues/89 in the review).

https://phabricator.mesosphere.com/D197

odenzo commented 7 years ago

I get the same errors, but with no scala options (e.g. GenBCode) 0.6 Scala 2.12.0

holograph commented 7 years ago

I've yet to be able to replicate this locally, but I'll keep trying. Thanks for the pointers, and if anything comes to mind (or you managed to reproduce this separately) please post it here!

holograph commented 7 years ago

OK, managed to find a workaround thanks to @jasongilanfarr's data on Marathon; I'm not clear on why this is required all of a sudden, but scala-compiler isn't part of the compilation classpath, and is consequently unavailable when expanding the macro. It is my understanding that this should be provided by the compiler instance hosting the macro, but I'll need to verify this with @xeno-by.

At any rate this can be worked around by manually adding a library dependency to the project containing the validators as follows (feedback would be appreciated):

// "provided" scope guarantees this will be available when compiling, but won't be transitively
// inherited by downstream projects.
libraryDependencies += "org.scala-lang" % "scala-compiler" % scalaVersion.value % "provided"

Sample output:

marathon(86586c3c8dd01cb60ff63a8cd1d64d5f231a3c4e)> show marathon/libraryDependencies
[info] * net.sourceforge.pmd:pmd-dist:5.4.2:cpd->default
[info] * com.sksamuel.scapegoat:scalac-scapegoat-plugin_2.11:1.2.1:compile
[info] * org.scala-lang:scala-library:2.11.8
[info] * com.typesafe.akka:akka-actor:2.4.10:compile
[info] * com.typesafe.akka:akka-slf4j:2.4.10:compile
[info] * com.typesafe.akka:akka-stream:2.4.10:compile
[info] * com.typesafe.akka:akka-http-experimental:2.4.10:compile
[info] * org.scala-lang.modules:scala-async:0.9.6:compile
[info] * io.spray:spray-client:1.3.3:compile
[info] * io.spray:spray-httpx:1.3.3:compile
[info] * mesosphere:chaos:0.8.7:compile
[info] * org.apache.mesos:mesos:1.1.0:compile
[info] * com.twitter.common.zookeeper:candidate:0.0.76:compile
[info] * joda-time:joda-time:2.9.4:compile
[info] * org.joda:joda-convert:1.8.1:compile
[info] * com.sun.jersey:jersey-servlet:1.18.5:compile
[info] * com.sun.jersey.contribs:jersey-multipart:1.18.5:compile
[info] * org.eclipse.jetty:jetty-servlets:9.3.6.v20151106:compile
[info] * com.fasterxml.uuid:java-uuid-generator:3.1.4:compile
[info] * org.javabits.jgrapht:jgrapht-core:0.9.3:compile
[info] * org.apache.hadoop:hadoop-hdfs:2.7.2:compile
[info] * org.apache.hadoop:hadoop-common:2.7.2:compile
[info] * commons-beanutils:commons-beanutils:1.9.2:compile
[info] * com.typesafe.play:play-json:2.5.8:compile
[info] * com.github.fge:json-schema-validator:2.2.6:compile
[info] * com.twitter:util-zk:6.34.0:compile
[info] * io.reactivex:rxscala:0.26.2:compile
[info] * mesosphere.marathon:ui:1.2.0:compile
[info] * io.dropwizard.metrics:metrics-graphite:3.1.2:compile
[info] * org.coursera:dropwizard-metrics-datadog:1.1.6:compile
[info] * mesosphere.marathon:api-console:3.0.8:compile
[info] * com.wix:accord-core:0.6:compile
[info] * org.apache.curator:curator-recipes:2.11.0:compile
[info] * org.apache.curator:curator-client:2.11.0:compile
[info] * org.apache.curator:curator-framework:2.11.0:compile
[info] * org.scala-lang.modules:scala-java8-compat:0.8.0:compile
[info] * com.typesafe.scala-logging:scala-logging:3.5.0:compile
[info] * net.logstash.logback:logstash-logback-encoder:4.7:compile
[info] * com.getsentry.raven:raven-logback:7.7.0:compile
[info] * de.heikoseeberger:akka-http-play-json:1.10.1:compile
[info] * org.gnieh:diffson:2.0.2:test
[info] * org.scalatest:scalatest:3.0.0:test
[info] * org.mockito:mockito-all:1.10.19:test
[info] * com.typesafe.akka:akka-testkit:2.4.10:test
[info] * junit:junit:4.12:test
[info] * org.psywerx.hairyfotr:linter:0.1.16:plugin->default(compile)
marathon(86586c3c8dd01cb60ff63a8cd1d64d5f231a3c4e)> set libraryDependencies in marathon += "org.scala-lang" % "scala-compiler" % "2.11.8" % "provided"
[info] Defining marathon/*:libraryDependencies
[info] The new value will be used by marathon/*:allDependencies, marathon/*:dependencyPositions, marathon/*:dependencyUpdatesData
[info] Reapplying settings...
[info] Set current project to marathon (in build file:/Users/tomerga/dev/marathon/)
marathon(86586c3c8dd01cb60ff63a8cd1d64d5f231a3c4e)> compile
[info] Formatting 85 Scala sources {file:/Users/tomerga/dev/marathon/}marathon(compile) ...
[info] Updating {file:/Users/tomerga/dev/marathon/}marathon...
[info] Resolving com.sun.jersey.contribs#jersey-guice;1.18.1 ...
[info] Reformatted 77 Scala sources {file:/Users/tomerga/dev/marathon/}marathon(compile).
[info] Resolving jline#jline;2.12.1 ...
[info] Done updating.
[warn] There may be incompatibilities among your library dependencies.
[warn] Here are some of the libraries that were evicted:
[warn]  * org.scala-lang.modules:scala-java8-compat_2.11:0.7.0 -> 0.8.0
[warn] Run 'evicted' to see detailed eviction warnings
[info] Compiling 508 Scala sources and 3 Java sources to /Users/tomerga/dev/marathon/target/scala-2.11/classes...
[success] Total time: 85 s, completed Nov 23, 2016 5:17:12 PM
xeno-by commented 7 years ago

@holograph I think that's yet another situation when missing transitive dependencies may cause spurious compilation failures. I guess that something as subtle as changing a backend may tip the scales.

About your question. When expanding macros, the macro engine provides a classloader that, in addition to the compilation classpath, includes scala-reflect.jar and scala-compiler.jar. This is how you can expand macros that have been compiled with scala-compiler.jar even when it is not on the compilation classpath of the project that uses macros.

What's going on here is unrelated, I think. While traversing the compilation classpath, the compiler may need to do some processing of the metadata it loads. It looks like the amount of processing required is different between the GenASM and GenBCode backends.

@lrytz @adriaanm @retronym Any ideas why the GenBCode backend would be more aggressive wrt transitive dependencies that the GenASM one?

retronym commented 7 years ago

Might be related to https://github.com/scalamacros/paradise/issues/68, by which use of typeOf with existentials / type aliases results in an unwanted dependency on internals.

⚡ git grep typeOf
core/src/main/scala-2.11/com/wix/accord/transform/MacroHelper.scala:          internal.boundedWildcardType( internal.typeBounds( typeOf[ Nothing ], typeOf[ Any ] ) )
core/src/main/scala/com/wix/accord/transform/ExpressionDescriber.scala:    private val descriptorTerm = typeOf[ com.wix.accord.dsl.Descriptor[_] ].typeSymbol.name.toTermName
core/src/main/scala/com/wix/accord/transform/PatternHelper.scala:      tpe.widen =:= typeOf[ Null ]
core/src/main/scala/com/wix/accord/transform/PatternHelper.scala:      tpe =:= typeOf[ Nothing ] || isNull
core/src/main/scala/com/wix/accord/transform/PatternHelper.scala:      if ( that == typeOf[ Null ] )
core/src/main/scala/com/wix/accord/transform/RuleFinder.scala:    def isValidationRule( tpe: Type ): Boolean = !tpe.isBottom && tpe <:< typeOf[ Validator[_] ]
core/src/main/scala/com/wix/accord/transform/RuleFinder.scala:    private val contextualizerTerm = typeOf[ dsl.Contextualizer[_] ].typeSymbol.name.toTermName
core/src/main/scala/com/wix/accord/transform/ValidationTransform.scala:    val vboTerm = typeOf[ dsl.ValidatorBooleanOps[_] ].typeSymbol.name.toTermName
/code/wix-accord on master*
⚡

Moving the {weak}typeOf calls to different locations can workaround the situation.

Now I'm not at all sure if this it he issue, or why the choice of backend makes a difference. If someone could prepare instructions on how to reproduce with a minimal client project of this macro, I can take a quick look.

retronym commented 7 years ago

In general, I wouldn't recommend using the experimental GenBCode backend in 2.11.x as there are known bugs which only fixed in 2.12.x. That said, this overall problem might still exist in 2.12.x, so I'm happy to help in that context.

retronym commented 7 years ago

I've lodged a compiler bug for this and will fix it shortly: https://github.com/scala/scala-dev/issues/275

holograph commented 7 years ago

Thanks @retronym, I'll track it and follow up if it seems resolved.

holograph commented 7 years ago

Remarking for milestone 0.6.1 (either for verification or to add a "known issues" section to the documentation)

holograph commented 7 years ago

Documentation updated to mention the workaround; removing milestone assignment and will keep open until a Scala version with the fix is released and confirmed.

holograph commented 7 years ago

Per the Scala repo this should've been fixed in 2.12.1, however I'm blocked from verifying this due to #103. Will follow up when there's news on that front.

holograph commented 7 years ago

I believe Scala 2.12.2 solves both issues, though I haven't been able to fully verify. Please reopen if you run into this issue again...