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

Use DependencyResolver instead of IvySbt#Module directly #86

Merged
merged 1 commit into from
Sep 13, 2023

Conversation

mdedetrich
Copy link
Contributor

@mdedetrich mdedetrich commented Sep 13, 2023

So this PR is the result of me going down a giant rabbit hole. The original improvement I was trying to do is replacing the usage of IvySbt#Module because I was experiencing sbt/sbt#3618 and the workaround at sbt/sbt#3618 (comment) for some reason didn't work (I guess sys.props was not being passed to sbt plugins due to classloader isolation???).

So after wasting a stupid amount of time figuring out if I could somehow pass the packaging.type manually into Ivy, I just figured why not just entirely replace IvySbt#Module with sbt's update task, that way we can also benefit from Coursier's faster dependency resolution (little tidbit, but sbt-license-report can end up taking ages in some cases due to Ivy)? Then this is where the rabbit hole began, I ended up hitting coursier/coursier#1790 (tl;dr version, Coursier doesn't properly grab the license information when using IvySbt#Module as a module). IvySbt#Module also needs to be used because certain dependencies don't hold the license info in pom.xml (see https://discord.com/channels/632150470000902164/922600050989875282/1151492070373085306).

So rather than using update I instead opted for IvyDependencyResolver. While it doesn't go the full way of just using update to get all of the dependencies and their respective licenses, the data structures returned by both DependencyResolution.update and sbt's update task are the same (i.e. UpdateReport) so once that coursier issue is solved a future PR to use update will be trivial.

The PR also drops support for SBT 0.13.x because its ancient and DependencyResolution doesn't exist there.

@mdedetrich mdedetrich force-pushed the use-dependency-resolver branch from eaf5337 to da82f08 Compare September 13, 2023 15:46
Copy link
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

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

Thanks @mdedetrich!

@eed3si9n eed3si9n merged commit 5bdab60 into sbt:main Sep 13, 2023
2 checks passed
@mdedetrich mdedetrich deleted the use-dependency-resolver branch September 13, 2023 16:13
ybasket added a commit to ybasket/sbt-license-report that referenced this pull request Nov 5, 2024
With the changes of sbt#86, the filtering of configurations broke and test dependencies got reported even though licenseConfigurations was set to Set("compile"). This is due to the fact that the resolved Ivy deps not properly mirror the configurations the dependency was defined for. By selecting the configurations from the report early on, we restore the filtering.
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.

2 participants