-
-
Notifications
You must be signed in to change notification settings - Fork 365
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
Allow Mill CLI to select the meta-build frame it operates on #2719
Conversation
Add a new CLI option `--frame` acception an `Int`. Default in `0` and means the root project, `1` is the parent meta-build, if defined, or the built-in bootstrap module, and so on. This is a first draft, to get more familiar with the recursive but mutable nature of our meta-build support. Don't hessitate to point out shortcomings. * Fixes #2658 Review by @lihaoyi
I applied one hack-ish way to avoid instantiating the higher level evaluators by just assigning `null`. This should be refactored, e.g. by representing a skipped frame by it's own case class.
The approach looks reasonable. Maybe one quibble is with the naming |
What about |
Sure that works. But I'd err on the side of "more verbose" since we don't expect this flag to be commonly used |
In the context of #1751 what I want is a way to run a target on all the levels, meta and non-meta. This:
requires me to know how many meta levels I have and also to run the target I'm thinking (and please let me know it has problems) on something like:
or something along those lines which would run a command on all levels, from meta-build to non-meta. |
@lolgab Yeah, I also want sometimes to be able to run something on all levels. This PR is a first step, to be able to request to run targets on any specific level, which was not possible before. Once, we have that working, we can add more features. From a high level view, what you want should be rather simple to implement, as we just need to aggregate all runs of all levels. But devil is in the details. Currently, we have different evaluators for each level, and the logic to aggregate results is also in the evaluator. Also parallelization logic is in the evaluator. So, some more work and refactoring is necessary. Whereas explicitly selecting one level is a rather easy goal to achieve. I'm even not sure if it is possible at all to run targets from all levels at the same time, as evaluation of meta-build can affect the effective module and target relations on dependent levels. So maybe, we will never be able to parallelize evaluation of different levels. |
Level 0 is the normal project, level 1 the first meta-build, and so on. | ||
The last level is the built-in synthetic meta-build which Mill uses to bootstrap the project.""" | ||
) | ||
val metaLevel: Option[Int] |
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.
Is there a reason why we don't use default values here? val metaLevel: Int = 0
which should be supported by mainargs?
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.
Defaults are provided in the apply
method.
metaLevel: Option[Int] = None
Using Option[Int]
in contrast to Int
is more explicit whether the user provided a value or not.
Add a new CLI option
--meta-level
accepting anInt
. Default is0
and means the root project,1
is the parent meta-build, if defined, or the built-in bootstrap module, and so on.This is a first draft, to get more familiar with the recursive but mutable nature of our meta-build support.
Don't hesitate to point out shortcomings.
--meta-level
) #2658Review by @lihaoyi
Example: Find version updates in the meta build
Here is some example output (applied to the mill repo):
Meta information about the build
I also added a new external module
mill.runner.MillBuild
to get some meta-information about the project, for now, the meta-module count or frame count.Here on a project with one meta-build:
Pull request: #2719