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

adding Coursier dependency resolution #4430

Open
wants to merge 4 commits into
base: develop
from

Conversation

Projects
None yet
5 participants
@andreaTP
Copy link
Contributor

andreaTP commented Oct 19, 2018

[disclaimer]
before merging make sure we default back to Ivy.

Here is the last bit on integrating the Coursier LM implementation to sbt/sbt.
This PR will be used to trigger the community build as well, possibly before merging.

cc. @eed3si9n @alexarchambault

@eed3si9n eed3si9n added ready review and removed ready labels Oct 19, 2018

@andreaTP

This comment has been minimized.

Copy link
Contributor

andreaTP commented Oct 25, 2018

despite the failre we obtained:

[info] Project sbt210-io-------------------: SUCCESS (project rebuilt ok)
[info] Project sbt210-util-----------------: SUCCESS (project rebuilt ok)
[info] Project sbt210-zinc-----------------: SUCCESS (project rebuilt ok)
[info] Project sbt211-io-------------------: SUCCESS (project rebuilt ok)
[info] Project sbt211-librarymanagement----: SUCCESS (project rebuilt ok)
[info] Project sbt211-util-----------------: SUCCESS (project rebuilt ok)
[info] Project sbt211-zinc-----------------: SUCCESS (project rebuilt ok)
[info] Project sbt212----------------------: SUCCESS (project rebuilt ok)
[info] Project sbt212-io-------------------: SUCCESS (project rebuilt ok)
[info] Project sbt212-librarymanagement----: SUCCESS (project rebuilt ok)
[info] Project sbt212-util-----------------: SUCCESS (project rebuilt ok)
[info] Project sbt212-util-interface-------: SUCCESS (project rebuilt ok)
[info] Project sbt212-zinc-----------------: SUCCESS (project rebuilt ok)

https://jenkins.scala-sbt.org:8592/job/sbt-validator/1133/console

Should be good enough to proceed, isn't is? 😄

@typesafe-tools

This comment has been minimized.

Copy link

typesafe-tools commented Oct 25, 2018

The validator has checked the following projects, tested using dbuild, projects built on top of each other.

Project Reference Commit
sbt pull/4430/head 8a8e33b
zinc develop sbt/zinc@c8e1f53
io develop sbt/io@e6529c0
librarymanagement pull/270/head sbt/librarymanagement@abc6d69
util develop sbt/util@cd67959

The result is: SUCCESS

The binaries are at https://dl.bintray.com/sbt/maven-snapshots
The new sbt version is: 1.3.0-bin-20181025T102644
(restart)

@typesafe-tools

This comment has been minimized.

Copy link

typesafe-tools commented Oct 25, 2018

A validation involving this pull request is in progress...

@leonardehrenfried

This comment has been minimized.

Copy link

leonardehrenfried commented Oct 25, 2018

This is a fantastic step! Thanks @andreaTP!

@typesafe-tools

This comment has been minimized.

Copy link

typesafe-tools commented Oct 25, 2018

The validator has checked the following projects, tested using dbuild, projects built on top of each other.

Project Reference Commit
sbt pull/4430/head 8a8e33b
zinc develop sbt/zinc@c8e1f53
io develop sbt/io@e6529c0
librarymanagement pull/270/head sbt/librarymanagement@789a4cc
util develop sbt/util@cd67959

The result is: SUCCESS

The binaries are at https://dl.bintray.com/sbt/maven-snapshots
The new sbt version is: 1.3.0-bin-20181025T151651
(restart)

@typesafe-tools

This comment has been minimized.

Copy link

typesafe-tools commented Oct 26, 2018

A validation involving this pull request is in progress...

@typesafe-tools

This comment has been minimized.

Copy link

typesafe-tools commented Oct 26, 2018

The validator has checked the following projects, tested using dbuild, projects built on top of each other.

Project Reference Commit
sbt pull/4430/head 1ad89a4
zinc develop sbt/zinc@c8e1f53
io develop sbt/io@e6529c0
librarymanagement pull/270/head sbt/librarymanagement@abc6d69
util develop sbt/util@cd67959

The result is: SUCCESS

The binaries are at https://dl.bintray.com/sbt/maven-snapshots
The new sbt version is: 1.3.0-bin-20181026T084504
(restart)

@andreaTP

This comment has been minimized.

Copy link
Contributor

andreaTP commented Oct 26, 2018

to go ahead I will need a release of LM ( M1 or something) comprehending the coursier PR.

@eed3si9n

This comment has been minimized.

Copy link
Member

eed3si9n commented Oct 29, 2018

See https://travis-ci.org/eed3si9n/sbt/builds/448039304

Out of 78 dependency-management tests, looks like only 30% of the tests are passing - https://travis-ci.org/eed3si9n/sbt

  • dependency-management/artifact
  • dependency-management/cache-classifiers
  • dependency-management/cache-local
  • dependency-management/cache-resolver
  • dependency-management/cache-update
  • dependency-management/cached-resolution-circular
  • dependency-management/cached-resolution-classifier
  • dependency-management/cached-resolution-configurations
  • dependency-management/cached-resolution-conflicts
  • dependency-management/cached-resolution-exclude
  • dependency-management/cached-resolution-force
  • dependency-management/cached-resolution-interproj
  • dependency-management/cached-resolution-overrides
  • dependency-management/chainresolver
  • dependency-management/circular-dependency
  • dependency-management/classifier
  • dependency-management/compiler-bridge-binary
  • dependency-management/configurations
  • dependency-management/configurations-to-retrieve
  • dependency-management/conflict
  • dependency-management/conflict-manager
  • dependency-management/conflict-manager-with-org
  • dependency-management/cross-ivy-maven
  • dependency-management/default-artifact
  • dependency-management/default-resolvers
  • dependency-management/deliver-artifacts
  • dependency-management/dynamic-revision
  • dependency-management/exclude-bundle
  • dependency-management/exclude-dependencies
  • dependency-management/exclude-scala
  • dependency-management/exclude-transitive
  • dependency-management/ext-pom-classifier
  • dependency-management/extra
  • dependency-management/force
  • dependency-management/force-update-period
  • dependency-management/gh-1484-npe
  • dependency-management/info
  • dependency-management/inline-dependencies-a
  • dependency-management/invalidate-internal
  • dependency-management/ivy-settings-a
  • dependency-management/ivy-settings-b
  • dependency-management/ivy-settings-c
  • dependency-management/ivy-settings-multi-a
  • dependency-management/ivy-settings-multi-b
  • dependency-management/latest-local-plugin
  • dependency-management/make-pom
  • dependency-management/make-pom-type
  • dependency-management/metadata-only-resolver
  • dependency-management/module-confs
  • dependency-management/module-name
  • dependency-management/multiple-classifiers
  • dependency-management/no-file-fails-publish
  • dependency-management/override
  • dependency-management/override2
  • dependency-management/parent-publish
  • dependency-management/pom-advanced
  • dependency-management/pom-classpaths
  • dependency-management/pom-packaging
  • dependency-management/pom-parent-pom
  • dependency-management/pom-scope
  • dependency-management/pom-type
  • dependency-management/provided
  • dependency-management/provided-multi
  • dependency-management/publish-to-maven-local-file
  • dependency-management/retrieve-managed-sync
  • dependency-management/scala-home
  • dependency-management/scala-organization
  • dependency-management/scala-organization-version-check
  • dependency-management/scala-tests-only
  • dependency-management/scala-version-check-exempt
  • dependency-management/snapshot-resolution
  • dependency-management/sources
  • dependency-management/t468
  • dependency-management/test-artifact
  • dependency-management/transitive-excludes
  • dependency-management/transitive-version-range
  • dependency-management/update-sbt-classifiers
  • dependency-management/url

Also note that bunch of other scripted tests are failing

[info] [error] (b / publish) java.lang.IllegalStateException: Ivy file not found in cache for org.example#b_2.12;0.1.0-SNAPSHOT!

@andreaTP andreaTP force-pushed the andreaTP:addingCoursier branch from 149cec4 to f4add1b Oct 30, 2018

@andreaTP

This comment has been minimized.

Copy link
Contributor

andreaTP commented Oct 30, 2018

@eed3si9n talking about:

[info] [error] (b / publish) java.lang.IllegalStateException: Ivy file not found in cache for org.example#b_2.12;0.1.0-SNAPSHOT!

this error appear because we are still using Ivy to publish, looks like Ivy expect to have fetched by itself dependencies, i.e. you will overcome it by re-resolving packages with Ivy once.

@typesafe-tools

This comment has been minimized.

Copy link

typesafe-tools commented Oct 30, 2018

A validation involving this pull request is in progress...

@typesafe-tools

This comment has been minimized.

Copy link

typesafe-tools commented Oct 30, 2018

The validator has checked the following projects, tested using dbuild, projects built on top of each other.

Project Reference Commit
sbt pull/4430/head f4add1b
zinc develop sbt/zinc@28c3522
io develop sbt/io@e6529c0
librarymanagement develop sbt/librarymanagement@92f8b75
util develop sbt/util@cd67959

The result is: SUCCESS

The binaries are at https://dl.bintray.com/sbt/maven-snapshots
The new sbt version is: 1.3.0-bin-20181030T091902
(restart)

@eed3si9n

This comment has been minimized.

Copy link
Member

eed3si9n commented Oct 30, 2018

I mentioned this on Gitter, but I'll repeat here.
Coursier plugin seems to generate ivy.xml in:

  def makeIvyXmlBefore[T](
    task: TaskKey[T],
    shadedConfigOpt: Option[(String, String)]
  ): Setting[Task[T]] =
    task := task.dependsOn(Def.task {
      val currentProject = {
        val proj = coursierProject.value
        val publications = coursierPublications.value
        proj.copy(publications = publications)
      }
      IvyXml.writeFiles(currentProject, shadedConfigOpt, ivySbt.value, streams.value.log)
    }).value
@alexarchambault

This comment has been minimized.

Copy link

alexarchambault commented Nov 19, 2018

FYI, I'm currently refactoring sbt-coursier so that:

  • it offers a module with implementations for sbt/librarymanagement,
  • it offers an sbt plugin, sbt-lm-coursier, setting dependencyResolution, that should be compatible almost as-is with the current sbt-coursier, passing the scripted tests of sbt-coursier too,
  • if possible, sbt-lm-coursier should shade coursier itself.

The first two points started fine in coursier/sbt-coursier#5, currently splitting that in clean commits in coursier/sbt-coursier#6 and subsequent PRs.

If all goes well, I'll probably rename the former sbt-coursier to sbt-coursier-legacy, and the new sbt-lm-coursier to sbt-coursier.

Then, I guess we can check if sbt-lm-coursier passes tests under sbt/src/sbt-test/dependency-management in sbt. If it does, then I guess we could consider including it as is in sbt, so that coursier is used by default in it. (Also, its DependencyResolution implem could be used here-or-there in sbt instead of the Ivy one.)

@alexarchambault

This comment has been minimized.

Copy link

alexarchambault commented Nov 19, 2018

^ I think that provides a clear path towards the use of coursier by sbt. Opinions?

@alexarchambault

This comment has been minimized.

Copy link

alexarchambault commented Nov 19, 2018

This all aims at finishing the prior work of @leonardehrenfried and @andreaTP.

@andreaTP

This comment has been minimized.

Copy link
Contributor

andreaTP commented Nov 20, 2018

@alexarchambault your effort is pretty much appreciated, but currently the blocker to have a proper implementation is that the logic for generating the POM file for publishing is pretty hard to replicate ( talking about this: https://github.com/coursier/sbt-coursier/blob/22595d07affff833f42755cb157ca91322267274/modules/sbt-coursier/src/main/scala/coursier/Tasks.scala#L237 ) cc. @eed3si9n

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment