autoformat on compile

[fommil]

this was possible in scalariform but scalafmt doesn't do it 😿

[fommil]

btw

(compile in Compile) := {
  runCommandAndRemaining("scalafmt")(state.value)
  (compile in Compile).value
}

is far too inefficient. I was thinking something that would reformat only files that are newer than their binary output (or if there is no binary output)

[olafurpg]

It's possible with sbt-scalafmt but not officially supported because I don't want to maintain it myself (I don't even use the sbt plugin). See one of the last bullets in http://scalameta.org/scalafmt/#sbt for a DIY solution. I'm happy to give collaborator rights to a person who sends a PR adding that gist to the main repo.

[fommil]

sweet! So basically it just needs somebody to add this to the sbt plugin

https://gist.github.com/olafurpg/e045ef9d8a4273bae3e2ccf610636d66

@hseeberger is this gist still good? Should we maybe just add it to the plugin?

[hseeberger]

@fommil yeah, this gist is still good and "heavily" used in production. It's also created by sbt-fresh. I'd say we add it once scalafmt is 1.0.

[pjrt]

For reference, this is why that gist hasn't been integrated: #597

If someone wants to pick up the SBT plugin, that would be great.

[fommil]

and don't forget the proposed concurrency fix (although I'm not sure how that fixes any problems)

[olafurpg]

Now that the sbt plugin no longer needs weird hacks to work on sbt 0.13, I invite anyone to contribute a reformatOnCompile setting if they're interested.

[sjrd]

Is like to warn anyone who wants to work on this that this feature fundamentally is at odds with sbt's model. Indeed, in order to do this, reformat most be a task that is an integral part of the task dependency graph, and it mutates files (source files to boot). Since the task dependency graph can be run in parallel, this will inevitably cause races on these files. compile is not necessarily the only task in your graph that reads source files.

I strongly advise against this feature, because it cannot be made correct. It will stay broken forever.

[hseeberger]

@sjrd, are you aware of the way sbt-scalariform supports format on compile? Do you think that approach is broken, too? If so, could you please give a concrete example?

[fommil]

Formatting will only affect files that have been changed, and before compile, so I don't see an issue. Scalariform does not cause problems.

[sjrd]

@hseeberger If it's this: https://github.com/sbt/sbt-scalariform/blob/master/src/main/scala/com/typesafe/sbt/SbtScalariform.scala then yes, it's broken too.

Let's say I have a custom task (potentially coming from a plugin), that reads the sources in Compile, reads their content, and produces something from that:

customTask in Compile := {
  val inputs = (sources in Compile).value
  val processed = readAndProcessInputs(inputs)
  val someOutput = file("...")
  IO.write(someOutput, processed)
  someOutput
}

and then I have another task that somehow depends on compile in Compile and customTask in Compile:

specialRun in Compile := {
  val cp = (fullClasspath in Compile).value
  val processedFile = (customTask in Compile).value
  run(cp, processedFile)
}

Then I have the following (simplified) dependency graph:

       sources
        /   \
       /     \
      /       \
     /         \
customTask    scalariformFormat
     |            |
     |         compile
     |            |
     |        fullClasspath
      \          /
       \        /
        \      /
      specialRun

As you can see, in that graph, customTask and scalariformFormat are allowed to (and likely will) run in parallel. In that case, customTask will try to read files that are simultaneously being written to by scalariformFormat.
==> Race

[fommil]

surely this is a bug report, or usecase to consider, for customTask. In sbt, mutation is introduced by any task and everybody just has to live with it.

[sjrd]

No you're looking at it the wrong way. In sbt, a task that mutates something is the bug. Commands are there when things need to be mutated.

[fommil]

lol, ok. If you say so. Every single Task I've ever written mutates things.

[sjrd]

You can produce new files of course (not written to by any other task), but you can never modify anything already produced by another task, nor input files (e.g., things that are checked in the repo).

[sjrd]

Also that's documented: http://www.scala-sbt.org/0.13/docs/Best-Practices.html#Don%E2%80%99t+%E2%80%9Cmutate%E2%80%9D+files
I'm not inventing anything just to please my own view of the world.

[fommil]

well I'm happy using formatters in this way. I'm used to working around incompatible tasks so if I ever need a customTask that depends on sources, I'll know to workaround this thanks.

[hseeberger]

Thanks a lot, @sjrd! Very interesting. While you are certainly right, I'm still in favor of using automated formatting on compile, because I have been using automated formatting via Scalariform and scalafmt for many years without any issues – most probably because there were no conflicting tasks in my builds.

[fommil]

this stopped working for me when upgrading to the latest, with a handrolled sbt plugin. cakesolutions/sbt-cake#2

https://github.com/cakesolutions/sbt-cake/tree/master/src/main/scala/org/scalafmt/sbt

Has anybody else seen this and been able to fix? There is a scripted test for this in the sbt-cake repo (currently commented out)

[olafurpg]

Reformat on compile is supported thanks to #1085