-
-
Notifications
You must be signed in to change notification settings - Fork 86
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
Add support for custom external references #428
Conversation
FYI, this issues Apache Log4j. |
@vy do you really need to be able to customize external references in each child module? defining a value in the parent is not sufficient? I understand the theory of this, but implementing it means that this plugins' code is doing Maven job in parallel to Maven (= mapping plugin execution configuration to objects): this is complex and fragile
your suspicion is not exactly right: the effective POM is the effective POM: it's your merge that does not mimic real Maven way of merging configuration from each level of effective Maven POM = exactly what I told before, you are trying to reimplement a Maven mechanism, that is risky |
CC: @ppkarwasz |
I believe the
Unfortunately I have no idea on how to implement this in Maven. @hboutemy any suggestions? The current order of execution is problematic, since the root project must do the same work as its submodules, including the resolution of the CycloneDX configuration of submodules. |
I think it is indeed a good idea to re-evaluate our assumptions. @stevespringett, you were the major lead in hinting us to place the VDR/VEX references in the following two places:
@stevespringett, can we live with only (1)? What would be the cons of dropping (2)?
@hboutemy, can we explicitly document this caveat of this feature and see how far we can go? Maybe even put a disclaimer stating the feature is experimental? |
Granularity would be the only con that I can see. By limiting to If the assumption is that a supplier or open source project (e.g. Apache Logging) wants to specify external references for VDR/VEX, then limiting to the first option should have no impact on their ability to do so. |
Yes, but it's not what Maven does currently: I finally opened the Maven Jira issue that I wanted to open for a few years to better document the current issues and future solutions https://issues.apache.org/jira/browse/MNG-7991
to me, adding external references makes only sense for supplier on what he provides: I don't see why a supplier would add references for his dependencies, given it would be specific only to his own use of the dependencies
I think this is what the current Apache Logging case is about, and a very good step forward |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Except the major API change, this looks good to me.
@hboutemy, I have updated the branch to only contain the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can merge the PR and do the changes after, but it's better to not do the unwanted changes for dependency artifacts
Signed-off-by: Volkan Yazıcı <[email protected]>
Signed-off-by: Volkan Yazıcı <[email protected]>
@hboutemy, I have addressed your remarks. Would you mind checking again, please? |
thanks a lot @vy |
This PR addresses #421 and adds the configured external references into two different places of the generated SBOM:
$.metadata.component.externalReferences
$.components[].externalReferences
(only for$.components
contained by the reactor)While (1) only requires the basic information about the current Maven module that is being processed, (2) was pretty tricky to get right. For (2), the parent module needs to obtain the external references configured for child modules. Hence, using the value injected into the
field of the mojo is of no use there. In
DefaultModelConverter#convert(MojoExecution, Artifact, CycloneDxSchema.Version, boolean)
, I locate the matching plugin and its matching execution, and merge the configuration of the two. Notice the.distinct()
during this merge – without that, merged external references contain duplicates. My suspicion isgetEffectiveMavenProject()
doesn't always return the effective POM.