diff --git a/subprojects/composite-builds/src/integTest/groovy/org/gradle/integtests/composite/CompositeBuildPropertiesIntegrationTest.groovy b/subprojects/composite-builds/src/integTest/groovy/org/gradle/integtests/composite/CompositeBuildPropertiesIntegrationTest.groovy new file mode 100644 index 000000000000..2fe408f77374 --- /dev/null +++ b/subprojects/composite-builds/src/integTest/groovy/org/gradle/integtests/composite/CompositeBuildPropertiesIntegrationTest.groovy @@ -0,0 +1,59 @@ +/* + * Copyright 2020 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.integtests.composite + +import org.gradle.integtests.fixtures.AbstractIntegrationSpec +import org.gradle.integtests.fixtures.executer.GradleContextualExecuter +import spock.lang.IgnoreIf + +@IgnoreIf({ GradleContextualExecuter.isInstant() }) +class CompositeBuildPropertiesIntegrationTest extends AbstractIntegrationSpec { + + def "included build properties take precedence over root build properties"() { + given: + createDir("included") { + file("gradle.properties") << """ + theProperty=included + """ + file("settings.gradle") << """ + println("included settings script: " + theProperty) + """ + file("build.gradle") << """ + println("included build script: " + theProperty) + """ + } + file("gradle.properties") << """ + theProperty=root + """ + settingsFile << """ + includeBuild 'included' + println("root settings script: " + theProperty) + """ + buildFile << """ + println("root build script: " + theProperty) + """ + + when: + run('help') + + then: + outputContains("root settings script: root") + outputContains("root build script: root") + outputContains("included settings script: included") + outputContains("included build script: included") + } +} diff --git a/subprojects/core/src/main/java/org/gradle/initialization/DefaultGradlePropertiesController.java b/subprojects/core/src/main/java/org/gradle/initialization/DefaultGradlePropertiesController.java new file mode 100644 index 000000000000..79856430eaf4 --- /dev/null +++ b/subprojects/core/src/main/java/org/gradle/initialization/DefaultGradlePropertiesController.java @@ -0,0 +1,114 @@ +/* + * Copyright 2020 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.initialization; + +import org.gradle.api.internal.properties.GradleProperties; + +import javax.annotation.Nullable; +import java.io.File; +import java.util.Map; + +public class DefaultGradlePropertiesController implements GradlePropertiesController { + + private State state = new NotLoaded(); + private final GradleProperties sharedGradleProperties = new SharedGradleProperties(); + private final IGradlePropertiesLoader propertiesLoader; + + public DefaultGradlePropertiesController(IGradlePropertiesLoader propertiesLoader) { + this.propertiesLoader = propertiesLoader; + } + + @Override + public GradleProperties getGradleProperties() { + return sharedGradleProperties; + } + + @Override + public void loadGradlePropertiesFrom(File settingsDir) { + state = state.loadGradlePropertiesFrom(settingsDir); + } + + private class SharedGradleProperties implements GradleProperties { + + @Nullable + @Override + public String find(String propertyName) { + return gradleProperties().find(propertyName); + } + + @Override + public Map mergeProperties(Map properties) { + return gradleProperties().mergeProperties(properties); + } + + private GradleProperties gradleProperties() { + return state.getGradleProperties(); + } + } + + private interface State { + + GradleProperties getGradleProperties(); + + State loadGradlePropertiesFrom(File settingsDir); + } + + private class NotLoaded implements State { + + @Override + public GradleProperties getGradleProperties() { + throw new IllegalStateException("GradleProperties has not been loaded yet."); + } + + @Override + public State loadGradlePropertiesFrom(File settingsDir) { + return new Loaded( + propertiesLoader.loadGradleProperties(settingsDir), + settingsDir + ); + } + } + + private static class Loaded implements State { + + private final GradleProperties gradleProperties; + private final File propertiesDir; + + public Loaded(GradleProperties gradleProperties, File propertiesDir) { + this.gradleProperties = gradleProperties; + this.propertiesDir = propertiesDir; + } + + @Override + public GradleProperties getGradleProperties() { + return gradleProperties; + } + + @Override + public State loadGradlePropertiesFrom(File settingsDir) { + if (!propertiesDir.equals(settingsDir)) { + throw new IllegalStateException( + String.format( + "GradleProperties has already been loaded from '%s' and cannot be loaded from '%s'.", + propertiesDir, settingsDir + ) + ); + } + return this; + } + } +} diff --git a/subprojects/core/src/main/java/org/gradle/initialization/GradlePropertiesController.java b/subprojects/core/src/main/java/org/gradle/initialization/GradlePropertiesController.java new file mode 100644 index 000000000000..a436c5e5ca62 --- /dev/null +++ b/subprojects/core/src/main/java/org/gradle/initialization/GradlePropertiesController.java @@ -0,0 +1,45 @@ +/* + * Copyright 2020 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.initialization; + +import org.gradle.api.internal.properties.GradleProperties; + +import java.io.File; + +/** + * Controls the state (not loaded / loaded) of the attached {@link GradleProperties} instance + * so that the set of Gradle properties is deterministically loaded only once per build. + */ +public interface GradlePropertiesController { + + /** + * The {@link GradleProperties} instance attached to this service. + */ + GradleProperties getGradleProperties(); + + /** + * Loads the immutable set of {@link GradleProperties} from the given directory and + * makes it available to the build. + * + * This method should be called only once per build but multiple calls with the + * same argument are allowed. + * + * @param settingsDir directory where to look for the {@code gradle.properties} file + * @throws IllegalStateException if called with a different argument in the same build + */ + void loadGradlePropertiesFrom(File settingsDir); +} diff --git a/subprojects/core/src/main/java/org/gradle/initialization/PropertiesLoadingSettingsProcessor.java b/subprojects/core/src/main/java/org/gradle/initialization/PropertiesLoadingSettingsProcessor.java new file mode 100644 index 000000000000..675567f844b7 --- /dev/null +++ b/subprojects/core/src/main/java/org/gradle/initialization/PropertiesLoadingSettingsProcessor.java @@ -0,0 +1,47 @@ +/* + * Copyright 2020 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.initialization; + +import org.gradle.StartParameter; +import org.gradle.api.internal.GradleInternal; +import org.gradle.api.internal.SettingsInternal; +import org.gradle.api.internal.initialization.ClassLoaderScope; + +public class PropertiesLoadingSettingsProcessor implements SettingsProcessor { + + private final SettingsProcessor delegate; + private final GradlePropertiesController gradlePropertiesController; + + public PropertiesLoadingSettingsProcessor( + SettingsProcessor delegate, + GradlePropertiesController gradlePropertiesController + ) { + this.delegate = delegate; + this.gradlePropertiesController = gradlePropertiesController; + } + + @Override + public SettingsInternal process( + GradleInternal gradle, + SettingsLocation settingsLocation, + ClassLoaderScope buildRootClassLoaderScope, + StartParameter startParameter + ) { + gradlePropertiesController.loadGradlePropertiesFrom(settingsLocation.getSettingsDir()); + return delegate.process(gradle, settingsLocation, buildRootClassLoaderScope, startParameter); + } +} diff --git a/subprojects/core/src/main/java/org/gradle/initialization/SettingsFactory.java b/subprojects/core/src/main/java/org/gradle/initialization/SettingsFactory.java index 1ed1bc5f5265..b1c9c5748d76 100644 --- a/subprojects/core/src/main/java/org/gradle/initialization/SettingsFactory.java +++ b/subprojects/core/src/main/java/org/gradle/initialization/SettingsFactory.java @@ -18,13 +18,13 @@ import org.gradle.StartParameter; import org.gradle.api.internal.DynamicObjectAware; -import org.gradle.internal.extensibility.ExtensibleDynamicObject; import org.gradle.api.internal.GradleInternal; import org.gradle.api.internal.SettingsInternal; import org.gradle.api.internal.initialization.ClassLoaderScope; import org.gradle.api.internal.initialization.ScriptHandlerFactory; import org.gradle.api.internal.initialization.ScriptHandlerInternal; import org.gradle.groovy.scripts.ScriptSource; +import org.gradle.internal.extensibility.ExtensibleDynamicObject; import org.gradle.internal.metaobject.DynamicObject; import org.gradle.internal.reflect.Instantiator; import org.gradle.internal.service.scopes.ServiceRegistryFactory; diff --git a/subprojects/core/src/main/java/org/gradle/internal/service/scopes/BuildScopeServices.java b/subprojects/core/src/main/java/org/gradle/internal/service/scopes/BuildScopeServices.java index 6a2c718be17e..2ac307a395b5 100644 --- a/subprojects/core/src/main/java/org/gradle/internal/service/scopes/BuildScopeServices.java +++ b/subprojects/core/src/main/java/org/gradle/internal/service/scopes/BuildScopeServices.java @@ -121,10 +121,12 @@ import org.gradle.initialization.ClassLoaderScopeListeners; import org.gradle.initialization.ClassLoaderScopeRegistry; import org.gradle.initialization.DefaultClassLoaderScopeRegistry; +import org.gradle.initialization.DefaultGradlePropertiesController; import org.gradle.initialization.DefaultGradlePropertiesLoader; import org.gradle.initialization.DefaultProjectDescriptorRegistry; import org.gradle.initialization.DefaultSettingsLoaderFactory; import org.gradle.initialization.DefaultSettingsPreparer; +import org.gradle.initialization.GradlePropertiesController; import org.gradle.initialization.IGradlePropertiesLoader; import org.gradle.initialization.InitScriptHandler; import org.gradle.initialization.InstantiatingBuildLoader; @@ -133,6 +135,7 @@ import org.gradle.initialization.ProjectAccessListener; import org.gradle.initialization.ProjectDescriptorRegistry; import org.gradle.initialization.ProjectPropertySettingBuildLoader; +import org.gradle.initialization.PropertiesLoadingSettingsProcessor; import org.gradle.initialization.RootBuildCacheControllerSettingsProcessor; import org.gradle.initialization.ScriptEvaluatingSettingsProcessor; import org.gradle.initialization.SettingsEvaluatedCallbackFiringSettingsProcessor; @@ -271,6 +274,18 @@ protected IsolatedAntBuilder createIsolatedAntBuilder(ClassPathRegistry classPat return new DefaultIsolatedAntBuilder(classPathRegistry, classLoaderFactory, moduleRegistry); } + protected GradleProperties createGradleProperties( + GradlePropertiesController gradlePropertiesController + ) { + return gradlePropertiesController.getGradleProperties(); + } + + protected GradlePropertiesController createGradlePropertiesController( + IGradlePropertiesLoader propertiesLoader + ) { + return new DefaultGradlePropertiesController(propertiesLoader); + } + protected IGradlePropertiesLoader createGradlePropertiesLoader() { return new DefaultGradlePropertiesLoader((StartParameterInternal) get(StartParameter.class)); } @@ -295,13 +310,6 @@ protected ProviderFactory createProviderFactory(ValueSourceProviderFactory value return new DefaultProviderFactory(valueSourceProviderFactory); } - protected GradleProperties createGradleProperties( - IGradlePropertiesLoader propertiesLoader, - BuildLayout buildLayout - ) { - return propertiesLoader.loadGradleProperties(buildLayout.getRootDirectory()); - } - protected ActorFactory createActorFactory() { return new DefaultActorFactory(get(ExecutorFactory.class)); } @@ -441,6 +449,7 @@ protected SettingsProcessor createSettingsProcessor( ScriptHandlerFactory scriptHandlerFactory, Instantiator instantiator, ServiceRegistryFactory serviceRegistryFactory, + GradlePropertiesController gradlePropertiesController, GradleProperties gradleProperties, BuildOperationExecutor buildOperationExecutor, TextFileResourceLoader textFileResourceLoader @@ -448,15 +457,18 @@ protected SettingsProcessor createSettingsProcessor( return new BuildOperationSettingsProcessor( new RootBuildCacheControllerSettingsProcessor( new SettingsEvaluatedCallbackFiringSettingsProcessor( - new ScriptEvaluatingSettingsProcessor( - scriptPluginFactory, - new SettingsFactory( - instantiator, - serviceRegistryFactory, - scriptHandlerFactory + new PropertiesLoadingSettingsProcessor( + new ScriptEvaluatingSettingsProcessor( + scriptPluginFactory, + new SettingsFactory( + instantiator, + serviceRegistryFactory, + scriptHandlerFactory + ), + gradleProperties, + textFileResourceLoader ), - gradleProperties, - textFileResourceLoader + gradlePropertiesController ) ) ), diff --git a/subprojects/core/src/test/groovy/org/gradle/initialization/DefaultGradlePropertiesControllerTest.groovy b/subprojects/core/src/test/groovy/org/gradle/initialization/DefaultGradlePropertiesControllerTest.groovy new file mode 100644 index 000000000000..368b6464c8a4 --- /dev/null +++ b/subprojects/core/src/test/groovy/org/gradle/initialization/DefaultGradlePropertiesControllerTest.groovy @@ -0,0 +1,101 @@ +/* + * Copyright 2020 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.initialization + +import org.gradle.api.internal.properties.GradleProperties +import spock.lang.Specification +import spock.lang.Unroll + +class DefaultGradlePropertiesControllerTest extends Specification { + + @Unroll + def "attached GradleProperties #method fails before loading"() { + + given: + def propertiesLoader = Mock(IGradlePropertiesLoader) + def subject = new DefaultGradlePropertiesController(propertiesLoader) + def properties = subject.gradleProperties + 0 * propertiesLoader.loadGradleProperties(_) + + when: + switch (method) { + case "find": properties.find("anything"); break + case "mergeProperties": properties.mergeProperties([:]); break + default: assert false + } + + then: + thrown(IllegalStateException) + + where: + method << ["find", "mergeProperties"] + } + + def "attached GradleProperties methods succeed after loading"() { + + given: + def settingsDir = new File('.') + def propertiesLoader = Mock(IGradlePropertiesLoader) + def subject = new DefaultGradlePropertiesController(propertiesLoader) + def properties = subject.gradleProperties + def loadedProperties = Mock(GradleProperties) + 1 * propertiesLoader.loadGradleProperties(settingsDir) >> loadedProperties + 1 * loadedProperties.mergeProperties(_) >> [property: '42'] + 1 * loadedProperties.find(_) >> '42' + + when: + subject.loadGradlePropertiesFrom(settingsDir) + + then: + properties.find("property") == '42' + properties.mergeProperties([:]) == [property: '42'] + } + + def "loadGradlePropertiesFrom is idempotent"() { + + given: + // use a different File instance for each call to ensure it is compared by value + def currentDir = { new File('.') } + def propertiesLoader = Mock(IGradlePropertiesLoader) + def subject = new DefaultGradlePropertiesController(propertiesLoader) + def loadedProperties = Mock(GradleProperties) + + when: "calling the method multiple times with the same value" + subject.loadGradlePropertiesFrom(currentDir()) + subject.loadGradlePropertiesFrom(currentDir()) + + then: + 1 * propertiesLoader.loadGradleProperties(currentDir()) >> loadedProperties + } + + def "loadGradlePropertiesFrom fails when called with different argument"() { + + given: + def settingsDir = new File('a') + def propertiesLoader = Mock(IGradlePropertiesLoader) + def subject = new DefaultGradlePropertiesController(propertiesLoader) + def loadedProperties = Mock(GradleProperties) + 1 * propertiesLoader.loadGradleProperties(settingsDir) >> loadedProperties + + when: + subject.loadGradlePropertiesFrom(settingsDir) + subject.loadGradlePropertiesFrom(new File('b')) + + then: + thrown(IllegalStateException) + } +} diff --git a/subprojects/instant-execution/src/main/kotlin/org/gradle/instantexecution/DefaultInstantExecution.kt b/subprojects/instant-execution/src/main/kotlin/org/gradle/instantexecution/DefaultInstantExecution.kt index 6eef2cc2d539..90f8c19190f9 100644 --- a/subprojects/instant-execution/src/main/kotlin/org/gradle/instantexecution/DefaultInstantExecution.kt +++ b/subprojects/instant-execution/src/main/kotlin/org/gradle/instantexecution/DefaultInstantExecution.kt @@ -27,6 +27,7 @@ import org.gradle.api.provider.Provider import org.gradle.api.provider.ValueSource import org.gradle.api.provider.ValueSourceParameters import org.gradle.execution.plan.Node +import org.gradle.initialization.GradlePropertiesController import org.gradle.initialization.InstantExecution import org.gradle.instantexecution.extensions.uncheckedCast import org.gradle.instantexecution.extensions.unsafeLazy @@ -70,7 +71,8 @@ class DefaultInstantExecution internal constructor( private val scopeRegistryListener: InstantExecutionClassLoaderScopeRegistryListener, private val beanConstructors: BeanConstructors, private val valueSourceProviderFactory: ValueSourceProviderFactory, - private val virtualFileSystem: VirtualFileSystem + private val virtualFileSystem: VirtualFileSystem, + private val gradlePropertiesController: GradlePropertiesController ) : InstantExecution { interface Host { @@ -101,6 +103,12 @@ class DefaultInstantExecution internal constructor( false } else -> { + + // TODO - should load properties from settings file directory + gradlePropertiesController.loadGradlePropertiesFrom( + startParameter.rootDirectory + ) + val fingerprintChangedReason = checkFingerprint() when { fingerprintChangedReason != null -> {