Skip to content
This repository has been archived by the owner on Apr 8, 2021. It is now read-only.

[0.10.0-RC1 + sbt 1.2.5+ regression] ignore cached resolution for the dedicated update report #184

Merged
merged 4 commits into from
Sep 7, 2019

Conversation

bjaglin
Copy link
Contributor

@bjaglin bjaglin commented Aug 27, 2019

Fixes empty dependencyTree on 0.10.0-RC1 with sbt 0.13.18, 1.2.5, 1.2.6, 1.2.7, 1.2.8 for projects with updateOptions.withCachedResolution(true).

sbt 1.3.0-RC4 does not exhibit that specific issue by default (although it has another one almost as big, the tree is flat because the caller info is missing from the report) as cached resolution is ignored when using coursier as librarymanagement, however the behavior is the same as 1.2.5+ when disabling coursier via useCoursier := false.

See commit message for more details.

That initial regression fix is clearly a hack, but I am not sure supporting cached resolution would be more robust given how opaque/internal artificial module descriptor names are. Also, from what I understand, cached resolution is on its way out with the usage of coursier by default in sbt 1.3, so this might not be the best time investment.

Related to #176.

Before 7992dc9, a custom, non-cached-resolution-aware update task was
used to generate the report that the tree is based on, effectively
ignoring the cached resolution flag at the project level.

Starting 7992dc9, this plugin, when run with sbt 0.13.8 or sbt 1.2.5+,
relies on cached-resolution-backed reports for projects that have
the engine enabled via `updateOptions`. Other 1.x releases are not
directly impacted as sbt had a buggy implementation of the feature
anyway, see sbt/sbt#3761.

Cached resolution has the side effect of generating an ivy report
with artificial module descriptors which makes it hard to reconstruct
the tree without inlining sbt internals (see below), so this
effectively ignores it *for the purpose of the tree generation*, even
if the project enabled it for the regular report.

ModuleId(
  org.scala-sbt.temp,
  temp-resolve-e2a956132f02c038285b41b374c02f5838076f37,
  1.0
)

https://github.com/sbt/librarymanagement/blob/984de6f/ivy/src/main/scala/sbt/internal/librarymanagement/ivyint/CachedResolutionResolveEngine.scala#L137
@bjaglin bjaglin changed the title ignore cached resolution for the dedicated update report [0.10.0-RC1 regression] ignore cached resolution for the dedicated update report Aug 27, 2019
@bjaglin bjaglin marked this pull request as ready for review August 27, 2019 21:50
@bjaglin bjaglin changed the title [0.10.0-RC1 regression] ignore cached resolution for the dedicated update report [0.10.0-RC1 + sbt 1.2.5+ regression] ignore cached resolution for the dedicated update report Aug 28, 2019
Copy link
Member

@jrudolph jrudolph left a comment

Choose a reason for hiding this comment

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

Thanks for spotting and fixing the problem. Much appreciated. I reverted the key rename, let's not do this now but otherwise, LGTM.

@jrudolph
Copy link
Member

jrudolph commented Sep 7, 2019

That initial regression fix is clearly a hack

I think it's perfectly fine :) No need to support cachedResolution for the dependency graph reports.

@jrudolph jrudolph merged commit 17d25cc into sbt:master Sep 7, 2019
@jrudolph
Copy link
Member

jrudolph commented Sep 7, 2019

Thanks again, @bjaglin!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants