Skip to content

Conversation

ckipp01
Copy link
Contributor

@ckipp01 ckipp01 commented Aug 2, 2022

This PR adds in support for the Mill build tool by utilizing
https://github.com/ckipp01/mill-scip. It's a pretty thin wrapper. You
can find more details about mill-scip in the repo but the general
workflow is:

  • It's an external Mill plugin that gathers everything needed to do a
    compile via Mill, but then adds some extra plugins/scalacOptions to
    ensure semanticDB gets produced
  • Once produced it uses scip-java as a library to actually produce the
    scip file.

The plugin isn't actually a dependency here which ensures we don't have
any cyclical dependency on anything, but rather just utilizes the
--import functionality of Mill and external plugins. I'll leave a
couple more comments on various parts of the PR.

Test plan

There is a test for the Mill integration

This PR adds in support for the Mill build tool by utilizing
https://github.com/ckipp01/mill-scip. It's a pretty thin wrapper. You
can find more details about mill-scip in the repo but the general
workflow is:

- It's an external Mill plugin that gathers everything needed to do a
  compile via Mill, but then adds some extra plugins/scalacOptions to
  ensure semanticDB gets produced
- Once produced it uses `scip-java` as a library to actually produce the
  scip file.

The plugin isn't actually a dependency here which ensures we don't have
any cyclical dependency on anything, but rather just utilizes the
`--import` functionality of Mill and external plugins. I'll leave a
couple more comments on various parts of the PR.
@@ -0,0 +1,32 @@
package tests

class MillBuildToolSuite extends BaseBuildToolSuite {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this doesn't pass right now because to my understanding the tests rely on being able to pass in targetRoot which mill-scip doesn't support. I didn't really see a reason to when creating it, but I guess I could add it if it's a must to be used here. Let me know, and I'll hold off until getting feedback.

val localMill = Files.isRegularFile(millFile)
val command =
if (localMill) {
"./mill"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is also one thing I need to test first locally to ensure proc'ing like this with ./mill actually works. I figured it's a good idea to try and just use the local mill if they have one.

@ckipp01
Copy link
Contributor Author

ckipp01 commented Aug 2, 2022

Sorry for the noise, created this on my main so closing in favor of #477

@ckipp01 ckipp01 closed this Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant