amaechler commented on code in PR #20764:
URL: https://github.com/apache/druid/pull/20764#discussion_r4402498221
##########
distribution/bin/check-licenses.py:
##########
@@ -422,23 +436,37 @@ def read_dependency_reports(dependency_reports_root,
compatible_license_names):
parser = DependencyReportParser(druid_module_name,
compatible_license_names)
dep_to_license = parser.parse(report_file)
reported_dep_to_licenses.update(dep_to_license)
+ unsupported_licenses.extend(parser.unsupported_licenses)
for key in dep_to_license:
reported_modules.setdefault(key,
set()).add(druid_module_name)
if len(reported_dep_to_licenses) == 0:
raise Exception("No dependency reports are found")
- return reported_dep_to_licenses, reported_modules
+ return reported_dep_to_licenses, reported_modules, unsupported_licenses
def check_licenses(license_yaml, dependency_reports_root,
exclude_module_prefixes):
# Build a dictionary to facilitate comparing reported licenses and
registered ones.
# These dictionaries are the mapping of (group_id, artifact_id, version)
to license_name.
with open(license_yaml, encoding='utf-8') as registry_file:
- licenses_list = list(yaml.load_all(registry_file,
Loader=yaml.FullLoader))
+ licenses_list = licenses_yaml.load_all(registry_file)
check_license_files(license_yaml, licenses_list)
# Build reported license dictionary.
compatible_license_names = build_compatible_license_names()
- reported_dep_to_licenses, reported_modules =
read_dependency_reports(dependency_reports_root, compatible_license_names)
+ reported_dep_to_licenses, reported_modules, unsupported_licenses =
read_dependency_reports(
+ dependency_reports_root, compatible_license_names
+ )
+
+ unsupported_licenses = find_unsupported_licenses(unsupported_licenses,
exclude_module_prefixes)
Review Comment:
nit: this exits before the mismatched and missing licenses are worked out,
so a run with several problems only shows the unknown licenses. Collecting them
and exiting once at the end, like the other checks, would save a CI round trip.
##########
distribution/bin/check-licenses.py:
##########
@@ -163,6 +164,10 @@ def handle_endtag(self, tag):
# print(json.dumps({"groupId": self.group_id, "artifactId":
self.artifact_id, "version": self.version, "classifier": self.classifier,
"type": self.dep_type, "license": self.license}))
if self.group_id.find("org.apache.druid") < 0:
self.dep_to_license[get_dep_key(self.group_id,
self.artifact_id, self.version)] = (self.license, self.druid_module_name)
+ # A license name missing from compatible_license_names
would otherwise leave the license unset,
+ # which silently skips the comparison against
licenses.yaml.
+ if self.license is None and self.unsupported_license_names:
Review Comment:
Building on the comment above: rather than tracking unknown names here,
`check_licenses()` could fail on any dependency from a released module whose
reported license is `None` (still skipping `-`). One rule would then catch
unknown names, GPL-only dependencies and dependencies whose POM declares no
license at all.
##########
distribution/bin/check-licenses.py:
##########
@@ -331,6 +336,9 @@ def build_compatible_license_names():
compatible_licenses['MIT-0'] = 'MIT No Attribution'
+ compatible_licenses['Zero-Clause BSD'] = 'Zero-Clause BSD'
+ compatible_licenses['0BSD'] = 'Zero-Clause BSD'
Review Comment:
`MIT No Attribution` and `CDDL 1.0` come out of this mapping but aren't keys
themselves, so an entry with `license_name: MIT No Attribution` fails the new
check, and `set_license` lets a later license overwrite them (`MIT-0` followed
by `Apache-2.0` ends up as Apache). Adding identity keys, as you did for
`Zero-Clause BSD`, fixes both:
```suggestion
compatible_licenses['Zero-Clause BSD'] = 'Zero-Clause BSD'
compatible_licenses['0BSD'] = 'Zero-Clause BSD'
compatible_licenses['MIT No Attribution'] = 'MIT No Attribution'
compatible_licenses['CDDL 1.0'] = 'CDDL 1.0'
```
##########
distribution/bin/check-licenses.py:
##########
@@ -222,8 +228,7 @@ def set_license(self, data):
try:
self.license = self.compatible_license_names[data]
except KeyError:
- print("Unsupported license: " + data)
- print("For:" + self.group_id + " " + self.artifact_id +
" in: "+ self.druid_module_name)
+ self.unsupported_license_names.append(data)
Review Comment:
License names containing "GPL" never get here: `set_license` skips them
first (`data.upper().find("GPL") < 0` above), so they don't land in
`unsupported_license_names`. That skip has been there since #7998 and looks
meant for dual-licensed dependencies, where Maven lists a GPL option next to a
compatible one (the logback EPL/LGPL case in the comment above), so the check
picks the compatible one. It has two side effects:
- A dependency reported only under a GPL-family name (`LGPL-2.1`, `GNU
General Public License, version 2 (GPL2)`) still ends up with `license = None`
and no unsupported entry, so the comparison against `licenses.yaml` is skipped
as before.
- The `CDDL/GPLv2+CE` and `CDDL + GPLv2 with classpath exception` mappings
in `build_compatible_license_names()` can never match, because the skip runs
before the lookup.
I checked both with the parser from this branch. Looking the name up first,
and only skipping names that are unknown and contain "GPL", would keep the
dual-licence behaviour and make those two mappings work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
