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

Move run-targets into RunModule #3090

Merged
merged 14 commits into from
Mar 26, 2024
Merged

Move run-targets into RunModule #3090

merged 14 commits into from
Mar 26, 2024

Conversation

lefou
Copy link
Member

@lefou lefou commented Mar 18, 2024

This PR moves the definition of the run targets from JavaModule into RunModule. Also some other targets where moved, e.g. mainClass, finalMainClassOpt and finalMainClass.

The advantage of having them in RunModule is, that one can easily define and configure additional runners as sub-modules.

Example: Define a daemon sub-module which acts as runner for the cli.DaemonTool class.

import mill._, scalalib._

object foo extends RootModule with JavaModule {
  def mainClass = "cli.Main"

  object daemon extends RunModule {
    def runClasspath = foo.runClasspath
    def localRunClasspath = foo.localRunClasspath
    def mainClass = "cli.DaemonTool"
  }
}

To preserve binary compatibility, all moved defs retain an override in JavaModule and forward to their super-def in RunModule. Therefore traits compiled against older versions of trait JavaModule should still be runnable with newer versions.

Some run-targets (run, runLocal and runBackground) previously required the compile target, since that also provided some zinc analysis which included some main class discovery. Since the goal was to decouple the run targets from the concept of compilation, I implemented a different discovery logic for main classes which uses the Scala API of scalap. This is completely newly written code but it's only a couple of lines and all existing tests (which also include main class discovery) succeeded. The advantage of the new logic is, that it should work with any given classpath and also for non-Scala classes. The new code is located in the zinc worker, since this is already shared between all RunModules, but there is no real need to have it there. To avoid increased complexity, I resisted to introduce a new shared worker just for the sake of technical independence, for now. The new allLocalMainClasses target can be used to find all main classes in the localRunClasspath. This is some useful information I needed now and then in projects, so it makes sense to have it in a dedicated target for better caching and easy showing.

I also introduced a new localRunClasspath target which abstracts away the classpath that is supposed to be produced by compilation. This is somewhat analogue to the testClassapth introdcued in PR #3064. Since both typically hold the same classpath, testClasspath by default uses the result of localRunClasspath. We probably could remove testClasspath altogether, but it's semantically bound to TestModule and isn't necessarily the same as localRunClasspath, so I think it's legit to keep it.

For consistency, I also added a localCompileClasspath which resembles the part of the localClasspath which is feed into the compiler. All added classpath targets also have a version for BSP (named with prefix bsp and returning a T[Agg[UnresolvedPath]]) when appropriate.

Depends on

Base automatically changed from test-classpath to main March 20, 2024 09:52
lefou added a commit that referenced this pull request Mar 20, 2024
This idea of this change is to restructure the `TestModule` to decouple
it from the concept of compilation. After this change, only a
`runClasspath` and a `testClasspath` is needed to run tests. This makes
adding test modules for the sake of using a different test framework a
breeze.

See the following example: the `test2` module is an additional test
framework on the same classes of `test`.

```scala
import mill._
import mill.scalalib._
import mill.scalalib.api.CompilationResult

object foo extends RootModule with ScalaModule {
  def scalaVersion = "2.13.11"
  def ivyDeps = Agg(
    ivy"com.lihaoyi::scalatags:0.12.0",
    ivy"com.lihaoyi::mainargs:0.6.2"
  )

  object test extends ScalaTests {
    def ivyDeps = Agg(
      ivy"com.lihaoyi::utest:0.7.11",
      ivy"org.scalatest::scalatest-freespec:3.2.18"
    )
    def testFramework = "utest.runner.Framework"
  }
  object test2 extends TestModule with TestModule.ScalaTest {
    override def compile: T[CompilationResult] = ???
    override def runClasspath: T[Seq[PathRef]] = foo.test.runClasspath()
    override def testClasspath = foo.test.testClasspath()
  }
}
```

Please note the `compile` target is a legacy to our binary-compatibility
promise. The target is not used directly in `TestModule`.

This pull request additionally contains the following changes:

* Introduces a new `RunModule` and moved some `run`-related task
previously in `TestModule` up.
* Extend `RunModule` in `JavaModule` to share run-releated targets and
resolve super-hierarchy
* Introduces a `WithZincWorker` as a shared base trait to resolve
super-hierarchies for using and overriding a common worker.

I plan to move more run-releated target from `JavaModule` to `RunModule`
in a subsequent PR. (See #3090)

See also the following discussion:
* #3076

Pull request: #3064
@lefou lefou marked this pull request as ready for review March 20, 2024 22:09
@lefou lefou requested a review from lihaoyi March 20, 2024 22:26
build.sc Outdated Show resolved Hide resolved
scalalib/src/mill/scalalib/JavaModule.scala Outdated Show resolved Hide resolved
scalalib/src/mill/scalalib/RunModule.scala Outdated Show resolved Hide resolved
@lefou lefou changed the title WIP Move run-targets into RunModule Move run-targets into RunModule Mar 20, 2024
@lefou lefou requested a review from lolgab March 21, 2024 08:56
@lefou
Copy link
Member Author

lefou commented Mar 21, 2024

This is good for a review. @lihaoyi @lolgab

@lefou lefou merged commit 36c5217 into main Mar 26, 2024
38 checks passed
@lefou lefou deleted the run-module branch March 26, 2024 07:16
@lefou lefou added this to the 0.11.8 milestone Mar 26, 2024
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