Make CompileLibraryResourcesTask and MergeResources task relocatable [141301405]

Fixed

Bug

Reviewed (L1)

Status Update

No update yet.

Description

ga...@google.com

created issue #1

Sep 19, 2019 03:15PM

Incremental state of LinkApplicationAndroidResourcesTask task should not be marked as @OutputDirectory, it should be @LocalState because it contains absolute paths.

When there is a remote build cache hit, local machine will get incremental state from the remote cache which contains absolute paths from e.g. CI server. Next run, which may be incremental, will use this invalid information, which may result in failures like this one:

> A failure occurred while executing com.android.build.gradle.internal.tasks.Workers$ActionFacade
> Android resource linking failed
AAPT: <CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4098: error: resource attr/flow_horizontalSeparator (aka <package-name>:attr/flow_horizontalSeparator) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4098: error: resource attr/flow_verticalSeparator (aka <package-name>:attr/flow_verticalSeparator) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4099: error: resource attr/flow_horizontalSeparator (aka <package-name>:attr/flow_horizontalSeparator) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4099: error: resource attr/flow_verticalSeparator (aka <package-name>:attr/flow_verticalSeparator) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4101: error: resource attr/flow_horizontalSeparator (aka <package-name>:attr/flow_horizontalSeparator) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4101: error: resource attr/flow_verticalSeparator (aka <package-name>:attr/flow_verticalSeparator) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4101: error: resource attr/motionProgress (aka <package-name>:attr/motionProgress) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4356: error: resource attr/motionProgress (aka <package-name>:attr/motionProgress) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4357: error: resource attr/motionProgress (aka <package-name>:attr/motionProgress) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4366: error: resource attr/motionProgress (aka <package-name>:attr/motionProgress) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4366: error: resource attr/waveDecay (aka <package-name>:attr/waveDecay) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4610: error: resource attr/motionPathRotate (aka <package-name>:attr/motionPathRotate) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4612: error: resource attr/motionProgress (aka <package-name>:attr/motionProgress) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4651: error: resource attr/motionProgress (aka <package-name>:attr/motionProgress) not found.
<CI workspace>/build/intermediates/incremental/mergeDebugResources/merged.dir/values/values.xml:4999: error: resource attr/duration (aka <package-name>:attr/duration) not found.
error: failed linking references.

Stacktrace is:
at com.android.builder.internal.aapt.v2.Aapt2Exception$Companion.create(Aapt2Exception.kt:45)
at com.android.builder.internal.aapt.v2.Aapt2Exception$Companion.create$default(Aapt2Exception.kt:39)
at com.android.build.gradle.internal.res.Aapt2ErrorUtils.rewriteException(Aapt2ErrorUtils.kt:195)
at com.android.build.gradle.internal.res.Aapt2ErrorUtils.rewriteLinkException(Aapt2ErrorUtils.kt:126)
at com.android.build.gradle.internal.res.LinkApplicationAndroidResourcesTask$AaptSplitInvoker.invokeAaptForSplit(LinkApplicationAndroidResourcesTask.kt:864)
at com.android.build.gradle.internal.res.LinkApplicationAndroidResourcesTask$AaptSplitInvoker.run(LinkApplicationAndroidResourcesTask.kt:737)
at com.android.build.gradle.internal.tasks.Workers$ActionFacade.run(Workers.kt:348)
at org.gradle.workers.internal.AdapterWorkAction.execute(AdapterWorkAction.java:50)
at org.gradle.workers.internal.DefaultWorkerServer.execute(DefaultWorkerServer.java:47)
at org.gradle.workers.internal.NoIsolationWorkerFactory$1$1$1.create(NoIsolationWorkerFactory.java:65)
at org.gradle.workers.internal.NoIsolationWorkerFactory$1$1$1.create(NoIsolationWorkerFactory.java:61)
at org.gradle.internal.classloader.ClassLoaderUtils.executeInClassloader(ClassLoaderUtils.java:98)
at org.gradle.workers.internal.NoIsolationWorkerFactory$1$1.execute(NoIsolationWorkerFactory.java:61)
at org.gradle.workers.internal.AbstractWorker$1.call(AbstractWorker.java:44)
at org.gradle.workers.internal.AbstractWorker$1.call(AbstractWorker.java:41)
at org.gradle.internal.operations.DefaultBuildOperationExecutor$CallableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:416)

Comments

im...@google.com <im...@google.com> #2Sep 23, 2019 01:50PM

There are two things that are happening here:
1. The error is saying that the attributes used were not defined, and they match exactly the error in

b/136103084

- which leads me to believe that perhaps between the builds the gradle version was updated and that's why the bug in constraint layout surfaced.
2. Aapt2 stores the absolute path in the compiled resources for reporting errors. This is why the <CI workspace> path is shown since that was stored in the file. In order to fix this we need to make the input resource-sets to tasks that compile resources as PathSensitivity.ABSOLUTE. A better non-temporary fix would be to make aapt2 accept storing non-absolute paths and for us to figure out the proper error reporting ourselves.

For 1 follow

b/136103084

, for 2 I'll do a temporary fix of using PathSensitivity.ABSOLUTE and I'll see what we can do to avoid storing the absolute paths.

[Deleted User] <[Deleted User]> #3Sep 23, 2019 06:22PM

Re 1., the build in question had just added


<attr name="motionProgress" />
<attr name="motionPathRotate" />
<attr name="waveDecay" />
<attr name="flow_verticalSeparator" />
<attr name="flow_horizontalSeparator" />

to app/src/main/res/values/attrs.xml. There was, however, a second com.android.application module that hadn't added that yet. If I'm understanding you correctly, you're saying that this wasn't really trying to reference a path that didn't exist locally (but did exist on the CI workspace), but rather it just _reports_ that path in an attempt to provide a user-friendly error report?

And as far as I can tell, both builds (the one that failed and the one that pushed to the cache) were on Gradle 5.6.2.

im...@google.com <im...@google.com> #4Sep 24, 2019 10:29AM

If you have two app modules and both have (direct or in-direct) dependency on the broken constraint layout then you either need to add those attrs to a common library module that they both depend on, or to both app modules.

And yes, aapt2 wasn't trying to access that path. It consumed a compiled file that had that path saved as the source path just for error messaging, but because the original file to-be-compiled was marked as PathSensitivity.Relative (while it should be Absolute). I'm gonna work on avoiding storing these absolute paths in the compiled res files, but until then for accurate error reporting we'll need to resolve to using PathSensitivity.Absolute for the resources to be compiled.

hu...@google.com <hu...@google.com> #5Nov 11, 2019 05:19PM

I'm renaming the bug title to make it more general, as this will probably be a multi-commit fix.

cr...@gmail.com <cr...@gmail.com> #6May 12, 2020 11:31AM

That "temporary fix of using PathSensitivity.ABSOLUTE" is 8 months old, any plan to fix it and make it Relative again?

im...@google.com <im...@google.com> #7May 12, 2020 01:10PM

@6 Thank you for checking in, we're working on it.

cr...@gmail.com <cr...@gmail.com> #8May 18, 2020 10:44AM

Great!
After applying

https://github.com/gradle/android-cache-fix-gradle-plugin (using AGP 4.0.0-rc01) 90% of the tasks not coming from the cache are CompileLibraryResourcesTask

im...@google.com <im...@google.com> #9Jul 13, 2020 02:36PM

Reassigned to lu...@google.com.

Reassigning to Luke who's working on relative path support in compiled res files.

lu...@google.com <lu...@google.com> Sep 7, 2020 01:46PM

Accepted by lu...@google.com.

ku...@gmail.com <ku...@gmail.com> #10May 5, 2021 10:09PM

Any update on this? Would be great if this could be cacheable

lu...@google.com <lu...@google.com> #11May 6, 2021 08:37AM

Hey Kurtis, we currently have this issue fixed for Linux and MacOS, there is still some more work to be done on Windows. If you are using Linux or MacOS you can add the below to your gradle.properties file which will make these tasks cacheable/relocatable:

android.experimental.enableSourceSetPathsMap=true
android.experimental.cacheCompileLibResources=true

ku...@gmail.com <ku...@gmail.com> #12May 6, 2021 05:50PM

Ah thanks, I see this is only on AGP 7.0, but unfortunately is not in 4.2. Will give it a try when testing AGP 7.

