Initial work for scoverage 2.0 #1547
Conversation
| @@ -573,7 +573,7 @@ object contrib extends MillModule { | |||
| override def compileIvyDeps = T{ | |||
| Agg( | |||
| // compile-time only, need to provide the correct scoverage version runtime | |||
| Deps.scalacScoveragePlugin, | |||
| Deps.scalacScoverageReporter, | |||
There was a problem hiding this comment.
I didn't fully understand this. Is this only being used to pull the version? I switched this to the reporter just to test it and it worked as well, but probably won't leave it as the reporter.
There was a problem hiding this comment.
The contrib.scoverage.worker is loaded by the contrib.scoverage module in an isolated classloader. The contrib.scoverage.api module is shared between both. So we can load the scoverage jars only when needed and isolated from other mill plugins. Technically, this allows us to use different scoverage versions in the same build. As Scoverage 2.x has an incompatible API, we need to create a new worker module working with Scoverage 2.x. We also do this in other plugin, e.g. contrib.playlib.
There is a major difference between the sbt plugin for Scoverage and this mill plugin. The sbt plugin is external to sbt and users can probably use the older versions supporting scoverage 1.x for a very long time. As our scoverage plugin is developed in-tree, we in general only support compatibility to the exact same mill version. (This is different for plugins developed outside of the tree.) So my question is, do we loose something with Scoverage 2.x? E.g. support for older Scala versions? If that should be the case (which I assume to be true), I'd say we should try a bit harder to keep support for Scoverage 1.x in the plugin. We already use a worker to load and use the scoverage API, so it should be pretty easy to have some switch for the dependencies depending on the actual used version and implement a new worker for Scoverage 2.x. |
| @@ -573,7 +573,7 @@ object contrib extends MillModule { | |||
| override def compileIvyDeps = T{ | |||
| Agg( | |||
| // compile-time only, need to provide the correct scoverage version runtime | |||
| Deps.scalacScoveragePlugin, | |||
| Deps.scalacScoverageReporter, | |||
There was a problem hiding this comment.
The contrib.scoverage.worker is loaded by the contrib.scoverage module in an isolated classloader. The contrib.scoverage.api module is shared between both. So we can load the scoverage jars only when needed and isolated from other mill plugins. Technically, this allows us to use different scoverage versions in the same build. As Scoverage 2.x has an incompatible API, we need to create a new worker module working with Scoverage 2.x. We also do this in other plugin, e.g. contrib.playlib.
| def report( | ||
| reportType: ReportType, | ||
| sources: Seq[os.Path], | ||
| dataDirs: Seq[os.Path], | ||
| sourceRoot: os.Path | ||
| )(implicit |
There was a problem hiding this comment.
As the worker API is shared between all worker implementations, we need to keep/support the sourceRoot parameter, in case we decide to keep the Scoverage 1.x worker.
Yea, this is exactly the case. In the 2.x series we'll no longer publish for 2.11. |
|
@ckipp01 Let me know if you need further guidance in creating a new worker module for Scoverage 2.x. Or just ping me in our Gitter channel. |
Sounds good. It'll probably be a bit before I get around to it. Once I get the green light on the direction in the pr to Dotty I'll probably swing back around to this, but I'll let you know. |
|
@ckipp01 Are you planning to get back to this? I may be interested in picking it up. |
Hey @kierendavies! I was planning on it, but no idea if it'd be soon or not. Seeing that scoverage 2.0.0 is release, by all means pick this up if you'd like. Feel free to ping me to review the scoverage parts or anything and I'll be happy to give some insight. If you do pick it up, just let me know so we don't duplicate work. |
|
Is anybody working on this? @ckipp01 or @kierendavies ? |
Not me. I don't really have any plans in the short-term to get back to this. |
|
As there is nobody currently working on this and we decided to keep the old worker but also as there are merge conflicts in this PR, I decided to re-start this work in a new branch and pull request: I heavily used this PR to identify the required changes. Thank you Chris (@ckipp01) for creating this PR! Closing this PR as it is superseded by #2010. |
scoverage 2.x makes a few changes to the structure of the scoverage project, including the artifacts, and also will allow for Scala 3 support once scala/scala3#13880 gets merged. I wasn't super familiar with how mill was using it, so this was just me learning a bit and updating some of the things I saw. However, it introduces a few questions that I'd love to get your thoughts on.
This is a breaking change that won't easily allow for users to use 1.x anymore. Are you ok with that? Once scoverage bumps to 2.x it will be pretty much impossible to support both 1.x and 2.x for integrations since the structure is quite different, even some of the package names, method signatures etc. In the sbt plugin once I officially bump I'll error out if someone manually tries to use 1.x in the 2.x series of the plugin. What are you thoughts on something like that in Mill?
I'll ask the other question in the parts of the code to make them clearer.
I'll leave this as a draft for now but will iterate on it since there is a few things we'll need to update if you're ok with the 2.x approach listed above.
-coverageflag and the datadir