fix: register src/paper/java as a Java source set on old architecture - #755
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #754 — 5.9.0 crashes on launch on the old architecture:
android/src/paper/javaholds five.javafiles —NativeSafeAreaContextSpec.javaplus the fourRNCSafeArea*Manager{Delegate,Interface}.java— alongside two.ktfiles. The AGP 9 DSL migration in 42c50a6 (#733) registered that directory only underkotlin.directories. On some AGP/Kotlin combinations the Kotlin compiler reads those.javafiles for resolution but never emits class files for them, and javac never sees the directory, so they're silently dropped from the AAR.SafeAreaContextModulethen can't resolve its superclass at runtime.This adds the directory to
java.directoriesas well, mirroring what the new-arch branch directly above already does. It's explicit either way, so it's correct regardless of which compiler picks the files up.On reproducibility: I could not reproduce the drop with the example's toolchain (AGP 8.12.0, Kotlin 2.1.20, Gradle 9.3.1) — a clean old-arch build there produces all five classes. But it reproduces in a minimal AGP 8.12 +
kotlin-androidlibrary (below), and the reporter hasdexdumpevidence of the classes being absent from every dex in a production AAB built with AGP 8.11.0 / RN 0.81.5 / Gradle 8.14.3. I haven't pinned down exactly which part of the toolchain flips the behavior, so I'd treat the exact trigger as unconfirmed — but the source set registration is wrong as written either way.Worth noting
android.ymldoes build the example with-PnewArchEnabled=false, but a missing class still compiles clean and nothing launches the app, so CI can't catch this class of regression today.Test Plan
Minimal AGP 8.12.0 +
kotlin-androidlibrary reproducing the config — a.javaand a.ktin a paper-style dir, plus a Kotlin class insrc/mainextending the Java one:Before (
kotlin.directoriesonly) — build succeeds, no warning:After:
Also verified the fix on the AGP 9.2.1 built-in-Kotlin path (where the current config instead fails loudly at compile time with
Unresolved reference 'PaperJavaSpec'), and that a clean old-arch build of the example still produces a complete AAR: