uschindler commented on a change in pull request #571: URL: https://github.com/apache/lucene/pull/571#discussion_r778931152
########## File path: gradle/java/modules.gradle ########## @@ -27,194 +29,167 @@ allprojects { modularity.inferModulePath.set(false) } - // Map convention configuration names to "modular" corresponding configurations. - Closure<String> moduleConfigurationNameFor = { String configurationName -> - return "module" + configurationName.capitalize().replace("Classpath", "Path") - } - - // - // For each source set, create explicit configurations for declaring modular dependencies. - // These "modular" configurations correspond 1:1 to Gradle's conventions but have a 'module' prefix - // and a capitalized remaining part of the conventional name. For example, an 'api' configuration in - // the main source set would have a corresponding 'moduleApi' configuration for declaring modular - // dependencies. - // - // Gradle's java plugin "convention" configurations extend from their modular counterparts - // so all dependencies end up on classpath by default for backward compatibility with other - // tasks and gradle infrastructure. // - // At the same time, we also know which dependencies (and their transitive graph of dependencies!) - // should be placed on module-path only. - // - // Note that an explicit configuration of modular dependencies also opens up the possibility of automatically - // validating whether the dependency configuration for a gradle project is consistent with the information in - // the module-info descriptor because there is a (nearly?) direct correspondence between the two: - // - // moduleApi - 'requires transitive' - // moduleImplementation - 'requires' - // moduleCompileOnly - 'requires static' + // Configure modular extensions for each source set. // project.sourceSets.all { SourceSet sourceSet -> - ConfigurationContainer configurations = project.configurations - - // Create modular configurations for convention configurations. - Closure<Configuration> createModuleConfigurationForConvention = { String configurationName -> - Configuration conventionConfiguration = configurations.maybeCreate(configurationName) - Configuration moduleConfiguration = configurations.maybeCreate(moduleConfigurationNameFor(configurationName)) - moduleConfiguration.canBeConsumed(false) - moduleConfiguration.canBeResolved(false) - conventionConfiguration.extendsFrom(moduleConfiguration) - - project.logger.info("Created module configuration for '${conventionConfiguration.name}': ${moduleConfiguration.name}") - return moduleConfiguration - } - - Configuration moduleApi = createModuleConfigurationForConvention(sourceSet.apiConfigurationName) - Configuration moduleImplementation = createModuleConfigurationForConvention(sourceSet.implementationConfigurationName) - moduleImplementation.extendsFrom(moduleApi) - Configuration moduleRuntimeOnly = createModuleConfigurationForConvention(sourceSet.runtimeOnlyConfigurationName) - Configuration moduleCompileOnly = createModuleConfigurationForConvention(sourceSet.compileOnlyConfigurationName) - // sourceSet.compileOnlyApiConfigurationName // This seems like a very esoteric use case, leave out. - - // Set up compilation module path configuration combining corresponding convention configurations. - Closure<Configuration> createResolvableModuleConfiguration = { String configurationName -> - Configuration conventionConfiguration = configurations.maybeCreate(configurationName) - Configuration moduleConfiguration = configurations.maybeCreate( - moduleConfigurationNameFor(conventionConfiguration.name)) - moduleConfiguration.canBeConsumed(false) - moduleConfiguration.canBeResolved(true) - moduleConfiguration.attributes { - // Prefer class folders over JARs. The exception is made for tests projects which require a composition - // of classes and resources, otherwise split into two folders. - if (project.name.endsWith(".tests")) { - attribute(LibraryElements.LIBRARY_ELEMENTS_ATTRIBUTE, objects.named(LibraryElements, LibraryElements.JAR)) - } else { - attribute(LibraryElements.LIBRARY_ELEMENTS_ATTRIBUTE, objects.named(LibraryElements, LibraryElements.CLASSES)) - } - } - - project.logger.info("Created resolvable module configuration for '${conventionConfiguration.name}': ${moduleConfiguration.name}") - return moduleConfiguration - } - - Configuration compileModulePathConfiguration = createResolvableModuleConfiguration(sourceSet.compileClasspathConfigurationName) - compileModulePathConfiguration.extendsFrom(moduleCompileOnly, moduleImplementation) - - Configuration runtimeModulePathConfiguration = createResolvableModuleConfiguration(sourceSet.runtimeClasspathConfigurationName) - runtimeModulePathConfiguration.extendsFrom(moduleRuntimeOnly, moduleImplementation) - // Create and register a source set extension for manipulating classpath/ module-path - ModularPathsExtension modularPaths = new ModularPathsExtension(project, sourceSet, - compileModulePathConfiguration, - runtimeModulePathConfiguration) + ModularPathsExtension modularPaths = new ModularPathsExtension(project, sourceSet) sourceSet.extensions.add("modularPaths", modularPaths) - // Customized the JavaCompile for this source set so that it has proper module path. + // LUCENE-10344: We have to provide a special-case extension for ECJ because it does not + // support all of the module-specific javac options. + ModularPathsExtension modularPathsForEcj = modularPaths + if (sourceSet.name == SourceSet.TEST_SOURCE_SET_NAME && project.path in [ + ":lucene:spatial-extras", + ":lucene:spatial3d", + ]) { + modularPathsForEcj = modularPaths.cloneWithMode(ModularPathsExtension.Mode.CLASSPATH_ONLY) + } + sourceSet.extensions.add("modularPathsForEcj", modularPathsForEcj) + + // TODO: the tests of these projects currently don't compile or work in + // module-path mode. Make the modular paths extension use class path only. + if (sourceSet.name == SourceSet.TEST_SOURCE_SET_NAME && project.path in [ + // Circular dependency between artifacts or source set outputs, + // causing package split issues at runtime. + ":lucene:core", + ":lucene:codecs", + ":lucene:test-framework", + ]) { + modularPaths.mode = ModularPathsExtension.Mode.CLASSPATH_ONLY + } + + // Configure the JavaCompile task associated with this source set. tasks.named(sourceSet.getCompileJavaTaskName()).configure({ JavaCompile task -> task.dependsOn modularPaths.compileModulePathConfiguration - // LUCENE-10327: don't allow gradle to emit an empty sourcepath as it would break compilation. + // LUCENE-10327: don't allow gradle to emit an empty sourcepath as it would break + // compilation of modules. task.options.setSourcepath(sourceSet.java.sourceDirectories) // Add modular dependencies and their transitive dependencies to module path. - task.options.compilerArgumentProviders.add((CommandLineArgumentProvider) { - def modularPathFiles = modularPaths.compileModulePathConfiguration.files - def extraArgs = [] - if (!modularPathFiles.isEmpty()) { - if (!modularPaths.hasModuleDescriptor()) { - // We're compiling a non-module so we'll bring everything on module path in - // otherwise things wouldn't be part of the resolved module graph. - extraArgs += ["--add-modules", "ALL-MODULE-PATH"] - } - - extraArgs += ["--module-path", modularPathFiles.join(File.pathSeparator)] - } - - task.logger.info("Module path for ${task.path}:\n " + modularPathFiles.sort().join("\n ")) - - return extraArgs - }) + task.options.compilerArgumentProviders.add(modularPaths.compilationArguments) // LUCENE-10304: if we modify the classpath here, IntelliJ no longer sees the dependencies as compile-time // dependencies, don't know why. if (!rootProject.ext.isIdea) { - // Modify the default classpath by removing anything already placed on module path. - // This could be done in a fancier way but a set difference is just fine for us here. Use a lazy - // provider to delay computation of the actual path. - task.classpath = files({ -> - def trimmedClasspath = sourceSet.compileClasspath - modularPaths.compileModulePathConfiguration - task.logger.info("Class path for ${task.path}:\n " + trimmedClasspath.files.sort().join("\n ")) - return trimmedClasspath - }) + task.classpath = modularPaths.compilationClasspath + } + + doFirst { + modularPaths.logCompilationPaths(logger) } }) + + // For source sets that contain a module descriptor, configure a jar task that combines + // classes and resources into a single module. + if (sourceSet.name != SourceSet.MAIN_SOURCE_SET_NAME) { + tasks.maybeCreate(sourceSet.getJarTaskName(), org.gradle.jvm.tasks.Jar).configure({ + archiveClassifier = sourceSet.name + from(sourceSet.output) + }) + } + } + + // Connect modular configurations between their "test" and "main" source sets, this reflects + // the conventions set by the Java plugin. + project.configurations { + moduleTestApi.extendsFrom moduleApi + moduleTestImplementation.extendsFrom moduleImplementation + moduleTestRuntimeOnly.extendsFrom moduleRuntimeOnly + moduleTestCompileOnly.extendsFrom moduleCompileOnly } + // Gradle's java plugin sets the compile and runtime classpath to be a combination + // of configuration dependencies and source set's outputs. For source sets with modules, + // this leads to split class and resource folders. // - // Configure the (default) test task to use module paths. + // We tweak the default source set path configurations here by assembling jar task outputs + // of the respective source set, instead of their source set output folders. We also attach + // the main source set's jar to the modular test implementation configuration. + SourceSet mainSourceSet = project.sourceSets.getByName(SourceSet.MAIN_SOURCE_SET_NAME) + boolean mainIsModular = mainSourceSet.modularPaths.hasModuleDescriptor() + boolean mainIsEmpty = mainSourceSet.allJava.isEmpty() + SourceSet testSourceSet = project.sourceSets.getByName(SourceSet.TEST_SOURCE_SET_NAME) + boolean testIsModular = testSourceSet.modularPaths.hasModuleDescriptor() + + // LUCENE-10304: if we modify the classpath here, IntelliJ no longer sees the dependencies as compile-time + // dependencies, don't know why. + if (!rootProject.ext.isIdea) { + def jarTask = project.tasks.getByName(mainSourceSet.getJarTaskName()) + def testJarTask = project.tasks.getByName(testSourceSet.getJarTaskName()) + + // Consider various combinations of module/classpath configuration between the main and test source set. + if (testIsModular) { + if (mainIsModular || mainIsEmpty) { + // If the main source set is empty, skip the jar task. + def jarTaskOutputs = mainIsEmpty ? [] : jarTask.outputs + + // Fully modular tests - must have no split packages, proper access, etc. + // Work around the split classes/resources problem by adjusting classpaths to + // rely on JARs rather than source set output folders. + testSourceSet.compileClasspath = project.objects.fileCollection().from( + jarTaskOutputs, + project.configurations.getByName(testSourceSet.getCompileClasspathConfigurationName()), + ) + testSourceSet.runtimeClasspath = project.objects.fileCollection().from( + jarTaskOutputs, + testJarTask.outputs, + project.configurations.getByName(testSourceSet.getRuntimeClasspathConfigurationName()), + ) + + project.dependencies { + moduleTestImplementation files(jarTaskOutputs) + moduleTestRuntimeOnly files(testJarTask.outputs) + } + } else { + // This combination simply does not make any sense (in my opinion). + throw GradleException("Test source set is modular and main source set is class-based, this makes no sense: " + project.path) + } + } else { + if (mainIsModular) { + // This combination is a potential candidate for patching the main sourceset's module with test classes. I could + // not resolve all the difficulties that arise when you try to do it though: + // - either a separate module descriptor is needed that opens test packages, adds dependencies via requires clauses + // or a series of jvm arguments (--add-reads, --add-opens, etc.) has to be generated and maintained. This is + // very low-level (ECJ doesn't support a full set of these instructions, for example). + // + // Fall back to classpath mode. + } else { + // This is the 'plain old classpath' mode: neither the main source set nor the test set are modular. + } + } + } + + // + // Configures a Test task associated with the provided source set to use module paths. // // There is no explicit connection between source sets and test tasks so there is no way (?) // to do this automatically, convention-style. - + // // This closure can be used to configure a different task, with a different source set, should we // have the need for it. Closure<Void> configureTestTaskForSourceSet = { Test task, SourceSet sourceSet -> task.configure { - Configuration modulePath = task.project.configurations.maybeCreate( - moduleConfigurationNameFor(sourceSet.getRuntimeClasspathConfigurationName())) + ModularPathsExtension modularPaths = sourceSet.modularPaths - task.dependsOn modulePath + dependsOn modularPaths // Add modular dependencies and their transitive dependencies to module path. - task.jvmArgumentProviders.add((CommandLineArgumentProvider) { - def extraArgs = [] - - // Determine whether the source set classes themselves should be appended - // to classpath or module path. - boolean sourceSetIsAModule = sourceSet.modularPaths.hasModuleDescriptor() - - if (!modulePath.isEmpty() || sourceSetIsAModule) { - if (sourceSetIsAModule) { - // Add source set outputs to module path. - extraArgs += ["--module-path", (modulePath + sourceSet.output.classesDirs).files.join(File.pathSeparator)] - // Ideally, we should only add the sourceset's module here, everything else would be resolved via the - // module descriptor. But this would require parsing the module descriptor and may cause JVM version conflicts - // so keeping it simple. - extraArgs += ["--add-modules", "ALL-MODULE-PATH"] - } else { - extraArgs += ["--module-path", modulePath.files.join(File.pathSeparator)] - // In this case we're running a non-module against things on the module path so let's bring in - // everything on module path into the resolution graph. - extraArgs += ["--add-modules", "ALL-MODULE-PATH"] - } - } + jvmArgumentProviders.add(modularPaths.runtimeArguments) - task.logger.info("Module path for ${task.path}:\n " + modulePath.files.sort().join("\n ")) - - return extraArgs - }) + // Modify the default classpath. + classpath = modularPaths.runtimeClasspath - - // Modify the default classpath by removing anything already placed on module path. - // This could be done in a fancier way but a set difference is just fine for us here. Use a lazy - // provider to delay computation of the actual path. - task.classpath = files({ -> - def trimmedClasspath = sourceSet.runtimeClasspath - modulePath - - boolean sourceSetIsAModule = sourceSet.modularPaths.hasModuleDescriptor() - if (sourceSetIsAModule) { - // also subtract the sourceSet's output directories. - trimmedClasspath = trimmedClasspath - sourceSet.output.classesDirs - } - - task.logger.info("Class path for ${task.path}:\n " + trimmedClasspath.files.sort().join("\n ")) - return trimmedClasspath - }) + doFirst { + modularPaths.logRuntimePaths(logger) + } } } - // Configure (tasks.test, sourceSets.test) - tasks.matching { it.name == "test" }.all { Test task -> + // Lazily configure (tasks.test, sourceSets.test) + tasks.matching { it.name ==~ /test(_[0-9]+)?/ }.all { Test task -> Review comment: We shoudl also check that instrumentation with Emma works. -- 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: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org