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

Add basic mill integration #532

Merged
merged 4 commits into from
Jun 11, 2018
Merged

Conversation

Baccata
Copy link
Contributor

@Baccata Baccata commented Jun 10, 2018

Adds a bloop.integrations.mill.Bloop mill external module that can be used to generate bloop config from mill. The support is limited to JVM modules at the moment, and does not fill the "resolution" configuration element.

See https://www.lihaoyi.com/mill/page/modules.html#external-modules

Usage :

  1. add import $ivy.`ch.epfl.scala::mill-bloop:$bloopVersion` to the build.sc of a project (it's probably possible to add it to a ~/.mill/ammonite/predef.sc or a similar file instead)

  2. run mill bloop.integrations.mill.Bloop/install from the directory of the mill project.

NB :

Due to some obscure bug that prevents me from safely running non-brewed bloop, I'm only able to run 1.0.0-M10, which fails to run the tests on my machine due to a bug in nuprocess.

@@ -66,4 +67,5 @@ object Dependencies {

val scalaNativeTools = "org.scala-native" %% "tools" % scalaNativeVersion
val scalaJsTools = "org.scala-js" %% "scalajs-tools" % scalaJsVersion
val mill = "com.lihaoyi" %% "mill-scalalib" % millVersion
Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably should tag this as "provided"

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think so. It depends on how mill implements plugin classloading, but if the mill classloader is the parent of your plugin classloader, it should be Provided 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 4943ed5 (tested locally)

java = javaConfig(),
test = testConfig(),
platform = Config.Platform.JVM,
nativeConfig = None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

filling this will depend on com-lihaoyi/mill#206

@jvican jvican changed the title Beginning of mill integration Add basic mill integration Jun 10, 2018
@jvican jvican added integrations mill community Any change or proposal that is contributed by the Open Source Community. labels Jun 10, 2018
Copy link
Contributor

@jvican jvican left a comment

Choose a reason for hiding this comment

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

The integration looks great and simple. Still some information to extract, but it has the bare minimum to already provide value to people, so I'll be merging today!

I first need to rebase the PR with the latest changes I did in another PR that will be merging soon. Those changes break some parts of the API you use here (we're preparing the final version of the bloop configuration file so we're polishing it). Do you have any more changes you want to include before I rebase?

By the way, I'm curious, how long does it take to generate the bloop configuration file?

Thanks @Baccata 🙏

@@ -66,4 +67,5 @@ object Dependencies {

val scalaNativeTools = "org.scala-native" %% "tools" % scalaNativeVersion
val scalaJsTools = "org.scala-js" %% "scalajs-tools" % scalaJsVersion
val mill = "com.lihaoyi" %% "mill-scalalib" % millVersion
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think so. It depends on how mill implements plugin classloading, but if the mill classloader is the parent of your plugin classloader, it should be Provided 👍

@Baccata
Copy link
Contributor Author

Baccata commented Jun 10, 2018

Do you have any more changes you want to include before I rebase?

Just pushed the "provided"

By the way, I'm curious, how long does it take to generate the bloop configuration file?

I nuked my mill cache, and running the Bloop/install task took 19 seconds. That included compilation of all my (non-trivial, 16 modules) build , and running all the upstream tasks. From then, if I remove the .bloop folder and regenerate, it seems to take ~3-5 seconds.

@jvican
Copy link
Contributor

jvican commented Jun 10, 2018

Does bloopInstall require the compilation of all your modules to generate bloop configuration files? It seems that should be improved upon. The sbt build, by default, does this too when you ask for the dependencyClasspath, but the plugin creates its own implementation of dependencyClasspath that doesn't require compilation.

Would you mind creating a ticket to keep track of that?

Copy link
Collaborator

@Duhemm Duhemm left a comment

Choose a reason for hiding this comment

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

Thanks a lot, I'm looking forward to trying that in Mill!

Does Mill support sources / resources generators? If it does, are they run when doing bloopInstall?

Also, could you please also add millBloop to the install alias and to allBloopReleases at the end of build.sbt?

@jvican
Copy link
Contributor

jvican commented Jun 10, 2018

(I'll be pushing to this branch in half an hour or so to rebase everything.)

@Baccata
Copy link
Contributor Author

Baccata commented Jun 10, 2018

Does bloopInstall require the compilation of all your modules to generate bloop configuration files

Nope, I made sure that's not the case. When I said "compilation of the build", I meant compilation of the build file itself and all the ammonite script it depends on, not the modules.

@Duhemm, Mill does provide a generatedSources task you use override with whatever behaviour. You can also write a resources generators very simply overriding the resources task to point to potential resource folders you might want to add. It's really super easy. Both tasks are run by the install task provided in this PR.

.resolveDeps(T.task { module.compileIvyDeps() ++ module.transitiveIvyDeps() })
.map(_.map(_.path).toSeq)

def transitiveClasspath(m: JavaModule): Task[Seq[Path]] = T.task {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jvican this is what I had to re-implement (rather than calling the mill one directly) to make sure the install task does not call upon the compile task

Copy link
Contributor

Choose a reason for hiding this comment

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

Great! I cannot get back to the rebase tonight but wil do tomorrow morning.

Baccata and others added 4 commits June 11, 2018 15:54
Adds a `bloop.integrations.mill.BloopModule` mill external module that
can be used to generate bloop config from mill.

See https://www.lihaoyi.com/mill/page/modules.html#external-modules

The suppot is limited to JVM project at the moment, and does not fill
the "resolution" configuration element.

Usage :

1. add ```import $ivy.``ch.epfl.scala::mill-bloop:$bloopVersion` ``` in
the build.sc of a project (it's probably possible to add it to a
`~/.mill/ammonite/predef.sc` or a similar file instead)

2. run `mill bloop.integrations.mill.BloopModule/install` from the
directory of the mill project.
@jvican jvican force-pushed the mill-integration branch from 4943ed5 to 7edd138 Compare June 11, 2018 20:37
@jvican jvican merged commit ab0c88f into scalacenter:master Jun 11, 2018
@jvican
Copy link
Contributor

jvican commented Jun 11, 2018

Done! Thanks again for the contribution @Baccata 👍

@Baccata Baccata deleted the mill-integration branch June 12, 2018 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Any change or proposal that is contributed by the Open Source Community. integrations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants