Skip to content

[issue-413] add workflow for circle conversion integration test #582

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

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

armintaenzertng
Copy link
Collaborator

fixes #413

@armintaenzertng armintaenzertng force-pushed the integrationTests branch 3 times, most recently from 446bb3d to 56cde6a Compare April 13, 2023 14:48
@armintaenzertng armintaenzertng marked this pull request as ready for review April 13, 2023 15:00
@armintaenzertng
Copy link
Collaborator Author

Progress on this is currently halted due to RDF not conserving the order of LicenseConjunction/Disjunctions.
General comparers like jd used here are not able to see MIT and GPL-2.0 and GPL-2.0 and MIT as equal.
Thus, we need a custom SPDX comparison. Maybe the java tools CompareDocs once this issue is solved.

@goneall
Copy link
Member

goneall commented Apr 24, 2023

I don't know if this helps - but here's a pointer to how the Java library handles this issue:

https://github.com/spdx/Spdx-Java-Library/blob/da021dffa3c2376f2aa2654863fb000be2aa7d2a/src/main/java/org/spdx/library/model/license/DisjunctiveLicenseSet.java#L106

We basically override the equals and hashcode methods to "flatten" the set and do a compare. Equals is overridden in most, if not all, of the license related classes.

maxhbr
maxhbr previously approved these changes Jun 12, 2023
Copy link
Member

@maxhbr maxhbr left a comment

Choose a reason for hiding this comment

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

LGTM

@maxhbr
Copy link
Member

maxhbr commented Jun 12, 2023

MIT and GPL-2.0 and GPL-2.0 and MIT as equal.

"fun" fact: I am not sure if the legal thread agrees that these two expressions are equal. As far as I remember, in some discussions it was stated that they should not be treated as interchangeable but instead if Legal decides to say "MIT and GPL-2.0" this specific expression needs to be preserved.

@armintaenzertng armintaenzertng force-pushed the integrationTests branch 3 times, most recently from 56cde6a to bf9e58e Compare June 23, 2023 08:48
Signed-off-by: Armin Tänzer <armin.taenzer@tngtech.com>
@armintaenzertng
Copy link
Collaborator Author

After discussing this again, a quick fix for the problem is to simply exclude AND and OR license expressions from the test file. With this, the core of the test is still intact and we don't lose very much test coverage.

@armintaenzertng armintaenzertng merged commit 5c99b5c into spdx:main Jun 27, 2023
@armintaenzertng armintaenzertng deleted the integrationTests branch June 27, 2023 07:43
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.

Add workflow for integration tests
4 participants