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

Move compatibility code from coursier-shared #5

Closed
wants to merge 1 commit into from

Conversation

leonardehrenfried
Copy link
Collaborator

In coursier/coursier#712 (comment) @dwijnand has asked for this code to be moved to this repository.

I'm a little unsure about how the code should be organised but I tried to follow the style of the rest.

I'm happy to rework this if it's against the spirit of the plugin.

@dwijnand
Copy link
Owner

dwijnand commented Jan 4, 2018

Hey @leonardehrenfried I started reviewing this today. It needs some work, but it's a great starting point.

The design of sbt-compat is that of backporting sbt 1's public API on top of sbt 0.13's implementation. Therefore most things should be in scala-sbt-0.13/compat.scala, and (almost) never in scala-sbt-1.0/compat.scala. That's ok because I think most of the things added to 1.0 compat aren't actually needed, only needsIvyXmlLocal and needsIvyXml, which seem more coursier-specific rather than "sbt-compat" generic.

@alexarchambault What in Coursier depends on coursier-shared? More specifically if I add sbt-compat to Coursier as a dependency of coursier-shared, what should I invoke in Coursier's build to test everything still compiles, for sbt 0.13 and sbt 1? Thanks.

@leonardehrenfried I hope to make more progress here tomorrow.

@alexarchambault
Copy link

alexarchambault commented Jan 4, 2018

@dwijnand The sbt plugins of coursier depend on it (more exactly, sbt-coursier directly depends on it, and the other sbt plugins depend on sbt-coursier). So a sbt ++2.12.4 sbt-plugins:compile (for sbt 1.x) or sbt ++2.10.7 sbt-plugins:compile (for sbt 0.13.x) should do.

@leonardehrenfried
Copy link
Collaborator Author

@dwijnand So I'm assuming stuff like this needs to go: https://github.com/dwijnand/sbt-compat/pull/5/files#diff-fceb3b502244ebeb2e5f778a738e4cc7R8

I can definitely sort that out.

The stuff I added to 0.13 all lives in the librarymanagement package, i think. Is that correct?

@dwijnand
Copy link
Owner

dwijnand commented Jan 5, 2018

The stuff I added to 0.13 all lives in the librarymanagement package, i think. Is that correct?

I think so.

Another thing I'm hesitant is a few of the implicit conversions. First there's a few implicit conversions for implicit classes, which I would assume would cause them to be in conflict. Then there's straight implicit conversions (ala JavaConversions) like configVectorToList and configListToVector. I'd like to review the motivating use cases for them, but yesterday I got stuck on loading the coursier build (coursier/coursier#728).

@dwijnand
Copy link
Owner

dwijnand commented Jan 8, 2018

I tried to add sbt-compat to coursier as a source dependency:

   lazy val `sbt-shared` = project
-  .dependsOn(coreJvm, cache)
+  .dependsOn(coreJvm, cache, RootProject(file("/d/sbt-compat")))
   .disablePlugins(ScriptedPlugin)
   .settings(
     plugin,

But when running sbt-shared/compile I get:

[error] (sbt-shared/*:coursierResolutions) coursier.ResolutionException: Encountered 1 error(s) in dependency resolution:
[error]     com.dwijnand:sbt-compat;sbtVersion=0.13;scalaVersion=2.10:1.0.0+23-34f5ff24+20180108-1428:
[error]         not found:
[error]             /Users/dnw/.ivy2/local/com.dwijnand/sbt-compat/scala_2.10/sbt_0.13/1.0.0+23-34f5ff24+20180108-1428/ivys/ivy.xml
[error]             https://repo1.maven.org/maven2/com/dwijnand/sbt-compat_2.10_0.13/1.0.0+23-34f5ff24+20180108-1428/sbt-compat-1.0.0+23-34f5ff24+20180108-1428.pom
[error]             https://dl.bintray.com/scalaz/releases/com/dwijnand/sbt-compat_2.10_0.13/1.0.0+23-34f5ff24+20180108-1428/sbt-compat-1.0.0+23-34f5ff24+20180108-1428.pom
[error]             https://oss.sonatype.org/content/repositories/releases/com/dwijnand/sbt-compat_2.10_0.13/1.0.0+23-34f5ff24+20180108-1428/sbt-compat-1.0.0+23-34f5ff24+20180108-1428.pom
[error]             https://oss.sonatype.org/content/repositories/snapshots/com/dwijnand/sbt-compat_2.10_0.13/1.0.0+23-34f5ff24+20180108-1428/sbt-compat-1.0.0+23-34f5ff24+20180108-1428.pom
[error]             https://repo.typesafe.com/typesafe/ivy-releases/com.dwijnand/sbt-compat/scala_2.10/sbt_0.13/1.0.0+23-34f5ff24+20180108-1428/ivys/ivy.xml
[error]             https://repo.scala-sbt.org/scalasbt/sbt-plugin-releases/com.dwijnand/sbt-compat/scala_2.10/sbt_0.13/1.0.0+23-34f5ff24+20180108-1428/ivys/ivy.xml
[error] Total time: 3 s, completed 08-Jan-2018 14:29:52

@alexarchambault
Copy link

I think source dependencies don't work well when sbt-coursier is enabled…

@dwijnand
Copy link
Owner

dwijnand commented Jan 8, 2018

Disabled sbt-coursier, disabled sbt-shading, commented out shading setup, commented out shaded configuration, now I'm getting:

[error] ## Exception when compiling 3 sources to /d/coursier/sbt-shared/target/scala-2.12/sbt-1.0/classes
[error] null
[error] scala.reflect.internal.Symbols$Symbol.rawInfo(Symbols.scala:1603)
[error] scala.reflect.internal.Symbols$Symbol.typeParams(Symbols.scala:1759)
[error] scala.reflect.internal.Types$NoArgsTypeRef.typeParams(Types.scala:1922)
[error] scala.reflect.internal.Symbols$Symbol.typeParams(Symbols.scala:1759)
[error] scala.reflect.internal.Types$NoArgsTypeRef.typeParams(Types.scala:1922)
[error] scala.reflect.internal.Symbols$Symbol.typeParams(Symbols.scala:1759)
[...]
[error] java.lang.StackOverflowError
[error] 	at scala.reflect.internal.Symbols$Symbol.rawInfo(Symbols.scala:1603)
[error] 	at scala.reflect.internal.Symbols$Symbol.typeParams(Symbols.scala:1759)
[error] 	at scala.reflect.internal.Types$NoArgsTypeRef.typeParams(Types.scala:1922)
[error] 	at scala.reflect.internal.Symbols$Symbol.typeParams(Symbols.scala:1759)
[error] 	at scala.reflect.internal.Types$NoArgsTypeRef.typeParams(Types.scala:1922)
[error] 	at scala.reflect.internal.Symbols$Symbol.typeParams(Symbols.scala:1759)

@dwijnand
Copy link
Owner

dwijnand commented Jan 9, 2018

Superseded by #7. Thanks @leonardehrenfried for bootstrapping this effort!

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.

None yet

3 participants