diff --git a/CHANGELOG.md b/CHANGELOG.md index 24d1cb9c..a4b08236 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Java Module Dependencies Gradle Plugin - Changelog +## Version 1.8 +* [#127](https://github.com/gradlex-org/java-module-dependencies/issues/127) Less configuration cache misses when modifying `module-info.java` (Thanks [TheGoesen](https://github.com/TheGoesen)) + ## Version 1.7.1 * Update module name mappings diff --git a/build.gradle.kts b/build.gradle.kts index acf3bbcb..fc2518c1 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -74,6 +74,11 @@ testing.suites.named("test") { systemProperty("gradleVersionUnderTest", gradleVersionUnderTest) exclude("**/*SamplesTest.class") // Not yet cross-version ready exclude("**/initialization/**") // Settings plugin only for Gradle 8.8+ + if (gradleVersionUnderTest == "7.4") { + // Configuration cache only "reliable" since 7.6 (?) + // https://github.com/gradlex-org/java-module-dependencies/issues/129 + exclude("**/configcache/**") + } } } } diff --git a/src/main/java/org/gradlex/javamodule/dependencies/JavaModuleDependenciesPlugin.java b/src/main/java/org/gradlex/javamodule/dependencies/JavaModuleDependenciesPlugin.java index 687a4b39..eec98333 100644 --- a/src/main/java/org/gradlex/javamodule/dependencies/JavaModuleDependenciesPlugin.java +++ b/src/main/java/org/gradlex/javamodule/dependencies/JavaModuleDependenciesPlugin.java @@ -229,14 +229,16 @@ private void setupOrderingCheckTasks(Project project, TaskProvider checkAl SourceSetContainer sourceSets = project.getExtensions().getByType(SourceSetContainer.class); ConfigurationContainer configurations = project.getConfigurations(); - sourceSets.all(sourceSet -> { + sourceSets.configureEach(sourceSet -> { TaskProvider checkModuleInfo = project.getTasks().register(sourceSet.getTaskName("check", "ModuleInfo"), ModuleDirectivesOrderingCheck.class, t -> { t.setGroup("java modules"); t.setDescription("Check order of directives in 'module-info.java' in '" + sourceSet.getName() + "' source set"); ModuleInfo moduleInfo = javaModuleDependencies.getModuleInfoCache().get().get(sourceSet, project.getProviders()); - - t.getModuleInfoPath().convention(moduleInfo.getFilePath().getAbsolutePath()); + File folder = javaModuleDependencies.getModuleInfoCache().get().getFolder(sourceSet, project.getProviders()); + if (folder != null) { + t.getModuleInfoPath().convention(new File(folder, "module-info.java").getAbsolutePath()); + } t.getModuleNamePrefix().convention(moduleInfo.moduleNamePrefix(project.getName(), sourceSet.getName(), false)); t.getModuleInfo().convention(moduleInfo); @@ -266,11 +268,11 @@ private void readModuleInfo(ModuleInfo.Directive moduleDirective, SourceSet sour } ModuleInfo moduleInfo = javaModuleDependenciesExtension.getModuleInfoCache().get().get(sourceSet, project.getProviders()); for (String moduleName : moduleInfo.get(moduleDirective)) { - declareDependency(moduleName, moduleInfo.getFilePath(), project, sourceSet, configuration, javaModuleDependenciesExtension); + declareDependency(moduleName, project, sourceSet, configuration, javaModuleDependenciesExtension); } } - private void declareDependency(String moduleName, File moduleInfoFile, Project project, SourceSet sourceSet, Configuration configuration, JavaModuleDependenciesExtension javaModuleDependencies) { + private void declareDependency(String moduleName, Project project, SourceSet sourceSet, Configuration configuration, JavaModuleDependenciesExtension javaModuleDependencies) { if (JDKInfo.MODULES.contains(moduleName)) { // The module is part of the JDK, no dependency required return; @@ -286,7 +288,7 @@ private List collectDepende File sourceSetDir = sourceSet.getJava().getSrcDirs().iterator().next().getParentFile(); File whiteboxModuleInfoFile = new File(sourceSetDir, "java9/module-info.java"); if (whiteboxModuleInfoFile.exists()) { - moduleInfo = new ModuleInfo(project.getProviders().fileContents(project.getLayout().getProjectDirectory().file(whiteboxModuleInfoFile.getAbsolutePath())).getAsText().get(), whiteboxModuleInfoFile); + moduleInfo = new ModuleInfo(project.getProviders().fileContents(project.getLayout().getProjectDirectory().file(whiteboxModuleInfoFile.getAbsolutePath())).getAsText().get()); } } return moduleInfo.get(directive).stream() diff --git a/src/main/java/org/gradlex/javamodule/dependencies/JavaModuleVersionsPlugin.java b/src/main/java/org/gradlex/javamodule/dependencies/JavaModuleVersionsPlugin.java index 6dc8d929..f0193cb5 100644 --- a/src/main/java/org/gradlex/javamodule/dependencies/JavaModuleVersionsPlugin.java +++ b/src/main/java/org/gradlex/javamodule/dependencies/JavaModuleVersionsPlugin.java @@ -132,7 +132,7 @@ private void registerCatalogTask(Project project) { moduleInfoFile = new File(srcDirSet, "java9/module-info.java"); } if (moduleInfoFile.exists()) { - ModuleInfo moduleInfo = new ModuleInfo(project.getProviders().fileContents(project.getLayout().getProjectDirectory().file(moduleInfoFile.getAbsolutePath())).getAsText().get(), moduleInfoFile); + ModuleInfo moduleInfo = new ModuleInfo(project.getProviders().fileContents(project.getLayout().getProjectDirectory().file(moduleInfoFile.getAbsolutePath())).getAsText().get()); t.getEntries().addAll(collectCatalogEntriesFromModuleInfos(javaModuleDependencies, moduleInfo.get(REQUIRES_TRANSITIVE))); t.getEntries().addAll(collectCatalogEntriesFromModuleInfos(javaModuleDependencies, moduleInfo.get(REQUIRES))); t.getEntries().addAll(collectCatalogEntriesFromModuleInfos(javaModuleDependencies, moduleInfo.get(REQUIRES_STATIC_TRANSITIVE))); diff --git a/src/main/java/org/gradlex/javamodule/dependencies/internal/utils/ModuleInfo.java b/src/main/java/org/gradlex/javamodule/dependencies/internal/utils/ModuleInfo.java index 5a3f6b1a..f4c362db 100644 --- a/src/main/java/org/gradlex/javamodule/dependencies/internal/utils/ModuleInfo.java +++ b/src/main/java/org/gradlex/javamodule/dependencies/internal/utils/ModuleInfo.java @@ -17,12 +17,12 @@ package org.gradlex.javamodule.dependencies.internal.utils; import javax.annotation.Nullable; -import java.io.File; import java.io.Serializable; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Objects; import static org.gradlex.javamodule.dependencies.internal.utils.ModuleNamingUtil.sourceSetToModuleName; @@ -43,7 +43,7 @@ public String literal() { public static final String RUNTIME_KEYWORD = "/*runtime*/"; - public static final ModuleInfo EMPTY = new ModuleInfo("", new File("")); + public static final ModuleInfo EMPTY = new ModuleInfo(""); private String moduleName = ""; private final List requires = new ArrayList<>(); @@ -52,12 +52,10 @@ public String literal() { private final List requiresStaticTransitive = new ArrayList<>(); private final List requiresRuntime = new ArrayList<>(); - private final File filePath; - public ModuleInfo(String moduleInfoFileContent, File filePath) { - this.filePath = filePath; + public ModuleInfo(String moduleInfoFileContent) { boolean insideComment = false; - for(String line: moduleInfoFileContent.split("\n")) { + for (String line : moduleInfoFileContent.split("\n")) { insideComment = parse(line, insideComment); } } @@ -108,10 +106,6 @@ public String moduleNamePrefix(String projectName, String sourceSetName, boolean return null; } - public File getFilePath() { - return filePath; - } - /** * @return true, if we are inside a multi-line comment after this line */ @@ -150,4 +144,29 @@ private boolean parse(String moduleLine, boolean insideComment) { } return moduleLine.lastIndexOf("/*") > moduleLine.lastIndexOf("*/"); } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ModuleInfo that = (ModuleInfo) o; + return Objects.equals(moduleName, that.moduleName) + && Objects.equals(requires, that.requires) + && Objects.equals(requiresTransitive, that.requiresTransitive) + && Objects.equals(requiresStatic, that.requiresStatic) + && Objects.equals(requiresStaticTransitive, that.requiresStaticTransitive) + && Objects.equals(requiresRuntime, that.requiresRuntime); + } + + @Override + public int hashCode() { + return Objects.hash( + moduleName, + requires, + requiresTransitive, + requiresStatic, + requiresStaticTransitive, + requiresRuntime + ); + } } diff --git a/src/main/java/org/gradlex/javamodule/dependencies/internal/utils/ModuleInfoCache.java b/src/main/java/org/gradlex/javamodule/dependencies/internal/utils/ModuleInfoCache.java index fc689b19..c9394087 100644 --- a/src/main/java/org/gradlex/javamodule/dependencies/internal/utils/ModuleInfoCache.java +++ b/src/main/java/org/gradlex/javamodule/dependencies/internal/utils/ModuleInfoCache.java @@ -16,7 +16,6 @@ package org.gradlex.javamodule.dependencies.internal.utils; -import org.gradle.api.file.RegularFileProperty; import org.gradle.api.logging.Logger; import org.gradle.api.model.ObjectFactory; import org.gradle.api.provider.Provider; @@ -69,6 +68,15 @@ public ModuleInfo get(SourceSet sourceSet, ProviderFactory providers) { return ModuleInfo.EMPTY; } + public File getFolder(SourceSet sourceSet, ProviderFactory providers) { + for (File folder : sourceSet.getJava().getSrcDirs()) { + if (maybePutModuleInfo(folder, providers)) { + return folder; + } + } + return null; + } + /** * @param projectRoot the project that should hold a Java module * @return parsed module-info.java for the given project assuming a standard Java project layout @@ -102,15 +110,17 @@ public String getCapability(String moduleName) { } private boolean maybePutModuleInfo(File folder, ProviderFactory providers) { - RegularFileProperty moduleInfoFile = getObjects().fileProperty(); - moduleInfoFile.set(new File(folder, "module-info.java")); - Provider moduleInfoContent = providers.fileContents(moduleInfoFile).getAsText(); - if (moduleInfoContent.isPresent()) { + Provider moduleInfoProvider = provideModuleInfo(folder, providers); + if (moduleInfoProvider.isPresent()) { if (!moduleInfo.containsKey(folder)) { - moduleInfo.put(folder, new ModuleInfo(moduleInfoContent.get(), moduleInfoFile.get().getAsFile())); + moduleInfo.put(folder, moduleInfoProvider.get() ); } return true; } return false; } + + private Provider provideModuleInfo(File folder, ProviderFactory providers) { + return providers.of(ValueSourceModuleInfo.class, spec -> spec.parameters(param -> param.getDir().set(folder))); + } } diff --git a/src/main/java/org/gradlex/javamodule/dependencies/internal/utils/ValueSourceModuleInfo.java b/src/main/java/org/gradlex/javamodule/dependencies/internal/utils/ValueSourceModuleInfo.java new file mode 100644 index 00000000..524d6b83 --- /dev/null +++ b/src/main/java/org/gradlex/javamodule/dependencies/internal/utils/ValueSourceModuleInfo.java @@ -0,0 +1,51 @@ +/* + * Copyright the GradleX team. + * + * Licensed 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. + */ + +package org.gradlex.javamodule.dependencies.internal.utils; + +import org.gradle.api.file.DirectoryProperty; +import org.gradle.api.provider.ValueSource; +import org.gradle.api.provider.ValueSourceParameters; +import org.jetbrains.annotations.Nullable; + +import java.io.File; +import java.io.IOException; +import java.util.Scanner; + +public abstract class ValueSourceModuleInfo implements ValueSource { + + interface Parameter extends ValueSourceParameters { + DirectoryProperty getDir(); + } + + @Override + public @Nullable ModuleInfo obtain() { + Parameter parameters = getParameters(); + File file = new File(parameters.getDir().get().getAsFile(), "module-info.java"); + if (file.isFile()) { + try { + try (Scanner scan = new Scanner(file)) { + scan.useDelimiter("\\Z"); + String content = scan.next(); + return new ModuleInfo(content); + } + } catch (IOException e) { + throw new RuntimeException(e); + } + } + return null; + } +} diff --git a/src/main/java/org/gradlex/javamodule/dependencies/tasks/ModuleDirectivesOrderingCheck.java b/src/main/java/org/gradlex/javamodule/dependencies/tasks/ModuleDirectivesOrderingCheck.java index 0ba5c10f..81006eeb 100644 --- a/src/main/java/org/gradlex/javamodule/dependencies/tasks/ModuleDirectivesOrderingCheck.java +++ b/src/main/java/org/gradlex/javamodule/dependencies/tasks/ModuleDirectivesOrderingCheck.java @@ -36,6 +36,7 @@ public abstract class ModuleDirectivesOrderingCheck extends DefaultTask { @Input + @Optional public abstract Property getModuleInfoPath(); @Input diff --git a/src/main/java/org/gradlex/javamodule/dependencies/tasks/ModulePathAnalysis.java b/src/main/java/org/gradlex/javamodule/dependencies/tasks/ModulePathAnalysis.java index bed5a2b0..19b434a1 100644 --- a/src/main/java/org/gradlex/javamodule/dependencies/tasks/ModulePathAnalysis.java +++ b/src/main/java/org/gradlex/javamodule/dependencies/tasks/ModulePathAnalysis.java @@ -75,7 +75,7 @@ public void report() throws IOException { if (file.exists()) { try(Stream lines = Files.lines(file.toPath())) { String fileContent = lines.collect(Collectors.joining("\n")); - ownModuleNamesPrefix = new ModuleInfo(fileContent, file).moduleNamePrefix(projectName, main.getName(), false); + ownModuleNamesPrefix = new ModuleInfo(fileContent).moduleNamePrefix(projectName, main.getName(), false); } break; } diff --git a/src/main/resources/org/gradlex/javamodule/dependencies/unique_modules.properties b/src/main/resources/org/gradlex/javamodule/dependencies/unique_modules.properties index 097059fd..a45f3e56 100644 --- a/src/main/resources/org/gradlex/javamodule/dependencies/unique_modules.properties +++ b/src/main/resources/org/gradlex/javamodule/dependencies/unique_modules.properties @@ -1151,7 +1151,7 @@ com.github.gv2011.util.gcol=com.github.gv2011:util-gcol com.github.gv2011.util.html.imp=com.github.gv2011:util-html com.github.gv2011.util.image=com.github.gv2011:util-image com.github.gv2011.util.json.imp=com.github.gv2011:util-json -com.github.gv2011.util.swing=com.github.gv2011:util-swing +com.github.gv2011.util.swing.imp=com.github.gv2011:util-swing com.github.gv2011.util.tika=com.github.gv2011:util-tika com.github.gw2toolbelt.gw2ml=com.github.gw2toolbelt.gw2ml:gw2ml com.github.hanshsieh.pixivj=com.github.hanshsieh:pixivj diff --git a/src/test/groovy/org/gradlex/javamodule/dependencies/test/ModuleInfoParseTest.groovy b/src/test/groovy/org/gradlex/javamodule/dependencies/test/ModuleInfoParseTest.groovy index 437c16a1..f2d35426 100644 --- a/src/test/groovy/org/gradlex/javamodule/dependencies/test/ModuleInfoParseTest.groovy +++ b/src/test/groovy/org/gradlex/javamodule/dependencies/test/ModuleInfoParseTest.groovy @@ -18,7 +18,7 @@ class ModuleInfoParseTest extends Specification { // requires com.bla.blub; requires transitive foo.bar.la; } - ''', new File('')) + ''') expect: moduleInfo.moduleNamePrefix("thing", "main", false) == "some" @@ -35,7 +35,7 @@ class ModuleInfoParseTest extends Specification { module some.thing { // module some.thing.else requires transitive foo.bar.la; } - ''', new File('')) + ''') expect: moduleInfo.moduleNamePrefix("thing", "main", false) == "some" @@ -55,7 +55,7 @@ class ModuleInfoParseTest extends Specification { */ requires static foo.bar.la; } - ''', new File('')) + ''') expect: moduleInfo.get(REQUIRES) == [] @@ -75,7 +75,7 @@ class ModuleInfoParseTest extends Specification { requires only.a.comment */ } - ''', new File('')) + ''') expect: moduleInfo.get(REQUIRES) == ["foo.bar.li"] @@ -91,7 +91,7 @@ class ModuleInfoParseTest extends Specification { module some.thing { requires /*runtime*/ foo.bar.lo; } - ''', new File('')) + ''') expect: moduleInfo.get(REQUIRES) == [] diff --git a/src/test/groovy/org/gradlex/javamodule/dependencies/test/configcache/ConfigurationCacheTest.groovy b/src/test/groovy/org/gradlex/javamodule/dependencies/test/configcache/ConfigurationCacheTest.groovy new file mode 100644 index 00000000..31f7f1d8 --- /dev/null +++ b/src/test/groovy/org/gradlex/javamodule/dependencies/test/configcache/ConfigurationCacheTest.groovy @@ -0,0 +1,96 @@ +package org.gradlex.javamodule.dependencies.test.configcache + +import org.gradlex.javamodule.dependencies.test.fixture.GradleBuild +import spock.lang.Specification + +import static org.gradle.util.GradleVersion.version + +class ConfigurationCacheTest extends Specification { + + @Delegate + GradleBuild build = new GradleBuild() + + final noCacheMessage = !gradleVersionUnderTest || version(gradleVersionUnderTest) >= version("8.8") + ? "Calculating task graph as no cached configuration is available for tasks: :app:compileJava" + : "Calculating task graph as no configuration cache is available for tasks: :app:compileJava" + + def "configurationCacheHit"() { + given: + libModuleInfoFile << 'module abc.lib { }' + + appModuleInfoFile << ''' + module abc.app { + requires abc.lib; + } + ''' + + def runner = runner('--configuration-cache',':app:compileJava') + when: + def result = runner.build() + + then: + result.output.contains(noCacheMessage) + + when: + result = runner.build() + + then: + result.output.contains("Reusing configuration cache.") + } + + def "configurationCacheHitIrrelevantChange"() { + libModuleInfoFile << 'module abc.lib { }' + appModuleInfoFile << ''' + module abc.app { + requires abc.lib; + } + ''' + + def runner = runner('--configuration-cache',':app:compileJava') + when: + def result = runner.build() + + then: + result.output.contains(noCacheMessage) + + when: + appModuleInfoFile.write(''' + module abc.app { + requires abc.lib; //This is a comment and should not break the configurationCache + } + ''') + result = runner.build() + + then: + result.output.contains("Reusing configuration cache.") + } + + def "configurationCacheMissRelevantChange"() { + given: + libModuleInfoFile << 'module abc.lib { }' + appModuleInfoFile << ''' + module abc.app { + requires abc.lib; + } + ''' + + def runner = runner('--configuration-cache',':app:compileJava') + when: + def result = runner.build() + + then: + result.output.contains(noCacheMessage) + + when: + appModuleInfoFile.write(''' + module abc.app { + //dependency removed; so thats indeed a configuration change + } + ''') + result = runner.build() + + then: + result.output.contains("Calculating task graph as configuration cache cannot be reused because a build logic input of type 'ValueSourceModuleInfo' has changed.\n") + } + +} diff --git a/src/test/groovy/org/gradlex/javamodule/dependencies/test/initialization/SettingsPluginTest.groovy b/src/test/groovy/org/gradlex/javamodule/dependencies/test/initialization/SettingsPluginTest.groovy index e88379bb..1f578d11 100644 --- a/src/test/groovy/org/gradlex/javamodule/dependencies/test/initialization/SettingsPluginTest.groovy +++ b/src/test/groovy/org/gradlex/javamodule/dependencies/test/initialization/SettingsPluginTest.groovy @@ -67,6 +67,104 @@ class SettingsPluginTest extends Specification { result.task(":lib:compileJava").outcome == SUCCESS } + def "configurationCacheHit"() { + given: + settingsFile << ''' + javaModules { + directory(".") { plugin("java-library") } + } + ''' + libModuleInfoFile << 'module abc.lib { }' + appModuleInfoFile << ''' + module org.gradlex.test.app { + requires abc.lib; + } + ''' + + + def runner = runner(':app:compileJava') + when: + def result = runner.build() + + then: + result.getOutput().contains("Calculating task graph as no cached configuration is available for tasks: :app:compileJava") + + when: + runner.build() // https://github.com/gradlex-org/java-module-dependencies/issues/128 + result = runner.build() + + then: + result.getOutput().contains("Reusing configuration cache.") + } + + def "configurationCacheHitIrrelevantChange"() { + given: + settingsFile << ''' + javaModules { + directory(".") { plugin("java-library") } + } + ''' + libModuleInfoFile << 'module abc.lib { }' + appModuleInfoFile << ''' + module org.gradlex.test.app { + requires abc.lib; + } + ''' + + def runner = runner(':app:compileJava') + when: + def result = runner.build() + + then: + result.getOutput().contains("Calculating task graph as no cached configuration is available for tasks: :app:compileJava") + + when: + runner.build() // https://github.com/gradlex-org/java-module-dependencies/issues/128 + appModuleInfoFile.write(''' + module org.gradlex.test.app { + requires abc.lib; //This is a comment and should not break the configurationCache + } + ''') + result = runner.build() + + then: + result.getOutput().contains("Reusing configuration cache.") + } + + def "configurationCacheMissRelevantChange"() { + given: + settingsFile << ''' + javaModules { + directory(".") { plugin("java-library") } + } + ''' + libModuleInfoFile << 'module abc.lib { }' + appModuleInfoFile << ''' + module org.gradlex.test.app { + requires abc.lib; + } + ''' + + def runner = runner(':app:compileJava') + when: + def result = runner.build() + + then: + result.getOutput().contains("Calculating task graph as no cached configuration is available for tasks: :app:compileJava") + + when: + runner.build() // https://github.com/gradlex-org/java-module-dependencies/issues/128 + appModuleInfoFile.write(''' + module org.gradlex.test.app { + //dependency removed; so thats indeed a configuration change + } + ''') + result = runner.build() + + then: + result.output.contains("Calculating task graph as configuration cache cannot be reused because a build logic input of type 'ValueSourceModuleInfo' has changed.\n") + } + def "automatically sets module for application plugin"() { given: settingsFile << '''