Skip to content

Stop adding self references and dependency constraints as direct dependencies - #670

Merged
skhokhlov merged 2 commits into
CycloneDX:masterfrom
tjog:gh-669
Sep 3, 2025
Merged

Stop adding self references and dependency constraints as direct dependencies#670
skhokhlov merged 2 commits into
CycloneDX:masterfrom
tjog:gh-669

Conversation

@tjog

@tjog tjog commented Aug 20, 2025

Copy link
Copy Markdown
Contributor

Fixes #669

Would be great if this makes 3.0.0

@tjog
tjog requested a review from a team as a code owner August 20, 2025 18:34
final ResolvedComponentResult dependencyComponent =
((ResolvedDependencyResult) dep).getSelected();
if (graphNode.id.equals(dependencyComponent.getId())) {
continue; // Skip self-references

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Im not sure how these appear, but this was a difference discovered between the direct task and aggregate task.

Due to line 105 here

continue;
}
dependenciesByBomRef.compute(bomRef, (key, existingDeps) -> {
final List<String> nextLevelDeps = dependency.getDependencies().stream()
.map(BomReference::getRef)
.filter(ref -> !ref.equals(key))
.collect(Collectors.toList());
if (existingDeps == null) {
return new TreeSet<>(nextLevelDeps);
} else {
existingDeps.addAll(nextLevelDeps);

The self references are hidden from the aggregate bom, but why even include them in the first place?

This in turn forced changes to how the test below asserts how many dependencies the root has. I think the test case is a bit weird. But maybe there is a good explanation for adding self-references currently? (And testing it)

@tjog

tjog commented Aug 21, 2025

Copy link
Copy Markdown
Contributor Author

I realize now after looking at open PRs that #641 incorporates the dependency constraint fix. Let me know if you want to prioritize that draft PR instead @skhokhlov

Comment thread src/test/groovy/org/cyclonedx/gradle/PluginConfigurationSpec.groovy
@skhokhlov

Copy link
Copy Markdown
Member

I realize now after looking at open PRs that #641 incorporates the dependency constraint fix. Let me know if you want to prioritize that draft PR instead @skhokhlov

@tjog it's fine to keep this PR instead

@tjog
tjog force-pushed the gh-669 branch 2 times, most recently from 1b20509 to 1c74ec6 Compare September 2, 2025 15:11
tjog added 2 commits September 3, 2025 11:15
Signed-off-by: Tobias Jogården <28024277+tjog@users.noreply.github.com>
This is to enable the direct bom task to also pass the dependency count test.

The cyclonedxBom aggregate task solved this by these lines in CyclonedxAggregateTask#mergeAll

```java
dependenciesByBomRef.compute(bomRef, (key, existingDeps) -> {
    final List<String> nextLevelDeps = dependency.getDependencies().stream()
          .map(BomReference::getRef)
          .filter(ref -> !ref.equals(key))
          .collect(Collectors.toList());
    if (existingDeps == null) {
        return new TreeSet<>(nextLevelDeps);
    } else {
        existingDeps.addAll(nextLevelDeps);
        return existingDeps;
    }
});
```

Signed-off-by: Tobias Jogården <28024277+tjog@users.noreply.github.com>
@tjog

tjog commented Sep 3, 2025

Copy link
Copy Markdown
Contributor Author

Hi, the failing test execution were due to the recent changes that require javaVersion = JavaVersion.current() below the test, which I had not rebased in until now. Next CI job should finish just fine.

@skhokhlov skhokhlov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for the fix!

@skhokhlov
skhokhlov merged commit 21d2bf6 into CycloneDX:master Sep 3, 2025
3 checks passed
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.

Dependency constraints incorrectly added as project direct dependency

2 participants