-
-
Notifications
You must be signed in to change notification settings - Fork 407
Support reading the Mill version from a using directive #4624
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
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The line `//> using mill.version <version>` can be used to specify the Mill version in the `build.mill` file. Removed the `--mill-version` options, since it's not a regular option that Mill supports.
Fixed precedence: `.mill-version` should be taken over `.config/mill-version`. Added some tests.
I said as much on the original discussion, but let's keep this PR open for now without merging it. I'm not ready to commit to using directives for all our in-build-file metadata config, although some of the code here may be reusable even if we go with a different approach |
lihaoyi
added a commit
that referenced
this pull request
Apr 23, 2025
… `import $ivy`, `mill-version` (#4969) This PR adds a YAML 1.2 header comment syntax that subsumes many of the ad-hoc configuration mechanisms we have today: `.mill-version`, `.mill-opts`, `.mill-jvm-opts`, `.mill-jvm-version`, `import $ivy`, and possibly more meta-build overrides or other configuration options in future. ```scala //| mill-version: 0.13.0 //| mill-jvm-version: 17 //| repositories: [$PWD_URI/custom-repo] //| mvnDeps: [org.thymeleaf:thymeleaf:3.1.1.RELEASE] package build import mill._, javalib._ import org.thymeleaf.TemplateEngine import org.thymeleaf.context.Context object foo extends JavaModule { def htmlSnippet = Task { val context = new Context context.setVariable("heading", "hello") context.setVariable("paragraph", "world") new TemplateEngine().process( """<div><h1 th:text="${heading}"></h1><p th:text="${paragraph}"></p></div>""", context ) } def resources = Task { os.write(Task.dest / "snippet.txt", htmlSnippet()) super.resources() ++ Seq(PathRef(Task.dest)) } } ``` The YAML header for any `.mill` file must be a line-comment block starting from the first line of the file and prefixed by `//|`. The prefix is chosen to avoid conflicts with normal comments (`//`), Scala-CLI directives (`//>`), and Java markdown comments (`///`), and to be reminiscent of Scala `|`s commonly used in `.stripMargin` We use YAML 1.2 (via `snakeyaml-engine`) rather than ScalaCLI directives because: - It can map more directly to Mill concepts: `mill-version`, `mill-jvm-version`, `mvnDeps`, and other config keys can be directly in YAML without any mapping. - YAML's hierarchical list/dict/primitive data model maps clearly to Mill's JSON data model for tasks, whereas Scala-CLI directives do not have such a clean correspondence and will require that we manually maintain an explicit mapping - YAML will also be more familiar to non-Scala developers than Scala-CLI's directive syntax, including all the edge cases that are moderately well defined (quoting? escaping? comments? type coercions? multi-line strings?) - Processing by third-party tools will be easier: anyone can write a Python script to strip the prefixes and parse out the YAML using pyyaml or yamlcore, or their equivalents in Node.js/Ruby/Rust/Go/whatever, whereas with Scala-CLI directives you are restricted to a single JVM implementation without spec or documentation All of these concerns were raised in the original Scala-CLI directives discussion but were ignored ([link](https://contributors.scala-lang.org/t/pre-sip-using-directives/5700/5?u=lihaoyi), [link](https://contributors.scala-lang.org/t/pre-sip-scala-cli-as-new-scala-command/5628/92?u=lihaoyi)), so that brings us to today where we have to diverge from their syntax. The choice of YAML over TOML was largely arbitrary. Python chose TOML for their script-header metadata in [PEP-723](https://peps.python.org/pep-0723/), whereas all Markdown implementations ([Github](https://docs.github.com/en/contributing/writing-for-github-docs/using-yaml-frontmatter), [RMarkdown](https://zsmith27.github.io/rmarkdown_crash-course/lesson-4-yaml-headers.html), [Jekyll](https://jekyllrb.com/docs/front-matter/)) chose YAML for their header-metadata syntax. YAML 1.2 seems to fix/mitigate most of the problems with YAML 1.1 (e.g. the `no == false` thing). Kotlin has their own syntax for file-level metadata [`@file:JvmName("Foo")`](https://kotlinlang.org/docs/annotations.html#annotation-use-site-targets), as does Swift [`@Metadata { @DocumentationExtension(mergeBehavior: override) }`](https://www.swift.org/documentation/docc/metadata) Builds on top of #4624 --------- Co-authored-by: Tobias Roeser <le.petit.fou@web.de> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Merged as part of #4969 |
lefou
added a commit
to lefou/mill
that referenced
this pull request
Apr 25, 2025
Those never made it into com-lihaoyi#4969 which was based on top of com-lihaoyi#4624
lefou
added a commit
that referenced
this pull request
Apr 2
8000
6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This experimentally adds support to read the Mill version from a using directive.
The line
//> using mill.version <version>
can be used to specify the Mill version in the
build.mill
file.$
-imports #3062Testing
To exercise that the change is working, I removed the
.config/mill-version
file and added the version to thebuild.mill
file.All CI jobs pass as before and run with the configured Mill version, so Mill version selection seems to work.
Why
using directives
?I wanted an way to configure the Mill version in the build script for a long time. Does it need to use the
using directive
? Maybe not.Using directives were newly invented by Scala CLI project. There were earlier versions supporting richer definitions, but the scala-cli devs settled with a rather primitive key-value format. I don't want to question that. Fact is, it makes processing easy, although it may not be suitable for all possible config values that may come in the future. Just let me highlight the major benefits:
//> using mill.version 0.12.8
Other changes
Removed the
--mill-version
options from the bootstrap scripts. Since it's not a regular option that Mill supports when run without a bootstrap script.Fixed the precedence of
.mill-version
over.config/mill-version
in Mill's own runtime check.Precedence
If a Mill version is given via multiple configuration options, the following precedence rules apply. First option wins:
MILL_VERSION
env var.mill-version
file.config/mill-verison
filemill.version
directive frombuild.mill
mill.version
directive frombuild.mill.scala
mill.version
directive frombuild.sc