From 4b54525b2be79c2860ba8da9cd30ce00727b40e7 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 28 Sep 2022 15:36:53 -0700 Subject: [PATCH] Fix nondeterminisim in xmlnotice SafePathPrefixes contains "prebuilts/" which is a prefix of another entry "prebuilts/module_sdk" which can both match the same path. SafePathPrefixes is a map, so the iteration order is nondeterminisitic. Move both SafePathPrefixes and SafePrebuiltPrefixes into lists that will always have a deterministic iteration order. Bug: 230357391 Test: build NOTICE.xml.gz multiple times Change-Id: Ibfcd6715b70f26164e0ef4d59f73b240f47f8db7 --- tools/compliance/noticeindex.go | 18 +++---- tools/compliance/policy_policy.go | 83 +++++++++++++------------------ 2 files changed, 43 insertions(+), 58 deletions(-) diff --git a/tools/compliance/noticeindex.go b/tools/compliance/noticeindex.go index 50f808bb16..86d42ac560 100644 --- a/tools/compliance/noticeindex.go +++ b/tools/compliance/noticeindex.go @@ -360,19 +360,17 @@ func (ni *NoticeIndex) getLibName(noticeFor *TargetNode, h hash) string { continue } } - for _, r := range OrderedSafePrebuiltPrefixes { - prefix := SafePrebuiltPrefixes[r] - match := r.FindString(licenseText) + for _, safePrebuiltPrefix := range safePrebuiltPrefixes { + match := safePrebuiltPrefix.re.FindString(licenseText) if len(match) == 0 { continue } - strip := SafePathPrefixes[prefix] - if strip { + if safePrebuiltPrefix.strip { // strip entire prefix match = licenseText[len(match):] } else { // strip from prebuilts/ until safe prefix - match = licenseText[len(match)-len(prefix):] + match = licenseText[len(match)-len(safePrebuiltPrefix.prefix):] } // remove LICENSE or NOTICE or other filename li := strings.LastIndex(match, "/") @@ -392,10 +390,10 @@ func (ni *NoticeIndex) getLibName(noticeFor *TargetNode, h hash) string { break } } - for prefix, strip := range SafePathPrefixes { - if strings.HasPrefix(p, prefix) { - if strip { - return p[len(prefix):] + for _, safePathPrefix := range safePathPrefixes { + if strings.HasPrefix(p, safePathPrefix.prefix) { + if safePathPrefix.strip { + return p[len(safePathPrefix.prefix):] } else { return p } diff --git a/tools/compliance/policy_policy.go b/tools/compliance/policy_policy.go index 2787698774..02d1d967ed 100644 --- a/tools/compliance/policy_policy.go +++ b/tools/compliance/policy_policy.go @@ -16,7 +16,6 @@ package compliance import ( "regexp" - "sort" "strings" ) @@ -30,35 +29,31 @@ var ( "toolchain": "toolchain", } - // SafePathPrefixes maps the path prefixes presumed not to contain any + // safePathPrefixes maps the path prefixes presumed not to contain any // proprietary or confidential pathnames to whether to strip the prefix // from the path when used as the library name for notices. - SafePathPrefixes = map[string]bool{ - "external/": true, - "art/": false, - "build/": false, - "cts/": false, - "dalvik/": false, - "developers/": false, - "development/": false, - "frameworks/": false, - "packages/": true, - "prebuilts/module_sdk/": true, - "prebuilts/": false, - "sdk/": false, - "system/": false, - "test/": false, - "toolchain/": false, - "tools/": false, + safePathPrefixes = []safePathPrefixesType{ + {"external/", true}, + {"art/", false}, + {"build/", false}, + {"cts/", false}, + {"dalvik/", false}, + {"developers/", false}, + {"development/", false}, + {"frameworks/", false}, + {"packages/", true}, + {"prebuilts/module_sdk/", true}, + {"prebuilts/", false}, + {"sdk/", false}, + {"system/", false}, + {"test/", false}, + {"toolchain/", false}, + {"tools/", false}, } - // SafePrebuiltPrefixes maps the regular expression to match a prebuilt + // safePrebuiltPrefixes maps the regular expression to match a prebuilt // containing the path of a safe prefix to the safe prefix. - SafePrebuiltPrefixes = make(map[*regexp.Regexp]string) - - // OrderedSafePrebuiltPrefixes lists the SafePrebuiltPrefixes ordered by - // increasing length. - OrderedSafePrebuiltPrefixes = make([]*regexp.Regexp, 0, 0) + safePrebuiltPrefixes []safePrebuiltPrefixesType // ImpliesUnencumbered lists the condition names representing an author attempt to disclaim copyright. ImpliesUnencumbered = LicenseConditionSet(UnencumberedCondition) @@ -89,6 +84,16 @@ var ( ImpliesShared = LicenseConditionSet(ReciprocalCondition | RestrictedCondition | WeaklyRestrictedCondition) ) +type safePathPrefixesType struct { + prefix string + strip bool +} + +type safePrebuiltPrefixesType struct { + safePathPrefixesType + re *regexp.Regexp +} + var ( anyLgpl = regexp.MustCompile(`^SPDX-license-identifier-LGPL.*`) versionedGpl = regexp.MustCompile(`^SPDX-license-identifier-GPL-\p{N}.*`) @@ -96,33 +101,15 @@ var ( ccBySa = regexp.MustCompile(`^SPDX-license-identifier-CC-BY.*-SA.*`) ) -// byIncreasingLength implements `sort.Interface` to order regular expressions by increasing length. -type byIncreasingLength []*regexp.Regexp - -func (l byIncreasingLength) Len() int { return len(l) } -func (l byIncreasingLength) Swap(i, j int) { l[i], l[j] = l[j], l[i] } -func (l byIncreasingLength) Less(i, j int) bool { - ri := l[i].String() - rj := l[j].String() - if len(ri) == len(rj) { - return ri < rj - } - return len(ri) < len(rj) -} - func init() { - for prefix := range SafePathPrefixes { - if strings.HasPrefix(prefix, "prebuilts/") { + for _, safePathPrefix := range safePathPrefixes { + if strings.HasPrefix(safePathPrefix.prefix, "prebuilts/") { continue } - r := regexp.MustCompile("^prebuilts/(?:runtime/mainline/)?" + prefix) - SafePrebuiltPrefixes[r] = prefix + r := regexp.MustCompile("^prebuilts/(?:runtime/mainline/)?" + safePathPrefix.prefix) + safePrebuiltPrefixes = append(safePrebuiltPrefixes, + safePrebuiltPrefixesType{safePathPrefix, r}) } - OrderedSafePrebuiltPrefixes = make([]*regexp.Regexp, 0, len(SafePrebuiltPrefixes)) - for r := range SafePrebuiltPrefixes { - OrderedSafePrebuiltPrefixes = append(OrderedSafePrebuiltPrefixes, r) - } - sort.Sort(byIncreasingLength(OrderedSafePrebuiltPrefixes)) } // LicenseConditionSetFromNames returns a set containing the recognized `names` and