Upgrade to sbt 1.2.6 #1954
Upgrade to sbt 1.2.6 #1954
Conversation
|
Hmmm… That feels dangerous. Is YUI compressor failing to work in sbt 1.2.6? Main issue I see: say someone is using CSP with hashes. The hash for the lift.js file will probably change if we use a different compressor, which will cause the page to fail to load correctly. Feels niche-y, but worth mentioning. |
|
Yeah. I don’t think it’s unreasonable to need to update CSP hashes when upgrading. Those hashes will change with any change to the code. The problem is the yui compressor plugin isn’t maintained by anyone other than us and is nontrivial enough to have run afoul of the sbt API - unlike the lift build plugin. We’ll get a better ROI from a plugin maintained by the community if it works. I did some finagling with it unsuccessfully. But I was in an impatient mood so maybe a second look will yield something. |
|
Nah, reasoning seems fair, if there's something else to switch to I say let's do it. |
The previous presence of the CVMapping helper means we didn't need %% previously
Previously, we could reference scala.xml as just "xml". This is no longer valid, it seems.
|
@Shadowfiend I think this build will pass now. Had to do some munging that I suspect is related to changes in how the default import visibility is set up for |
project/Dependencies.scala
Outdated
| @@ -83,7 +83,9 @@ object Dependencies { | |||
| lazy val mockito_all = "org.mockito" % "mockito-all" % "1.9.0" % "test" | |||
| lazy val scalacheck = "org.specs2" %% "specs2-scalacheck" % "3.8.6" % "test" | |||
| lazy val specs2 = "org.specs2" %% "specs2-core" % "3.8.6" % "test" | |||
| lazy val specs2Main = "org.specs2" %% "specs2-core" % "3.8.6" | |||
There was a problem hiding this comment.
Why did we need this main-classpath variant?
There was a problem hiding this comment.
webkit provides some things that depend on specs2 (e.g. MockWeb)
build.sbt
Outdated
| (sources / aggregate) in (Compile, doc), | ||
| aggregatedSetting(dependencyClasspath in(Compile, doc)), | ||
| publishArtifact := false | ||
| )*/ |
There was a problem hiding this comment.
Is this something we can nuke now?
There was a problem hiding this comment.
Ah, yes, I had planned to replace this with the unidoc plugin but under either implementation I'm getting..... fun errrors:
[error] (lift-mapper / Compile / doc) java.net.URISyntaxException: Illegal character in path at index 45: http://www.scala-lang.org/api/SettingKey(This / This / This / version)/scala/xml/NodeSeq.html
build.sbt
Outdated
| specs2.copy(configurations = Some("provided")), | ||
| specs2Matchers.copy(configurations = Some("provided")), | ||
| specs2Main, | ||
| specs2MatchersMain, |
There was a problem hiding this comment.
The new one puts it on the classpath, while the old one marked it provided (~= not on the classpath via sbt, but on the classpath via external means?). Not fully following the difference in what we were doing vs what we're doing now.
There was a problem hiding this comment.
Ahhhhh right you are.
|
Looks like maybe
|
|
:shakefist: |
This is consistent with the previous behavior
| aggregatedSetting(dependencyClasspath in(Compile, doc)), | ||
| publishArtifact := false | ||
| )*/ | ||
| .enablePlugins(ScalaUnidocPlugin) |
There was a problem hiding this comment.
Just to check---is this still officially a workaround for scala/scala-dev#249 per the comment above it? Happy to make this edit.
There was a problem hiding this comment.
Oh. To be clear this comment about the Scala bug is for the block above, not the block below 😬
There was a problem hiding this comment.
These lines are about the compiled mega docs. It’s currently misbehaving but I think this PR is still a win.
|
Going to let this sit for a few but probably merging by EOD. |
|
|
This PR gets the Lift build running under SBT 1.2.6, aside from the yui compressor plugin. We'll want to replace that with something from sbt-web, I suspect.