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

Fix UpdateReport to be compatible with dependency-graph #156

Merged
merged 1 commit into from
Nov 21, 2019

Conversation

eed3si9n
Copy link
Collaborator

Fixes coursier/coursier#1375
Ref sbt/sbt#4706 / sbt/sbt#4688
Ref sbt/sbt-dependency-graph#178

Currently the UpdateReport returned by Coursier is missing callers from the direct dependencies. This is evident from the fact that thisModule's information is not passed. Another missing information in the UpdateReport is ModuleReport that originates from subproject dependencies (aka inter-project dependencies). These two missing info result in broken rendering for sbt-dependency-graph.

This commit attempts to fix them by passing them through to SbtUpdateReport. See the scripted test for confirmation.

before (1.3.3)

sbt:graphtest> app/dependencyTree
[info] com.example:app_2.13:0.1.0-SNAPSHOT

after

sbt:graphtest> app/dependencyTree
[info] com.example:app_2.13:0.1.0-SNAPSHOT [S]
[info]   +-com.example:util_2.13:0.1.0-SNAPSHOT
[info]     +-com.example:core_2.13:0.1.0-SNAPSHOT
[info]       +-com.typesafe.akka:akka-actor_2.13:2.5.25 (evicted by: 2.5.26)
[info]       +-com.typesafe.akka:akka-actor_2.13:2.5.26
[info]       | +-com.typesafe:config:1.3.3
[info]       | +-org.scala-lang.modules:scala-java8-compat_2.13:0.9.0
[info]       |
[info]       +-com.typesafe.akka:akka-stream_2.13:2.5.26
[info]         +-com.typesafe.akka:akka-actor_2.13:2.5.26
[info]         | +-com.typesafe:config:1.3.3
[info]         | +-org.scala-lang.modules:scala-java8-compat_2.13:0.9.0
[info]         |
[info]         +-com.typesafe.akka:akka-protobuf_2.13:2.5.26
[info]         +-com.typesafe:ssl-config-core_2.13:0.3.8
[info]         | +-com.typesafe:config:1.3.3
[info]         | +-org.scala-lang.modules:scala-parser-combinators_2.13:1.1.2
[info]         |
[info]         +-org.reactivestreams:reactive-streams:1.0.2
[info]

Fixes coursier/coursier#1375
Ref sbt/sbt#4706 / sbt/sbt#4688
Ref sbt/sbt-dependency-graph#178

Currently the UpdateReport returned by Coursier is missing callers from the direct dependencies. This is evident from the fact that `thisModule`'s information is not passed. Another missing information in the UpdateReport is ModuleReport that originates from subproject dependencies (aka inter-project dependencies). These two missing info result in broken rendering for sbt-dependency-graph.

This commit attemps to fix them by passing them through to SbtUpdateReport. See the scripted test for confirmation.
@alexarchambault
Copy link
Member

Thanks for that, @eed3si9n!

@alexarchambault alexarchambault merged commit dc5b9ec into coursier:master Nov 21, 2019
@jrudolph
Copy link

Great, thanks, @eed3si9n.

@dwijnand
Copy link
Contributor

So this is in sbt-coursier 2.0.0-RC5-1. So does that mean to have a working sbt-dependency-graph in an sbt 1.3 project I need to add sbt-coursier? Or, put simply, what's the TL;DR for "unbreaking" sbt-dependency-graph in sbt 1.3?

@jrudolph
Copy link

It probably needs an sbt release to pull in these changes.

@ches
Copy link

ches commented Nov 25, 2019

Sorry this probably isn't the best place to ask, but I think all the stakeholder parties are here: has any meta issue been raised about bringing dependency tree/graph functionality into sbt proper? (I agree with Dale in sbt/sbt#4688 (comment)). I can do so if not (sbt/sbt?).

It's core functionality of many build tools—or a first-party, often built-in plugin—including Maven and Gradle. Especially with Coursier integration in sbt 1.3, it seems wasteful at best to have implementations in sbt-dependency-graph and sbt-coursier, and plumbing in sbt, where the domain boundaries have become tricky to understand for a would-be contributor/fixer.

I can't personally volunteer sbt maintainers to take it on, nor @jrudolph or @alexarchambault to support/maintain it in a central home, but maybe a discussion can start about what's best for all, including users.

(And thank you @eed3si9n for this fix)

@dwijnand
Copy link
Contributor

Opened sbt/sbt#5244.

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.

update report is missing callers for direct dependencies and subproject dependencies
5 participants