Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Define a way for users to extend the top-level module? #1612

Closed
lihaoyi opened this issue Dec 12, 2021 · 21 comments
Closed

Define a way for users to extend the top-level module? #1612

lihaoyi opened this issue Dec 12, 2021 · 21 comments
Milestone

Comments

@lihaoyi
Copy link
Member

lihaoyi commented Dec 12, 2021

Currently a user can define tasks top-level, and can define tasks and inherit things in submodules, but there is no way to inherit things in the top-level module. Apart from being a bit irregular, this means the common SBT style of doing sbt run sbt compile in a trivial one-module project cannot be done without prefixes like ./mill app.run ./mill app.compile

I'm honestly not 100% what this should look like; some options are:

  • We define a special syntax used for inheriting modules, e.g. import $extends.mill.scalalib.ScalaModule``, and have that be spliced into the source code wrapper we run the scripts with. Downstream compilation and usage remains unchanged.

  • We let people annotate a module as "top-level", e.g. via a special name object build extends ScalaModule{} or a marker trait object foo extends ScalaModule with TopLevelModule, and automatically recurse into the module's contents as part of our target resolve logic

  • Something else?

This would make working with Mill builds a bit more regular, especially for small ones, and help simplify the getting started experience for Mill, bringing it up to par with SBT

@heksesang
Copy link
Contributor

The approach with marker trait seems less "magical" than the special import syntax, so it would be easier to understand and make the getting started experience better for new users.

@lefou
Copy link
Member

lefou commented Jan 20, 2022

Best is, we don't need any magic at all. The marker trait is the right direction. We should also review MainModule which currently is our top level module and provides additional commands. Maybe, we can just re-use it here?

@lolgab
Copy link
Member

lolgab commented Jan 29, 2022

I find having a special object in the build as a TopLevelModule very confusing..
What happens if you have the same task both in the special object and in the top level? Do you fail? It seems to me that we are merging two unrelated scopes together, the root scope and the special object one.
What about a mill argument to decide what type is the root module and then allow the overrides in place? (I'm assuming this is possible in ammonite)
Imagining that you set the root module to be a ScalaModule then you need to define

def scalaVersion = "2.13.8"

in the top level for it to compile.
Seems clean enough to me.

@lefou
Copy link
Member

lefou commented Jan 29, 2022

That's why I suggested to kind of reuse MainModule, as this is what provides all the current top level tasks. When we detect this situation, we could simply don't wrap it around but instead instantiate it directly. I see this issue in context of potentially other changes we want to apply, e.g. decoupling from Ammonite and handling $-special imports directly (com-lihaoyi/Ammonite#1228), so we might introduce an analyze phase anyway.

@lefou
Copy link
Member

lefou commented Jan 29, 2022

@lolgab Ah, now I think I see what you mean. I think when we use a special top level module, we should simply ignore or reject other top level tasks.

@lolgab
Copy link
Member

lolgab commented Jan 29, 2022

But I don't see the point of having all the extra code, concepts and complexity if then users need to write the same build file but with a with TopLevelModule and see errors when there are top level tasks just to make you run tasks without a app. prefix.
Instead having a build file like:

def scalaVersion = "2.13.8"

instead of the current:

object app extends ScalaModule {
  def scalaVersion = "2.13.8"
}

would make the newcomer experience nicer.
Also it would be source compatible with current mill (you will not need to move all your existing tasks to another special object if you want to use the top level module).

Also putting your code in the root is already supported via:

def millSourcePath = super.millSourcePath / os.up

@lihaoyi
Copy link
Member Author

lihaoyi commented Jan 30, 2022

The question is how does Mill know the top level module is a ScalaModule? It could be a JavaModule, a ScalaNativeModule, or something user defined: I often have a FooScalaModule defined in my Mill projects which is ScalaModule with my project-specific customizations, and I'd like my top-level module to inherit that stuff just as my nested modules inherit it

Having the top level module hardcoded to ScalaModule would work in trivial cases, but that approach doesnt transition smoothly as the build gets more complex

@lolgab
Copy link
Member

lolgab commented Jan 30, 2022

@lihaoyi My suggestion is for the supertype to be coming from the mill configuration (like the .mill-jvm-args) and to default to the current type that Mill root. I don't know how feasible is this, though.

@lefou
Copy link
Member

lefou commented Jan 30, 2022

First I want to say having to explicitly declare a top level module isn't IMHO a big deal for newcomer or expert. It's only one line above and a curly brace below the other code, so I think we should not over-use this as couter-argument. What is the alternative? I never liked the much heavier paradigm shift needed in sbt when transitioning between a single and multi project setups. Also having an explicit top module type makes it easy for anybody far away from an IDE to reason about the project and the available targets. So, having that extra line is an advantage in my eyes, and much better than yet another magic import.

Speaking about transition cost between single and multi projects. It's rather straight forward to add just a second top level module or a sub-module, so it's nothing a beginner tries to avoid, which is a good thing too.

Also, an explicit top level module allows us to start with customizations already applied, e.g. when the top module comes from an $-import and is shared in a repo or organization or distributed via Maven. This includes to possibility to use completely different (from scalalib) toolchains altogether.

Essentially, the only thing we need to change in mill itself is how it's handling the build script. And this brings the question whether there are alternative approaches? I think one alternative is (what @lolgab suggested too) to just handle the Mill cli differently. When we assume or configure a prefix, Mill could automatically handle all cli arguments that don't resolve by applying this prefix. Example: A simple project defines a object foo extends JavaModule, and the cli invocation could be mill compile, so Mill treat it like mill foo.compile.

The pros of this approach is, that we don't need to change anything that affects the compilation and reflection of the build file.
The cons is, that it is a bit more magical (maybe it's not magical but just new) and we need to deal with ambiguity, by ignoring, or warning or error-ing out. Also we need to think how we configure the prefix? By annotation, by magic import or by marker trait.

And last not least, another thought where I'm not sure it's cleanly possible, but which might feel intuitive. Instead of a marker trait, we could just match on name, e.g. build, which currently denotes the actual top level trait. Once we find a user defined object build we could just treat it as the new top level module.

@lefou
Copy link
Member

lefou commented Jan 30, 2022

Just to make this clear. For me, this RFC is not so much about making the build file smaller but about having a more convenient Mill cli.

@lefou
Copy link
Member

lefou commented Jan 30, 2022

@lihaoyi My suggestion is for the supertype to be coming from the mill configuration (like the .mill-jvm-args) and to default to the current type that Mill root. I don't know how feasible is this, though.

I don't think this is a good idea. The build.sc is a much better place for such essential information. And the Scala language is the best tool to declare extensions. Also, for loading the top level type you probably want to use $-imports.

@lolgab
Copy link
Member

lolgab commented Jan 30, 2022

Just to make this clear. For me, this PRC is not so much about making the build file smaller but about having a more convenient Mill cli.

This is probably the point of friction that makes us having different ideas.
I think that the current cli is good enough and not having to write an extra build. prefix in commands is not a good reason to add a new concept in mill for people to know.
But I may be wrong.
About the .mill-jvm-args, you're right on it being limiting. Let's drop that from the table.

@lihaoyi
Copy link
Member Author

lihaoyi commented Jan 30, 2022

I think having a top-level object build extends FooModule{} is an acceptable price worth paying. Having it in an external config file is definitely possible, but it kind of goes against how the "rest" of Mill works: most of it lives in the build.sc, rather than any other config file. It's similar to having some other magic import or annotation syntax, mentioned in the initial description.

I'm OK with matching on the module name build, the only complaint then would be that it's too easy to accidentally lose all the miscellaneous stuff that MainModule provides: show, visualize, etc. if your new object build does not provide them. Perhaps in the interest of conformity, we could declare "it must be named build and must somehow inherit from MainModule", and anything that only partially satisfies this we can provide a nice error message. That way everyone's builds which have top-level stuff will look about the same object build extends ... with MainModule{ ... }.

@lefou
Copy link
Member

lefou commented Jan 30, 2022

As a side note: I like to see Mill as having one single source of truth, the build.sc. (Almost) everything else is included from it. The only exceptions are the Mill version, JVM arguments and potential predef files.

This is an huge advantage over many other build tools and it's impact is probably underestimated. It helps to easily understand build setups and assist reproducibility.

@lefou
Copy link
Member

lefou commented Jan 30, 2022

I think having a top-level object build extends FooModule{} is an acceptable price worth paying. Having it in an external config file is definitely possible, but it kind of goes against how the "rest" of Mill works: most of it lives in the build.sc, rather than any other config file. It's similar to having some other magic import or annotation syntax, mentioned in the initial description.

I'm OK with matching on the module name build, the only complaint then would be that it's too easy to accidentally lose all the miscellaneous stuff that MainModule provides: show, visualize, etc. if your new object build does not provide them. Perhaps in the interest of conformity, we could declare "it must be named build and must somehow inherit from MainModule", and anything that only partially satisfies this we can provide a nice error message. That way everyone's builds which have top-level stuff will look about the same object build extends ... with MainModule{ ... }.

I can see we get some clearer idea how it could work, nice. Although the explicit extends MainModule is somewhat ideomatic, as Mill tries to follow the Scala Language when possible, I'd also expect this detail to be some frequently poping up issue in the chat.

About the various commands, I remember from my early Mill days that we had a discussion about moving some of them into generic cli arguments (probably because I asked that). After that, my understanding was then, that we prefer "Evaluator Commands" because they are also reachable from the Mill REPL. But we have other issues with these kind of commands and will replace them by something more main-args @main style. And the REPL isn't guaranteed to stay too, IIUC, at least it isn't used much by us devs. :-) I just wanted to drop this here.

@lefou
Copy link
Member

lefou commented May 3, 2022

One idea is to have a def mainModule: Module = build (or something in that spirit) in the synthetic outer module, which represents the module which is used by Mill as the top level one.

By overriding it, we could easily replace the "top level" module.

import $file.`my-plugin`

override def mainModule = `my-plugin`.MyCoolTopLevelModule

This won't work out of the box. What we really want is an object, but that could be worked out, I guess.

@lefou
Copy link
Member

lefou commented May 3, 2022

... as a consequence, other top-level defs would be no longer reachable.

@lefou
Copy link
Member

lefou commented Jul 15, 2022

While thinking about a solution to

I came across the using directive, which is used by Scala-cli and available as a stand-alone Java library (https://github.com/VirtusLab/using_directives), and which could be a lightweight replacement for Ammonite magic imports ($-imports). If we decide to use it and proof of concepts work, we could as well think of supporting some other settings directly via using directives. This may as well include a toggle, whether we want to use a default wrapper object build, as it is now, or not. We could even allow three variants:

  • wrap (default) - wrap with build which extends MainModule
  • wrap-and-extends - explicitly specify an extra trait(s) we want to extend from (additionally to MainModule)
  • no-wrap - don't wrap but use a build object found in the script - this would allow very custom setups, as it will disable built-in commands. We could make all existing commands from MainModule available via an external module, so we won't lose any functionality.

@lefou
Copy link
Member

lefou commented Jul 18, 2022

Directly extending a given module:

//> using extends "mill.scalalib.ScalaModule"

override def scalaVersion = // ...

Extending a (local) self-defined module: This one will probably not work, as Common is defined inside of the object being created. But it may work, if Common is defined in an included file.

//> using extends "Common"

trait Common extends ScalaModule {
  def scalaVersion = // ...
}

def ivyDeps = Agg( // ...

I'm not sure we want to allow extending without a MainModule. If we do, Mill cli would no longer provide built-in commands like mill show. But we could expose MainModule as external module, so the user could run mill mill.main.MainModule/show.

//> using extends.only "mill.scalalib.ScalaModule"

def scalaVersion = // ...

@lefou
Copy link
Member

lefou commented Jul 18, 2022

While the above examples would allow slick single module projects, we could also go the other way, by not wrapping at all.

//> using mill.entry `TopModule`

trait TopModule extends mill.define.Module {

}

@lihaoyi
Copy link
Member Author

lihaoyi commented Apr 23, 2023

Done in #2377

You can now extends RootModule to mark a module as top-level

import mill._, scalalib._

object foo extends RootModule with ScalaModule {
  def scalaVersion = "2.13.8"
  def ivyDeps = Agg(
    ivy"com.lihaoyi::scalatags:0.8.2",
    ivy"com.lihaoyi::mainargs:0.4.0"
  )
}

@lihaoyi lihaoyi closed this as completed Apr 23, 2023
@lefou lefou added this to the 0.11.0-M8 milestone Apr 23, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants