bharos commented on code in PR #13257: URL: https://github.com/apache/gravitino/pull/13257#discussion_r4065589832
########## dev/release/maven/test-legal-files.gradle: ########## @@ -0,0 +1,296 @@ +/* + * 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. + */ +import java.nio.charset.StandardCharsets +import java.util.zip.ZipEntry +import java.util.zip.ZipFile +import java.util.zip.ZipOutputStream +import org.gradle.api.tasks.SourceSetContainer +import org.gradle.api.tasks.bundling.War + +// Synthetic JARs keep generator checks independent of dependency downloads. +def generatorType = project(':api').tasks.named('generateMavenLegalFiles').get().class.superclass +def fixtureGenerator = tasks.register('mavenLegalTestFixture', generatorType) +def legalTests = tasks.register('testMavenLegalFiles') { + group = 'verification' + description = 'Checks Maven legal templates, source mappings and packaging edge cases.' + doLast { + def templates = file('dev/release/maven') + def expectFailure = { String message, Closure action -> + boolean failed = false + try { + action.call() + } catch (IllegalArgumentException error) { + assert error.message.contains(message): error.message + failed = true + } + assert failed: "Expected failure: ${message}" + } + ['LICENSE', 'NOTICE'].each { name -> + assert file(name).text.trim().startsWith(new File(templates, name).text.trim()): "Maven ${name} differs from the root prefix" + } + def normalizedRootNotice = file('NOTICE').text.replaceAll(/\s+/, ' ') + def modules = subprojects.findAll { it.tasks.findByName('generateMavenLegalFiles') != null } + modules.each { module -> + module.tasks.named('generateMavenLegalFiles').get().sourceNotices.get().each { name -> + new File(templates, "NOTICE.${name}").readLines().findAll { it.trim() }.each { line -> + assert normalizedRootNotice.contains(line.trim().replaceAll(/\s+/, ' ')): "Root NOTICE is missing ${name}: ${line}" + } + } + def shadow = module.tasks.findByName('shadowJar') + def slf4jPatterns = shadow == null ? [] : shadow.excludes.findAll { it.startsWith('org/slf4j/') } + if (!slf4jPatterns.isEmpty()) { + assert slf4jPatterns.toSet() == ['org/slf4j/**'].toSet(): "${module.path}: review legal exclusions when changing the SLF4J pattern" + assert module.tasks.named('generateBundledLegalFiles').get().excludedGroups.get().contains('org.slf4j'): "${module.path}: excluded SLF4J classes must not retain dependency notices" + } + } + def sourceOwners = modules.collectEntries { module -> + [(module): module.extensions.getByType(SourceSetContainer).getByName('main').allJava.files.collect { it.canonicalFile }.toSet()] + } + def reviewedWithoutNotice = [ + // These upstream trees contain LICENSE but no NOTICE: + // https://github.com/lance-format/lance-namespace/tree/v0.0.20 + // https://github.com/lance-format/lance-namespace-impls/tree/e3e202f2f655c92ee9427ab671226e1ed945a629 + 'Lance Namespace (https://github.com/lance-format/lance-namespace-impls)': [ + 'lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/JsonArrowSchemaConverter.java', + 'lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/PageUtil.java' + ], + // https://github.com/trinodb/trino/tree/435 has only a server dependency NOTICE, + // which does not apply to these copied classes. + 'Trino': [ + 'trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/hive/SortingColumn.java', + 'trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/util/json/AbstractTypedJacksonModule.java', + 'trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/util/json/BlockJsonSerde.java', + 'trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/util/json/TypeDeserializer.java', + 'trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/util/json/TypeSignatureDeserializer.java' + ] + ].collectEntries { component, paths -> [(component): paths.collect { file(it).canonicalFile }.toSet()] } + def sourceNoticeFor = { String component, Collection sources -> + def notice = templates.listFiles().find { it.name.startsWith('NOTICE.') && it.readLines().first().startsWith(component) } + if (notice == null && !reviewedWithoutNotice.getOrDefault(component, Collections.emptySet()).containsAll(sources)) { + throw new IllegalArgumentException("Review upstream NOTICE for ${component}: add a template or document the source-specific absence of an applicable notice") + } + notice + } + expectFailure('Review upstream NOTICE for Unknown source') { + sourceNoticeFor('Unknown source', [file('unknown/Source.java').canonicalFile]) + } + expectFailure('Review upstream NOTICE for Lance Namespace') { + sourceNoticeFor('Lance Namespace (https://github.com/lance-format/lance-namespace-impls)', [file('lance/NewSource.java').canonicalFile]) + } + file('LICENSE').text.split(/\n\s*\n/).each { section -> + def lines = section.readLines().collect { it.trim() } + def sources = lines.findAll { it.startsWith('./') && it.contains('/src/main/java/') }.collect { file(it).canonicalFile } + def compiledSources = sources.findAll { source -> sourceOwners.values().any { it.contains(source) } } + if (compiledSources.isEmpty()) return Review Comment: Fixed. Missing or unowned source paths now fail the check. The supported Trino skip option still works. ########## build.gradle.kts: ########## @@ -565,27 +798,111 @@ subprojects { from(tasks["javadoc"]) } - tasks.withType<Jar> { - into("META-INF") { - from(rootDir) { - if (name == "sourcesJar") { - include("LICENSE") - include("NOTICE") - } else if (project.name == "web") { - include("web/web/LICENSE.bin") - rename("LICENSE.bin", "LICENSE") - include("web/web/NOTICE.bin") - rename("NOTICE.bin", "NOTICE") - } else { - include("LICENSE.bin") - rename("LICENSE.bin", "LICENSE") - include("NOTICE.bin") - rename("NOTICE.bin", "NOTICE") + // These notices cover copied production sources, not dependencies declared only in the POM. + val sourceNoticeNames = mapOf( + ":api" to listOf("spark", "iceberg"), + ":common" to listOf("iceberg", "hadoop"), + ":core" to listOf("spark", "iceberg", "kafka", "aws"), + ":clients:client-java" to listOf("iceberg"), + ":catalogs:catalog-common" to listOf("doris"), + ":catalogs:hive-metastore-common" to listOf("iceberg"), + ":catalogs:catalog-lakehouse-iceberg" to listOf("iceberg"), + ":catalogs:catalog-lakehouse-paimon" to listOf("paimon"), + ":spark-connector:spark-3.5" to listOf("iceberg"), + ":spark-connector:spark-4.0" to listOf("iceberg"), + ":server-common" to listOf("hadoop"), + ":iceberg:iceberg-common" to listOf("iceberg"), + ":iceberg:iceberg-rest-server" to listOf("iceberg"), + ":authorizations:authorization-ranger" to listOf("ranger") + )[project.path].orEmpty() + + val mavenLegalFiles = tasks.register<GenerateJarLegalFiles>("generateMavenLegalFiles") { + templates.set(rootProject.layout.projectDirectory.dir("dev/release/maven")) + sourceNotices.set(sourceNoticeNames) + outputArchive.set(layout.buildDirectory.file("generated/maven-legal/main.zip")) + } + val javadocLegalFiles = tasks.register<GenerateJarLegalFiles>("generateJavadocLegalFiles") { + templates.set(rootProject.layout.projectDirectory.dir("dev/release/maven")) + // SparkTransformConverter's Iceberg-derived findWidth method is private and absent from Javadoc. + sourceNotices.set( + when (project.path) { + ":spark-connector:spark-3.5", ":spark-connector:spark-4.0" -> sourceNoticeNames.filterNot { it == "iceberg" } Review Comment: Kept the exclusion and added a single assertion that `findWidth` remains private, with a message to review the notice if it changes. -- 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]
