-
-
Notifications
You must be signed in to change notification settings - Fork 366
Bump mill to version 1.0.6 #1862
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
base: dev
Are you sure you want to change the base?
Conversation
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
|
@Readon @KireinaHoro |
|
if i remember right, it would.
we need to take more time to have it tested.
…---Original---
From: ***@***.***>
Date: Sun, Jan 4, 2026 17:58 PM
To: ***@***.***>;
Cc: ***@***.******@***.***>;
Subject: Re: [SpinalHDL/SpinalHDL] Bump mill to version 1.0.6 (PR #1862)
Dolu1990 left a comment (SpinalHDL/SpinalHDL#1862)
@Readon @KireinaHoro
Any objection ?
If i remember well, this could break the SpinalHDL integration from source in peoples mill project ? (big mill API change)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Yes -- @inochisa can you please check if we would still be able to include SpinalHDL as a submodule? Ideally the bumped version should still allow using it like in this project, sorry that I don't have time to give you a more stripped-down version... |
|
Thanks -- @inochisa can you check to make sure that the workflow of using SpinalHDL as a git submodule would still work? Sorry I don't have a cut-down example at the moment, a big example would be in this project. |
|
@KireinaHoro I guess it is broken at least for now. I got the following error It seems like this is mentioned in com-lihaoyi/mill#5091, as you have known before. |
|
@inochisa thanks for the check! So my idea when I opened the issue was that, I’ll have some time to downgrade the error as mill upstream suggested, and then do the version bump on our side. Unfortunately I just didn’t get the time. Would you have the interest to tinker with mill a bit? IIRC the change should be small, just disabling a check. Thanks and happy new year! |
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
|
@KireinaHoro At least for now, with PR com-lihaoyi/mill#6504, I have managed to compile my code with SpinalHDL repository. You can check https://github.com/project-inochi/misc-ips/tree/mill-subproject for reference |
Thanks! I really like the simplicity of the syntax and structure. I haven't attempted to build locally just yet, since the mill PR is not merged yet. I'll test locally as soon as that one is merged upstream. |
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.
Pull request overview
This PR upgrades Mill build tool from version 0.11.6 to 1.0.6, migrating the build configuration from the legacy build.sc format to the new build.mill format introduced in Mill 1.0.x. The migration includes updating API usage to match Mill 1.0.x conventions.
Changes:
- Upgraded Mill version from 0.11.6 to 1.0.6
- Migrated build configuration from
build.sctobuild.millwith Mill 1.0.x API updates - Added package declarations to project files for better modularity
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| .mill-version | Updated Mill version from 0.11.6 to 1.0.6 |
| build.sc | Removed legacy build.sc file (replaced by build.mill) |
| build.mill | New build configuration with Mill 1.0.x API (ivy→mvn, T→Task, Agg→Seq, ModuleRef removal) |
| project/package.mill | Added package declaration for build.project package |
| project/Version.mill | Added package declaration and updated to work with new build.mill structure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val MvnDeps = Seq( | ||
| mvn"org.scala-lang:scala-library:${scalaVersion}", | ||
| mvn"org.scalactic:scalactic::3.2.10", | ||
| mvn"net.java.dev.jna:jna:5.12.1", | ||
| mvn"net.java.dev.jna:jna-platform:5.12.1", | ||
| mvn"org.slf4j:slf4j-api:2.0.5", | ||
| mvn"org.scala-lang.modules::scala-xml:1.3.0" | ||
| ) |
Copilot
AI
Jan 21, 2026
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.
The variable MvnDeps is defined but never used. This appears to be a bug in the migration from Mill 0.11.x to 1.0.x. In the original code, IvyDeps was defined but individual modules called super.ivyDeps() which would inherit dependencies from the SbtModule trait.
With Mill 1.0.x, you need to override def mvnDeps in the SpinalModule trait to return these base dependencies. Change this from val MvnDeps = Seq(...) to def mvnDeps = super.mvnDeps() ++ Seq(...) so that child modules can properly inherit and extend these base dependencies.
| def defaultCrossSegments = Seq(SpinalVersion.compilers.head) | ||
| } | ||
| trait Tester extends SpinalModule with SpinalPublishModule { | ||
| import moduleRefs._ |
Copilot
AI
Jan 21, 2026
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.
The original Tester trait in build.sc included override def millSourcePath = os.pwd / "tester" which is missing in the migrated code. This override is important if the tester module's source path differs from the default convention. Without this override, Mill may not be able to locate the tester module's source files correctly.
| import moduleRefs._ | |
| import moduleRefs._ | |
| override def millSourcePath = os.pwd / "tester" |
Closes #1803
Context, Motivation & Description
Since the mill is bump to 1.0.0 release version, I think it is a good time point to bump the mill version.
Impact on code generation
I am not sure, but I think this PR should have no impact on code generation
Checklist
/** */?