janhoy commented on code in PR #4569: URL: https://github.com/apache/solr/pull/4569#discussion_r3519895627
########## gradle/validation/rat-config.xml: ########## @@ -0,0 +1,63 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> + +<!-- + Custom RAT license families supplementing the built-in defaults. +--> +<rat-config> + <families> + <family id="MIT" name="The MIT License"/> + <family id="BSD4" name="Original BSD License (with advertising clause)"/> + <family id="BSD-modi" name="Modified BSD License"/> + <family id="GEN" name="Generated"/> + </families> + + <licenses> + <!-- BSD 4-clause: presence of advertising clause is a policy violation --> + <license family="BSD4" id="BSD4" name="BSD 4-Clause (advertising clause)"> + <text>All advertising materials</text> + </license> + + <!-- Various BSD-like third-party libraries with non-standard copyright headers --> + <license family="BSD-modi" id="BSD-modi" name="Modified BSD License"> + <any> + <text>THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS</text> <!-- d3.js --> + <text>THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS</text> <!-- highlight.js --> + </any> + </license> + + <!-- Inline MIT comment used by ui-grid bundled assets --> + <license family="MIT" id="MIT-uigrid" name="MIT (ui-grid)"> + <text> ; License: MIT</text> + </license> + + <!-- Machine-generated source files that carry no license header --> + <license family="GEN" id="GEN" name="Generated"> + <any> + <!-- none so far --> Review Comment: Why include it if there are no rules? ########## gradle/validation/validate-source-patterns.gradle: ########## @@ -171,32 +157,33 @@ class ValidateSourcePatternsTask extends DefaultTask { def javadocsPattern = ~$/(?sm)^\Q/**\E(.*?)\Q*/\E/$; def javaCommentPattern = ~$/(?sm)^\Q/*\E(.*?)\Q*/\E/$; def xmlCommentPattern = ~$/(?sm)\Q<!--\E(.*?)\Q-->\E/$; - def lineSplitter = ~$/[\r\n]+/$; - def licenseMatcher = Defaults.createDefaultMatcher(); def validLoggerPattern = ~$/(?s)\b(private\s|static\s|final\s){3}+\s*Logger\s+\p{javaJavaIdentifierStart}+\s+=\s+\QLoggerFactory.getLogger(MethodHandles.lookup().lookupClass());\E/$; def validLoggerNamePattern = ~$/(?s)\b(private\s|static\s|final\s){3}+\s*Logger\s+log+\s+=\s+\QLoggerFactory.getLogger(MethodHandles.lookup().lookupClass());\E/$; def packagePattern = ~$/(?m)^\s*package\s+org\.apache.*;/$; def xmlTagPattern = ~$/(?m)\s*<[a-zA-Z].*/$; def extendsLuceneTestCasePattern = ~$/public.*?class.*?extends.*?LuceneTestCase[^\n]*?\n/$; def validSPINameJavadocTag = ~$/(?s)\s*\*\s*@lucene\.spi\s+\{@value #NAME\}/$; - def isLicense = {matcher, ratDocument -> - licenseMatcher.reset() - return lineSplitter.split(matcher.group(1)).any {licenseMatcher.match(ratDocument, it)} + // Detects whether a comment block contains Apache license header text. + def isLicense = { matcher -> + def content = matcher.group(1) + content.contains('Licensed to the Apache Software Foundation') || Review Comment: A comment by claude that I have not been able to validate but here it is: The new string-based isLicense only recognizes Apache header text, while the old RAT default matchers also caught MIT/BSD-style javadoc headers. Coverage narrows slightly; probably an acceptable trade-off ########## gradle/validation/rat-sources.gradle: ########## @@ -104,84 +108,99 @@ allprojects { description = 'Runs Apache Rat checks.' ratClasspath.from(rootProject.configurations.ratDeps) + ratConfig.set(rootProject.layout.projectDirectory.file("gradle/validation/rat-config.xml")) def defaultScanFileTree = project.fileTree(projectDir, { - // Only check files tracked by git — skip untracked/gitignored files - // (IDE artifacts, AI assistant configs, etc.) def trackedFiles = gitTrackedFiles(projectDir) if (trackedFiles != null) { - exclude { element -> - // Allow directories through (they are just containers), exclude untracked files - !element.isDirectory() && !trackedFiles.contains(element.relativePath.pathString) - } + // Git checkout: only scan files in the git index — naturally excludes build + // output, gitignored files, and untracked files without explicit patterns. + include { f -> f.isDirectory() || trackedFiles.contains(f.relativePath.pathString) } + + // The following are excluded from the source release, thus no header needed + exclude "dev-docs" + exclude "**/AGENTS.md" + exclude "**/.*" // e.g. .gradle, .github, .junie, .gitignore, etc. + } else { + // Source release (no git): scan everything, exclude generated/ignored paths manually. + include "**" } - // Exclude the build directory — even though it's not git-tracked, Gradle - // validates input/output overlaps at configuration time before the git - // filter closure runs, so this must be excluded explicitly. exclude project.layout.buildDirectory.get().asFile.name - // similarly, at least one subproject writes to .gradle/ for config/setup - exclude ".gradle/**" // Don't check any of the subprojects - they have their own rat tasks. - exclude subprojects.collect {it.projectDir.name} - - // The git index filter above excludes untracked files. These include - // patterns select the file types that should carry license headers. - include "**/*.xml" - include "**/*.md" - include "**/*.py" - include "**/*.sh" - include "**/*.bat" - include "**/*.gradle" - - // Include selected patterns from any source folders. We could make this - // relative to source sets but it seems to be of little value - all our source sets - // live under 'src' anyway. - include "src/**" - exclude "src/**/*.png" - exclude "src/**/*.txt" - exclude "src/**/*.zip" - exclude "src/**/*.properties" - exclude "src/**/*.utf8" - exclude "src/**/*.svg" - exclude "src/**/*.csv" - - // TODO: SOLR-15601: Some of these should carry the license, perhaps? - exclude "**/*.html" + exclude subprojects.collect { + projectDir.toPath().relativize(it.projectDir.toPath()).toString().replace('\\', '/') + } + + // Binary and media types — no text-based headers possible. + exclude "**/*.png" + exclude "**/*.jpg" + exclude "**/*.gif" + exclude "**/*.svg" + exclude "**/*.ico" + exclude "**/*.bin" + exclude "**/*.pem" + exclude "**/*.pdf" Review Comment: Could all these excludes have been defined in `rat-config.xml` centrally? ########## gradle/validation/rat-sources.gradle: ########## @@ -104,84 +108,99 @@ allprojects { description = 'Runs Apache Rat checks.' ratClasspath.from(rootProject.configurations.ratDeps) + ratConfig.set(rootProject.layout.projectDirectory.file("gradle/validation/rat-config.xml")) def defaultScanFileTree = project.fileTree(projectDir, { - // Only check files tracked by git — skip untracked/gitignored files - // (IDE artifacts, AI assistant configs, etc.) def trackedFiles = gitTrackedFiles(projectDir) if (trackedFiles != null) { - exclude { element -> - // Allow directories through (they are just containers), exclude untracked files - !element.isDirectory() && !trackedFiles.contains(element.relativePath.pathString) - } + // Git checkout: only scan files in the git index — naturally excludes build + // output, gitignored files, and untracked files without explicit patterns. + include { f -> f.isDirectory() || trackedFiles.contains(f.relativePath.pathString) } + + // The following are excluded from the source release, thus no header needed + exclude "dev-docs" + exclude "**/AGENTS.md" + exclude "**/.*" // e.g. .gradle, .github, .junie, .gitignore, etc. + } else { + // Source release (no git): scan everything, exclude generated/ignored paths manually. + include "**" Review Comment: Claude had a comment on this change: `.gradle/` is no longer excluded in the no-git path (the old code excluded `.gradle/**` unconditionally; the new `**/.*` exclude only lives inside the if (trackedFiles != null) branch). In an extracted source release the running Gradle invocation itself creates `.gradle/` and `buildSrc/.gradle` with files like `checksums.lock` and `file-system.probe` that the extension-based excludes don't catch — RAT could flag them during smoketesting. -- 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]