ga...@gmail.com <ga...@gmail.com> #13Jul 29, 2021 06:54AM

I've tried out the flags and it's working very well. There is just one cache miss which is the MergeResources task in com.android.application modules. I'm seeing that the rawLocalResourcesProcessRes uses absolute paths. In com.android.library the task cache is relocatable though.

ku...@gmail.com <ku...@gmail.com> #14Aug 5, 2021 07:33PM

I also notice the same behavior. Is there another ticket for tracking the work of making MergeResources use relative paths?

eu...@gmail.com <eu...@gmail.com> #15Sep 30, 2021 05:50PM

I'm seeing that the rawLocalResourcesProcessRes uses absolute paths. In com.android.library the task cache is relocatable though.

I see the opposite. rawLocalResourcesProcessRes with absolute paths in MergeResources is still not relocatable:

:module:mergeReleaseUnitTestResources // on the same commit

Appending input file fingerprints for 'rawLocalResourcesProcessRes' to build cache key: 887de39fc62d5f1ffaa09199a31cdfa0 - ABSOLUTE_PATH{/home/ekrivobokov/projects/module/src/test/res='/home/ekrivobokov/projects/module/src/test/res' / MISSING, /home/ekrivobokov/projects/module/src/testRelease/res='/home/ekrivobokov/projects/module/src/testRelease/res' / MISSING}

Appending input file fingerprints for 'rawLocalResourcesProcessRes' to build cache key: 4bca0657315f28731d11494d8a552adf - ABSOLUTE_PATH{/Users/ekrivobokov/projects/module/src/test/res='/Users/ekrivobokov/projects/module/src/test/res' / MISSING, /Users/ekrivobokov/projects/module/src/testRelease/res='/Users/ekrivobokov/projects/module/src/testRelease/res' / MISSING}

ch...@instacart.com <ch...@instacart.com> #16Oct 14, 2021 09:08PM

#11 Applying the flags does not fix it:
android.experimental.enableSourceSetPathsMap=true
android.experimental.cacheCompileLibResources=true

Still seeing MergeDebugResources as absolute path on AGP 7.0.3 (Gradle 7.3-rc-1) (Linux Ubuntu)

ch...@instacart.com <ch...@instacart.com> #17Nov 2, 2021 10:38PM

Still no update on this, if you won't fix, can you at least let us fix this using the cache-fix-plugin

ga...@google.com <ga...@google.com> #18Nov 2, 2021 10:48PM

The bug is still open and we are working on it. Once we make more progress, an update will be posted here. Thanks.

ch...@instacart.com <ch...@instacart.com> #19Nov 3, 2021 04:56AM

Thank you!

pj...@grubhub.com <pj...@grubhub.com> #20Nov 29, 2021 02:13PM

With the flags enabled see that
LinkApplicationAndroidResourcesTask
got CACHED
but the
MergeResources
did not

BR

AGP 7.0.3 Gradle 7.2

ey...@gmail.com <ey...@gmail.com> #21Jan 28, 2022 09:14PM

Using AGP 7.2.0-beta01 I get a warning:

WARNING: Android plugin 7.2.0-beta01 has experimental support for using relative path sensitivity with CompileLibraryResourcesTask inputs which will provide more build cache hits and improve build speed.  Set 'android.experimental.cacheCompileLibResources=true' and 'android.experimental.enableSourceSetPathsMap=true' in gradle.properties to enable this support.

Even though I have the following in my gradle.properties:

android.experimental.cacheCompileLibResources=true
android.experimental.enableSourceSetPathsMap=true

ga...@gradle.com <ga...@gradle.com> #22Jan 28, 2022 09:26PM

That warning is not coming from AGP - it's coming from

https://github.com/gradle/android-cache-fix-gradle-plugin. If this is reproducible, please raise an issue there and we'll take a look at it.

lu...@google.com <lu...@google.com> #23May 3, 2022 04:36PM

Marked as fixed.

Marking this issue as fixed, relative path normalisation are enabled by default in AGP 7.2 for MergeResources and CompileLibraryResourcesTask.

no...@gradle.com <no...@gradle.com> #24Jun 15, 2022 12:15PM

This is still happening in AGP 7.2. I've opened this issue to track: https://issuetracker.google.com/issues/236001083