diff --git a/subprojects/build-cache/src/integTest/groovy/org/gradle/caching/configuration/BuildCacheCompositeConfigurationIntegrationTest.groovy b/subprojects/build-cache/src/integTest/groovy/org/gradle/caching/configuration/BuildCacheCompositeConfigurationIntegrationTest.groovy index e9a0c7fa34dd..6e117c5376fd 100644 --- a/subprojects/build-cache/src/integTest/groovy/org/gradle/caching/configuration/BuildCacheCompositeConfigurationIntegrationTest.groovy +++ b/subprojects/build-cache/src/integTest/groovy/org/gradle/caching/configuration/BuildCacheCompositeConfigurationIntegrationTest.groovy @@ -21,6 +21,7 @@ import org.gradle.integtests.fixtures.AbstractIntegrationSpec import org.gradle.integtests.fixtures.BuildOperationsFixture import org.gradle.integtests.fixtures.TestBuildCache import org.gradle.internal.operations.trace.BuildOperationTrace +import spock.lang.Issue /** * Tests build cache configuration within composite builds and buildSrc. */ @@ -29,7 +30,9 @@ class BuildCacheCompositeConfigurationIntegrationTest extends AbstractIntegratio def operations = new BuildOperationsFixture(executer, testDirectoryProvider) def setup() { - executer.withBuildCacheEnabled() + executer.beforeExecute { + withBuildCacheEnabled() + } } def "can configure with settings.gradle"() { @@ -96,7 +99,7 @@ class BuildCacheCompositeConfigurationIntegrationTest extends AbstractIntegratio and: def finalizeOps = operations.all(FinalizeBuildCacheConfigurationBuildOperationType) - finalizeOps.size() == 2 + finalizeOps.size() == 5 def pathToCacheDirMap = finalizeOps.collectEntries { [ it.details.buildPath, @@ -106,10 +109,55 @@ class BuildCacheCompositeConfigurationIntegrationTest extends AbstractIntegratio pathToCacheDirMap == [ ":": mainCache.cacheDir, + ":i1": mainCache.cacheDir, + ":i1:buildSrc": mainCache.cacheDir, + ":i2": mainCache.cacheDir, ":buildSrc": buildSrcCache.cacheDir ] } + @Issue("https://github.com/gradle/gradle/issues/4216") + def "build cache service is closed only after all included builds are finished"() { + def localCache = new TestBuildCache(file("local-cache")) + + buildTestFixture.withBuildInSubDir() + ['i1', 'i2'].each { + multiProjectBuild(it, ['first', 'second']) { + buildFile << """ + gradle.startParameter.setTaskNames(['build']) + println gradle + allprojects { + apply plugin: 'java-library' + } + """ + } + } + + settingsFile << localCache.localCacheConfiguration() << """ + includeBuild "i1" + includeBuild "i2" + """ + buildFile << """ + apply plugin: 'java' + println gradle + gradle.startParameter.setTaskNames(['build']) + processResources { + dependsOn gradle.includedBuild('i1').task(':processResources') + dependsOn gradle.includedBuild('i2').task(':processResources') + } + """ + + expect: + succeeds '--parallel' + + when: + ['i1', 'i2'].each { + file("$it/src/test/java/DummyTest.java") << "public class DummyTest {}" + } + then: + succeeds '--parallel' + } + private static String customTaskCode(String val = "foo") { """ @CacheableTask diff --git a/subprojects/build-cache/src/main/java/org/gradle/caching/internal/controller/RootBuildCacheConfigurationRef.java b/subprojects/build-cache/src/main/java/org/gradle/caching/internal/controller/RootBuildCacheConfigurationRef.java new file mode 100644 index 000000000000..157015c48609 --- /dev/null +++ b/subprojects/build-cache/src/main/java/org/gradle/caching/internal/controller/RootBuildCacheConfigurationRef.java @@ -0,0 +1,43 @@ +/* + * Copyright 2017 the original author or authors. + * + * 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.gradle.caching.internal.controller; + +import org.gradle.caching.configuration.internal.BuildCacheConfigurationInternal; + +public class RootBuildCacheConfigurationRef { + + private BuildCacheConfigurationInternal buildCacheConfiguration; + + public void set(BuildCacheConfigurationInternal buildCacheConfiguration) { + // This instance ends up in build/gradle scoped services for nesteds + // We don't want to invoke close at that time. + // Instead, close it at the root. + this.buildCacheConfiguration = buildCacheConfiguration; + } + + public BuildCacheConfigurationInternal getForNonRootBuild() { + if (!isSet()) { + throw new IllegalStateException("Root build cache configuration not yet assigned"); + } + + return buildCacheConfiguration; + } + + public boolean isSet() { + return buildCacheConfiguration != null; + } +} diff --git a/subprojects/build-cache/src/main/java/org/gradle/caching/internal/controller/RootBuildCacheControllerRef.java b/subprojects/build-cache/src/main/java/org/gradle/caching/internal/controller/RootBuildCacheControllerRef.java deleted file mode 100644 index f6170d22d6e5..000000000000 --- a/subprojects/build-cache/src/main/java/org/gradle/caching/internal/controller/RootBuildCacheControllerRef.java +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Copyright 2017 the original author or authors. - * - * 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.gradle.caching.internal.controller; - -import javax.annotation.Nullable; - -public class RootBuildCacheControllerRef { - - private BuildCacheController buildCacheController; - - public void set(BuildCacheController buildCacheController) { - // This instance ends up in build/gradle scoped services for nesteds - // We don't want to invoke close at that time. - // Instead, close it at the root. - this.buildCacheController = new CloseShieldBuildCacheController(buildCacheController); - } - - public BuildCacheController getForNonRootBuild() { - if (!isSet()) { - throw new IllegalStateException("Root build cache controller not yet assigned"); - } - - return buildCacheController; - } - - public boolean isSet() { - return buildCacheController != null; - } - - private static class CloseShieldBuildCacheController implements BuildCacheController { - private final BuildCacheController delegate; - - private CloseShieldBuildCacheController(BuildCacheController delegate) { - this.delegate = delegate; - } - - @Override - @Nullable - public T load(BuildCacheLoadCommand command) { - return delegate.load(command); - } - - @Override - public void store(BuildCacheStoreCommand command) { - delegate.store(command); - } - - @Override - public void close() { - } - } - -} diff --git a/subprojects/build-cache/src/main/java/org/gradle/caching/local/internal/DirectoryBuildCacheService.java b/subprojects/build-cache/src/main/java/org/gradle/caching/local/internal/DirectoryBuildCacheService.java index 3d71a58165a7..2e23e23ec86d 100644 --- a/subprojects/build-cache/src/main/java/org/gradle/caching/local/internal/DirectoryBuildCacheService.java +++ b/subprojects/build-cache/src/main/java/org/gradle/caching/local/internal/DirectoryBuildCacheService.java @@ -46,7 +46,7 @@ public class DirectoryBuildCacheService implements LocalBuildCacheService, Build private final PersistentCache persistentCache; private final BuildCacheTempFileStore tempFileStore; private final String failedFileSuffix; - private final ReadWriteLock lock = new ReentrantReadWriteLock(); + private static final ReadWriteLock LOCK = new ReentrantReadWriteLock(); public DirectoryBuildCacheService(PathKeyFileStore fileStore, PersistentCache persistentCache, BuildCacheTempFileStore tempFileStore, String failedFileSuffix) { this.fileStore = fileStore; @@ -96,11 +96,11 @@ public void loadLocally(final BuildCacheKey key, final Action read persistentCache.withFileLock(new Runnable() { @Override public void run() { - lock.readLock().lock(); + LOCK.readLock().lock(); try { loadInsideLock(key, reader); } finally { - lock.readLock().unlock(); + LOCK.readLock().unlock(); } } }); @@ -157,11 +157,11 @@ public void storeLocally(final BuildCacheKey key, final File file) { persistentCache.withFileLock(new Runnable() { @Override public void run() { - lock.writeLock().lock(); + LOCK.writeLock().lock(); try { storeInsideLock(key, file); } finally { - lock.writeLock().unlock(); + LOCK.writeLock().unlock(); } } }); diff --git a/subprojects/build-cache/src/main/java/org/gradle/caching/local/internal/DirectoryBuildCacheServiceFactory.java b/subprojects/build-cache/src/main/java/org/gradle/caching/local/internal/DirectoryBuildCacheServiceFactory.java index c26edf4fa790..379131cbfba4 100644 --- a/subprojects/build-cache/src/main/java/org/gradle/caching/local/internal/DirectoryBuildCacheServiceFactory.java +++ b/subprojects/build-cache/src/main/java/org/gradle/caching/local/internal/DirectoryBuildCacheServiceFactory.java @@ -102,7 +102,9 @@ private static void checkDirectory(File directory) { } } else { if (!directory.mkdirs()) { - throw new UncheckedIOException(String.format("Could not create cache directory: %s", directory)); + if (!directory.isDirectory()) { + throw new UncheckedIOException(String.format("Could not create cache directory: %s", directory)); + } } } } diff --git a/subprojects/core/src/main/java/org/gradle/caching/internal/BuildCacheServices.java b/subprojects/core/src/main/java/org/gradle/caching/internal/BuildCacheServices.java index 858f5e39b2c8..d9d00b9a1e02 100644 --- a/subprojects/core/src/main/java/org/gradle/caching/internal/BuildCacheServices.java +++ b/subprojects/core/src/main/java/org/gradle/caching/internal/BuildCacheServices.java @@ -20,7 +20,7 @@ import org.gradle.caching.configuration.internal.BuildCacheServiceRegistration; import org.gradle.caching.configuration.internal.DefaultBuildCacheConfiguration; import org.gradle.caching.configuration.internal.DefaultBuildCacheServiceRegistration; -import org.gradle.caching.internal.controller.RootBuildCacheControllerRef; +import org.gradle.caching.internal.controller.RootBuildCacheConfigurationRef; import org.gradle.caching.internal.tasks.BuildCacheTaskServices; import org.gradle.caching.local.DirectoryBuildCache; import org.gradle.caching.local.internal.DirectoryBuildCacheFileStoreFactory; @@ -42,8 +42,8 @@ public final class BuildCacheServices extends AbstractPluginServiceRegistry { @Override public void registerBuildTreeServices(ServiceRegistration registration) { registration.addProvider(new Object() { - RootBuildCacheControllerRef createRootBuildCacheControllerRef() { - return new RootBuildCacheControllerRef(); + RootBuildCacheConfigurationRef createRootBuildCacheConfigurationRef() { + return new RootBuildCacheConfigurationRef(); } }); } diff --git a/subprojects/core/src/main/java/org/gradle/caching/internal/tasks/BuildCacheTaskServices.java b/subprojects/core/src/main/java/org/gradle/caching/internal/tasks/BuildCacheTaskServices.java index 66a4bf2a7602..617e2ae2ab67 100644 --- a/subprojects/core/src/main/java/org/gradle/caching/internal/tasks/BuildCacheTaskServices.java +++ b/subprojects/core/src/main/java/org/gradle/caching/internal/tasks/BuildCacheTaskServices.java @@ -29,7 +29,7 @@ import org.gradle.caching.internal.controller.BuildCacheControllerFactory; import org.gradle.caching.internal.controller.BuildCacheControllerFactory.BuildCacheMode; import org.gradle.caching.internal.controller.BuildCacheControllerFactory.RemoteAccessMode; -import org.gradle.caching.internal.controller.RootBuildCacheControllerRef; +import org.gradle.caching.internal.controller.RootBuildCacheConfigurationRef; import org.gradle.caching.internal.tasks.origin.TaskOutputOriginFactory; import org.gradle.initialization.buildsrc.BuildSourceBuilder; import org.gradle.internal.SystemProperties; @@ -85,19 +85,17 @@ BuildCacheController createBuildCacheController( BuildOperationExecutor buildOperationExecutor, InstantiatorFactory instantiatorFactory, GradleInternal gradle, - RootBuildCacheControllerRef rootControllerRef + RootBuildCacheConfigurationRef rootConfigurationRef ) { - if (isRoot(gradle) || isRootBuildSrc(gradle) || isGradleBuildTaskRoot(rootControllerRef)) { - return doCreateBuildCacheController(serviceRegistry, buildCacheConfiguration, buildOperationExecutor, instantiatorFactory, gradle); - } else { - // must be an included build - return rootControllerRef.getForNonRootBuild(); - } + BuildCacheConfigurationInternal usedConfiguration = + isRoot(gradle) || isRootBuildSrc(gradle) || isGradleBuildTaskRoot(rootConfigurationRef) + ? buildCacheConfiguration : rootConfigurationRef.getForNonRootBuild(); + return doCreateBuildCacheController(serviceRegistry, usedConfiguration, buildOperationExecutor, instantiatorFactory, gradle); } - private boolean isGradleBuildTaskRoot(RootBuildCacheControllerRef rootControllerRef) { + private boolean isGradleBuildTaskRoot(RootBuildCacheConfigurationRef rootControllerRef) { // GradleBuild tasks operate with their own build session and tree scope. - // Therefore, they have their own RootBuildCacheControllerRef. + // Therefore, they have their own RootBuildCacheConfigurationRef. // This prevents them from reusing the build cache configuration defined by the root. // There is no way to detect that a Gradle instance represents a GradleBuild invocation. // If there were, that would be a better heuristic than this. diff --git a/subprojects/core/src/main/java/org/gradle/initialization/RootBuildCacheControllerSettingsProcessor.java b/subprojects/core/src/main/java/org/gradle/initialization/RootBuildCacheControllerSettingsProcessor.java index 738616282641..80e5589c34e3 100644 --- a/subprojects/core/src/main/java/org/gradle/initialization/RootBuildCacheControllerSettingsProcessor.java +++ b/subprojects/core/src/main/java/org/gradle/initialization/RootBuildCacheControllerSettingsProcessor.java @@ -20,8 +20,8 @@ import org.gradle.api.internal.GradleInternal; import org.gradle.api.internal.SettingsInternal; import org.gradle.api.internal.initialization.ClassLoaderScope; -import org.gradle.caching.internal.controller.BuildCacheController; -import org.gradle.caching.internal.controller.RootBuildCacheControllerRef; +import org.gradle.caching.configuration.internal.BuildCacheConfigurationInternal; +import org.gradle.caching.internal.controller.RootBuildCacheConfigurationRef; public class RootBuildCacheControllerSettingsProcessor implements SettingsProcessor { @@ -36,13 +36,13 @@ public SettingsInternal process(GradleInternal gradle, SettingsLocation settings SettingsInternal settings = delegate.process(gradle, settingsLocation, buildRootClassLoaderScope, startParameter); // The strategy for sharing build cache configuration across included builds in a composite, - // requires that the cache configuration be finalized (and cache controller available) + // requires that the cache configuration be finalized // before configuring them. This achieves that. if (gradle.getParent() == null) { - BuildCacheController rootController = gradle.getServices().get(BuildCacheController.class); - RootBuildCacheControllerRef rootControllerRef = gradle.getServices().get(RootBuildCacheControllerRef.class); - rootControllerRef.set(rootController); + BuildCacheConfigurationInternal rootConfiguration = gradle.getServices().get(BuildCacheConfigurationInternal.class); + RootBuildCacheConfigurationRef rootConfigurationRef = gradle.getServices().get(RootBuildCacheConfigurationRef.class); + rootConfigurationRef.set(rootConfiguration); } return settings;