镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

[build] Isolate Java naming policy in build tools - #13537

Open
jonathanpeppers wants to merge 7 commits into
mainfrom
jonathanpeppers-assembly-naming-policy-preparation
Open

jonathanpeppers wants to merge 7 commits into
mainfrom
jonathanpeppers-assembly-naming-policy-preparation

Conversation

@jonathanpeppers

Copy link
Copy Markdown
Member

Stacked on #13525. This layer contains no manifest deletion or generator behavior fix.

AssemblyModifierPipeline still wrote the process-wide Java naming policy after its naming consumers were removed. Delete that write and unused task input. Pass policy explicitly in the binding generator, retaining its existing LowercaseCrc64 behavior, and ban the implicit-policy APIs in both build projects.

Keep existing public naming overloads for runtime and external tooling callers. Add one focused test showing explicit policy calls ignore changes to global state. Task MT remains disabled.

Local validation: SDK build and 112 tests passed

Commands used this worktree's pinned SDK and rebuilt outputs:

.\bin\Debug\dotnet\dotnet.exe test tests\Java.Interop.Tools.JavaCallableWrappers-Tests\Java.Interop.Tools.JavaCallableWrappers-Tests.csproj -v:minimal -nr:false
# Passed: 21 tests.

.\bin\Debug\dotnet\dotnet.exe test tests\generator-Tests\generator-Tests.csproj -v:minimal --filter 'FullyQualifiedName~CodeGeneratorTests|FullyQualifiedName~ManagedTests' -nr:false -p:JavaSdkDirectory=D:\android-toolchain\jdk-21
# Passed: 91 tests.

.\dotnet-local.cmd build Microsoft.Android.slnx -m:4 -nr:false -v:minimal -p:AndroidSdkDirectory=D:\android-toolchain\sdk -p:AndroidNdkDirectory=D:\android-toolchain\ndk -p:JavaSdkDirectory=D:\android-toolchain\jdk-21
# Passed: 0 errors; 28 existing warnings.

The first parallel test-build attempt hit a shared-output file lock; the naming suite passed when rerun after the generator build.

Host-only app checks also passed for both supported policies:

$app = 'C:\Users\jopepper\.copilot\session-state\e8665303-0f94-4ad8-8f58-138d624db48c\files\naming-audit\app\NamingPolicyProbe.csproj'
foreach ($policy in @('LowercaseCrc64', 'Crc64')) {
    .\dotnet-local.cmd build $app -t:Rebuild "-p:AndroidPackageNamingPolicy=$policy" -p:AndroidSdkDirectory=D:\android-toolchain\sdk -p:AndroidNdkDirectory=D:\android-toolchain\ndk -p:JavaSdkDirectory=D:\android-toolchain\jdk-21 -nr:false -v:minimal
}
# Both passed. Java packages and matching manifest activity names remained
# crc640607b65c3a40cd67 and scrc64ca4c22b77fd46e81 respectively.

Inspection of 253 rebuilt SDK tool assemblies found no path from their naming calls to the static policy getter. No device tests were run; this changes host build tools only.

Base automatically changed from jonathanpeppers-obsolete-manifest-removal to main November 13, 2026 09:54
jonathanpeppers and others added 7 commits November 13, 2026 10:54
Assembly modification no longer generates Java names. Retain the task input for compatibility without changing process-wide naming policy.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use the existing banned API analyzer to reject direct policy reads and writes. Preserve imported warning-as-error settings so RS0030 remains an error.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Delete the no-op input and its MSBuild binding. Remove the earlier input-setting test; the banned API analyzer still rejects global policy access in tasks.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add explicit-policy naming overloads while preserving existing runtime and tooling entry points. Migrate the generator and legacy manifest helper callers, and ban the implicit-policy APIs in both build projects.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Retain the missing policy-isolation check without repeating policy, nested-name, registered-name and framework-name coverage.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Merge naming restrictions into BannedSymbols.txt and reuse the common analyzer import. Update generator resolutions required by the existing Cecil restrictions without changing naming policy selection.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Share the operation TypeDefinitionCache through generation options so importer interface, base-type and field resolutions are cached rather than bypassing the banned Resolve API through the module resolver.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@simonrozsival
simonrozsival force-pushed the jonathanpeppers-assembly-naming-policy-preparation branch from 7b43e0d to 328be58 Compare November 13, 2026 09:54
@jonathanpeppers
jonathanpeppers marked this pull request as ready for review November 13, 2026 16:35
Copilot AI balanced review requested due to automatic review settings November 13, 2026 16:35
@jonathanpeppers jonathanpeppers added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Nov 13, 2026
Comment on lines -64 to -65
JavaNativeTypeManager.PackageNamingPolicy = Enum.TryParse (PackageNamingPolicy, out PackageNamingPolicyEnum pnp) ? pnp : PackageNamingPolicyEnum.LowercaseCrc64;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was static state that wouldn't work under -mt.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are recorded for the changes isolating Java naming policy from process-wide state.

No open issues 🎉

What changed in this PR

Isolates Java naming policy in host build tools from process-wide state, building on the manifest cleanup in #13525 while preserving existing public naming overloads.

Changes:

  • Removes the unused naming-policy task input and global-state write.
  • Passes LowercaseCrc64 explicitly in the binding generator and shares its type-resolution cache.
  • Adds analyzer guards against implicit-policy APIs and a regression test for explicit-policy isolation.
File Description
tests/​Java.Interop.Tools.JavaCallableWrappers-Tests/​Java.Interop.Tools.JavaCallableWrappers/​JavaNativeTypeManagerTests.cs Tests that explicit naming policy ignores global-state changes.
src/​Xamarin.Android.Build.Tasks/​Xamarin.Android.Common.targets Stops passing the unused naming-policy task input.
src/​Xamarin.Android.Build.Tasks/​Xamarin.Android.Build.Tasks.csproj Preserves inherited warnings-as-errors settings.
src/​Xamarin.Android.Build.Tasks/​Tasks/​AssemblyModifierPipeline.cs Removes the policy input and process-wide mutation.
src/​Java.Interop.Tools.TypeNameMappings/​Java.Interop.Tools.TypeNameMappings/​JavaNativeTypeManager.cs Adds explicit-policy overloads while retaining existing APIs.
src/​generator/​Java.Interop.Tools.Generator.Importers/​CecilApiImporter.cs Routes type resolution through the shared cache.
src/​generator/​generator.csproj Enables banned-API analyzer rules.
src/​generator/​CodeGenerator.cs Uses explicit naming policy, shares the cache, and removes redundant resolution.
src/​generator/​CodeGenerationOptions.cs Provides the shared type-resolution cache.
build-tools/​banned-apis/​BannedSymbols.txt Bans global naming-state access and implicit-policy APIs in consuming build projects.

Review effort: Balanced

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants