Skip to content

Detect native source file relocation during recompilation #29557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Detect native source file relocation during recompilation #29557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
270bc11
Introduce failing test case regarding relocating sources
lacasseio Jun 15, 2024
9fb28dd
Use normal FileCollection property creation
lacasseio Jun 15, 2024
6978ea7
Use injected ObjectFactory instead of Project#getObject()
lacasseio Jun 15, 2024
c34505a
Cleanup unused/broken API
lacasseio Jun 15, 2024
c6cf507
Restore binary compatibility and remove unused internal class
big-guy Jun 17, 2024
897f0d4
Finalize source file collection
big-guy Jun 21, 2024
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
32 changes: 32 additions & 0 deletions .../src/integTest/groovy/org/gradle/language/cpp/CppIncrementalCompileIntegrationTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,36 @@ class CppIncrementalCompileIntegrationTest extends AbstractInstalledToolChainInt

sharedLibrary("build/lib/main/release/hello").assertExists()
}

@ToBeFixedForConfigurationCache
def "recompile when only relative path of source file changes"() {
def compileTaskName = "compileReleaseCpp" // any compile task works
def lib = new CppLib()
settingsFile << "rootProject.name = 'hello'"

given:
lib.writeToProject(testDirectory)

and:
def fileToRelocate = file("src/main/cpp/relocate.cpp")
fileToRelocate.createNewFile()

and:
buildFile << """
apply plugin: 'cpp-library'
"""

and:
succeeds compileTaskName
result.assertTaskExecuted(":$compileTaskName")

and:
succeeds compileTaskName
result.assertTaskSkipped(":$compileTaskName")

expect:
fileToRelocate.moveToDirectory(file("src/main/cpp/subdir"))
succeeds compileTaskName
result.assertTaskNotSkipped(":$compileTaskName")
}
}
24 changes: 15 additions & 9 deletions ...ive/src/main/java/org/gradle/language/nativeplatform/tasks/AbstractNativeCompileTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,23 @@ public abstract class AbstractNativeCompileTask extends DefaultTask {
private final ConfigurableFileCollection includes;
private final ConfigurableFileCollection systemIncludes;
private final ConfigurableFileCollection source;
private final Map<String, String> macros = new LinkedHashMap<String, String>();
private final Map<String, String> macros = new LinkedHashMap<>();
private final ListProperty<String> compilerArgs;
private final IncrementalCompilerBuilder.IncrementalCompiler incrementalCompiler;

public AbstractNativeCompileTask() {
ObjectFactory objectFactory = getProject().getObjects();
this.includes = getProject().files();
this.systemIncludes = getProject().files();
this.includes = getObjects().fileCollection();
this.systemIncludes = getObjects().fileCollection();
dependsOn(includes);
dependsOn(systemIncludes);

this.source = getTaskFileVarFactory().newInputFileCollection(this);
this.objectFileDir = objectFactory.directoryProperty();
this.compilerArgs = getProject().getObjects().listProperty(String.class);
this.targetPlatform = objectFactory.property(NativePlatform.class);
this.toolChain = objectFactory.property(NativeToolChain.class);
this.source = getObjects().fileCollection();
this.source.finalizeValueOnRead();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

❌ @big-guy Unless the bug with finalizeValueOnRead() where it discards task dependencies when read was fixed, this can cause issues. I don't think I had a chance to open the issue (or it was in our shared document), but when the file collection is realized, the task dependencies are dropped. I don't remember if it was a bug or intended behaviour. It would be worth checking that is someone early reads the source, everything works fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have some code on v7.6.4 that will exhibit the explained behaviour above. If it was fixed on 8.x, backporting this fix to 7.6.x will need to be adjusted.


this.objectFileDir = getObjects().directoryProperty();
this.compilerArgs = getObjects().listProperty(String.class);
this.targetPlatform = getObjects().property(NativePlatform.class);
this.toolChain = getObjects().property(NativeToolChain.class);
this.incrementalCompiler = getIncrementalCompilerBuilder().newCompiler(this, source, includes.plus(systemIncludes), macros, toolChain.map(new Transformer<Boolean, NativeToolChain>() {
@Override
public Boolean transform(NativeToolChain nativeToolChain) {
Expand All @@ -101,6 +102,11 @@ protected TaskFileVarFactory getTaskFileVarFactory() {
throw new UnsupportedOperationException();
}

@Inject
protected ObjectFactory getObjects() {
throw new UnsupportedOperationException();
}

@Inject
protected IncrementalCompilerBuilder getIncrementalCompilerBuilder() {
throw new UnsupportedOperationException();
Expand Down
73 changes: 0 additions & 73 deletions ...jects/core/src/main/java/org/gradle/api/internal/file/CachingTaskInputFileCollection.java
View file
Open in desktop

This file was deleted.

15 changes: 1 addition & 14 deletions subprojects/core/src/main/java/org/gradle/api/internal/file/DefaultProjectLayout.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,16 @@
import org.gradle.api.Project;
import org.gradle.api.Task;
import org.gradle.api.Transformer;
import org.gradle.api.file.ConfigurableFileCollection;
import org.gradle.api.file.Directory;
import org.gradle.api.file.DirectoryProperty;
import org.gradle.api.file.FileCollection;
import org.gradle.api.file.ProjectLayout;
import org.gradle.api.file.RegularFile;
import org.gradle.api.internal.file.collections.MinimalFileSet;
import org.gradle.api.internal.provider.MappingProvider;
import org.gradle.api.internal.provider.PropertyHost;
import org.gradle.api.internal.provider.Providers;
import org.gradle.api.internal.tasks.TaskDependencyFactory;
import org.gradle.api.provider.Provider;
import org.gradle.api.tasks.util.PatternSet;
import org.gradle.internal.Factory;

import java.io.File;

Expand All @@ -41,16 +37,12 @@ public class DefaultProjectLayout implements ProjectLayout, TaskFileVarFactory {
private final DirectoryProperty buildDir;
private final FileResolver fileResolver;
private final TaskDependencyFactory taskDependencyFactory;
private final Factory<PatternSet> patternSetFactory;
private final PropertyHost propertyHost;
private final FileCollectionFactory fileCollectionFactory;
private final FileFactory fileFactory;

public DefaultProjectLayout(File projectDir, FileResolver fileResolver, TaskDependencyFactory taskDependencyFactory, Factory<PatternSet> patternSetFactory, PropertyHost propertyHost, FileCollectionFactory fileCollectionFactory, FilePropertyFactory filePropertyFactory, FileFactory fileFactory) {
public DefaultProjectLayout(File projectDir, FileResolver fileResolver, TaskDependencyFactory taskDependencyFactory, FileCollectionFactory fileCollectionFactory, FilePropertyFactory filePropertyFactory, FileFactory fileFactory) {
this.fileResolver = fileResolver;
this.taskDependencyFactory = taskDependencyFactory;
this.patternSetFactory = patternSetFactory;
this.propertyHost = propertyHost;
this.fileCollectionFactory = fileCollectionFactory;
this.fileFactory = fileFactory;
this.projectDir = fileFactory.dir(projectDir);
Expand All @@ -67,11 +59,6 @@ public DirectoryProperty getBuildDirectory() {
return buildDir;
}

@Override
public ConfigurableFileCollection newInputFileCollection(Task consumer) {
return new CachingTaskInputFileCollection(fileResolver, patternSetFactory, taskDependencyFactory, propertyHost);
}

@Override
public FileCollection newCalculatedInputFileCollection(Task consumer, MinimalFileSet calculatedFiles, FileCollection... inputs) {
return new CalculatedTaskInputFileCollection(taskDependencyFactory, consumer.getPath(), calculatedFiles, inputs);
Expand Down
10 changes: 0 additions & 10 deletions subprojects/core/src/main/java/org/gradle/api/internal/file/TaskFileVarFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,10 @@
package org.gradle.api.internal.file;

import org.gradle.api.Task;
import org.gradle.api.file.ConfigurableFileCollection;
import org.gradle.api.file.FileCollection;
import org.gradle.api.internal.file.collections.MinimalFileSet;

public interface TaskFileVarFactory {
/**
* Creates a {@link ConfigurableFileCollection} that can be used as a task input.
*
* <p>The implementation may apply caching to the result, so that the matching files are calculated during file snapshotting and the result cached in memory for when it is queried again, either during task action execution or in order to calculate some other task input value.
*
* <p>Use this collection only for those files that are not expected to change during task execution, such as task inputs.
*/
ConfigurableFileCollection newInputFileCollection(Task consumer);

/**
* Creates a {@link FileCollection} that represents some task input that is calculated from one or more other file collections.
*
Expand Down
4 changes: 2 additions & 2 deletions ...re/src/main/java/org/gradle/internal/service/scopes/WorkerSharedProjectScopeServices.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ ObjectFactory createObjectFactory(InstantiatorFactory instantiatorFactory, Servi

@Provides
DefaultProjectLayout createProjectLayout(FileResolver fileResolver, FileCollectionFactory fileCollectionFactory, TaskDependencyFactory taskDependencyFactory,
FilePropertyFactory filePropertyFactory, Factory<PatternSet> patternSetFactory, PropertyHost propertyHost, FileFactory fileFactory) {
return new DefaultProjectLayout(projectDir, fileResolver, taskDependencyFactory, patternSetFactory, propertyHost, fileCollectionFactory, filePropertyFactory, fileFactory);
FilePropertyFactory filePropertyFactory, FileFactory fileFactory) {
return new DefaultProjectLayout(projectDir, fileResolver, taskDependencyFactory, fileCollectionFactory, filePropertyFactory, fileFactory);
}
}
86 changes: 0 additions & 86 deletions ...re/src/test/groovy/org/gradle/api/internal/file/CachingTaskInputFileCollectionTest.groovy
View file
Open in desktop

This file was deleted.

5 changes: 2 additions & 3 deletions ...rojects/core/src/test/groovy/org/gradle/api/internal/file/DefaultProjectLayoutTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@

package org.gradle.api.internal.file

import org.gradle.api.internal.provider.PropertyHost

import org.gradle.api.internal.tasks.TaskDependencyFactory
import org.gradle.internal.Factory
import org.gradle.test.fixtures.file.TestFile
import org.gradle.test.fixtures.file.TestNameTestDirectoryProvider
import org.junit.Rule
Expand All @@ -36,7 +35,7 @@ class DefaultProjectLayoutTest extends Specification {

def setup() {
projectDir = tmpDir.createDir("project")
layout = new DefaultProjectLayout(projectDir, TestFiles.resolver(projectDir), Stub(TaskDependencyFactory), Stub(Factory), Stub(PropertyHost), TestFiles.fileCollectionFactory(projectDir), TestFiles.filePropertyFactory(projectDir), TestFiles.fileFactory())
layout = new DefaultProjectLayout(projectDir, TestFiles.resolver(projectDir), Stub(TaskDependencyFactory), TestFiles.fileCollectionFactory(projectDir), TestFiles.filePropertyFactory(projectDir), TestFiles.fileFactory())
}

def "can query the project directory"() {
Expand Down
3 changes: 1 addition & 2 deletions subprojects/core/src/test/groovy/org/gradle/api/internal/project/DefaultProjectSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import org.gradle.api.internal.provider.PropertyHost
import org.gradle.api.internal.tasks.DefaultTaskDependencyFactory
import org.gradle.api.internal.tasks.TaskContainerInternal
import org.gradle.api.model.ObjectFactory
import org.gradle.api.tasks.util.internal.PatternSets
import org.gradle.configuration.internal.ListenerBuildOperationDecorator
import org.gradle.internal.Factory
import org.gradle.internal.instantiation.InstantiatorFactory
Expand Down Expand Up @@ -263,7 +262,7 @@ class DefaultProjectSpec extends Specification {
@Provides
DefaultProjectLayout createProjectLayout(FileResolver fileResolver, FileCollectionFactory fileCollectionFactory) {
def filePropertyFactory = new DefaultFilePropertyFactory(PropertyHost.NO_OP, fileResolver, fileCollectionFactory)
return new DefaultProjectLayout(fileResolver.resolve("."), fileResolver, DefaultTaskDependencyFactory.withNoAssociatedProject(), PatternSets.getNonCachingPatternSetFactory(), PropertyHost.NO_OP, fileCollectionFactory, filePropertyFactory, filePropertyFactory)
return new DefaultProjectLayout(fileResolver.resolve("."), fileResolver, DefaultTaskDependencyFactory.withNoAssociatedProject(), fileCollectionFactory, filePropertyFactory, filePropertyFactory)
}
})

Expand Down
2 changes: 1 addition & 1 deletion subprojects/core/src/test/groovy/org/gradle/api/internal/project/DefaultProjectTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ class DefaultProjectTest extends Specification {
ModelSchemaStore modelSchemaStore = Stub(ModelSchemaStore)
serviceRegistryMock.get((Type) ModelSchemaStore) >> modelSchemaStore
serviceRegistryMock.get(ModelSchemaStore) >> modelSchemaStore
serviceRegistryMock.get((Type) DefaultProjectLayout) >> new DefaultProjectLayout(rootDir, TestFiles.resolver(rootDir), Stub(TaskDependencyFactory), Stub(Factory), Stub(PropertyHost), Stub(FileCollectionFactory), TestFiles.filePropertyFactory(), TestFiles.fileFactory())
serviceRegistryMock.get((Type) DefaultProjectLayout) >> new DefaultProjectLayout(rootDir, TestFiles.resolver(rootDir), Stub(TaskDependencyFactory), Stub(FileCollectionFactory), TestFiles.filePropertyFactory(), TestFiles.fileFactory())

build.getProjectEvaluationBroadcaster() >> Stub(ProjectEvaluationListener)
build.getParent() >> null
Expand Down
3 changes: 1 addition & 2 deletions subprojects/core/src/testFixtures/groovy/org/gradle/util/TestUtil.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import org.gradle.api.internal.tasks.TaskDependencyFactory
import org.gradle.api.internal.tasks.properties.annotations.OutputPropertyRoleAnnotationHandler
import org.gradle.api.model.ObjectFactory
import org.gradle.api.provider.ProviderFactory
import org.gradle.api.tasks.util.internal.PatternSets
import org.gradle.cache.internal.TestCrossBuildInMemoryCacheFactory
import org.gradle.internal.hash.ChecksumService
import org.gradle.internal.hash.HashCode
Expand Down Expand Up @@ -169,7 +168,7 @@ class TestUtil {
@Provides
ProjectLayout createProjectLayout() {
def filePropertyFactory = new DefaultFilePropertyFactory(PropertyHost.NO_OP, fileResolver, fileCollectionFactory)
return new DefaultProjectLayout(fileResolver.resolve("."), fileResolver, DefaultTaskDependencyFactory.withNoAssociatedProject(), PatternSets.getNonCachingPatternSetFactory(), PropertyHost.NO_OP, fileCollectionFactory, filePropertyFactory, filePropertyFactory)
return new DefaultProjectLayout(fileResolver.resolve("."), fileResolver, DefaultTaskDependencyFactory.withNoAssociatedProject(), fileCollectionFactory, filePropertyFactory, filePropertyFactory)
}

@Provides
Expand Down