Populate dependencies from module graph #109

matmannion · 2025-01-06T09:50:04Z

Populates dependencies in the bom output by using the details from the configuration report.

This relies on an sbt.internal class (SbtUpdateReport) which is what is used by the built-in dependency graph/tree plugin; I'm not sure how I feel about that and maybe it'd be better to just re-implement the logic that it uses (https://github.com/sbt/sbt/blob/1.10.x/main/src/main/scala/sbt/internal/graph/backend/SbtUpdateReport.scala) - happy to take views on that.

Dependency has equality based on the ref alone, so calling .distinct on the result from dependenciesForConfiguration is sufficient to ensure that only one ends up in the output.

fixes #88

raboof

Awesome, thanks for looking into this!

This relies on an sbt.internal class (SbtUpdateReport) which is what is used by the built-in dependency graph/tree plugin; I'm not sure how I feel about that and maybe it'd be better to just re-implement the logic that it uses (https://github.com/sbt/sbt/blob/1.10.x/main/src/main/scala/sbt/internal/graph/backend/SbtUpdateReport.scala) - happy to take views on that.

It looks small enough that re-implementing the logic might be safer - or does it end up 'pulling in' a lot of other internals?

As it makes the generated sbom considerably larger, I think we should make populating the dependencies section optional (default enabled).

.gitignore

raboof · 2025-01-06T09:56:48Z

README.md

@@ -89,7 +89,7 @@ executed.
 [Scripted](https://www.scala-sbt.org/1.x/docs/Testing-sbt-plugins.html) is a tool that allow you to test sbt plugins.
 For each test it is necessary to create a specially crafted project. These projects are inside src/sbt-test directory.

-Scripted tests are run using `scripted` command.
+Scripted tests are run using `scripted` command. Note that these fail on JDK 21 due to the old version of sbt.


should we create an issue for this capturing what exactly is going wrong and what the plan to fix that should be?

I don't think there's much we can do about it as it's the version of Scala 2.12 pulled in by sbt, so other than moving to a more recent baseline (which feels like a possibly breaking major change) I thought maybe documenting here for local development was sufficient?

Documenting it is a great improvement. I've opened #110 to discuss going further

matmannion · 2025-01-06T10:50:43Z

Awesome, thanks for looking into this!

This relies on an sbt.internal class (SbtUpdateReport) which is what is used by the built-in dependency graph/tree plugin; I'm not sure how I feel about that and maybe it'd be better to just re-implement the logic that it uses (https://github.com/sbt/sbt/blob/1.10.x/main/src/main/scala/sbt/internal/graph/backend/SbtUpdateReport.scala) - happy to take views on that.

It looks small enough that re-implementing the logic might be safer - or does it end up 'pulling in' a lot of other internals?

As it makes the generated sbom considerably larger, I think we should make populating the dependencies section optional (default enabled).

Makes sense - I've done both of these.

raboof

Looks good to me! We should research the licensing question before merging, but I don't necessarily want to burden you with that, I can look into it later.

raboof · 2025-01-06T10:55:15Z

README.md

+| includeBomToolVersion    | Boolean | `true`                                                                 | include tool version in bom                                     |
+| includeBomHashes         | Boolean | `true`                                                                 | include artifact hashes in bom                                  |
+| enableBomSha3Hashes      | Boolean | `true`                                                                 | enable the generation of sha3 hashes (not available on java 8)  |
+| includeBomDependencyTree | Boolean | `true`                                                                 | include dependency tree in bom (bomSchemaVersion 1.1 and later) |


(I'm not sure the detail 'bomSchemaVersion 1.1 and later' is needed here, but OK)

src/main/scala/com/github/sbt/sbom/BomExtractor.scala

raboof · 2025-01-06T11:03:42Z

src/main/scala/com/github/sbt/sbom/BomExtractor.scala

+          dependency
+        }
+
+    // https://github.com/sbt/sbt/blob/1.10.x/main/src/main/scala/sbt/internal/graph/backend/SbtUpdateReport.scala


Hmm. Sorry to go back-and-forth on this a bit, but I just noticed that this is including Apache-licensed code into an MIT-licensed project. That might be possible (they're both premissively-licensed, after all), but we'd want to double-check that adding this comment is sufficient, or that we'd want to do a bit more to make sure this is responsibly tracked.

That makes sense, thank you for checking this as well

In preparation of including a file with Apache-2.0-licensed code from sbt in sbt#109 https://reuse.software/

raboof

I took the liberty of breaking out the sbt code to its own file (so it can have its own license header).

I proposed a general approach to license headers in #111, but I don't think this PR necessarily needs to be blocked on that. So this PR LGTM (thanks!) but I'll give other maintainers some time to chime in before merging.

matmannion · 2025-01-08T09:08:00Z

Thanks so much @raboof, breaking into its own file seems like a pragmatic solution until we can get the SPDX headers in

lhns

Looks very good! Thank you!

In preparation of including a file with Apache-2.0-licensed code from sbt in sbt#109 https://reuse.software/

* Add explicit license headers In preparation of including a file with Apache-2.0-licensed code from sbt in #109 https://reuse.software/ * Add reuse check GitHub action

…ndencyTree setting (default true)

Since it's licensed under the Apache 2.0 license

raboof reviewed Jan 6, 2025

View reviewed changes

raboof force-pushed the populate-dependencies branch from 0d1696c to 969d7fa Compare January 7, 2025 22:47

raboof added a commit to raboof/sbt-bom that referenced this pull request Jan 7, 2025

Add explicit license headers

901d063

In preparation of including a file with Apache-2.0-licensed code from sbt in sbt#109 https://reuse.software/

raboof mentioned this pull request Jan 7, 2025

Add explicit license headers #111

Merged

raboof force-pushed the populate-dependencies branch 4 times, most recently from 5ae3656 to 224f7e2 Compare January 7, 2025 23:32

raboof approved these changes Jan 8, 2025

View reviewed changes

lhns approved these changes Jan 8, 2025

View reviewed changes

raboof changed the title ~~Populate dependencies from module graph, fixes #108~~ Populate dependencies from module graph Jan 8, 2025

raboof added a commit to raboof/sbt-bom that referenced this pull request Jan 10, 2025

Add explicit license headers

a1af6ad

In preparation of including a file with Apache-2.0-licensed code from sbt in sbt#109 https://reuse.software/

raboof added a commit that referenced this pull request Jan 10, 2025

Add explicit license headers (#111)

ac0e6ea

* Add explicit license headers In preparation of including a file with Apache-2.0-licensed code from sbt in #109 https://reuse.software/ * Add reuse check GitHub action

raboof force-pushed the populate-dependencies branch 3 times, most recently from 1c84bed to e0af721 Compare January 13, 2025 08:35

Populate dependencies from module graph, fixes sbt#108

d3fc07c

raboof force-pushed the populate-dependencies branch from e0af721 to 783b8c4 Compare January 13, 2025 08:38

matmannion and others added 4 commits January 13, 2025 10:01

PR feedback: pull SbtUpdateTask code into project; add includeBomDepe…

7a7da9f

…ndencyTree setting (default true)

PR feedback: dependencies -> dependencyTree

e87c6c4

Move code taken from sbt to its own file

e7f42b4

Since it's licensed under the Apache 2.0 license

Add explicit license headers

d95b03b

raboof force-pushed the populate-dependencies branch from 783b8c4 to d95b03b Compare January 13, 2025 09:01

raboof merged commit dfce9ff into sbt:main Jan 13, 2025
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Populate dependencies from module graph #109

Populate dependencies from module graph #109

matmannion commented Jan 6, 2025 •

edited by raboof

Loading

raboof left a comment

raboof Jan 6, 2025

matmannion Jan 6, 2025

raboof Jan 6, 2025

matmannion commented Jan 6, 2025

raboof left a comment

raboof Jan 6, 2025

raboof Jan 6, 2025

matmannion Jan 6, 2025

raboof left a comment •

edited

Loading

matmannion commented Jan 8, 2025

lhns left a comment

Populate dependencies from module graph #109

Populate dependencies from module graph #109

Conversation

matmannion commented Jan 6, 2025 • edited by raboof Loading

raboof left a comment

Choose a reason for hiding this comment

raboof Jan 6, 2025

Choose a reason for hiding this comment

matmannion Jan 6, 2025

Choose a reason for hiding this comment

raboof Jan 6, 2025

Choose a reason for hiding this comment

matmannion commented Jan 6, 2025

raboof left a comment

Choose a reason for hiding this comment

raboof Jan 6, 2025

Choose a reason for hiding this comment

raboof Jan 6, 2025

Choose a reason for hiding this comment

matmannion Jan 6, 2025

Choose a reason for hiding this comment

raboof left a comment • edited Loading

Choose a reason for hiding this comment

matmannion commented Jan 8, 2025

lhns left a comment

Choose a reason for hiding this comment

matmannion commented Jan 6, 2025 •

edited by raboof

Loading

raboof left a comment •

edited

Loading