[TrimmableTypeMap] Fix stale legacy JCW contamination when switching typemap flavors#11098
Open
simonrozsival wants to merge 4 commits intomainfrom
Open
[TrimmableTypeMap] Fix stale legacy JCW contamination when switching typemap flavors#11098simonrozsival wants to merge 4 commits intomainfrom
simonrozsival wants to merge 4 commits intomainfrom
Conversation
simonrozsival
force-pushed
the
dev/simonrozsival/fix-trimmable-register-guard
branch
from
de004d7 to
36b07f1
Compare
simonrozsival
changed the title
simonrozsival
force-pushed
the
dev/simonrozsival/fix-trimmable-register-guard
branch
from
36b07f1 to
e628b09
Compare
…typemap flavors When switching MonoAndroidTypeMapFlavor between legacy and trimmable without a clean build, stale legacy JCWs with Runtime.register(typeName, klass, methods) calls persisted in the android/src/ intermediate directory alongside trimmable JCWs that only use Runtime.registerNatives(). Both got compiled into the APK, causing the legacy registration path to execute at runtime — a path incompatible with trimming. Root cause: the trimmable JCW generator wrote to typemap/java/ and then copied to android/src/, but never cleaned android/src/ first. Legacy JCWs with different package names survived the copy overlay. Three fixes: 1. Write trimmable JCWs directly to _AndroidIntermediateJavaSourceDirectory (android/src/) instead of a separate typemap/java/ directory, eliminating the copy step entirely. This is the same directory that _FindJavaStubFiles globs from, so no files can be missed or stale. 2. Add MonoAndroidTypeMapFlavor to the build properties cache so switching between legacy and trimmable triggers _CleanIntermediateIfNeeded, removing all stale artifacts from the previous flavor. 3. Always wire registerJniNativesFn so that if a stale JCW somehow calls Runtime.register(), TrimmableTypeMapTypeManager.RegisterNativeMembers throws UnreachableException with a clear diagnostic instead of the C++ side silently dropping the call. Also adds a unit test verifying trimmable JCWs never emit Runtime.register(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
force-pushed
the
dev/simonrozsival/fix-trimmable-register-guard
branch
from
e628b09 to
d3cfd10
Compare
simonrozsival
added
trimmable-type-map
copilot
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an incremental-build hazard where switching _AndroidTypeMapImplementation between llvm-ir and trimmable could leave stale legacy JCWs in android/src/, causing Runtime.register() to get compiled into a trimmable APK and break trimming expectations at runtime.
Changes:
- Emit trimmable JCWs directly into
_AndroidIntermediateJavaSourceDirectoryto avoid stale overlay artifacts. - Add
_AndroidTypeMapImplementationto the build properties cache so implementation switches trigger_CleanIntermediateIfNeeded. - Always wire
registerJniNativesFnduring runtime initialization and add unit + integration coverage for the regression.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/JcwJavaSourceGeneratorTests.cs | Adds a regression assertion that trimmable JCWs never emit the legacy Runtime.register(...) pattern. |
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets | Includes _AndroidTypeMapImplementation in _PropertyCacheItems so switching typemap flavors invalidates intermediates. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs | Adds an integration test verifying _CleanIntermediateIfNeeded runs when _AndroidTypeMapImplementation changes. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets | Directs typemap generation to write Java sources into android/src/ and removes the copy/overlay step. |
| src/Mono.Android/Android.Runtime/JNIEnvInit.cs | Removes the trimmable-mode guard so registerJniNativesFn is always set. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Switching
_AndroidTypeMapImplementationbetweenllvm-irandtrimmableon an incremental build could leave stale legacy Java callable wrappers underandroid/src/. That meant a trimmable build could still compile oldRuntime.register (...)calls into the APK and fall back to the legacy reflection-based registration path.Fix
This PR closes the gap in three places:
_AndroidTypeMapImplementationto the build property cache so implementation changes trigger_CleanIntermediateIfNeeded_AndroidIntermediateJavaSourceDirectoryinstead oftypemap/javaplus a copy/overlay stepregisterJniNativesFnduring runtime initialization so unexpected legacyRuntime.register (...)usage is surfaced through the normal runtime path instead of being silently skippedTests
JcwJavaSourceGeneratorTests.Generate_AcwType_NeverCallsRuntimeRegisterBuildTest.SwitchingTypeMapImplementationTriggersCleanThe integration test also verifies that switching both directions (
llvm-ir<->trimmable) forces the intermediate clean, while a no-change rebuild still skips it.