Including source file information with residual names that overlap input names is not correctly represented [279702361]

Fixed

Bug

Status Update

No update yet.

Description

ze...@google.com

created issue #1

Apr 26, 2023 07:56AM

Implementing b/214018111 shows an issue found in ClassNameMinifierOriginalClassNameTest.

Here the expected encoding of the mapping would become:

com.android.tools.r8.naming.testclasses.A -> com.android.tools.r8.naming.testclasses.A:
# {"id":"sourceFile","fileName":"A.java"}

com.android.tools.r8.naming.testclasses.B -> com.android.tools.r8.naming.testclasses.A:
    1:1:void com.android.tools.r8.naming.testclasses.A.<init>():0:0 -> <init>
    1:1:void <init>():0 -> <init>
    void foo() -> a

But the overlap in residual name A causes the map to be clobbered. The source for original type A is needed, but it has no residual output so the mapping should be something like:

com.android.tools.r8.naming.testclasses.A -> removed:
# {"id":"sourceFile","fileName":"A.java"}

com.android.tools.r8.naming.testclasses.B -> com.android.tools.r8.naming.testclasses.A:
    1:1:void com.android.tools.r8.naming.testclasses.A.<init>():0:0 -> <init>
    1:1:void <init>():0 -> <init>
    void foo() -> a

Comments

ap...@google.com <ap...@google.com> #2Apr 26, 2023 09:08AM

Will be this fix also part of 4.2 please?

ap...@google.com <ap...@google.com> #3Apr 26, 2023 09:08AM

for the "out" bundle issue, this seems indeed an issue. for the second one reported adding "debugUpdateAPKArtifact" to the path, this WAI as there can more more than one transformer and we need to automatically separate them so they do not use the same output folder. for #3, I will look at it but it might be difficult to change it at this time.

ap...@google.com <ap...@google.com> #4Apr 27, 2023 07:37AM

I understand that if there is more transformer is necessary to make every folder different, but there is the main problem, that the final one (last transform) is inside of the intermediate folder. The last transformation should end in the standard location, right? Maybe also with the default name not?

ap...@google.com <ap...@google.com> #5May 1, 2023 12:08PM

Hello, this is a P1 S1 issue for a while now, and it's blocking us from upgrading to AGP 4.2 and further. What's the current status? And do you have plans to backport it to 4.2.x?

ap...@google.com <ap...@google.com> #6May 1, 2023 12:08PM

no we don't have a plan to backport this. why is this blocking you from upgrading ?

ap...@google.com <ap...@google.com> #7May 1, 2023 12:08PM

We sign apk's and bundles via internal service, and i don't know another way to interact with artifacts; Yesterday i ended up with some ugly workaround: https://github.com/avito-tech/avito-android/pull/1085/files#diff-b898cc20fedebe394439bb37ffb91d3366a1f9a6d359dbdebca79af8a079f911 I'd be glad if you'll point me out how can it be done better.

ap...@google.com <ap...@google.com> #8May 1, 2023 12:08PM

I commented on the merge request but to summarize :

you can set your signing tasks output folder to wherever you need to be :

artifacts.use(target.tasks.signedApkTaskProvider(this)).configure { signedDirProperty.set(File("/path/to/where/you/want/your/signed/files")) }

if you don't set it, then we will set a directory automatically.

ap...@google.com <ap...@google.com> #9May 1, 2023 12:10PM

Thanks, Jerome!

ap...@google.com <ap...@google.com> #10May 2, 2023 08:41AM

I checked that we do have correct behavior for FILE based artifacts like bundle :

> Task :app:debugUpdateArtifact
originalArtifact: /usr/local/google/home/jedo/src/studio-main/out/apiTests/Kotlin/bugTest/app/build/intermediates/bundle/debug/signDebugBundle/app-debug.aab
updatedArtifact: /usr/local/google/home/jedo/src/studio-main/out/apiTests/Kotlin/bugTest/app/build/outputs/bundle/debug/app-debug.aab

however, we still seem to not be consistent for the DIRECTORY based artifacts like APK:

> Task :app:debugUpdateAPKArtifact
Input folder: /usr/local/google/home/jedo/src/studio-main/out/apiTests/Kotlin/bugTest/app/build/outputs/apk/debug
Output folder: /usr/local/google/home/jedo/src/studio-main/out/apiTests/Kotlin/bugTest/app/build/intermediates/apk/debug
Input file: /usr/local/google/home/jedo/src/studio-main/out/apiTests/Kotlin/bugTest/app/build/outputs/apk/debug/app-debug.apk

Alex, can you have a look ?

sg...@google.com <sg...@google.com> #11May 8, 2023 04:22PM

Marked as fixed.

correction, I was not correct about FILE being correct.

Instead of :

updatedArtifact: /usr/local/google/home/jedo/src/studio-main/out/apiTests/Kotlin/bugTest/app/build/outputs/bundle/debug/app-debug.aab

it should be

updatedArtifact: /usr/local/google/home/jedo/src/studio-main/out/apiTests/Kotlin/bugTest/app/build/outputs/app-debug.aab
``

gr...@gmail.com <gr...@gmail.com> #12Mar 9, 2024 09:40AM

Two requests are created as they are API changes b/241581686 (set artifact name for transformation) and b/241582907 (change string to OutputFile)

gr...@gmail.com <gr...@gmail.com> #13Mar 9, 2024 09:41AM

Fix will be available in Android Studio F/AGP 8.0 Canary 2 Release