chores(analyzer): Print warning if duplicate packages

[bennati]

When duplicate packages or projects are found in the dependency tree, print a warning instead of throwing an exception and inteerrupting the scan. Duplicate packages may arise when the same package is imported twice, in these cases the dependency tree will be completed as the package is imported at least once.

Please ensure that your pull request adheres to our contribution guidelines. Thank you!

[sschuberth]

Besides the current build errors, I'm actually not sure if we want the change. I currently don't oversee all side effects of allowing duplicate identifiers.

[bennati]

I appreciate the need of understanding in detail all side effects of allowing duplicate identifiers. What this proposal is meant for, is making the user aware of the possibility of these side effects, which can be manually corrected, while allowing the scan to complete in case side effects are unlikely.

In my opinion, the scan should crash only if there is a non-recoverable error. Having ORT produce a notice file with potential errors is still safer than having no notice file.

[fviernau]

Besides the current build errors, I'm actually not sure if we want the change. I currently don't oversee all side effects of allowing duplicate identifiers.

I agree, that side-effect are hard to oversee and probably huge. Also I am strongly for keeping identifiers unique. What in my view would be the only option is not adding duplicate IDs to the result in the first place, and instead add an OrtIssue. This way it is indicated that something is missing while ids remain unique.

[bennati]

Is this what you had in mind?

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 67.67%. Comparing base (546550e) to head (344d6cb). Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #9260   +/-   ##
=========================================
  Coverage     67.67%   67.67%           
  Complexity     1187     1187           
=========================================
  Files           239      239           
  Lines          7796     7796           
  Branches        900      900           
=========================================
  Hits           5276     5276           
  Misses         2153     2153           
  Partials        367      367           

Flag	Coverage Δ
funTest-non-docker	34.71% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[fviernau]

Is this what you had in mind?

No it isn't. It disregards that:

• Duplicates can be across projects / packages.
• Removing afterwards probably has a different effect, than skipping over it when attempting to add it in the first place.
• Also the scopes are already constructed, which contain reference to projects or packages and which correspond to a
  project each.

...but, I also do not see a do-able way how to implement what I had in mind.

[bennati]

Ok, let me know if any better solution than this comes to mind which is also feasible and I can try to implement it. Otherwise let's please merge this one so I can finally generate some notice file for my projects.

[fviernau]

Ok, let me know if any better solution than this comes to mind which is also feasible and I can try to implement it.

I won't have the time to look into this anytime soon.

Otherwise let's please merge this one so I can finally generate some notice file for my projects.

IMO it should not be merged, due to not understood side effects and possible broken referential integrity.

Would you be willing to invest the time / effort to find another solution @bennati ?

[bennati]

I can develop a different solution, but first I need someone to explain how the solution should work and the ORT maintainers to commit to merge it once developed.

[bennati]

Any update on this?

[sschuberth]

Any update on this?

I believe this PR needs to make more clear which of the three different root causes for duplicate identifiers it targets to address.

In any case, I believe it's wrong to unconditionally remove all duplicates from packages as that, depending on the root cause, could result in a loss of information.

Instead, to get started with addressing the general topic, I plan to revive https://github.com/oss-review-toolkit/ort/pull/6533 which targets the case where duplicates occur between projects and packages.

[dimitris-iliou]

We have identified another instance related to the RapidJSON library that may need to be addressed. Specifically, the OSS Review Toolkit (ORT) reported a failure because it detected a duplicate dependency across two distinct packages:

[[Package(id=Identifier(type=SpdxDocumentFile, namespace=, name=rapidjson, version=v1.1.0), 
purl=pkg:generic/[email protected]?download_url=https://github.com/Tencent/rapidjson/archive/refs/tags/v1.1.0.tar.gz,
 cpe=null, authors=[], declaredLicenses=[MIT], 
declaredLicensesProcessed=ProcessedDeclaredLicense(spdxExpression=MIT, mapped={}, unmapped=[]), 
concludedLicense=MIT, description=A fast JSON parser/generator for C++ with both SAX/DOM style API, 
homepageUrl=https://rapidjson.org/, binaryArtifact=RemoteArtifact(url=, hash=Hash(value=, algorithm=)), 
sourceArtifact=RemoteArtifact(url=https://github.com/Tencent/rapidjson/archive/refs/tags/v1.1.0.tar.gz, hash=Hash(value=, 
algorithm=)), vcs=VcsInfo(type=Git, url=ssh://[email protected]:3389/professional-services/hdo/hdo-client.git, 
revision=2e21d71c1840c8d795ef73247b73b052acd73b8e, path=external/rapidjson), vcsProcessed=VcsInfo(type=Git, 
url=ssh://[email protected]:3389/professional-services/hdo/hdo-client.git, 
revision=2e21d71c1840c8d795ef73247b73b052acd73b8e, path=external/rapidjson), isMetadataOnly=false, 
isModified=false, sourceCodeOrigins=null), Package(id=Identifier(type=SpdxDocumentFile, namespace=, 
name=rapidjson, version=v1.1.0), purl=pkg:generic/[email protected]?
download_url=https://github.com/Tencent/rapidjson/archive/refs/tags/v1.1.0.tar.gz, cpe=null, authors=[], 
declaredLicenses=[MIT], declaredLicensesProcessed=ProcessedDeclaredLicense(spdxExpression=MIT, mapped={}, 
unmapped=[]), concludedLicense=MIT, description=A fast JSON parser/generator for C++ with both SAX/DOM style API, 
homepageUrl=https://rapidjson.org/, binaryArtifact=RemoteArtifact(url=, hash=Hash(value=, algorithm=)), 
sourceArtifact=RemoteArtifact(url=https://github.com/Tencent/rapidjson/archive/refs/tags/v1.1.0.tar.gz, hash=Hash(value=,
 algorithm=)), vcs=VcsInfo(type=Git, url=ssh://[email protected]:3389/professional-
services/dra/clients/vehicle.git, revision=0b876fe71ff6056818fe4eebe0db31e21751679c, path=external/rapidjson), 
vcsProcessed=VcsInfo(type=Git, url=ssh://[email protected]:3389/professional-services/dra/clients/vehicle.git,
 revision=0b876fe71ff6056818fe4eebe0db31e21751679c, path=external/rapidjson), isMetadataOnly=false, 
isModified=false, sourceCodeOrigins=null)]]