[AGP] Add generated source directory to IDE model (Variant API) [232805583]

Fixed

Bug

Status Update

No update yet.

Description

[Deleted User]

created issue #1

May 17, 2022 10:51AM

The new Variant API SourceDirectoriesImpl.addGeneratedSourceDirectory() allows to add a custom task generating source files that need to be compiled within the build process, but does not add the generated source directory to the IDE model in TaskProviderBasedDirectoryEntryImpl.

It is used in replacement of the old API BaseVariantImpl.registerJavaGeneratingTask() that automatically added the generated source directories to the IDE model.

It seems to me that the old behavior should be maintained, i.e., either:

set shouldBeAddedToIdeModel to true by default in the constructor of TaskProviderBasedDirectoryEntryImpl,
pass shouldBeAddedToIdeModel = true when creating TaskProviderBasedDirectoryEntryImpl in addGeneratedSourceDirectory,
add an optional argument to addGeneratedSourceDirectory for letting the user set the desired value of shouldBeAddedToIdeModel.

Note that it should go the same way with SourceDirectoriesImpl.addStaticSourceDirectory().

Also, I would personally not set the path of the output directory here if it has already been set by the user before calling addGeneratedSourceDirectory, since this overwriting is not documented and not obvious to the user. Or similarly to shouldBeAddedToIdeModel, this behavior (setting or not the output directory) could be controlled through an optional argument to addGeneratedSourceDirectory.

Please consider this other issue too that might be relevant as well (I did not try to reproduce it), it apppears not only in TaskProviderBasedDirectoryEntryImpl but also in ProviderBasedDirectoryEntryImpl.

Studio Build: #AI-213.7172.25.2211.8571212 (Android Studio Electric Eel | 2022.1.1 Canary 2)

Version of Gradle Plugin: 7.4.0-alpha02

Version of Gradle: 7.5-rc-1

Version of Java: 1.8

OS: Ubuntu 18.04

Comments

mk...@google.com <mk...@google.com> May 18, 2022 12:05PM

Assigned to an...@google.com.

je...@google.com <je...@google.com> #2May 24, 2022 08:02PM

Reassigned to al...@google.com.

Thank you for the report and the supplied dump with the reproduction. I can confirm that the NPE is also present on HEAD, and I will look into it.

Quite a few of the class files in the dump has malformed inner-class attribute, where R8 reports e.g.:

Info in /tmp/tmpo67ucguh/program.jar:ofifmwrrhhmodsc/ψξψξψξξ.class:
Malformed inner-class attribute:
	outerTypeInternal: ofifmwrrhhmodsc/ψξψξψξξ
	innerTypeInternal: ofifmwrrhhmodsc/ψξψξψξξ$ψξξψξξξ
	innerName: ψξψξψξξ$ψξξψξξξ

These are ignored, so they should not be the reason for the NPE, but something to also look into.

al...@google.com <al...@google.com> Jun 3, 2022 03:53PM

Accepted by al...@google.com.

[Deleted User] <[Deleted User]> #3Jul 1, 2022 06:56PM

Project: r8
Branch: main

commit e59500ca4445f5fb5d2ccc1114acbf66df11d196
Author: Søren Gjesse <sgjesse@google.com>
Date: Tue Mar 21 09:48:55 2023

Insert patched frame instructions in the right place

Fixes:

b/274337639

Change-Id: Iae204ed529f9af5bf5fa3eaacba45128dfa6fd9e

M src/main/java/com/android/tools/r8/graph/LazyCfCode.java
M src/test/java/com/android/tools/r8/cf/frames/InitBeforeNewInInstructionStreamTest.java

https://r8-review.googlesource.com/77390

al...@google.com <al...@google.com> #4Jul 22, 2022 06:50PM

Project: r8
Branch: main

commit 780c2e83c8c033e7594d61894fa1b3dbcad6630b
Author: Søren Gjesse <sgjesse@google.com>
Date: Tue Mar 21 09:48:13 2023

Add reproduction of

b/274337639

Bug:

b/274337639

Change-Id: I7573883d6a649eed47b10d7bf0cdea2e11710268

M build.gradle
M src/main/java/com/android/tools/r8/cf/code/frame/UninitializedNew.java
A src/test/java/com/android/tools/r8/cf/frames/InitBeforeNewInInstructionStreamTest.java

https://r8-review.googlesource.com/77389

al...@google.com <al...@google.com> #5Sep 16, 2022 11:13PM

Marked as fixed.

I will backport this to the 8.0 and 4.0 branches.

mi...@gmail.com <mi...@gmail.com> #6Sep 18, 2022 01:17PM

Thank you very much for taking a look at this!

Is this fix related to the nullpointer or the malformed inner-class attributes ?

Do you have a separated ticket for it? Could you share it?

cm...@google.com <cm...@google.com> #7Oct 10, 2022 04:54PM

Accepted by cm...@google.com.

Project: r8
Branch: 4.0

commit 6edb00fb23e41b28d4794cfa69d706d5c5aa3fba
Author: Søren Gjesse <sgjesse@google.com>
Date: Tue Mar 21 16:55:01 2023

Version 4.0.56

Bug:

b/274337639

Change-Id: I88a20fc922ce892d38fa3a34f9a3f1f4714354ae

M src/main/java/com/android/tools/r8/Version.java

https://r8-review.googlesource.com/77447

cm...@google.com <cm...@google.com> #8Oct 12, 2022 03:03PM

Marked as fixed.

Project: r8
Branch: 4.0

commit d9215af3b02f64d1207b9ebd63a6925229307b9d
Author: Søren Gjesse <sgjesse@google.com>
Date: Tue Mar 21 16:54:50 2023

Insert patched frame instructions in the right place

Bug:

b/274337639

https://r8-review.googlesource.com/77446

fr...@gmail.com <fr...@gmail.com> #9Oct 13, 2022 03:50PM

Project: r8
Branch: 4.0

commit 4fda9b9062c871ba0e31cb68d1518b13b993692f
Author: Søren Gjesse <sgjesse@google.com>
Date: Tue Mar 21 16:54:33 2023

Add reproduction of

b/274337639

Bug:

b/274337639

https://r8-review.googlesource.com/77445

Issue 232805583

Description

Issue summary

Comments

mk...@google.com <mk...@google.com> May 18, 2022 12:05PM

je...@google.com <je...@google.com> #2May 24, 2022 08:02PM

al...@google.com <al...@google.com> Jun 3, 2022 03:53PM

[Deleted User] <[Deleted User]> #3Jul 1, 2022 06:56PM

al...@google.com <al...@google.com> #4Jul 22, 2022 06:50PM

al...@google.com <al...@google.com> #5Sep 16, 2022 11:13PM

mi...@gmail.com <mi...@gmail.com> #6Sep 18, 2022 01:17PM

cm...@google.com <cm...@google.com> #7Oct 10, 2022 04:54PM

cm...@google.com <cm...@google.com> #8Oct 12, 2022 03:03PM

fr...@gmail.com <fr...@gmail.com> #9Oct 13, 2022 03:50PM

Add comment

Issue metadata