rmetzger commented on a change in pull request #13796:
URL: https://github.com/apache/flink/pull/13796#discussion_r517179535
##########
File path:
tools/ci/java-ci-tools/src/main/java/org/apache/flink/tools/ci/licensecheck/LicenseChecker.java
##########
@@ -120,87 +122,100 @@ private int ensureRequiredNoticeFiles(Multimap<String,
IncludedDependency> modul
return severeIssueCount;
}
- private static String getModuleFromNoticeFile(File noticeFile) {
- File moduleFile = noticeFile.getParentFile() // META-INF
- .getParentFile() // resources
- .getParentFile() // main
- .getParentFile() // src
- .getParentFile(); // <-- module name
- return moduleFile.getName();
+ private static String getModuleFromNoticeFile(Path noticeFile) {
+ Path moduleDirectory = noticeFile.getParent() // META-INF
+ .getParent() // resources
+ .getParent() // main
+ .getParent() // src
+ .getParent(); // <-- module name
+ return moduleDirectory.getFileName().toString();
}
- private int checkNoticeFile(Multimap<String, IncludedDependency>
modulesWithShadedDependencies, File noticeFile) throws IOException {
+ private static int checkNoticeFile(Multimap<String, IncludedDependency>
modulesWithShadedDependencies, Path noticeFile) throws IOException {
int severeIssueCount = 0;
String moduleName = getModuleFromNoticeFile(noticeFile);
// 1st line contains module name
- String noticeContents = readFile(noticeFile.toPath());
- if (!noticeContents.startsWith(moduleName)) {
- String firstLine = noticeContents.substring(0,
noticeContents.indexOf('\n'));
- LOG.warn("Expected first file of notice file to start
with module name. moduleName={}, firstLine={}", moduleName, firstLine);
+ List<String> noticeContents = Files.readAllLines(noticeFile);
+
+ if (noticeContents.isEmpty()) {
+ LOG.error("Notice file empty {}", noticeFile);
+ severeIssueCount++;
+ }
+
+ // first line must be the module name.
+ if (!noticeContents.get(0).equals(moduleName)) {
+ LOG.warn("Expected first file of notice file to start
with module name. moduleName={}, firstLine={}", moduleName,
noticeContents.get(0));
}
// collect all declared dependencies from NOTICE file
Set<IncludedDependency> declaredDependencies = new HashSet<>();
- try (BufferedReader br = new BufferedReader(new
StringReader(noticeContents))) {
- String line;
- while ((line = br.readLine()) != null) {
- Matcher noticeDependencyMatcher =
NOTICE_DEPENDENCY_PATTERN.matcher(line);
- if (noticeDependencyMatcher.find()) {
- String groupId =
noticeDependencyMatcher.group(1);
- String artifactId =
noticeDependencyMatcher.group(2);
- String version =
noticeDependencyMatcher.group(3);
- if (groupId == null && artifactId ==
null && version == null) { // "bundles" case
- groupId =
noticeDependencyMatcher.group(5);
- artifactId =
noticeDependencyMatcher.group(6);
- version =
noticeDependencyMatcher.group(7);
- }
- IncludedDependency toAdd =
IncludedDependency.create(groupId, artifactId, version);
- if (!declaredDependencies.add(toAdd)) {
- LOG.warn("Dependency {} has
been declared twice in module {}", toAdd, moduleName);
- }
+ for (String line : noticeContents) {
+ Matcher noticeDependencyMatcher =
NOTICE_DEPENDENCY_PATTERN.matcher(line);
+ if (noticeDependencyMatcher.find()) {
+ String groupId =
noticeDependencyMatcher.group(1);
+ String artifactId =
noticeDependencyMatcher.group(2);
+ String version =
noticeDependencyMatcher.group(3);
+ if (groupId == null && artifactId == null &&
version == null) { // "bundles" case
+ groupId =
noticeDependencyMatcher.group(5);
+ artifactId =
noticeDependencyMatcher.group(6);
+ version =
noticeDependencyMatcher.group(7);
+ }
+ IncludedDependency toAdd =
IncludedDependency.create(groupId, artifactId, version);
+ if (!declaredDependencies.add(toAdd)) {
+ LOG.error("Dependency {} has been
declared twice in module {}", toAdd, moduleName);
+ severeIssueCount++;
}
}
}
// print all dependencies missing from NOTICE file
- Set<IncludedDependency> expectedDependencies = new
HashSet<>(modulesWithShadedDependencies.get(moduleName));
- expectedDependencies.removeAll(declaredDependencies);
- for (IncludedDependency missingDependency :
expectedDependencies) {
- LOG.error("Could not find dependency {} in NOTICE file
{}", missingDependency, noticeFile);
- severeIssueCount++;
+ Collection<IncludedDependency> expectedDependencies =
modulesWithShadedDependencies.get(moduleName);
+ for (IncludedDependency expectedDependency :
expectedDependencies) {
+ if (!declaredDependencies.contains(expectedDependency))
{
+ LOG.error("Could not find dependency {} in
NOTICE file {}", expectedDependency, noticeFile);
+ severeIssueCount++;
+ }
}
- if (!MODULES_DEFINING_EXCESS_DEPENDENCIES.contains(moduleName))
{
- // print all dependencies defined in NOTICE file, which
were not expected
- Set<IncludedDependency> excessDependencies = new
HashSet<>(declaredDependencies);
-
excessDependencies.removeAll(modulesWithShadedDependencies.get(moduleName));
- for (IncludedDependency excessDependency :
excessDependencies) {
- LOG.warn("Dependency {} is mentioned in NOTICE
file {}, but is not expected there", excessDependency, noticeFile);
+ boolean moduleDefinesExcessDependencies =
MODULES_DEFINING_EXCESS_DEPENDENCIES.contains(moduleName);
+
+ // print all dependencies defined in NOTICE file, which were
not expected
+ Set<IncludedDependency> excessDependencies = new
HashSet<>(declaredDependencies);
Review comment:
🤦
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
