Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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"() {
Expand Down Expand Up @@ -96,7 +99,7 @@ class BuildCacheCompositeConfigurationIntegrationTest extends AbstractIntegratio

and:
def finalizeOps = operations.all(FinalizeBuildCacheConfigurationBuildOperationType)
finalizeOps.size() == 2
finalizeOps.size() == 5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a break for build scans.

We either need to roll this back (e.g. don't fire this event for non root builds), or we have to update build scans to ignore these events.

def pathToCacheDirMap = finalizeOps.collectEntries {
[
it.details.buildPath,
Expand All @@ -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'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need --parallel as the tasks for included builds are run in parallel anyway.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, @adammurdoch. Removed.


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
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -96,11 +96,11 @@ public void loadLocally(final BuildCacheKey key, final Action<? super File> 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();
}
}
});
Expand Down Expand Up @@ -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();
}
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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();
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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;
Expand Down