Calculate BOM representation for each IU only once #3914
Conversation
| @@ -57,6 +58,8 @@ public class TychoProjectDependenciesConverter extends DefaultProjectDependencie | |||
| @Inject | |||
| private MavenProjectDependencyProcessor dependencyProcessor; | |||
|
|
|||
| private final Map<IInstallableUnit, List<String>> bomRepresentations = new HashMap<>(); | |||
There was a problem hiding this comment.
Better use a ConcurentHashMap to support parallel builds
There was a problem hiding this comment.
A fair point. I just double-checked and the map is indeed shared across the entire build.
| bomRefs.add(bomRef); | ||
| } | ||
| return bomRefs; // mutable! | ||
| return bomRefs; // mutable! |
There was a problem hiding this comment.
The APi says the returned list is mutable, so either drop the mutability (and use a read-only list) or return a copy of the cached value.
There was a problem hiding this comment.
It's been a few months, but I believe mutability was required in an earlier implementation because CycloneDX would add element to this list. But now it's only used within loops, so I think dropping this property is the best way to go.
tycho-sbom/src/main/java/org/eclipse/tycho/sbom/TychoProjectDependenciesConverter.java
Show resolved
Hide resolved
|
Apologies, I missclicked and deleted the branch by accident... |
|
@ptziegler can you rebase / resolve conflict? |
| String bomRef = modelConverter.generatePackageUrl(project.getArtifact()); | ||
| if (bomRef == null) { | ||
| LOG.error("Unable to calculate BOM for: " + project); | ||
| return List.copyOf(bomRefs); |
There was a problem hiding this comment.
| return List.copyOf(bomRefs); | |
| return List.of(); |
There was a problem hiding this comment.
I never really liked List.copyOf() and instead I use Collections.emptyList() to make it clear that this is an empty list.
| return List.copyOf(bomRefs); | ||
| } | ||
| bomRefs.add(bomRef); | ||
| return List.copyOf(bomRefs); |
There was a problem hiding this comment.
| return List.copyOf(bomRefs); | |
| return List.of(bomRef); |
There was a problem hiding this comment.
This won't compile as bomRefs is already a list. But to avoid copying the array, I now use Collections.unmodifiableList(...)
It's just a single element, nevermind.
| LOG.error("Unable to calculate BOM for: " + project); | ||
| return List.copyOf(bomRefs); | ||
| } | ||
| bomRefs.add(bomRef); |
There was a problem hiding this comment.
| bomRefs.add(bomRef); |
| return bomRepresentations.computeIfAbsent(iu, ignore -> { | ||
| final MavenSession mavenSession = legacySupport.getSession(); | ||
| final List<MavenProject> reactorProjects = mavenSession.getAllProjects(); | ||
| final List<String> bomRefs = new ArrayList<>(); |
There was a problem hiding this comment.
| final List<String> bomRefs = new ArrayList<>(); |
tycho-sbom/src/main/java/org/eclipse/tycho/sbom/TychoProjectDependenciesConverter.java
Show resolved
Hide resolved
The BOM representation is currently calculated twice for each artifact. Within a reactor build, such IUs should always produce the same BOM representation and should therefore be cached. Resolves eclipse-tycho#3911
laeubi
added
the
backport-to-tycho-4.0.x
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
The BOM representation is currently calculated twice for each artifact. Within a reactor build, such IUs should always produce the same BOM representation and should therefore be cached.
Resolves #3911