Skip to content

Commit 0e88248

Browse files
elrayleelrayle
authored
Merge pull request #42 from github/elr/fix-only
fix bug where GPL-2.0 failed to match GPL-2.0-only
2 parents 9f2f2c0 + e08d1d6 commit 0e88248

File tree

5 files changed

+76
-24
lines changed

5 files changed

+76
-24
lines changed

‎spdxexp/compare.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
package spdxexp
22

3+
// The compare methods determine if two ranges are greater than, less than or equal within the same license group.
4+
// NOTE: Ranges are organized into groups (referred to as license groups) of the same base license (e.g. GPL).
5+
// Groups have sub-groups of license versions (referred to as the range) where each member is considered
6+
// to be the same version (e.g. {GPL-2.0, GPL-2.0-only}). The sub-groups are in ascending order within
7+
// the license group, such that the first sub-group is considered to be less than the second sub-group,
8+
// 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}).
9+
10+
// compareGT returns true if the first range is greater than the second range within the same license group; otherwise, false.
311
func compareGT(first *node, second *node) bool {
412
if !first.isLicense() || !second.isLicense() {
513
return false
@@ -13,6 +21,7 @@ func compareGT(first *node, second *node) bool {
1321
return firstRange.location[versionGroup] > secondRange.location[versionGroup]
1422
}
1523

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

38+
// compareEQ returns true if the first and second range are the same range within the same license group; otherwise, false.
2939
func compareEQ(first *node, second *node) bool {
3040
if !first.isLicense() || !second.isLicense() {
3141
return false
3242
}
43+
if first.lic.license == second.lic.license {
44+
return true
45+
}
46+
3347
firstRange := getLicenseRange(*first.license())
3448
secondRange := getLicenseRange(*second.license())
3549

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

56+
// sameLicenseGroup returns false if either license isn't in a range or the two ranges are
57+
// not in the same license group (e.g. group GPL != group Apache); otherwise, true
4258
func sameLicenseGroup(firstRange *licenseRange, secondRange *licenseRange) bool {
4359
if firstRange == nil || secondRange == nil || firstRange.location[licenseGroup] != secondRange.location[licenseGroup] {
4460
return false

‎spdxexp/compare_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,12 @@ func TestCompareGT(t *testing.T) {
2020
{"expect greater than: AGPL-3.0 > AGPL-1.0", getLicenseNode("AGPL-3.0", false), getLicenseNode("AGPL-1.0", false), true},
2121
{"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},
2222
{"expect equal: GPL-2.0-or-later > GPL-2.0", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0", false), false},
23+
{"expect equal: GPL-2.0-only > GPL-2.0", getLicenseNode("GPL-2.0-only", false), getLicenseNode("GPL-2.0", false), false},
2324
{"expect equal: GPL-3.0 > GPL-3.0", getLicenseNode("GPL-3.0", false), getLicenseNode("GPL-3.0", false), false},
25+
{"expect equal: MIT > MIT", getLicenseNode("MIT", false), getLicenseNode("MIT", false), false},
2426
{"expect less than: MPL-1.0 > MPL-2.0", getLicenseNode("MPL-1.0", false), getLicenseNode("MPL-2.0", false), false},
2527
{"incompatible: MIT > ISC", getLicenseNode("MIT", false), getLicenseNode("ISC", false), false},
28+
{"incompatible: MIT > GPL-2.0-only", getLicenseNode("MIT", false), getLicenseNode("GPL-2.0-only", false), false},
2629
{"incompatible: OSL-1.0 > OPL-1.0", getLicenseNode("OSL-1.0", false), getLicenseNode("OPL-1.0", false), false},
2730
{"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?
2831
}
@@ -49,9 +52,12 @@ func TestCompareEQ(t *testing.T) {
4952
{"expect greater than: AGPL-3.0 == AGPL-1.0", getLicenseNode("AGPL-3.0", false), getLicenseNode("AGPL-1.0", false), false},
5053
{"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},
5154
{"expect equal: GPL-2.0-or-later > GPL-2.0", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0", false), true},
55+
{"expect equal: GPL-2.0-only == GPL-2.0", getLicenseNode("GPL-2.0-only", false), getLicenseNode("GPL-2.0", false), true},
5256
{"expect equal: GPL-3.0 == GPL-3.0", getLicenseNode("GPL-3.0", false), getLicenseNode("GPL-3.0", false), true},
57+
{"expect equal: MIT == MIT", getLicenseNode("MIT", false), getLicenseNode("MIT", false), true},
5358
{"expect less than: MPL-1.0 == MPL-2.0", getLicenseNode("MPL-1.0", false), getLicenseNode("MPL-2.0", false), false},
5459
{"incompatible: MIT == ISC", getLicenseNode("MIT", false), getLicenseNode("ISC", false), false},
60+
{"incompatible: MIT == GPL-2.0-only", getLicenseNode("MIT", false), getLicenseNode("GPL-2.0-only", false), false},
5561
{"incompatible: OSL-1.0 == OPL-1.0", getLicenseNode("OSL-1.0", false), getLicenseNode("OPL-1.0", false), false},
5662
{"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?
5763
}
@@ -78,9 +84,12 @@ func TestCompareLT(t *testing.T) {
7884
{"expect greater than: AGPL-3.0 < AGPL-1.0", getLicenseNode("AGPL-3.0", false), getLicenseNode("AGPL-1.0", false), false},
7985
{"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},
8086
{"expect greater than: GPL-2.0-or-later == GPL-2.0", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0", false), false},
87+
{"expect equal: GPL-2.0-only < GPL-2.0", getLicenseNode("GPL-2.0-only", false), getLicenseNode("GPL-2.0", false), false},
8188
{"expect equal: GPL-3.0 < GPL-3.0", getLicenseNode("GPL-3.0", false), getLicenseNode("GPL-3.0", false), false},
89+
{"expect equal: MIT < MIT", getLicenseNode("MIT", false), getLicenseNode("MIT", false), false},
8290
{"expect less than: MPL-1.0 < MPL-2.0", getLicenseNode("MPL-1.0", false), getLicenseNode("MPL-2.0", false), true},
8391
{"incompatible: MIT < ISC", getLicenseNode("MIT", false), getLicenseNode("ISC", false), false},
92+
{"incompatible: MIT < GPL-2.0-only", getLicenseNode("MIT", false), getLicenseNode("GPL-2.0-only", false), false},
8493
{"incompatible: OSL-1.0 < OPL-1.0", getLicenseNode("OSL-1.0", false), getLicenseNode("OPL-1.0", false), false},
8594
{"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?
8695
}

‎spdxexp/license.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,13 @@ func getExceptions() []string {
640640
}
641641
}
642642

643+
// licenseRanges returns a list of license ranges.
644+
//
645+
// Ranges are organized into groups (referred to as license groups) of the same base license (e.g. GPL).
646+
// Groups have sub-groups of license versions (referred to as the range) where each member is considered
647+
// to be the same version (e.g. {GPL-2.0, GPL-2.0-only}). The sub-groups are in ascending order within
648+
// the license group, such that the first sub-group is considered to be less than the second sub-group,
649+
// 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}).
643650
func licenseRanges() [][][]string {
644651
return [][][]string{
645652
{

‎spdxexp/node.go

Lines changed: 40 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func (n *node) isLicense() bool {
8989
return n.role == licenseNode
9090
}
9191

92-
// Return the value of the license field.
92+
// license returns the value of the license field.
9393
// See also reconstructedLicenseString()
9494
func (n *node) license() *string {
9595
if !n.isLicense() {
@@ -167,7 +167,7 @@ func (n *node) reconstructedLicenseString() *string {
167167
return nil
168168
}
169169

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

187187
// ---------------------- Comparator Methods ----------------------
188188

189-
// Return true if two licenses are compatible; otherwise, false.
189+
// licensesAreCompatible returns true if two licenses are compatible; otherwise, false.
190+
// Two licenses are compatible if they are the same license or if they are in the same
191+
// license group and they meet one of the following rules:
192+
//
193+
// * both licenses have the `hasPlus` flag set to true
194+
// * the first license has the `hasPlus` flag and the second license is in the first license's range or greater
195+
// * the second license has the `hasPlus` flag and the first license is in the second license's range or greater
196+
// * both licenses are in the same range
190197
func (nodes *nodePair) licensesAreCompatible() bool {
198+
// checking ranges is expensive, so check for simple cases first
191199
if !nodes.firstNode.isLicense() || !nodes.secondNode.isLicense() {
192200
return false
193201
}
202+
if !nodes.exceptionsAreCompatible() {
203+
return false
204+
}
205+
if nodes.licensesExactlyEqual() {
206+
return true
207+
}
208+
209+
// simple cases don't apply, so check license ranges
210+
// NOTE: Ranges are organized into groups (referred to as license groups) of the same base license (e.g. GPL).
211+
// Groups have sub-groups of license versions (referred to as the range) where each member is considered
212+
// to be the same version (e.g. {GPL-2.0, GPL-2.0-only}). The sub-groups are in ascending order within
213+
// the license group, such that the first sub-group is considered to be less than the second sub-group,
214+
// 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}).
194215
if nodes.secondNode.hasPlus() {
195216
if nodes.firstNode.hasPlus() {
196-
// first+, second+
217+
// first+, second+ just need to be in same range group
197218
return nodes.rangesAreCompatible()
198219
}
199-
// first, second+
220+
// first, second+ requires first to be in range of second
200221
return nodes.identifierInRange()
201222
}
202223
// else secondNode does not have plus
203224
if nodes.firstNode.hasPlus() {
204-
// first+, second
225+
// first+, second requires second to be in range of first
205226
revNodes := &nodePair{firstNode: nodes.secondNode, secondNode: nodes.firstNode}
206227
return revNodes.identifierInRange()
207228
}
208-
// first, second
209-
return nodes.licensesExactlyEqual()
229+
// first, second requires both to be in same range group
230+
return nodes.rangesEqual()
210231
}
211232

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

225-
// Return true if two licenses are compatible in the context of their ranges; otherwise, false.
247+
// licenseRefsAreCompatible returns true if two licenses are in the same license group (e.g. all "GPL" licenses are in the same
248+
// license group); otherwise, false.
226249
func (nodes *nodePair) rangesAreCompatible() bool {
227-
if nodes.licensesExactlyEqual() {
228-
// licenses specify ranges exactly the same (e.g. Apache-1.0+, Apache-1.0+)
229-
return true
230-
}
231-
232250
firstNode := *nodes.firstNode
233251
secondNode := *nodes.secondNode
234252

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

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

250-
if !compareGT(simpleLicense, plusLicense) && !compareEQ(simpleLicense, plusLicense) {
251-
return false
252-
}
253-
254-
// With simpleLicense >= plusLicense, licenses are compatible, as long as, any exception
255-
// is also compatible
256-
return nodes.exceptionsAreCompatible()
257-
268+
return compareGT(simpleLicense, plusLicense) || compareEQ(simpleLicense, plusLicense)
258269
}
259270

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

276287
return *nodes.firstNode.exception() == *nodes.secondNode.exception()
288+
}
277289

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

280-
// Return true if the licenses are the same; otherwise, false
296+
// licensesExactlyEqual returns true if the licenses are the same; otherwise, false
281297
func (nodes *nodePair) licensesExactlyEqual() bool {
282298
return strings.EqualFold(*nodes.firstNode.reconstructedLicenseString(), *nodes.secondNode.reconstructedLicenseString())
283299
}

‎spdxexp/satisfies_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ func TestValidateLicenses(t *testing.T) {
1818
{"All invalid", []string{"MTI", "Apche-2.0", "0xDEADBEEF", ""}, false, []string{"MTI", "Apche-2.0", "0xDEADBEEF", ""}},
1919
{"All valid", []string{"MIT", "Apache-2.0", "GPL-2.0"}, true, []string{}},
2020
{"Some invalid", []string{"MTI", "Apche-2.0", "GPL-2.0"}, false, []string{"MTI", "Apche-2.0"}},
21+
{"GPL-2.0", []string{"GPL-2.0"}, true, []string{}},
22+
{"GPL-2.0-only", []string{"GPL-2.0-only"}, true, []string{}},
2123
}
2224

2325
for _, test := range tests {
@@ -110,6 +112,8 @@ func TestSatisfies(t *testing.T) {
110112
{"GPL-1.0+ satisfies [GPL-2.0+]", "GPL-1.0+", []string{"GPL-2.0+"}, true, nil},
111113
{"! GPL-1.0 satisfies [GPL-2.0+]", "GPL-1.0", []string{"GPL-2.0+"}, false, nil},
112114
{"GPL-2.0-only satisfies [GPL-2.0-only]", "GPL-2.0-only", []string{"GPL-2.0-only"}, true, nil},
115+
{"GPL-2.0 satisfies [GPL-2.0-only]", "GPL-2.0", []string{"GPL-2.0-only"}, true, nil},
116+
{"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},
113117
{"GPL-3.0-only satisfies [GPL-2.0+]", "GPL-3.0-only", []string{"GPL-2.0+"}, true, nil},
114118

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

0 commit comments

Comments
 (0)