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

Resolved external artifacts reconciliation throwing IllegalStateException #115

Open
vajain-1982 opened this issue Mar 4, 2024 · 22 comments

Comments

@vajain-1982
Copy link

Hi,
External artifiacts reconcile throws illegalStateException https://github.com/spdx/spdx-gradle-plugin/blob/main/src/main/java/org/spdx/sbom/gradle/utils/SpdxDocumentBuilder.java#L174
This could happen if same componentIdentfier resolves to two different files ( possible due to transitive dependencies of different jars). In this case task simple fails as it couldn't make a decision which one is right.
Shouldn't this be handled gracefully or given an option to user to pick one ( first | second) in case such condition occurs.
Observed this issue for with netty jars which are platform dependent ,where component identifier is same.

cannot merge duplicate /Users/.gradle/caches/modules-2/files-2.1/io.netty/netty-transport-native-epoll/4.1.105.Final/888ba2b17e360367e01a380fbbd048266cb92305/netty-transport-native-epoll-4.1.105.Final-linux-riscv64.jar and /Users/.gradle/caches/modules-2/files-2.1/io.netty/netty-transport-native-epoll/4.1.105.Final/2958234697ebe6a61d2f43257fe1ed6d19a46b57/netty-transport-native-epoll-4.1.105.Final-linux-aarch_64.jar

@loosebazooka
Copy link
Collaborator

loosebazooka commented Mar 4, 2024

Yeah I think we never found a good example of when that happens. Can you include a minimal sample project where this error occurs, that'll help us address it better?

There might be some underlying variant information that we can use. Component Id might be insufficient.

@vajain-1982
Copy link
Author

I don't have good sample example for this . jars for different arch are coming from my organizations custom repo ( can't share details) , any other detail like resolved artifact object structure through debug can help to fix this ?

@loosebazooka
Copy link
Collaborator

I think we need to rewrite a lot of the code to be "variant-aware" but that is hard because I haven't found a good example where multiple variants of the same componentId are included in a build, variants are picked and the rest are ignored. Without that I can't exactly design for it.

@vajain-1982
Copy link
Author

Understood , will check if I can build such case with sample public repo .
But for the mean time can we handle the error gracefully or provide a way/config to handle the conflict due to duplicate componentIdentifier. Currently any such case fails the complete task.

@loosebazooka
Copy link
Collaborator

Is your build using multiple configurations in the sbom configuration? This is the only way I can think of to bring in multiple variants into a project and create this clash?

@vajain-1982
Copy link
Author

vajain-1982 commented Mar 7, 2024

my repo has a dependency on an organization level auth library which has library dependency on netty with different arch and as I also have included the netty it downloads the other arch lib too , Since the component identifier is same , plugin fails.
I agree with you that it's hard to handle all such cases and probably requires a lot of work , but I am kind of not in favour of failing for such cases instead plugin should run with warning , recommending user to fix this to achieve better /verbose sbom generation.
Let me know your thought on this.

@loosebazooka
Copy link
Collaborator

loosebazooka commented Mar 7, 2024

Oh yeah the error message could be better. I don't think we can act on it though. The fix is likely excluding the unused netty dependency somehow. But variant selection should already do that which is why this is very confusing to me

@vajain-1982
Copy link
Author

I am thinking of more like a warning instead of error. This would inform the user about issue but still generate sbom with best effort approach.

@loosebazooka
Copy link
Collaborator

Fundamentally this points to an issue with the user's build. Unless I'm mistaken, two variants of the same component cannot be part of a single configuration. We can provide a better error message, but other than that, I don't see any changes.

@kamildoleglo
Copy link

kamildoleglo commented Mar 13, 2024

Same here. In our case it's Netty DNS resolver:

