Skip to content

Commit

Permalink
Merge pull request #42 from github/elr/fix-only
Browse files Browse the repository at this point in the history 
fix bug where GPL-2.0 failed to match GPL-2.0-only
  • Loading branch information
@elrayle
elrayle authored Mar 21, 2023
2 parents 9f2f2c0 + e08d1d6 commit 0e88248
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 24 deletions.
16 changes: 16 additions & 0 deletions spdxexp/compare.go
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
package spdxexp

// The compare methods determine if two ranges are greater than, less than or equal within the same license group.
// NOTE: Ranges are organized into groups (referred to as license groups) of the same base license (e.g. GPL).
// Groups have sub-groups of license versions (referred to as the range) where each member is considered
// to be the same version (e.g. {GPL-2.0, GPL-2.0-only}). The sub-groups are in ascending order within
// the license group, such that the first sub-group is considered to be less than the second sub-group,
// and so on. (e.g. {{GPL-1.0}, {GPL-2.0, GPL-2.0-only}} implies {GPL-1.0} < {GPL-2.0, GPL-2.0-only}).

// compareGT returns true if the first range is greater than the second range within the same license group; otherwise, false.
func compareGT(first *node, second *node) bool {
if !first.isLicense() || !second.isLicense() {
return false
Expand All @@ -13,6 +21,7 @@ func compareGT(first *node, second *node) bool {
return firstRange.location[versionGroup] > secondRange.location[versionGroup]
}

// compareLT returns true if the first range is less than the second range within the same license group; otherwise, false.
func compareLT(first *node, second *node) bool {
if !first.isLicense() || !second.isLicense() {
return false
Expand All @@ -26,10 +35,15 @@ func compareLT(first *node, second *node) bool {
return firstRange.location[versionGroup] < secondRange.location[versionGroup]
}

// compareEQ returns true if the first and second range are the same range within the same license group; otherwise, false.
func compareEQ(first *node, second *node) bool {
if !first.isLicense() || !second.isLicense() {
return false
}
if first.lic.license == second.lic.license {
return true
}

firstRange := getLicenseRange(*first.license())
secondRange := getLicenseRange(*second.license())

Expand All @@ -39,6 +53,8 @@ func compareEQ(first *node, second *node) bool {
return firstRange.location[versionGroup] == secondRange.location[versionGroup]
}

// sameLicenseGroup returns false if either license isn't in a range or the two ranges are
// not in the same license group (e.g. group GPL != group Apache); otherwise, true
func sameLicenseGroup(firstRange *licenseRange, secondRange *licenseRange) bool {
if firstRange == nil || secondRange == nil || firstRange.location[licenseGroup] != secondRange.location[licenseGroup] {
return false
Expand Down
9 changes: 9 additions & 0 deletions spdxexp/compare_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@ func TestCompareGT(t *testing.T) {
{"expect greater than: AGPL-3.0 > AGPL-1.0", getLicenseNode("AGPL-3.0", false), getLicenseNode("AGPL-1.0", false), true},
{"expect equal: GPL-2.0-or-later > GPL-2.0-only", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0-only", false), false},
{"expect equal: GPL-2.0-or-later > GPL-2.0", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0", false), false},
{"expect equal: GPL-2.0-only > GPL-2.0", getLicenseNode("GPL-2.0-only", false), getLicenseNode("GPL-2.0", false), false},
{"expect equal: GPL-3.0 > GPL-3.0", getLicenseNode("GPL-3.0", false), getLicenseNode("GPL-3.0", false), false},
{"expect equal: MIT > MIT", getLicenseNode("MIT", false), getLicenseNode("MIT", false), false},
{"expect less than: MPL-1.0 > MPL-2.0", getLicenseNode("MPL-1.0", false), getLicenseNode("MPL-2.0", false), false},
{"incompatible: MIT > ISC", getLicenseNode("MIT", false), getLicenseNode("ISC", false), false},
{"incompatible: MIT > GPL-2.0-only", getLicenseNode("MIT", false), getLicenseNode("GPL-2.0-only", false), false},
{"incompatible: OSL-1.0 > OPL-1.0", getLicenseNode("OSL-1.0", false), getLicenseNode("OPL-1.0", false), false},
{"not simple license: (MIT OR ISC) > GPL-3.0", getLicenseNode("(MIT OR ISC)", false), getLicenseNode("GPL-3.0", false), false}, // TODO: should it raise error?
}
Expand All @@ -49,9 +52,12 @@ func TestCompareEQ(t *testing.T) {
{"expect greater than: AGPL-3.0 == AGPL-1.0", getLicenseNode("AGPL-3.0", false), getLicenseNode("AGPL-1.0", false), false},
{"expect equal: GPL-2.0-or-later > GPL-2.0-only", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0-only", false), true},
{"expect equal: GPL-2.0-or-later > GPL-2.0", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0", false), true},
{"expect equal: GPL-2.0-only == GPL-2.0", getLicenseNode("GPL-2.0-only", false), getLicenseNode("GPL-2.0", false), true},
{"expect equal: GPL-3.0 == GPL-3.0", getLicenseNode("GPL-3.0", false), getLicenseNode("GPL-3.0", false), true},
{"expect equal: MIT == MIT", getLicenseNode("MIT", false), getLicenseNode("MIT", false), true},
{"expect less than: MPL-1.0 == MPL-2.0", getLicenseNode("MPL-1.0", false), getLicenseNode("MPL-2.0", false), false},
{"incompatible: MIT == ISC", getLicenseNode("MIT", false), getLicenseNode("ISC", false), false},
{"incompatible: MIT == GPL-2.0-only", getLicenseNode("MIT", false), getLicenseNode("GPL-2.0-only", false), false},
{"incompatible: OSL-1.0 == OPL-1.0", getLicenseNode("OSL-1.0", false), getLicenseNode("OPL-1.0", false), false},
{"not simple license: (MIT OR ISC) == GPL-3.0", getLicenseNode("(MIT OR ISC)", false), getLicenseNode("GPL-3.0", false), false}, // TODO: should it raise error?
}
Expand All @@ -78,9 +84,12 @@ func TestCompareLT(t *testing.T) {
{"expect greater than: AGPL-3.0 < AGPL-1.0", getLicenseNode("AGPL-3.0", false), getLicenseNode("AGPL-1.0", false), false},
{"expect greater than: GPL-2.0-or-later < GPL-2.0-only", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0-only", false), false},
{"expect greater than: GPL-2.0-or-later == GPL-2.0", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0", false), false},
{"expect equal: GPL-2.0-only < GPL-2.0", getLicenseNode("GPL-2.0-only", false), getLicenseNode("GPL-2.0", false), false},
{"expect equal: GPL-3.0 < GPL-3.0", getLicenseNode("GPL-3.0", false), getLicenseNode("GPL-3.0", false), false},
{"expect equal: MIT < MIT", getLicenseNode("MIT", false), getLicenseNode("MIT", false), false},
{"expect less than: MPL-1.0 < MPL-2.0", getLicenseNode("MPL-1.0", false), getLicenseNode("MPL-2.0", false), true},
{"incompatible: MIT < ISC", getLicenseNode("MIT", false), getLicenseNode("ISC", false), false},
{"incompatible: MIT < GPL-2.0-only", getLicenseNode("MIT", false), getLicenseNode("GPL-2.0-only", false), false},
{"incompatible: OSL-1.0 < OPL-1.0", getLicenseNode("OSL-1.0", false), getLicenseNode("OPL-1.0", false), false},
{"not simple license: (MIT OR ISC) < GPL-3.0", getLicenseNode("(MIT OR ISC)", false), getLicenseNode("GPL-3.0", false), false}, // TODO: should it raise error?
}
Expand Down
7 changes: 7 additions & 0 deletions spdxexp/license.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,13 @@ func getExceptions() []string {
}
}

// licenseRanges returns a list of license ranges.
//
// Ranges are organized into groups (referred to as license groups) of the same base license (e.g. GPL).
// Groups have sub-groups of license versions (referred to as the range) where each member is considered
// to be the same version (e.g. {GPL-2.0, GPL-2.0-only}). The sub-groups are in ascending order within
// the license group, such that the first sub-group is considered to be less than the second sub-group,
// and so on. (e.g. {{GPL-1.0}, {GPL-2.0, GPL-2.0-only}} implies {GPL-1.0} < {GPL-2.0, GPL-2.0-only}).
func licenseRanges() [][][]string {
return [][][]string{
{
Expand Down
64 changes: 40 additions & 24 deletions spdxexp/node.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (n *node) isLicense() bool {
return n.role == licenseNode
}

// Return the value of the license field.
// license returns the value of the license field.
// See also reconstructedLicenseString()
func (n *node) license() *string {
if !n.isLicense() {
Expand Down Expand Up @@ -167,7 +167,7 @@ func (n *node) reconstructedLicenseString() *string {
return nil
}

// Sort an array of license and license reference nodes alphabetically based
// sortLicenses sorts an array of license and license reference nodes alphabetically based
// on their reconstructedLicenseString() representation. The sort function does not expect
// expression nodes, but if one is in the nodes list, it will sort to the end.
func sortLicenses(nodes []*node) {
Expand All @@ -186,29 +186,51 @@ func sortLicenses(nodes []*node) {

// ---------------------- Comparator Methods ----------------------

// Return true if two licenses are compatible; otherwise, false.
// licensesAreCompatible returns true if two licenses are compatible; otherwise, false.
// Two licenses are compatible if they are the same license or if they are in the same
// license group and they meet one of the following rules:
//
// * both licenses have the `hasPlus` flag set to true
// * the first license has the `hasPlus` flag and the second license is in the first license's range or greater
// * the second license has the `hasPlus` flag and the first license is in the second license's range or greater
// * both licenses are in the same range
func (nodes *nodePair) licensesAreCompatible() bool {
// checking ranges is expensive, so check for simple cases first
if !nodes.firstNode.isLicense() || !nodes.secondNode.isLicense() {
return false
}
if !nodes.exceptionsAreCompatible() {
return false
}
if nodes.licensesExactlyEqual() {
return true
}

// simple cases don't apply, so check license ranges
// NOTE: Ranges are organized into groups (referred to as license groups) of the same base license (e.g. GPL).
// Groups have sub-groups of license versions (referred to as the range) where each member is considered
// to be the same version (e.g. {GPL-2.0, GPL-2.0-only}). The sub-groups are in ascending order within
// the license group, such that the first sub-group is considered to be less than the second sub-group,
// and so on. (e.g. {{GPL-1.0}, {GPL-2.0, GPL-2.0-only}} implies {GPL-1.0} < {GPL-2.0, GPL-2.0-only}).
if nodes.secondNode.hasPlus() {
if nodes.firstNode.hasPlus() {
// first+, second+
// first+, second+ just need to be in same range group
return nodes.rangesAreCompatible()
}
// first, second+
// first, second+ requires first to be in range of second
return nodes.identifierInRange()
}
// else secondNode does not have plus
if nodes.firstNode.hasPlus() {
// first+, second
// first+, second requires second to be in range of first
revNodes := &nodePair{firstNode: nodes.secondNode, secondNode: nodes.firstNode}
return revNodes.identifierInRange()
}
// first, second
return nodes.licensesExactlyEqual()
// first, second requires both to be in same range group
return nodes.rangesEqual()
}

// licenseRefsAreCompatible returns true if two license references are compatible; otherwise, false.
func (nodes *nodePair) licenseRefsAreCompatible() bool {
if !nodes.firstNode.isLicenseRef() || !nodes.secondNode.isLicenseRef() {
return false
Expand All @@ -222,13 +244,9 @@ func (nodes *nodePair) licenseRefsAreCompatible() bool {
return compatible
}

// Return true if two licenses are compatible in the context of their ranges; otherwise, false.
// licenseRefsAreCompatible returns true if two licenses are in the same license group (e.g. all "GPL" licenses are in the same
// license group); otherwise, false.
func (nodes *nodePair) rangesAreCompatible() bool {
if nodes.licensesExactlyEqual() {
// licenses specify ranges exactly the same (e.g. Apache-1.0+, Apache-1.0+)
return true
}

firstNode := *nodes.firstNode
secondNode := *nodes.secondNode

Expand All @@ -238,7 +256,7 @@ func (nodes *nodePair) rangesAreCompatible() bool {
// When both licenses allow later versions (i.e. hasPlus==true), being in the same license
// group is sufficient for compatibility, as long as, any exception is also compatible
// Example: All Apache licenses (e.g. Apache-1.0, Apache-2.0) are in the same license group
return sameLicenseGroup(firstRange, secondRange) && nodes.exceptionsAreCompatible()
return sameLicenseGroup(firstRange, secondRange)
}

// identifierInRange returns true if the (first) simple license is in range of the (second)
Expand All @@ -247,14 +265,7 @@ func (nodes *nodePair) identifierInRange() bool {
simpleLicense := nodes.firstNode
plusLicense := nodes.secondNode

if !compareGT(simpleLicense, plusLicense) && !compareEQ(simpleLicense, plusLicense) {
return false
}

// With simpleLicense >= plusLicense, licenses are compatible, as long as, any exception
// is also compatible
return nodes.exceptionsAreCompatible()

return compareGT(simpleLicense, plusLicense) || compareEQ(simpleLicense, plusLicense)
}

// exceptionsAreCompatible returns true if neither license has an exception or they have
Expand All @@ -274,10 +285,15 @@ func (nodes *nodePair) exceptionsAreCompatible() bool {
}

return *nodes.firstNode.exception() == *nodes.secondNode.exception()
}

// rangesEqual returns true if the licenses are in the same range; otherwise, false
// (e.g. GPL-2.0-only == GPL-2.0)
func (nodes *nodePair) rangesEqual() bool {
return compareEQ(nodes.firstNode, nodes.secondNode)
}

// Return true if the licenses are the same; otherwise, false
// licensesExactlyEqual returns true if the licenses are the same; otherwise, false
func (nodes *nodePair) licensesExactlyEqual() bool {
return strings.EqualFold(*nodes.firstNode.reconstructedLicenseString(), *nodes.secondNode.reconstructedLicenseString())
}
4 changes: 4 additions & 0 deletions spdxexp/satisfies_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ func TestValidateLicenses(t *testing.T) {
{"All invalid", []string{"MTI", "Apche-2.0", "0xDEADBEEF", ""}, false, []string{"MTI", "Apche-2.0", "0xDEADBEEF", ""}},
{"All valid", []string{"MIT", "Apache-2.0", "GPL-2.0"}, true, []string{}},
{"Some invalid", []string{"MTI", "Apche-2.0", "GPL-2.0"}, false, []string{"MTI", "Apche-2.0"}},
{"GPL-2.0", []string{"GPL-2.0"}, true, []string{}},
{"GPL-2.0-only", []string{"GPL-2.0-only"}, true, []string{}},
}

for _, test := range tests {
Expand Down Expand Up @@ -110,6 +112,8 @@ func TestSatisfies(t *testing.T) {
{"GPL-1.0+ satisfies [GPL-2.0+]", "GPL-1.0+", []string{"GPL-2.0+"}, true, nil},
{"! GPL-1.0 satisfies [GPL-2.0+]", "GPL-1.0", []string{"GPL-2.0+"}, false, nil},
{"GPL-2.0-only satisfies [GPL-2.0-only]", "GPL-2.0-only", []string{"GPL-2.0-only"}, true, nil},
{"GPL-2.0 satisfies [GPL-2.0-only]", "GPL-2.0", []string{"GPL-2.0-only"}, true, nil},
{"GPL-2.0 AND GPL-2.0-only satisfies [GPL-2.0-only]", "GPL-2.0 AND GPL-2.0-only", []string{"GPL-2.0-only"}, true, nil},
{"GPL-3.0-only satisfies [GPL-2.0+]", "GPL-3.0-only", []string{"GPL-2.0+"}, true, nil},

{"! GPL-2.0 satisfies [GPL-2.0+ WITH Bison-exception-2.2]",
Expand Down

0 comments on commit 0e88248

Please sign in to comment.