Fixes issue with file hashing cache

Normalization rules were not being properly captured when calculating the
cache key for file hashes during runtime classpath normalization.  This was
causing incorrect build cache misses under the right sequence of events.

Fixes #16144

subprojects/core/src/integTest/groovy/org/gradle/normalization/ConfigureRuntimeClasspathNormalizationIntegrationTest.groovy

@@ -20,8 +20,13 @@ import org.gradle.integtests.fixtures.AbstractIntegrationSpec
 import org.gradle.integtests.fixtures.ToBeFixedForConfigurationCache
 import org.gradle.integtests.fixtures.UnsupportedWithConfigurationCache
 import org.gradle.test.fixtures.file.TestFile
+import org.gradle.util.TextUtil
+import spock.lang.Issue
 import spock.lang.Unroll
 
+import java.util.jar.Attributes
+import java.util.jar.Manifest
+
 @Unroll
 class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractIntegrationSpec {
 
@@ -80,6 +85,7 @@ class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractInte
         'nested in dir jars' | 'ignoredResourceInNestedInDirJar' | 'notIgnoredResourceInNestedInDirJar' | Api.ANNOTATION
     }
 
+    @ToBeFixedForConfigurationCache(because = "classpath normalization")
     @Unroll
     def "can ignore manifest attributes in #tree on runtime classpath"() {
         def project = new ProjectWithRuntimeClasspathNormalization(Api.RUNTIME).withManifestAttributesIgnored()
@@ -96,7 +102,7 @@ class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractInte
         skipped(project.customTask)
 
         when:
-        manifestResource.replaceContents("Manifest-Version: 1.0\nImplementation-Version: 1.0.1")
+        manifestResource.changeAttributes((IMPLEMENTATION_VERSION): "1.0.1")
         succeeds project.customTask
         then:
         skipped(project.customTask)
@@ -124,7 +130,7 @@ class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractInte
         skipped(project.customTask)
 
         when:
-        manifestResource.replaceContents("Manifest-Version: 1.0\nImplementation-Version: 1.0.1")
+        manifestResource.changeAttributes((IMPLEMENTATION_VERSION): "1.0.1")
         succeeds project.customTask
         then:
         skipped(project.customTask)
@@ -152,7 +158,7 @@ class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractInte
         skipped(project.customTask)
 
         when:
-        manifestResource.replaceContents("Manifest-Version: 1.0\nImplementation-Version: 1.0.1")
+        manifestResource.changeAttributes((IMPLEMENTATION_VERSION): "1.0.1")
         project.jarManifestProperties.replaceContents("implementation-version=1.0.1")
         succeeds project.customTask
         then:
@@ -235,8 +241,8 @@ class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractInte
         change                      | config                                                    | api
         'ignore file'               | "ignore '**/some-other-file.txt'"                         | Api.RUNTIME
         'ignore file'               | "ignore '**/some-other-file.txt'"                         | Api.ANNOTATION
-        'ignore manifest attribute' | "metaInf { ignoreAttribute 'Implementation-version' }"    | Api.RUNTIME
-        'ignore manifest attribute' | "metaInf { ignoreAttribute 'Implementation-version' }"    | Api.ANNOTATION
+        'ignore manifest attribute' | "metaInf { ignoreAttribute '${IMPLEMENTATION_VERSION}' }"    | Api.RUNTIME
+        'ignore manifest attribute' | "metaInf { ignoreAttribute '${IMPLEMENTATION_VERSION}' }"    | Api.ANNOTATION
         'ignore property'           | "properties { ignoreProperty 'timestamp' }"               | Api.RUNTIME
         'ignore property'           | "properties { ignoreProperty 'timestamp' }"               | Api.ANNOTATION
     }
@@ -466,16 +472,56 @@ class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractInte
         executedAndNotSkipped(project.customTask)
     }
 
+    @ToBeFixedForConfigurationCache(because = "classpath normalization")
+    @Issue('https://github.com/gradle/gradle/issues/16144')
+    def "changing normalization configuration rules changes build cache key (#description)"() {
+        def project = new ProjectWithRuntimeClasspathNormalization(Api.RUNTIME)
+        project.propertiesFileInJar.changeProperty(ALSO_IGNORE_ME, 'some value')
+        project.buildFile << """
+            normalization {
+                runtimeClasspath {
+                    if (project.hasProperty('${enableFilterFlag}')) {
+                        ${normalizationRule}
+                    }
+                }
+            }
+        """
+
+        when:
+        args('--build-cache')
+        succeeds 'clean', project.customTask
+        then:
+        executedAndNotSkipped(project.customTask)
+
+        when:
+        args("-P${enableFilterFlag}", '--build-cache')
+        succeeds 'clean', project.customTask
+        then:
+        executedAndNotSkipped(project.customTask)
+
+        where:
+        enableFilterFlag        | normalizationRule                                                         | description
+        PROPERTIES_FILTER_FLAG  | "properties('**/foo.properties') { ignoreProperty '${ALSO_IGNORE_ME}' }"  | 'properties rule'
+        META_INF_FILTER_FLAG    | "metaInf { ignoreAttribute '${IMPLEMENTATION_VERSION}' }"                 | 'meta-inf rule'
+        FILE_FILTER_FLAG        | "ignore '**/ignored.txt'"                                                 | 'ignore rule'
+    }
+
     static final String IGNORE_ME = 'ignore-me'
+    static final String ALSO_IGNORE_ME = 'also-ignore-me'
     static final String IGNORE_ME_TOO = 'ignore-me-too'
     static final String DONT_IGNORE_ME = 'dont-ignore-me'
+    static final String IMPLEMENTATION_VERSION = Attributes.Name.IMPLEMENTATION_VERSION.toString()
+    static final String PROPERTIES_FILTER_FLAG = "filterProperties"
+    static final String META_INF_FILTER_FLAG = "filterMetaInf"
+    static final String FILE_FILTER_FLAG = "filterFile"
 
     enum Api {
         RUNTIME, ANNOTATION
     }
 
     class ProjectWithRuntimeClasspathNormalization {
         final TestFile root
+        final TestFile buildCacheDir
         TestResource ignoredResourceInDirectory
         TestResource notIgnoredResourceInDirectory
         TestResource ignoredResourceInJar
@@ -484,9 +530,9 @@ class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractInte
         TestResource notIgnoredResourceInJar
         TestResource notIgnoredResourceInNestedJar
         TestResource notIgnoredResourceInNestedInDirJar
-        TestResource jarManifest
+        ManifestResource jarManifest
         TestResource jarManifestProperties
-        TestResource manifestInDirectory
+        ManifestResource manifestInDirectory
         PropertiesResource propertiesFileInDir
         PropertiesResource propertiesFileInJar
         PropertiesResource propertiesFileInNestedJar
@@ -499,10 +545,22 @@ class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractInte
         private TestFile nestedInDirJarContents
         private final String projectName
         final TestFile buildFile
+        final TestFile settingsFile
 
         ProjectWithRuntimeClasspathNormalization(String projectName = null, Api api) {
             this.projectName = projectName
             this.root = projectName ? file(projectName) : temporaryFolder.testDirectory
+            this.buildCacheDir = testDirectory.file("build-cache")
+
+            def buildCachePath = TextUtil.normaliseFileSeparators(buildCacheDir.absolutePath)
+
+            settingsFile = root.file('settings.gradle') << """
+                buildCache {
+                    local {
+                        directory = file('${buildCachePath}')
+                    }
+                }
+            """
 
             buildFile = root.file('build.gradle') << """
                 apply plugin: 'base'
@@ -520,15 +578,15 @@ class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractInte
                 notIgnoredResourceInDirectory = new TestResource(file("not-ignored.txt") << "This should not be ignored")
                 nestedInDirJar = file('nestedInDir.jar')
                 propertiesFileInDir = new PropertiesResource(file('some/path/to/foo.properties'), [(IGNORE_ME): 'this should be ignored', (DONT_IGNORE_ME): 'this should not be ignored'])
-                manifestInDirectory = new TestResource(file('META-INF/MANIFEST.MF') << "Manifest-Version: 1.0\nImplementation-Version: 1.0.0")
+                manifestInDirectory = new ManifestResource(file('META-INF/MANIFEST.MF')).withAttributes((IMPLEMENTATION_VERSION): "1.0.0")
             }
             nestedJarContents = root.file('libraryContents').create {
                 ignoredResourceInNestedJar = new TestResource(file('some/package/ignored.txt') << "This should be ignored", this.&createJar)
                 notIgnoredResourceInNestedJar = new TestResource(file('some/package/not-ignored.txt') << "This should not be ignored", this.&createJar)
                 propertiesFileInNestedJar = new PropertiesResource(file('some/path/to/foo.properties'), [(IGNORE_ME): 'this should be ignored', (DONT_IGNORE_ME): 'this should not be ignored'], this.&createJar)
             }
             libraryJarContents = root.file('libraryContents').create {
-                jarManifest = new TestResource(file('META-INF/MANIFEST.MF') << "Manifest-Version: 1.0\nImplementation-Version: 1.0.0", this.&createJar)
+                jarManifest = new ManifestResource(file('META-INF/MANIFEST.MF'), this.&createJar).withAttributes((IMPLEMENTATION_VERSION): "1.0.0")
                 jarManifestProperties = new TestResource(file('META-INF/build-info.properties') << "implementation-version=1.0.0", this.&createJar)
                 ignoredResourceInJar = new TestResource(file('some/package/ignored.txt') << "This should be ignored", this.&createJar)
                 notIgnoredResourceInJar = new TestResource(file('some/package/not-ignored.txt') << "This should not be ignored", this.&createJar)
@@ -550,6 +608,7 @@ class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractInte
                             .withNormalizer(ClasspathNormalizer)
                         outputs.file(outputFile)
                             .withPropertyName("outputFile")
+                        outputs.cacheIf { true }
 
                         doLast {
                             outputFile.text = "done"
@@ -558,6 +617,7 @@ class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractInte
                 """
             } else {
                 return """
+                    @CacheableTask
                     class CustomTask extends DefaultTask {
                         @OutputFile File outputFile = new File(temporaryDir, "output.txt")
                         @Classpath FileCollection classpath = project.layout.files("classpath/dirEntry", "library.jar")
@@ -632,7 +692,7 @@ class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractInte
                 normalization {
                     runtimeClasspath {
                         metaInf {
-                            ignoreAttribute "Implementation-Version"
+                            ignoreAttribute "${IMPLEMENTATION_VERSION}"
                         }
                     }
                 }
@@ -707,6 +767,44 @@ class ConfigureRuntimeClasspathNormalizationIntegrationTest extends AbstractInte
         }
     }
 
+    class ManifestResource extends TestResource {
+        Map<String, String> attributes
+
+        ManifestResource(TestFile backingFile, Closure onChange = {}) {
+            super(backingFile, onChange)
+        }
+
+        ManifestResource withAttributes(Map<String, String> attributes) {
+            this.attributes = attributes
+            def manifest = new Manifest()
+            def mainAttributes = manifest.getMainAttributes()
+            mainAttributes.put(Attributes.Name.MANIFEST_VERSION, "1.0")
+            attributes.each {name, value ->
+                mainAttributes.put(new Attributes.Name(name), value)
+            }
+            backingFile.withOutputStream {os ->
+                manifest.write(os)
+            }
+            return this
+        }
+
+        ManifestResource changeAttributes(Map<String, String> attributes) {
+            withAttributes(attributes)
+            changed()
+            return this
+        }
+
+        @Override
+        void changeContents() {
+            throw new UnsupportedOperationException()
+        }
+
+        @Override
+        void add() {
+            throw new UnsupportedOperationException()
+        }
+    }
+
     class PropertiesResource extends TestResource {
         String comment = ""
 

subprojects/normalization-java/src/main/java/org/gradle/api/internal/changedetection/state/IgnoringResourceHasher.java

@@ -33,6 +33,8 @@ public IgnoringResourceHasher(ResourceHasher delegate, ResourceFilter resourceFi
 
     @Override
     public void appendConfigurationToHasher(Hasher hasher) {
+        delegate.appendConfigurationToHasher(hasher);
+        hasher.putString(getClass().getName());
         resourceFilter.appendConfigurationToHasher(hasher);
     }
 

subprojects/normalization-java/src/main/java/org/gradle/api/internal/changedetection/state/MetaInfAwareClasspathResourceHasher.java

@@ -52,6 +52,7 @@ public MetaInfAwareClasspathResourceHasher(ResourceHasher delegate, ResourceEntr
     @Override
     public void appendConfigurationToHasher(Hasher hasher) {
         delegate.appendConfigurationToHasher(hasher);
+        hasher.putString(getClass().getName());
         attributeResourceFilter.appendConfigurationToHasher(hasher);
     }
 

subprojects/normalization-java/src/main/java/org/gradle/api/internal/changedetection/state/PropertiesFileAwareClasspathResourceHasher.java

@@ -58,6 +58,8 @@ public PropertiesFileAwareClasspathResourceHasher(ResourceHasher delegate, Map<S
 
     @Override
     public void appendConfigurationToHasher(Hasher hasher) {
+        delegate.appendConfigurationToHasher(hasher);
+        hasher.putString(getClass().getName());
         propertiesFilePatterns.forEach(hasher::putString);
         propertiesFileFilters.values().forEach(resourceEntryFilter -> resourceEntryFilter.appendConfigurationToHasher(hasher));
     }

subprojects/normalization-java/src/main/java/org/gradle/api/internal/changedetection/state/UnionResourceEntryFilter.java

@@ -37,6 +37,7 @@ public boolean shouldBeIgnored(String entry) {
 
     @Override
     public void appendConfigurationToHasher(Hasher hasher) {
+        hasher.putString(getClass().getName());
         filters.forEach(resourceEntryFilter -> resourceEntryFilter.appendConfigurationToHasher(hasher));
     }
 }

subprojects/normalization-java/src/test/groovy/org/gradle/api/internal/changedetection/state/IgnoringResourceHasherTest.groovy

@@ -19,6 +19,7 @@ package org.gradle.api.internal.changedetection.state
 import org.gradle.internal.file.FileMetadata
 import org.gradle.internal.file.impl.DefaultFileMetadata
 import org.gradle.internal.hash.HashCode
+import org.gradle.internal.hash.Hasher
 import org.gradle.internal.snapshot.RegularFileSnapshot
 import spock.lang.Specification
 
@@ -50,4 +51,14 @@ class IgnoringResourceHasherTest extends Specification {
         and:
         hash == null
     }
+
+    def "delegate configuration is added to hasher"() {
+        def configurationHasher = Mock(Hasher)
+
+        when:
+        hasher.appendConfigurationToHasher(configurationHasher)
+
+        then:
+        1 * delegate.appendConfigurationToHasher(configurationHasher)
+    }
 }

subprojects/normalization-java/src/test/groovy/org/gradle/api/internal/changedetection/state/MetaInfAwareClasspathResourceHasherTest.groovy

@@ -21,6 +21,7 @@ import org.gradle.api.internal.file.archive.ZipEntry
 import org.gradle.internal.file.FileMetadata
 import org.gradle.internal.file.impl.DefaultFileMetadata
 import org.gradle.internal.hash.HashCode
+import org.gradle.internal.hash.Hasher
 import org.gradle.internal.snapshot.RegularFileSnapshot
 import org.junit.Rule
 import org.junit.rules.TemporaryFolder
@@ -389,6 +390,18 @@ class MetaInfAwareClasspathResourceHasherTest extends Specification {
         hash3 == hash4
     }
 
+    def "delegate configuration is added to hasher"() {
+        def configurationHasher = Mock(Hasher)
+        def delegate = Mock(ResourceHasher)
+        useDelegate(delegate)
+
+        when:
+        hasher.appendConfigurationToHasher(configurationHasher)
+
+        then:
+        1 * delegate.appendConfigurationToHasher(configurationHasher)
+    }
+
     void populateAttributes(Attributes attributes, Map<String, Object> attributesMap) {
         attributesMap.each { String name, Object value ->
             if (value instanceof String) {

subprojects/normalization-java/src/test/groovy/org/gradle/api/internal/changedetection/state/PropertiesFileAwareClasspathResourceHasherTest.groovy

@@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableSet
 import com.google.common.collect.Maps
 import com.google.common.io.ByteStreams
 import org.gradle.api.internal.file.archive.ZipEntry
+import org.gradle.internal.hash.Hasher
 import org.gradle.internal.hash.Hashing
 import org.gradle.internal.hash.HashingOutputStream
 import org.gradle.internal.snapshot.RegularFileSnapshot
@@ -257,6 +258,17 @@ class PropertiesFileAwareClasspathResourceHasherTest extends Specification {
         SnapshotContext.FILE_SNAPSHOT | "key"    | 'keyWithBadEscapeSequence\\uxxxx=some value'
     }
 
+    def "delegate configuration is added to hasher"() {
+        def configurationHasher = Mock(Hasher)
+        delegate = Mock(ResourceHasher)
+
+        when:
+        filteredHasher.appendConfigurationToHasher(configurationHasher)
+
+        then:
+        1 * delegate.appendConfigurationToHasher(configurationHasher)
+    }
+
     enum SnapshotContext {
         ZIP_ENTRY, FILE_SNAPSHOT
     }