Cross build for SBT 1.0.0-RC3

[liff]

Uses scalariform for 2.12 from com.github.machaval organization (see #55 and #36) so perhaps this is not the final form.

[godenji]

"org.scalariform" %% "scalariform" % "0.2.0"

Has been released, includes changes from the com.github.machaval fork, cross published for 2.10/11/12.

Please amend PR accordingly. Thanks

[godenji]

thanks, the reason for mentioning cross publishing is that library dependencies can now simply be:

libraryDependencies += "org.scalariform" %% "scalariform" % "0.2.0"

[liff]

Yeah, seems I failed to properly parse your last comment, sorry. Update on its way.

I was trying to look for 0.2.0 for 2.10 on Maven Central but I suppose its indexing hasn't caught up yet or something.

[godenji]

right, it's not yet up on maven central, but the dependency resolves (0.2.0 was published an hour or so ago)

[godenji]

@liff thanks for the update.

Scala 2.12.3 will soon be released (likely within days), I'd like to hold off on publishing a new sbt-scalariform release until that time (we're also waiting on a PR that brings embedded scalariform in Scala IDE up-to-date with latest version of scalariform).

More later.

[godenji]

@liff thanks for the contribution, will be part of the 1.8.0 release (probably in a couple of weeks when sbt 1.0 lands) .

[liff]

From my understanding of the announcement you don't necessarily have to wait for 1.0 final to release for it.

[godenji]

Sure, but latest changes need to be vetted before releasing 1.8.0 to make sure everything is working properly, which is not the case right now, seems to be broken.

Did you try out 1.0.0-RC3 + scala 2.12 against PR changes? Everything's fine for 0.13.16 + 2.10.6, but when doing a ^publishLocal from sbt-scalariform and creating a sample sbt project with:

build.properties > 1.0.0-RC3
project/plugins.sbt > addSbtPlugin("org.scalariform" % "sbt-scalariform" % "1.8.0-SNAPSHOT")
src/main/scala > Sample.scala

formatting fails with:

[error] jawn.IncompleteParseException: exhausted input
[error] at jawn.Parser.parse(Parser.scala:362)
[error] at jawn.SyncParser.parse(SyncParser.scala:24)
[error] at jawn.SupportParser.parseUnsafe(SupportParser.scala:12)
[error] at jawn.SupportParser.parseUnsafe$(SupportParser.scala:11)
[error] at sjsonnew.support.scalajson.unsafe.Parser$.parseUnsafe(Parser.scala:8)
[error] at sbt.util.CacheStore$.$anonfun$jvalueIsoString$2(CacheStore.scala:17)
[error] at sjsonnew.IsoString$$anon$1.from(IsoString.scala:27)
[error] at sbt.util.PlainInput.read(Input.scala:29)
[error] at sbt.util.FileBasedStore.$anonfun$read$1(CacheStore.scala:61)
[error] at sbt.io.Using.apply(Using.scala:23)
[error] at sbt.util.FileBasedStore.read(CacheStore.scala:61)
[error] at sbt.util.Changed.uptodate(Tracked.scala:225)
[error] at sbt.util.Changed.$anonfun$apply$1(Tracked.scala:214)
[error] at com.typesafe.sbt.Scalariform$.apply(Scalariform.scala:62)
...

[liff]

I didn't try actually. Assumed it'd just work. I'll check it out in the evening.

[godenji]

Assumed it'd just work

Famous last words ;-)

Think your code is fine, looks like real cause of the exception may be upstream in sjsonnew. There are at least two problems that I can see.

First, in sbt 1.0 the empty file case seems to not be properly encoded by sjsonnew for jawn to consume. See sbt 1.0 Changed[0] vs. sbt 0.13.16 Changed[0]. In the latter the uptodate method catches IO exception whereas the former optimistically assumes that the (potentially empty) file can be read in as the target type.

Second, to workaround the first problem, even when stuffing dummy data (like "alignParameters=false") into target/sbt-scalariform/streams/compile/scalariformFormat/\$global/streams/scalariform-preferences, the data is for some reason not cached -- compile and scalariformFormat format sources over and over despite the preferences not having been changed.

Please try to sort this out, will likely involve modifying your code to workaround upstream issues. If you issue a new PR, please do so against my fork in the 1.8.0 branch, have some new functionality built on top of current master that will go into 1.8.0 release. That or fixup your fork and I'll clone it to test out changes prior to your issuing a PR to master.

p.s. you can run ^scripted from my fork on 1.8.0; if the tests pass then we're good.

Thanks

[liff]

Opened a PR about the caching issue. I think the exception handling thing might be just a regression in SBT 1.0. I'll ask around in gitter tomorrow or open an issue.

[godenji]

We're getting there, your PR to my fork solves the caching issue. Now we need to sort out the empty cache file scenario (which is always the case on fresh project, or after an sbt clean).

SBT 1.0 final is scheduled to be released in a couple of weeks. Hopefully they can be more defensive here. That, or we try to workaround the issue by checking the empty file case on our end.

[liff]

It looks like this is going to be more complicated than even fixing the SBT regression. Changed is going to be deprecated but apparently without fixing the exception behavior in 1.0.0.

The suggested replacement is to use Tracked.inputChanged but its innards have also changed beween 0.13 and 1.0. In 0.13 the Equiv provided for the cached type is used to find out if the object changed. In 1.0 the logic has been changed to use hash code. Due to the somewhat curious way Equiv is defined for IFormattingPreferences (i.e. empty Map and defaults are equivalent), using inputChanged doesn't really work in 1.0 either.

I suppose one option could be to always write out the preferences in full so that the Equiv trick is not needed. What do you think?

[godenji]

Agreed, 0.13 to 1.0 is full of pitfalls (dealing with similar problems trying to port sbt-eclipse to 1.0).

I have tried with Tracked.inputChanged (see below), always returns true regardless of whether or not formatting preferences have changed. I am not sure what is happening here, ideally we could use inputChanged instead of the following workaround:

Use our own internal definition of Changed with try/catch clause added (works, have in place on local branch), and then remove it when 0.13 support is dropped. At that point everything can be moved back under src/main/scala and a 1.0-specific implementation can be used.

Tracked.inputChanged attempt

private[sbt] class Changed[O: Equiv: JsonFormat](store: File) {

  def apply(ifChanged: O => Boolean, ifUnchanged: O => Boolean): O => Boolean = value => {
      val f = Tracked.inputChanged[O, Boolean](store) { case (changed, _) =>
        changed
      }   
      f(value)
    }   
  }

  def changed[O: Equiv: JsonFormat](store: File) = new Changed[O](CacheStore(store))

[liff]

Yeah, Tracked.inputChanged doesn't work because (from what I can tell, check the links in my post above) it doesn't really use the Equiv instance in 1.0.

Using a custom Changed implementation is something I tried as well earlier but deemed a bit too hacky. However, having gone through all this, it's starting to look like the easiest option :)

[godenji]

Life is too short, let's just get this PR fixed up and deal with perfection later (i.e. when sbt 0.13 can be dropped). Unless you have time that is, feel free to try to get inputChanged working alongside 0.13 if you like ;-)

I pushed patched Changed implementation to my fork on 1.8.0 branch. When you get the chance can you pull my latest changes and confirm that ^scripted tests pass for both 0.13.16 and 1.0.0-RC3?

[godenji]

@liff Tests pass, think we're good to go for 1.8.0

[liff]

Sorry, I was busy with other stuff the past couple of days.

Perhaps the Travis build should still be updated? The matrix from sbt-pgp should be a good template.