Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

A typelevel sbt plugin for unifying the partial unification setting #69

Closed
kailuowang opened this issue Apr 6, 2017 · 13 comments
Closed

Comments

@kailuowang
Copy link
Contributor

There are several typelevel projects that need to manage user experience around SI-2712.
Currently, there is fiadliel/sbt-partial-unification which tries to provide an easy way to enable partial unification for Scala projects, i.e. set the scalacOption for 2.11.10+ and use the s2712 fix plugin by @milessabin for 2.10.8 and 2.11.8. However this plugin has some issues and the owner didn't respond to a PR that address them.
Shall we start a new sbt plugin under typelevel to provide the same thing? If so, shall we consider making it part of sbt-catalysts or a new repo?

related issue: typelevel/cats#1583

@milessabin
Copy link
Member

Surely this is just a handful of lines of goop in a build.sbt isn't it? Is a plugin really worth the effort?

@kailuowang
Copy link
Contributor Author

kailuowang commented Apr 6, 2017

It is just a handful lines of code. But since we are going to put this burden onto users, I still think there is a meaningful difference between copying a handful of code and one line of addSbtPlugin.
@non, since you originally suggested a plugin here, do you have any input on this?

@tpolecat
Copy link
Member

tpolecat commented Apr 6, 2017

It's non-obvious goop and I recall being enraged at some point, but yeah maybe a plugin isn't worth the effort since the doc for it would have to explain most of the implementation. A blog post on how to do it might be as effective.

@milessabin
Copy link
Member

Is this just for support for 2.10.x? If it is then we should reconsider typelevel/scala#122.

@kailuowang
Copy link
Contributor Author

kailuowang commented Apr 6, 2017

For the most part. But also this plugin helps projects that cross build.
Without a plugin, our projects have to include a documentation more or less like the following:

This library will be easier to use if you turn on partial unification for your scalac.
If you are on 2.10.6 please use this plugin in your plugins.sbt

addCompilerPlugin("com.milessabin" % "si2712fix-plugin_2.10.6" % "1.2.0")

If you are on 2.11.x please make sure you upgrade to 2.11.10+ and enable this scalac flag

scalacOptions += "-Ypartial-unification"

If you are on 2.12.x pelase enabled the flag as above.

if you are cross building you would need these lines of code goop

  def pluginOrFlag(scalaVersion: String, pluginModule: ModuleID): Either[ModuleID, String] = {
    val pluginDependency = Left(compilerPlugin(pluginModule cross CrossVersion.full))
    val flag = Right("-Ypartial-unification")
    val VersionNumber((major +: minor +: patch +: _), _, _) = scalaVersion

    (major, minor, patch) match {
      case (2, 10, p) if p >= 6 => pluginDependency
      case (2, 11, 8) => pluginDependency
      case (2, 11, p) if p >= 9 => flag
      case (2, 12, _) => flag
      case (2, 13, _) => flag
      case _ => sys.error(s"scala version $scalaVersion is not supported.")
    }
  }

libraryDependencies ++= pluginOrFlag(scalaVersion.value, partialUnificationModule.value).left.toSeq,
    scalacOptions ++= pluginOrFlag(scalaVersion.value, partialUnificationModule.value).right.toSeq

<<<<<<<<<<<<<<<<<<<<<<<<<<<

I think that's a lot for the users to consume.

@tpolecat
Copy link
Member

tpolecat commented Apr 6, 2017

Hm, yeah that's a lot of goop. I switch my vote to 👍

@adelbertc
Copy link

👍 from me

@milessabin
Copy link
Member

Very disappointing to see so much enthusiasm for this and so little for TLS for 2.10.x.

@kailuowang
Copy link
Contributor Author

@milessabin from my part, I need this for pushing through for cats 1.0.0-RC1, this sounds an easier and quicker solution than TLS 2.10.x. Not saying that we shouldn't pursue TLS 2.10.x

@tpolecat
Copy link
Member

tpolecat commented Apr 6, 2017

I guess I see this as a way for 2.10 projects to limp along a bit longer, as opposed to TLS 2.10 which seems like a bigger and more long-term commitment. I want my reality to be rid of 2.10 by the end of the year (if not earlier) so quick fixes don't bother me.

@fiadliel
Copy link

fiadliel commented Apr 7, 2017

That SBT project was somehow marked as "not watching", so didn't see the change.

I've merged those changes. I previously had seen limited interest in the plugin, but I can either keep it updated, or you can do whatever you want with a fork.

I'm also very much open to whatever seems reasonable with adding some level of support for TLS - at the very least, it should do the right thing if scalaOrganization is modified; it could in theory modify scalaOrganization itself, though it increases the feature profile, and is more likely to conflict with some other setting that people want in their builds.

@kailuowang
Copy link
Contributor Author

Thanks very much for merging and releasing @fiadliel. I'd rather continue using yours than forking. I am going to close this issue.

@Blaisorblade
Copy link

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants