Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add asDottyDep to sbt plugin #9974

Closed

Conversation

bishabosha
Copy link
Member

@bishabosha bishabosha commented Oct 9, 2020

This adds another convenience method to the plugin, now that we have the tasty reader in Scala 2. Which lets you do e.g.

ThisBuild / scalaVersion := "2.13.4-bin-8891679"

Global / resolvers += "scala-integration".at(
   "https://scala-ci.typesafe.com/artifactory/scala-integration/")

lazy val shared = project
  .settings(scalaVersion := "0.27.0-RC1")

lazy val app = project
  .dependsOn(shared)
  .settings(
    libraryDependencies += ("com.lihaoyi" %% "fansi" % "0.2.9").asDottyDep("0.27.0-RC1")
  )

@bishabosha bishabosha requested a review from smarter October 9, 2020 14:12
@bishabosha
Copy link
Member Author

bishabosha commented Oct 9, 2020

will need to add scripted tests, and rename it for the scala3 change

@bishabosha bishabosha requested a review from julienrf October 9, 2020 14:15
Copy link
Contributor

@julienrf julienrf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think withDottyCompat could handle both cases: when the Scala version is 2.13 it changes the binary version of the dependency to 0.27, and when the Scala version is 0.27 it changes the binary version of the dependency to 2.13?

Otherwise, I’m not a big fan of truncated names, I prefer asDottyDependency over asDottyDep, even though it’s longer.

Does it work in combination with Scala.js as well?

* libraryDependencies ~= (_.map(_.withDottyCompat(scalaVersion.value)))
* }}}
*/
def asDottyDep(scalaVersion: String): ModuleID = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we have a simpler implementation by using the cross operation? https://www.scala-sbt.org/1.x/docs/Cross-Build.html#More+about+using+cross-built+libraries

@smarter
Copy link
Member

smarter commented Oct 14, 2020

Do you think withDottyCompat could handle both cases: when the Scala version is 2.13 it changes the binary version of the dependency to 0.27, and when the Scala version is 0.27 it changes the binary version of the dependency to 2.13?

The current behavior of withDottyCompat was designed so that you could:

  • take an existing Scala 2 project
  • add withDottyCompat to all its Scala 2 dependencies without breaking anything
  • then change its crossScalaVersions to include Dotty and compiling with Dotty would just work

What you're proposing would break the crossScalaVersions usecase: suddenly adding withDottyCompat would mean that when the project is compiled with Scala 2, it would use a Scala 3 dependency and vice-versa which would be very confusing.

So basically there's various usecases we need to keep in mind:

  • a scala 3 only project that depends on a scala 2 only project
  • a scala 2 only project that depends on a scala 3 only project
  • a cross-compiling project that depends on a cross-compiling project (this "just works" without doing anything)
  • a cross-compiling project that depends on a scala 2 only project
  • a cross-compiling project that depends on a scala 3 only project
  • ...and then there's scala-js/native which add another layer of fun

I think ideally we would only have one method to deal with all of those, with one or more parameters, I don't really know what that would look like but here's a tentative list of requirement:

  • We shouldn't have "dotty" in the name of that method since we won't use this name for scala 3 externally.
  • withDottyCompat is an extension method on ModuleID but we might want to give up on that and use a regular method that takes the ModuleID as parameter as it might be less confusing
  • Whatever we choose, it needs to be something that sbt upstream is willing to support since sbt-dotty should be going away ASAP, so this needs to be discussed with the sbt people.

Also I'd like to hear @sjrd opinion on this :).

@eed3si9n
Copy link
Member

For sbt, one of my criteria is that the build is repeatable regardless of the state of Maven Central.
This means that if a particular build works, the same commit id should resolve the same way regardless of if fansi adds Scala 2.13 or Scala 3.x artifacts in a later date.

Expanding the definition of cross might be a good direction to explore. For example, if we could add something like .cross(use3For2_13) as @julienrf suggested, we can reuse %% to mean use Dotty only for 2.13 cross building, and that should be a repeatable build.

@bishabosha
Copy link
Member Author

so should this be closed in favour of the other solutions?

@smarter
Copy link
Member

smarter commented Nov 2, 2020

Well we still need a place for discussion I guess, maybe an issue on the sbt repo.

@bishabosha
Copy link
Member Author

Well we still need a place for discussion I guess, maybe an issue on the sbt repo.

we should probably keep it here because it seems sbt feature requests should be in their discuss forum

@eed3si9n
Copy link
Member

eed3si9n commented Nov 3, 2020

we should probably keep it here because it seems sbt feature requests should be in their discuss forum

Feel free to open an issue on sbt/sbt for this since it's a task that we're planning to deliver. I normally discourage ppl from opening an issue for blue sky ideas that no one works on.

@bishabosha
Copy link
Member Author

close for sbt/sbt#6080

@bishabosha bishabosha closed this Nov 11, 2020
@bishabosha bishabosha deleted the plugin/add-asScala3Dependency branch September 10, 2021 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants