Skip to content
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

rewrite mill build scripts for chisel5. #3045

Merged
merged 8 commits into from
Apr 13, 2023
Merged

rewrite mill build scripts for chisel5. #3045

merged 8 commits into from
Apr 13, 2023

Conversation

sequencer
Copy link
Member

@sequencer sequencer commented Mar 1, 2023

This PR cleaned up the old build script, split it to common.sc and builds.sc two files. common.sc is used for declare the dependencies and project structual for each modules, which can be reused by other projects for building from source. build.sc is used for build project in tree.

Contributor Checklist

  • Did you add Scaladoc to every public function/method?
  • Did you add at least one test demonstrating the PR?
  • Did you delete any extraneous printlns/debugging code?
  • Did you specify the type of improvement?
  • Did you add appropriate documentation in docs/src?
  • Did you state the API impact?
  • Did you specify the code generation impact?
  • Did you request a desired merge strategy?
  • Did you add text to be included in the Release Notes for this change?

Type of Improvement

API Impact

Backend Code Generation Impact

Desired Merge Strategy

Release Notes

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels?
  • Did you mark the proper milestone (Bug fix: 3.4.x, [small] API extension: 3.5.x, API modification or big change: 3.6.0)?
  • Did you review?
  • Did you check whether all relevant Contributor checkboxes have been checked?
  • Did you do one of the following when ready to merge:
    • Squash: You/ the contributor Enable auto-merge (squash), clean up the commit message, and label with Please Merge.
    • Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

@sequencer sequencer marked this pull request as ready for review March 1, 2023 18:41
@sequencer sequencer requested a review from jackkoenig March 3, 2023 22:49
@sequencer sequencer force-pushed the mill_chisel5 branch 2 times, most recently from 498af00 to 554c284 Compare March 7, 2023 03:24
@sequencer sequencer force-pushed the mill_chisel5 branch 2 times, most recently from eb9aa3c to 4279fb5 Compare March 15, 2023 00:58
@sequencer sequencer force-pushed the mill_chisel5 branch 2 times, most recently from 225d2eb to 04580b9 Compare April 10, 2023 10:56
@sequencer
Copy link
Member Author

@jackkoenig request for review.

Copy link
Contributor

@jackkoenig jackkoenig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned to you on Slack, this needs to be rebased on #3174 which adds a new firtoolVersion: Option[String] to the BuildInfo. You can just make it None as it really only needs to have a value when publishing and we still use SBT for that (for now).

It's great to see this working again, please add it to CI so we know if it breaks in the future.

Also, I'm not necessarily against supporting multi-project builds binding Chisel from source via the mill rules, but I'm not keen on making it a supported API. I think anyone building from source will have to accept that the build rules can change at any time for any reason.

@jackkoenig jackkoenig added the Internal Internal change, does not affect users, will be included in release notes label Apr 12, 2023
This PR cleaned up the old build script, split it to common.sc and
builds.sc two files. common.sc is used for declare the dependencies and
project structual for each modules, which can be reused by other
projects for building from source. build.sc is used for build project in
tree.
This commit makes chisel3['2.13.10'] being able to compile.
Mill's BuildInfo can't handle anything but Strings.
@jackkoenig jackkoenig merged commit 8283ec1 into main Apr 13, 2023
@jackkoenig jackkoenig deleted the mill_chisel5 branch April 13, 2023 04:35
@ZenithalHourlyRate
Copy link
Contributor

Can this be backported to branch 3.6.x? chisel 3.6.0 already adds firtoolVersion in BuildInfo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internal Internal change, does not affect users, will be included in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants