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 value instead of DependencyResolution #87

Open
mdedetrich opened this issue Sep 13, 2023 · 3 comments
Open

Use value instead of DependencyResolution #87

mdedetrich opened this issue Sep 13, 2023 · 3 comments

Comments

@mdedetrich
Copy link
Contributor

As explained in #86 it would be ideal to just use the update task in order to retrieve the dependencies from the report however we are blocked by coursier/coursier#1790 (tl;dr coursier doesn't populate license information from ivy.xml descriptor files).

Using Ivy Resolution is also causing other problems, i.e. for dependencies that use packaging.type there is a workaround at sbt/sbt#3618 (comment) however this doesn't seem to work with ivy resolution done by sbt plugins (I think that because of classloader isolation maybe the system properties aren't propogating?)

@raboof
Copy link

raboof commented Jul 2, 2024

Great work. Just to move the needle on this a bit, I shared what I think is the change you had in mind at #123 - and it indeed demonstrates the problem of not having licensing information populated (which is presumably caused by coursier/coursier#1790, haven't dug into that part yet).

If you're into that sort of thing, you can set scriptedLaunchOpts += "-Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=5005" and then scripted dumpLicenseReport/default-report in sbt to see the problem with a debugger attached.

@raboof
Copy link

raboof commented Jul 2, 2024

It indeed looks like https://github.com/coursier/sbt-coursier/blob/main/modules/lm-coursier/src/main/scala/lmcoursier/internal/SbtUpdateReport.scala#L124 only looks at licenses defined in the pom of the artifact itself (e.g. it sees the license of junit) but if that is missing it does not look in the parent pom (e.g. commons-lang3 and jackson-databind have no licenses)

@raboof
Copy link

raboof commented Jul 3, 2024

so I guess the question is where we should implement https://maven.apache.org/pom.html#inheritance . Without understanding the context too well, it seems reasonable to do this at https://github.com/coursier/sbt-coursier/blob/main/modules/lm-coursier/src/main/scala/lmcoursier/internal/SbtUpdateReport.scala#L255, i.e. after looking up the project in the Resolution, but before passing it to moduleReport. Keeping it this local also might allow us to avoid implementing inheritence fully, and instead only inherit the fields that end up in ModuleReport.

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

No branches or pull requests

2 participants