Merge pull request #182 from gradle/no/mergesourcesetfolders

Disable caching of MergeSourceSetFolders

Author: ghale

Diff for: README.md

@@ -125,6 +125,6 @@ room {
 * There can only be a single schema export directory for the project - you cannot configure variant-specific export
 directories.  Schemas exported from different variants will be merged in the directory specified in the "room" extension.
 
-### MergeNativeLibsWorkaround, StripDebugSymbols, MergeJavaResources
+### MergeNativeLibs, StripDebugSymbols, MergeJavaResources, and MergeSourceSetFolders Workarounds
 
-It has been observed that caching the `MergeNativeLibsTask`, `StripDebugSymbols`, and `MergeJavaResources` tasks rarely provide any significant positive avoidance savings.  In fact, they frequently provide negative savings, especially when fetched from a remote cache node.  As such, these workarounds actually disable caching for these tasks.
+It has been observed that caching the `MergeNativeLibsTask`, `StripDebugSymbols`, `MergeJavaResources`, and `MergeSourceSetFolders` tasks rarely provide any significant positive avoidance savings.  In fact, they frequently provide negative savings, especially when fetched from a remote cache node.  As such, these workarounds actually disable caching for these tasks.

Diff for: src/main/groovy/org/gradle/android/AndroidCacheFixPlugin.groovy

@@ -9,6 +9,7 @@ import org.gradle.android.workarounds.MergeJavaResourcesWorkaround
 import org.gradle.android.workarounds.MergeNativeLibsWorkaround
 import org.gradle.android.workarounds.MergeResourcesWorkaround
 import org.gradle.android.workarounds.CompileLibraryResourcesWorkaround_4_2
+import org.gradle.android.workarounds.MergeSourceSetFoldersWorkaround
 import org.gradle.android.workarounds.StripDebugSymbolsWorkaround
 import org.gradle.android.workarounds.SystemPropertiesCompat
 import org.gradle.android.workarounds.RoomSchemaLocationWorkaround
@@ -48,6 +49,7 @@ class AndroidCacheFixPlugin implements Plugin<Project> {
             return Arrays.<Workaround>asList(
                 new MergeJavaResourcesWorkaround(),
                 new MergeNativeLibsWorkaround(),
+                new MergeSourceSetFoldersWorkaround(),
                 new RoomSchemaLocationWorkaround(),
                 new CompileLibraryResourcesWorkaround_4_0(),
                 new CompileLibraryResourcesWorkaround_4_2(),

Diff for: src/main/groovy/org/gradle/android/workarounds/MergeSourceSetFoldersWorkaround.groovy

@@ -0,0 +1,26 @@
+package org.gradle.android.workarounds
+
+import com.android.build.gradle.tasks.MergeSourceSetFolders
+import org.gradle.android.AndroidIssue
+import org.gradle.api.Project
+
+/**
+ * Disables caching of the MergeSourceSetFolders task which is mostly disk bound and unlikely to provide positive
+ * performance benefits.
+ */
+@AndroidIssue(introducedIn = "3.5.0", fixedIn = [], link = "https://issuetracker.google.com/issues/194804421")
+class MergeSourceSetFoldersWorkaround implements Workaround {
+    private static final String CACHING_ENABLED_PROPERTY = "org.gradle.android.cache-fix.MergeSourceSetFolders.caching.enabled"
+
+    @Override
+    void apply(WorkaroundContext context) {
+        context.project.tasks.withType(MergeSourceSetFolders).configureEach {
+            outputs.doNotCacheIf("Caching MergeSourceSetFolders is unlikely to provide positive performance results.", {true })
+        }
+    }
+
+    @Override
+    boolean canBeApplied(Project project) {
+        return !SystemPropertiesCompat.getBoolean(CACHING_ENABLED_PROPERTY, project)
+    }
+}

Diff for: src/test/groovy/org/gradle/android/CrossVersionOutcomeAndRelocationTest.groovy

@@ -265,19 +265,19 @@ class CrossVersionOutcomeAndRelocationTest extends AbstractTest {
         builder.expect(':app:kaptDebugKotlin', FROM_CACHE)
         builder.expect(':app:kaptGenerateStubsReleaseKotlin', FROM_CACHE)
         builder.expect(':app:kaptReleaseKotlin', FROM_CACHE)
-        builder.expect(':app:mergeDebugAssets', FROM_CACHE)
+        builder.expect(':app:mergeDebugAssets', SUCCESS)
         builder.expect(':app:mergeDebugJavaResource', SUCCESS)
-        builder.expect(':app:mergeDebugJniLibFolders', FROM_CACHE)
-        builder.expect(':app:mergeDebugShaders', FROM_CACHE)
+        builder.expect(':app:mergeDebugJniLibFolders', SUCCESS)
+        builder.expect(':app:mergeDebugShaders', SUCCESS)
         builder.expect(':app:mergeDexRelease', FROM_CACHE)
         builder.expect(':app:mergeExtDexDebug', FROM_CACHE)
         builder.expect(':app:mergeExtDexRelease', FROM_CACHE)
         builder.expect(':app:mergeLibDexDebug', FROM_CACHE)
         builder.expect(':app:mergeProjectDexDebug', FROM_CACHE)
-        builder.expect(':app:mergeReleaseAssets', FROM_CACHE)
+        builder.expect(':app:mergeReleaseAssets', SUCCESS)
         builder.expect(':app:mergeReleaseJavaResource', SUCCESS)
-        builder.expect(':app:mergeReleaseJniLibFolders', FROM_CACHE)
-        builder.expect(':app:mergeReleaseShaders', FROM_CACHE)
+        builder.expect(':app:mergeReleaseJniLibFolders', SUCCESS)
+        builder.expect(':app:mergeReleaseShaders', SUCCESS)
         builder.expect(':app:mergeRoomSchemaLocations', SUCCESS)
         builder.expect(':app:packageDebug', SUCCESS)
         builder.expect(':app:packageRelease', SUCCESS)
@@ -325,16 +325,16 @@ class CrossVersionOutcomeAndRelocationTest extends AbstractTest {
         builder.expect(':library:kaptGenerateStubsReleaseKotlin', FROM_CACHE)
         builder.expect(':library:kaptReleaseKotlin', FROM_CACHE)
         builder.expect(':library:mergeDebugJavaResource', SUCCESS)
-        builder.expect(':library:mergeDebugJniLibFolders', FROM_CACHE)
-        builder.expect(':library:mergeDebugShaders', FROM_CACHE)
+        builder.expect(':library:mergeDebugJniLibFolders', SUCCESS)
+        builder.expect(':library:mergeDebugShaders', SUCCESS)
         builder.expect(':library:mergeReleaseJavaResource', SUCCESS)
-        builder.expect(':library:mergeReleaseJniLibFolders', FROM_CACHE)
+        builder.expect(':library:mergeReleaseJniLibFolders', SUCCESS)
         builder.expect(':library:mergeReleaseResources', FROM_CACHE)
-        builder.expect(':library:mergeReleaseShaders', FROM_CACHE)
-        builder.expect(':library:packageDebugAssets', FROM_CACHE)
+        builder.expect(':library:mergeReleaseShaders', SUCCESS)
+        builder.expect(':library:packageDebugAssets', SUCCESS)
         builder.expect(':library:packageDebugRenderscript', NO_SOURCE)
         builder.expect(':library:packageDebugResources', FROM_CACHE)
-        builder.expect(':library:packageReleaseAssets', FROM_CACHE)
+        builder.expect(':library:packageReleaseAssets', SUCCESS)
         builder.expect(':library:packageReleaseRenderscript', NO_SOURCE)
         builder.expect(':library:packageReleaseResources', FROM_CACHE)
         builder.expect(':library:mergeRoomSchemaLocations', SUCCESS)

Diff for: src/test/groovy/org/gradle/android/WorkaroundTest.groovy

@@ -12,11 +12,11 @@ class WorkaroundTest extends Specification {
         workarounds.collect { it.class.simpleName.replaceAll(/Workaround/, "") }.sort() == expectedWorkarounds.sort()
         where:
         androidVersion  | expectedWorkarounds
-        "7.0.0"         | ['MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_7_0']
-        "4.2.2"         | ['MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_4_2', 'MergeResources']
-        "4.1.3"         | ['MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_4_0', 'MergeResources']
-        "4.0.2"         | ['MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_4_0', 'MergeResources']
-        "3.6.4"         | ['MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs']
-        "3.5.4"         | ['MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs']
+        "7.0.0"         | ['MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_7_0']
+        "4.2.2"         | ['MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_4_2', 'MergeResources']
+        "4.1.3"         | ['MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_4_0', 'MergeResources']
+        "4.0.2"         | ['MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs', 'CompileLibraryResources_4_0', 'MergeResources']
+        "3.6.4"         | ['MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs']
+        "3.5.4"         | ['MergeSourceSetFolders', 'MergeJavaResources', 'RoomSchemaLocation', 'StripDebugSymbols', 'MergeNativeLibs']
     }
 }