-
Notifications
You must be signed in to change notification settings - Fork 17
Add settings for modules supporting Scala Native #31
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
Conversation
@@ -147,6 +147,17 @@ object ScalaModulePlugin extends AutoPlugin { | |||
|
|||
lazy val scalaModuleSettingsJVM: Seq[Setting[_]] = scalaModuleOsgiSettings | |||
|
|||
lazy val scalaModuleSettingsNative: Seq[Settings[_]] = Seq( | |||
scalaVersion := "2.11.11", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be left to the individual projects to specify.
if (!scalaVersion.value.startsWith("2.11")) | ||
libraryDependencies.value.filterNot(_.organization == "org.scala-native") | ||
else libraryDependencies.value | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this extremely weird. You're trying to bypass Native compilation if the specified scalaVersion
is not compatible with Scala Native. But that, in itself, is a workaround for the first problem: the project should not even try to update/compile/test
for such a Scala version. The build matrix should be configured so that the Native projects are only built on the appropriate versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is a workaround to the fact that ScalaNative plugins are going to be in the classpath and SBT fails to resolve them when the Scala version is different than 2.11.x
. This is regardless of the fact that the previous settings and it's the only way I managed to make scala-parser-combinators
cross-compile.
the project should not even try to update/compile/test for such a Scala version
Of course!, I would love it if sbt-crossproject
allowed to disable the native build, which is what this hack packaged as scalaModuleSettingsNative
tries to do so probably it's better to PR something less hacky in that other plugin. If you have any ideas I'd be happy to help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That does not make any sense. People with Scala.js cross-projects can publish the JVM part of their libraries for new binary-incompatible versions of Scala before Scala.js has been released for those new versions. And that's exactly the same resolve mechanism used by Scala Native (I should know, I co-designed sbt-crossproject
as a generalization of Scala.js' cross-project).
You must simply not run fooNative/compile
at all when you have done ++2.12.2
. It's as simple as that.
The most likely reason you think this doesn't work is because you aggregate
all the fooJVM
/fooJS
/fooNative
in a single foo
, and then of course foo/test
will insist on running fooNative/test
. The solution is simple: do not use aggregation in those contexts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, it actually makes sense, you just have explained it. That's how scala-parser-combinators
was configured before I made any change to it (aggregating all the modules), also the compilation is driven by sbt +compile
without strictly selecting a scala version with ++2.11.11
because this plugin sets up the matix.
Really, as I said, this comes mostly motivated by the initial PR referenced up there, might it be the case of only that project? probably, would it make sense to not aggregate modules in scala-parser-combinators
? Maybe, it would make very cumbersome to work with it. I'm going to close this PR, feel free to continue the conversation in the original PR.
you aggregate all the fooJVM/fooJS/fooNative in a single foo, and then of course foo/test will insist on running fooNative/test
That, in my view, it's a sign that there is something lacking in there. SBT gives the ability of cross-compiling to different Scala versions, sbt-crossproject
attempts to give the ability of cross-compiling to different platforms. Aggregating projects is a common thing, workarounds like this one or like the one you mention (yeah, it's just another workaround) and not excuses to say it's simple as that (oh yes, there are other workarounds, like creating phantom fooJS
, fooJVM
modules, etc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's as simple as that because aggregation and +
have always been broken in sbt, except for the most simple setups. (++
is fine). You don't need platform cross-compilation to be in a scenario where part of the projects in your build will not compile against all Scala versions. For example, if you write an sbt plugin, and extract its core logic in a separate project, you can cross-compile the core logic for all versions of Scala, but the sbt plugin can only build on 2.10.x (and now 2.12 with sbt 1.x).
Therefore, I argue that relying on aggregation and/or +
was broken first, and adding cross-projects into the mix did not make the situation any worse. In fact, even though I've never used sbt-doge, I suspect it would solve the present situation with cross-projects just as well as the sbt plugin/core logic situation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I defo agree on the fact that cross compilation in multi-module projects is broken once you pass the trivial setup. I did used in the past sbt-doge
and solves the problem, but also did not think that adding it to the parser combinators module would be a good idea. Besides, the pure existance of a plugin like sbt-doge
is a clear sign of something lacking in SBT ecosystem.
Anyway, hopefully something that would be fixed (or less hacky) in SBT 1.0...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sbt-doge was created to experiment with how to solve the problem, and wasn't merged into sbt 0.13 to avoid a breaking anyone relying on the old semantics. It has since been merged into sbt 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sbt-doge ... has since been merged into sbt 1
TIL! 🎉
From scala/scala-parser-combinators#118, this PR defines a
scalaModuleSettingsNative
for scala modules that want to cross-compile to Scala Native