SBT plugin is looking for a new maintainer #597

Closed
olafurpg opened this Issue Nov 19, 2016 · 11 comments

Comments

Projects
None yet
3 participants
@olafurpg
Member

olafurpg commented Nov 19, 2016

I have decided to remove the SBT plugin. Why? I don't personally use the SBT plugin, the plugin has a few issues and maintaining the plugin in my free time has turned out to be an unrewarding experience. I have several times asked for help to maintain the plugin, but unfortunately with no luck so far.

It appears many people (and organizations) use the sbt plugin. I'm opening this issue to invite anyone who is interested to lend an extra hand to help bring the SBT plugin back. I would love to welcome you as a collaborator to the project, give you merge rights, and help bring you up to speed on making the SBT plugin a success.

To show your interest in maintaining the SBT plugin, you can do any of the following

  • send a PR reverting 308f393 and 983b043. This is probably the easiest option, but comes with the baggage of issues such as #485, which is caused by a regression in sbt 0.13.12. The implementation is mostly borrowed from the scalariform sbt plugin but is slightly more complicated because scalafmt is 2.11 only and sbt plugins must compile against 2.10.
  • send a PR implementing a new SBT plugin from scratch that uses the bootstrap module. The bootstrap module can potentially fix issues such as #485 by using the coursier api as a replacement for SBT's update mechanism (the root cause of #485). Don't hesitate to ask for more details.
  • send a PR creating a new SBT plugin that leverages the "synthetic projects" feature in sbt 0.13.13 to create an isolated classpath to invoke the scalafmt CLI
  • send a PR implementing a new SBT plugin with whatever approach you like. However, please note that sbt plugins must compile against 2.10 and scalafmt is 2.11 only because of its dependency on scala.meta. Backporting scala.meta to 2.10 has so far been unsuccessful, scalameta/scalameta#295

If you opt to implement a new SBT plugin, please include scripted tests like the previous plugin had.

Migrating away from the SBT plugin

Use the CLI: https://olafurpg.github.io/scalafmt/#CLI

To enforce scalafmt in a project like was possible with the sbt plugin, my personal recommendation is to

  • put a tiny scalafmt bootstrap script into your repo as explained here,
  • configure the project setting in .scalafmt.conf as explained here,
  • run ./scalafmt instead of sbt scalafmt and
  • run ./scalafmt --test instead of sbt scalafmtTest.
  • In CI, you can cache the downloaded jars in $HOME/.coursier/bootstrap.
  • It takes a few seconds to create a bootstrap script for a new scalafmt version, as long as you have coursier installed.

For your convenience, here is a bootstrap script for scalafmt 0.4.10: https://github.com/olafurpg/scalafmt/files/601602/scalafmt.zip

@RomanIakovlev

This comment has been minimized.

Show comment
Hide comment
@RomanIakovlev

RomanIakovlev Nov 20, 2016

Contributor

I wonder if it's a good idea to combine coursier-style fetching and launching the scalafmt artifact with sbt plugin. I mean, one can write a sbt plugin that uses the coursier to fetch and run scalafmt in the context of particular sbt project. Scalafmt will be launched as a separate process, so all issues with classpaths will go away, but it will still enjoy the sbt integration. I could give it a try if you think it makes sense.

Contributor

RomanIakovlev commented Nov 20, 2016

I wonder if it's a good idea to combine coursier-style fetching and launching the scalafmt artifact with sbt plugin. I mean, one can write a sbt plugin that uses the coursier to fetch and run scalafmt in the context of particular sbt project. Scalafmt will be launched as a separate process, so all issues with classpaths will go away, but it will still enjoy the sbt integration. I could give it a try if you think it makes sense.

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Nov 20, 2016

Member

@RomanIakovlev I think that approach could work. The bootstrap module already implements most of the coursier logic to classload a specific version of scalafmt and inject the correct in/out/err streams. See the example test here: https://github.com/olafurpg/scalafmt/blob/master/bootstrap/src/test/scala/org/scalafmt/bootstrap/BootstrapTest.scala

The bootstrap module should be able to cross-compile to 2.10 and 2.11 so you should be able to call it from an sbt plugin. Please give it a try if you like :)

Member

olafurpg commented Nov 20, 2016

@RomanIakovlev I think that approach could work. The bootstrap module already implements most of the coursier logic to classload a specific version of scalafmt and inject the correct in/out/err streams. See the example test here: https://github.com/olafurpg/scalafmt/blob/master/bootstrap/src/test/scala/org/scalafmt/bootstrap/BootstrapTest.scala

The bootstrap module should be able to cross-compile to 2.10 and 2.11 so you should be able to call it from an sbt plugin. Please give it a try if you like :)

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Nov 20, 2016

Member

You are of course also free to roll your own implementation of using the coursier api to invoke scalafmt, whatever you prefer!

Member

olafurpg commented Nov 20, 2016

You are of course also free to roll your own implementation of using the coursier api to invoke scalafmt, whatever you prefer!

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Nov 20, 2016

Member

@RomanIakovlev You might also be interested in the approach explained here: sbt/sbt#2786 (comment)

Member

olafurpg commented Nov 20, 2016

