Remove Java.Interop.GenericMarshaler#1450
Open
simonrozsival wants to merge 1 commit into
Open
Conversation
Remove the standalone generic JNI marshaling helper assembly and its performance tests. Keep Java.Interop-Tests compiling by moving the few needed helper wrappers into the test assembly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR removes the standalone Java.Interop.GenericMarshaler assembly (and its performance benchmark project) as part of the repo reduction effort in #1448, while keeping required generic marshaling test coverage by introducing a small test-local replacement for the few extension methods the unit tests still need.
Changes:
- Removed the
src/Java.Interop.GenericMarshalerproject and theJava.Interop-PerformanceTestsproject (and associated sources/CI/docs references). - Updated
Java.Interop-Teststo stop referencingJava.Interop.GenericMarshalerand added a test-localJniPeerMembersExtensionshelper to cover required generic marshaling paths. - Removed the
InternalsVisibleToentry forJava.Interop.GenericMarshalerfromJava.Interop.
Show a summary per file
| File | Description |
|---|---|
| tests/Java.Interop-Tests/Java.Interop/TestType.cs | Drops Java.Interop.GenericMarshaler using; continues to exercise generic marshaling via test-local extensions. |
| tests/Java.Interop-Tests/Java.Interop/JniPeerMembersTests.cs | Drops Java.Interop.GenericMarshaler using; keeps tests using extension-method-based generic marshal paths. |
| tests/Java.Interop-Tests/Java.Interop/JniPeerMembersExtensions.cs | Adds test-local extension methods replacing the limited generic marshaling helpers previously sourced from the deleted assembly. |
| tests/Java.Interop-Tests/Java.Interop/CallVirtualFromConstructorBase.cs | Drops Java.Interop.GenericMarshaler using; continues constructor/method invocation coverage via test-local extensions. |
| tests/Java.Interop-Tests/Java.Interop-Tests.csproj | Removes project reference to Java.Interop.GenericMarshaler. |
| tests/Java.Interop-PerformanceTests/java/com/xamarin/interop/performance/JavaTiming.java | Deletes Java timing helper source as part of removing the performance test project. |
| tests/Java.Interop-PerformanceTests/Java.Interop/TimingTests.cs | Deletes performance benchmark test suite. |
| tests/Java.Interop-PerformanceTests/Java.Interop/JavaVMFixture.cs | Deletes performance-test JVM fixture. |
| tests/Java.Interop-PerformanceTests/Java.Interop/JavaTiming.cs | Deletes managed wrapper used by performance benchmarks (and its generic marshal benchmark paths). |
| tests/Java.Interop-PerformanceTests/Java.Interop/JavaPrimitiveArraysTiming.cs | Deletes primitive array lookup/creation performance benchmarks. |
| tests/Java.Interop-PerformanceTests/Java.Interop-PerformanceTests.targets | Deletes MSBuild target used to build the performance-test JAR. |
| tests/Java.Interop-PerformanceTests/Java.Interop-PerformanceTests.csproj | Deletes the performance test project definition. |
| src/Java.Interop/Properties/AssemblyInfo.cs | Removes InternalsVisibleTo for Java.Interop.GenericMarshaler. |
| src/Java.Interop.GenericMarshaler/Java.Interop.GenericMarshaler/JniPeerInstanceMethodsExtensions.tt | Deletes T4 template which generated the generic marshaling extension surface. |
| src/Java.Interop.GenericMarshaler/Java.Interop.GenericMarshaler.csproj | Deletes the Java.Interop.GenericMarshaler project file. |
| Java.Interop.sln | Removes the deleted projects from the solution. |
| build-tools/automation/templates/core-tests.yaml | Removes CI execution of the deleted performance test project. |
| .github/skills/build-and-test/SKILL.md | Removes mention of Java.Interop-PerformanceTests from the documented common test projects list. |
| .github/copilot-instructions.md | Removes mention of Java.Interop-PerformanceTests from repo testing structure documentation. |
Copilot's findings
- Files reviewed: 19/20 changed files
- Comments generated: 0
Member
Author
|
/review |
|
✅ Java.Interop PR Reviewer completed successfully! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ LGTM
Clean removal of Java.Interop.GenericMarshaler and Java.Interop-PerformanceTests. The test-local JniPeerMembersExtensions helper covers exactly the 5 extension methods still used by tests, with proper [DynamicallyAccessedMembers] annotations and try/finally cleanup for DestroyGenericArgumentState.
Summary of changes verified:
- All
using Java.Interop.GenericMarshaler;removed; call sites resolve the extension methods from the new test-local helper in the same namespace ✅ InternalsVisibleToforGenericMarshalerremoved fromAssemblyInfo.cs✅- Solution, CI pipeline, and documentation references all cleaned up ✅
- Pre-existing missing
EndProjectforJava.Interop.Tools.Maven-Testsin.slnfixed incidentally — nice catch 👍 - No remaining references to
GenericMarshaleranywhere in the repo ✅
Positive callouts:
- PR description is thorough — documents downstream dotnet/android impact
- Validation section includes test counts
- The replacement helper is minimal and well-scoped to only what tests actually need
CI note: Only license/cla and copilot-pull-request-reviewer checks are visible — no build/test CI run yet. The author reports local test success (671 passed, 0 failed), but a green CI run would be nice before merge.
| Severity | Count |
|---|---|
| ❌ error | 0 |
| 0 | |
| 💡 suggestion | 1 |
Generated by Java.Interop PR Reviewer for issue #1450 · ● 3.5M
Comment on lines
+48
to
+49
| } | ||
| public static unsafe void InvokeGenericVirtualVoidMethod< |
There was a problem hiding this comment.
🤖 💡 Formatting — Missing blank line between FinishGenericCreateInstance and InvokeGenericVirtualVoidMethod. All other method boundaries in this file have a separating blank line (lines 22–24, 73–75, 99–101); this one is the exception.
Rule: Minimal diffs / consistency
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.
Part of #1448.
This removes the standalone
Java.Interop.GenericMarshalerhelper assembly and its performance benchmark project.Java.Interop-Testsstill exercises the few generic marshaling paths it needs via a small test-local helper instead of referencing the deleted assembly.Also removes the now-obsolete CI/docs/metadata references and the
InternalsVisibleToentry forJava.Interop.GenericMarshaler.Validation:
git diff --check origin/mainSDKROOT=$(xcrun --sdk macosx --show-sdk-path) dotnet test tests/Java.Interop-Tests/Java.Interop-Tests.csprojNote for dotnet/android codeflow:
Xamarin.Android-Tests.slnandtests/Mono.Android-Tests/Java.Interop-Tests/Java.Interop-Tests.NET.csproj; those will need to be removed or updated when this flows.