-
Notifications
You must be signed in to change notification settings - Fork 613
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
Migrate to mill 0.12.5 and refactor mill scripts #4616
Conversation
4a56e09
to
baab46e
Compare
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.
I didn't expect introduced so many package.mill
files, but anyway that looks good to me since it's recommended by the official mill.
But My key concern is package.mill
always requires importing from the root build.mill
, I understand this is because the cross dependencies of these modules. But is possible to only declare the dependencies requirements from each sub-folder and fill by the build.mill
, like what my common.sc
originally did.
The second nit is, I think the build.mill
is still too much of things, please reduce it by splitting it into multiple files, I think the core build.mill
should only maintain the publish flow.
|
||
// Java Codegen for all declared functions. | ||
// All of these functions are not private API which is subject to change. | ||
trait CIRCTPanamaBinding extends HasJextractGeneratedSources { |
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.
HasJextractGeneratedSources
, you can add a panama.mill
file storing it, In the future, a contribution point is push our Jextract
+ Panama
as plugin to official mill plugin.
Signed-off-by: unlsycn <[email protected]>
Signed-off-by: unlsycn <[email protected]>
Signed-off-by: unlsycn <[email protected]>
Signed-off-by: unlsycn <[email protected]>
Signed-off-by: unlsycn <[email protected]>
Thanks for this. I'm seeing both failures during the publish step [1], during nightly ci [2], and locally when switching branches. The switching branches I'm confused about, but this is acting like there is a stale file that it is trying to compile which was removed:
Any idea what is going on? Given that CI is actually broken, can this be blamelessly reverted while it gets figured out? |
I think it was blamed to random issue from mill side, may GitHub Action + mill don't work well together.
I personally think it just only a small tweak on CI, ask @unlsycn fix this today, otherwise revert this. |
This PR migrates to Mill 0.12.5 and refactors the Mill build scripts according to the recommended practice, which includes the following changes:
All modules except
chisel
have been split into separate package.mill in subfolders, and the publish flow has been separated intorelease.mill
.build.mill
contains onlythe utility traits and modules for easy reuse in packages and the
chisel
module, which Mill requires to be declared inside the top-level module.Due to
RootModule
andCross
cannot be applied to the same module com-lihaoyi/mill#3693, CrossModule have been moved to{module}.cross
(e.g.firrtl[2.13.15] => firrtl.cross[2.13.15]
as a workaround, and it does not affect the directory hierarchy.Contributor Checklist
docs/src
?Type of Improvement
Desired Merge Strategy
Release Notes
Reviewer Checklist (only modified by reviewer)
3.6.x
,5.x
, or6.x
depending on impact, API modification or big change:7.0
)?Enable auto-merge (squash)
, clean up the commit message, and label withPlease Merge
.Create a merge commit
.