Add coursier implementation of dependency resolution API #190
Conversation
|
@leonardehrenfried I can't review it in detail right now, but that looks really cool. About the ivy2 cache, if you meant Is the code from |
I was thinking about both. Right now the Ivy dep resolution includes them by default. I would also think that
Yes, it's intentionally a dumb c&p so sharing code is made easier. I would love a reusable module that I can depend on! |
|
@leonardehrenfried Thanks for taking on this task!
For Ivy it looks like this: def extraInputHash: Long = {
import AltLibraryManagementCodec._
Hasher.hash(owner.configuration) match {
case Success(keyHash) => keyHash.toLong
case Failure(_) => 0L
}
} |
|
If we are talking about bringing Coursier into to the librarymanagement code base (as opposed to living in the plugin space as alternative lm) one of my concerns about Coursier is its dependencies (see coursier/coursier#175, coursier/coursier#25, com-lihaoyi/Ammonite#644, etc). @alexarchambault Do you have rough timeline for 1.0 and 1.1? |
|
Coursier 1.0.0 has been released today, so I will update the PR accordingly. Handily, this includes a module that contains all the sbt <-> coursier conversion, so the diff will become smaller. @eed3si9n I understand your reservation about pulling in Scalaz. In this comment @alexarchambault says that he will be working on removing the scalaz dependency. Do you feel it's condition of this PR being merged or is it simply a strong preference? |
|
Since Alex has been publicly on board with reducing the dependencies, I'd much rather wait until it's done. |
|
@eed3si9n Yep, it's something I'd like to tackle quickly. |
|
Is there any progress on this awesome effort? |
|
There is some progress, but there are a couple of issues that block me from progressing further:
So in summary, there are a few things that need to fall into place for this to happen. If people want to help out they could work on removing Scalaz from coursier. *Excellent argument for sbt-release-early. |
|
For future reference, here is the compilation/linking error that I'm getting with coursier 1.0.0: As I said upthread I think it's a problem with |
|
After having taken a look here https://repo1.maven.org/maven2/io/get-coursier/sbt-shared_2.12_1.0/1.0.0/sbt-shared-1.0.0.pom I see that I'll raise ticket with coursier. |
leonardehrenfried
force-pushed
the
coursier
branch
2 times, most recently
from
ba9c9b7
to
3c0f51c
Compare
|
Wouldn't adding coursier as a git submodule be ok, rather than copy-pasting |
@eed3si9n I haven't actually run the scripted tests locally but will try. Should this issue be even possible on CI with a clean
@alexarchambault I would be open to this but I'm not sure what the policy for this is. Maybe Eugene or Dale can chime in. |
|
sbt's license is https://github.com/sbt/sbt/blob/1.x/LICENSE + https://www.lightbend.com/contribute/cla. This grants copyright to Lightbend, so we probably should get the written ok from @alexarchambault on this. My general preference is that we host librarymanagement code in sbt/librarymanagement repo. I'd also be happy to give repo rights to both of you assuming this PR goes through. |
I am not sure exactly. One thing I noticed is that |
|
@eed3si9n I tried putting Can you take a look at the diff of my branch of sbt and check if there is anything in there that could cause the failure: sbt/sbt@1.x...leonardehrenfried:coursier Here is the latest run of the scripted test of that branch: https://travis-ci.org/leonardehrenfried/sbt/jobs/360044405 |
|
Something is not hooked up correctly either in your lm or sbt. I added the following line to debug your classpath: diff --git a/scripted/sbt/src/main/scala/sbt/scriptedtest/ScriptedTests.scala b/scripted/sbt/src/main/scala/sbt/scriptedtest/ScriptedTests.scala
index dd9de5bc1..7672a65eb 100644
--- a/scripted/sbt/src/main/scala/sbt/scriptedtest/ScriptedTests.scala
+++ b/scripted/sbt/src/main/scala/sbt/scriptedtest/ScriptedTests.scala
@@ -322,6 +322,7 @@ final class ScriptedTests(
IO.copyDirectory(originalDir, tempTestDir)
val runTest = () => {
+ IO.write(tempTestDir / "project" / "p.sbt", """logLevel := Level.Debug""")and I am not seeing sbt JARs passed into Zinc: compare that with 1.x branch: |
Why should @alexarchambault grant any copyright to Lightbend? sbt is currently using a fork of ivy (sbt/ivy) and happily using ivy's Apache license. There should be no problem that |
There's a difference in the way software is licensed, binary is used, and how the source is controlled. If we use Ivy as an example:
For Coursier, similarly, @alexarchambault should rightfully retain the copyright etc of the Coursier itself. (At this point, I don't see the need to fork Coursier). But if any source code becomes part of this pull request, then we request BSD-3 with Lightbend CLA to be applied for the glue code. Since Leonard is not the original author of FromSbt.scala and ToSbt.scala, I thought we would eventually require written consent of the CLA. |
Sure, just wanted to emphasize that the glue code should not be checked in the coursier repository, and therefore there's no need to ask Alex to re-license coursier. My opinion as an outsider is that the big benefit of using coursier, aside from the fact that it's way better than ivy, is that the sbt team doesn't need to maintain it -- it only needs to take care of the code that integrates with it, which is already an advantage over maintaining sbt/ivy! Having coursier versioned and licensed as an independent tool makes things much easier 😄. |
|
We might be able to sidestep the entire issue by me trying a little harder to include
The same would then also be needed for |
|
Ok, I just tried it out and it doesn't work because that would resolve the following:
When in actual fact the correct URL is:
|
|
Maybe I need to add it as an sbt-plugin of lmCoursier and of sbt itself. But I'm not sure what is worse: Copying two classes or trying to get sbt to transitively include an sbt plugin. |
|
I figured out why the scripted tests break. The dependency @eed3si9n @alexarchambault I'm not sure what the correct behaviour is here. Should provided dependencies be transitively resolved? |
|
See https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html
|
|
https://github.com/sbt/sbt/blob/v1.1.2/main/src/main/scala/sbt/Defaults.scala#L2068-L2072 val sbtdeps =
(sbtDependency in pluginCrossBuild).value.withConfigurations(Some(Provided.name))
val pluginAdjust =
if (isPlugin) sbtdeps +: base
else baseThe code above adds sbt as a provided dependency to the metabuild's root project directly. So I am not sure why you'd need to resolve it transitively. |
|
Then I'm officially out of ideas why this doesn't work. If those jars are manually added to the classpath, how come that the compiler cannot find |
|
(I'm fine with signing the CLA if that's needed or to clarify things.) |
|
@leonardehrenfried would you be able to join our sbt meeting? Eugene is not available next week Wednesday, so perhaps Wednesday 13 June? We'd love to get a chance to chat with you (and @alexarchambault too if you'd like to join!) about this PR. Alternatively or in addition: are you going to be at Scala Days NYC? |
|
Hi, sorry for not following up on this PR. I'd gladly join a an sbt meeting either at the coming week or the week after that. Sadly, I was too snowed under at work to attend scala days in Berlin even though it happened quite close to where I live. :| |
|
So, to answer your question: no, unfortunately I won't be at Scala Days NYC. |
|
this can be closed now. |
|
Closing this in favor of #270 |
This implements a Coursier implementation of the dependency management API.
This exploratory implementation is somewhat beta as some functionality is missing. I am opening this pull request in order to gauge interest in the wider community in this approach and whether it's worth me spending time polishing this further.
With only this PR, sbt itself will not include a way of using this. A separate PR on sbt/sbt is needed for this.
General approach
I've tried to use as much as possible from the coursier plugin itself. In particular the classes
FromSbtandToSbtare straight up copies from their repository.Not implemented yet/problems
I am hoping that @dwijnand, @eed3si9n and @alexarchambault can provide feedback on how to implement the missing features.
Questions
UpdateReportModuleDescriptor.extraInputHash?As I said above I would love some feedback about whether this has a chance of being merged or if there are any blockers that need to be removed first.
References