cannot merge duplicate /Users/kdoleglo/.gradle/caches/modules-2/files-2.1/io.netty/netty-resolver-dns-native-macos/4.1.107.Final/93db846854c5dbcf3f7690049da954b9a2decc2e/netty-resolver-dns-native-macos-4.1.107.Final-osx-aarch_64.jar and /Users/kdoleglo/.gradle/caches/modules-2/files-2.1/io.netty/netty-resolver-dns-native-macos/4.1.107.Final/ca071cb93bd52f4193a980158b97e3d068d22d18/netty-resolver-dns-native-macos-4.1.107.Final-osx-x86_64.jar

@loosebazooka
Copy link
Collaborator

@kamildoleglo can you point me to a sample project. I'd love to solve this, but I need an example to work with

@kamildoleglo
Copy link

kamildoleglo commented Mar 13, 2024

I'll try to make one. While I agree with you that probably

two variants of the same component cannot be part of a single configuration

I use several configurations and I want all of them documented in one file:

configurations.set(listOf(project.configurations.runtimeClasspath.name, <some more configurations here>))

so that may trigger this behaviour

EDIT: Actually no, I only left one configuration there and it still fails with the same message

@loosebazooka
Copy link
Collaborator

yeah multiple configurations will definitely trigger it. But I'm more curious about the single configuration case.

@kamildoleglo
Copy link

Reproduced here: https://github.com/kamildoleglo/spdx-repro
The template was generated automatically by Gradle, I just added some dependencies

@loosebazooka
Copy link
Collaborator

@vlsi fyi what we were talking about last week

@loosebazooka
Copy link
Collaborator

Ah so this is actually not variants, this is just multiple classifiers of the same object, which could(?) end up being easier to handle.

@vlsi
Copy link

vlsi commented Mar 13, 2024

Dependencies with classifiers should better be avoided, especially with Gradle (e.g. gradle/gradle#16665 (comment)). The reasoning is that "classifiers" have no metadata. Classifiers have little to no meaning, and they are hard to process dependency-wise.

@kamildoleglo
Copy link

kamildoleglo commented Mar 13, 2024

I agree with that, however this is a valid Gradle configuration and script. IMHO SPDX generation for valid Gradle scripts shouldn't fail in any case. Emitting a warning or providing a way / requiring to manually fix the resulting output is fine as well

@loosebazooka
Copy link
Collaborator

@goneall what's the maven plugin do here to represent multiple artifacts with a classifier? Is there an spdx field for this?

For instance the case of a project depending on both:

  • io.netty:netty-resolver-dns-native-macos:4.1.107.Final:osx-aarch_64
  • io.netty:netty-resolver-dns-native-macos:4.1.107.Final:osx-x86_64

the spdx entry is something like

name = `io.netty:netty-resolver-dns-native-macos`
version = `4.1.107.Final`

my best guess is just append to version so it's like 4.1.107.Final:osx-aarch_64 ?

@goneall
Copy link
Member

goneall commented Mar 14, 2024

what's the maven plugin do here to represent multiple artifacts with a classifier?

I'm not completely sure - it's quite possible there is an issue in the Maven plugin related to generating multiple artifacts as well. If we had a Maven POM file example, I could try it out and see how it behaves.

Is there an spdx field for this?

I don't believe there is. We could add a filed such as "target architecture" for a package, but I don't think that would cover all the uses of the classifier.

my best guess is just append to version so it's like 4.1.107.Final:osx-aarch_64 ?

This seems like a good approach since the qualifier could be considered a refinement of the version (even though it doesn't strictly follow the SemVer syntax/semantics).

@loosebazooka
Copy link
Collaborator

I don't believe there is. We could add a filed such as "target architecture" for a package, but I don't think that would cover all the uses of the classifier.

Yeah I don't think target architecture would work. We can't really know what the classifier's meaning is, and adding logic feels like we'll probably get it wrong.

Lemme see if I can recreate this problem in maven.

@loosebazooka
Copy link
Collaborator

loosebazooka commented Mar 15, 2024

@goneall, filed spdx/spdx-maven-plugin#164

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

5 participants