@RomanIakovlev You might also be interested in the approach explained here: sbt/sbt#2786 (comment)

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Nov 20, 2016

Member

With that approach, the plugin could be implemented almost as easily as

import sbt._, Keys._

object ScalafmtPlugin extends AutoPlugin {
  override def extraProjects: Seq[Project] =
    List("scalafmt") map generateProject

  def generateProject(id: String): Project =
    Project(id, file(id)).
      settings(
        name := id,
        scalaVersion := "2.11.8",
        libraryDependencies += "com.geirsson" % "scalafmt-cli_2.11" % "0.4.10"
      )
}
Member

olafurpg commented Nov 20, 2016

With that approach, the plugin could be implemented almost as easily as

import sbt._, Keys._

object ScalafmtPlugin extends AutoPlugin {
  override def extraProjects: Seq[Project] =
    List("scalafmt") map generateProject

  def generateProject(id: String): Project =
    Project(id, file(id)).
      settings(
        name := id,
        scalaVersion := "2.11.8",
        libraryDependencies += "com.geirsson" % "scalafmt-cli_2.11" % "0.4.10"
      )
}
@RomanIakovlev

This comment has been minimized.

Show comment
Hide comment
@RomanIakovlev

RomanIakovlev Nov 20, 2016

Contributor

I'm probably missing something obvious, but I haven't worked with synthetic projects in sbt 0.13.13 yet, so I don't see how they'll help in this situation. I think I'll try the coursier-based approach first, but will look at synthetic projects after I'll have a proof of concept running.

Contributor

RomanIakovlev commented Nov 20, 2016

I'm probably missing something obvious, but I haven't worked with synthetic projects in sbt 0.13.13 yet, so I don't see how they'll help in this situation. I think I'll try the coursier-based approach first, but will look at synthetic projects after I'll have a proof of concept running.

@clhodapp

This comment has been minimized.

Show comment
Hide comment
@clhodapp

clhodapp Nov 26, 2016

If anyone wants to riff on my hack at https://github.com/twitter/bijection/blob/469154b01dd852bd9e3b34dd8993b032c36c0c75/project/ScalaFmt.scala, feel free. The output is still a little janky, but it seems to work. I went with the generated project idea.

clhodapp commented Nov 26, 2016

If anyone wants to riff on my hack at https://github.com/twitter/bijection/blob/469154b01dd852bd9e3b34dd8993b032c36c0c75/project/ScalaFmt.scala, feel free. The output is still a little janky, but it seems to work. I went with the generated project idea.

@clhodapp

This comment has been minimized.

Show comment
Hide comment
@clhodapp

clhodapp Nov 26, 2016

To explain what's going on more fully: I'm setting up a synthetic stub project which just pulls down the scalafmt CLI. I've set it up so that the run task for that project invokes the main method for the CLI and then set up a task on each project that will invoke run on the stub project, passing the target project's scala sources as arguments.

To explain what's going on more fully: I'm setting up a synthetic stub project which just pulls down the scalafmt CLI. I've set it up so that the run task for that project invokes the main method for the CLI and then set up a task on each project that will invoke run on the stub project, passing the target project's scala sources as arguments.

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Nov 27, 2016

Member

@clhodapp Cool! I'm glad to hear that approach seems to work, thanks for sharing. Do you often run scalafmt from a subproject instead of all projects in the build?

If not, you might get away with some of the unmanagedSources complexity by not passing in the --files flag and using instead the project.git = true setting in .scalafmt.conf. For more details, see https://olafurpg.github.io/scalafmt/#project

Member

olafurpg commented Nov 27, 2016

@clhodapp Cool! I'm glad to hear that approach seems to work, thanks for sharing. Do you often run scalafmt from a subproject instead of all projects in the build?

If not, you might get away with some of the unmanagedSources complexity by not passing in the --files flag and using instead the project.git = true setting in .scalafmt.conf. For more details, see https://olafurpg.github.io/scalafmt/#project

olafurpg added a commit that referenced this issue Dec 16, 2016

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Dec 16, 2016

Member

I decided to add an sbt plugin using the synthetic project idea. Using this approach, the plugin is just a tiny wrapper around the command line interface and should therefore have a lower maintenance cost. For detailed comparison: #610

Member

olafurpg commented Dec 16, 2016

I decided to add an sbt plugin using the synthetic project idea. Using this approach, the plugin is just a tiny wrapper around the command line interface and should therefore have a lower maintenance cost. For detailed comparison: #610

olafurpg added a commit that referenced this issue Dec 17, 2016

Revive the sbt plugin (#610)
* Revert "Remove SBT plugin, see #597. (#598)"

This reverts commit 308f393.

* Revert "Don't run scripted tests on travis"

This reverts commit 983b043.

* Implement sbt plugin as a thin wrapper around CLI.

* Use sbt-buildinfo so that I can use sbt-scalafmt

* Polish sbt pluging and documentation.

* Fix git diff bug.

* Re-enable scoverage plugin to stop flaky tests.

I have no idea why this seems to fix the flaky tests.
@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Dec 23, 2016

Member

0.5 was released with a new sbt plugin merged in #610.

Member

olafurpg commented Dec 23, 2016

0.5 was released with a new sbt plugin merged in #610.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment