Address build log warnings - #11625
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce local build-log warnings by cleaning up XML documentation issues, removing unused usings, and adding targeted warning suppressions across installer, runtime, and test infrastructure code.
Changes:
- Suppress or resolve documentation-related warnings (e.g., CS1591) and improve XML doc completeness/accuracy.
- Add targeted analyzer suppressions / nullability tweaks in runtime + test utilities.
- Remove unused usings / tighten some nullable flows.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Installer.Common/Xamarin.Installer.Common.csproj | Adds CS1591 suppression to reduce doc-warning noise. |
| src/Xamarin.Installer.AndroidSDK/Xamarin.Installer.AndroidSDK/Repository.cs | Removes incorrect/obsolete XML doc parameter tag. |
| src/Xamarin.Installer.AndroidSDK/Xamarin.Installer.AndroidSDK/JavaDependencyInstaller.cs | Adjusts temp-file creation to address compiler warning. |
| src/Xamarin.Installer.AndroidSDK/Xamarin.Installer.AndroidSDK/IProgressMonitor.cs | Completes XML docs for progress-reporting parameters. |
| src/Xamarin.Installer.AndroidSDK/Xamarin.Installer.AndroidSDK/DirectorySizeMonitoringTimer.cs | Fixes XML doc parameter name/description. |
| src/Xamarin.Installer.AndroidSDK/Xamarin.Installer.AndroidSDK/Common/HttpClientProvider.cs | Adds missing XML docs for optional parameters. |
| src/Xamarin.Installer.AndroidSDK/Xamarin.Installer.AndroidSDK/Common/Archive.cs | Adjusts XML documentation text to avoid doc warnings. |
| src/Xamarin.Installer.AndroidSDK/Xamarin.Installer.AndroidSDK/Common/AndroidLicensesStorage.cs | Adds missing XML doc for a boolean parameter. |
| src/Xamarin.Installer.AndroidSDK/Xamarin.Installer.AndroidSDK/AndroidSDKInstaller.cs | Fixes/extends XML docs (param tags, formatting). |
| src/Xamarin.Installer.AndroidSDK/Xamarin.Installer.AndroidSDK.csproj | Adds CS1591 suppression to reduce doc-warning noise. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/SolutionBuilder.cs | Adds nullability annotation to help static analysis. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Android/AndroidSdkResolver.cs | Suppresses CA1416 around Windows registry usage in tests. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/InlineData.cs | Removes unused using. |
| src/Mono.Android/Mono.Android.csproj | Removes a compile item from Mono.Android build inputs. |
| src/Mono.Android/Java.Interop/TypeManager.cs | Tightens nullable flow around typemap resolution. |
| src/Mono.Android/Android.Runtime/RuntimeNativeMethods.cs | Updates P/Invoke signature nullability for typemap lookup. |
| src/Mono.Android/Android.Runtime/JNIEnv.cs | Qualifies RuntimeFeature usage and adjusts nullable return types. |
| src/Mono.Android/Android.Runtime/AndroidRuntime.cs | Removes unused usings; qualifies RuntimeHelpers usage. |
| build-tools/jnienv-gen/Generator.cs | Suppresses an “unassigned field” warning in generator model types. |
Copilot's findings
- Files reviewed: 19/19 changed files
- Comments generated: 4
| static Type? GetJavaToManagedTypeCore (string? class_name) | ||
| { | ||
| if (TypeManagerMapDictionaries.JniToManaged.TryGetValue (class_name, out Type? type)) { | ||
| if (class_name is not null && TypeManagerMapDictionaries.JniToManaged.TryGetValue (class_name, out Type? type)) { | ||
| return type; | ||
| } |
| <Compile Include="Microsoft.Android.Runtime\ManagedTypeManager.cs" /> | ||
| <Compile Include="Microsoft.Android.Runtime\ManagedTypeMapping.cs" /> | ||
| <Compile Include="Microsoft.Android.Runtime\JavaMarshalValueManager.cs" /> | ||
| <Compile Include="Microsoft.Android.Runtime\RuntimeFeature.cs" /> | ||
| <Compile Include="Microsoft.Android.Runtime\SimpleValueManager.cs" /> | ||
| <Compile Include="Microsoft.Android.Runtime\SingleUniverseTypeMap.cs" /> |
There was a problem hiding this comment.
@copilot we need to figure out how to remove it AND make sure it's accessible everywhere where it's needed (via InternalsVisibleTo I imagine). This way we don't have the same class in both Mono.Android and Mono.Android.Runtime. Only the Mono.Android.Runtime should be used everywhere.
There was a problem hiding this comment.
Done in the latest commit. RuntimeFeature.cs is now removed from Mono.Android.csproj — Mono.Android.Runtime already had [assembly: InternalsVisibleTo("Mono.Android, ...")] in its AssemblyInfo.cs.in, so AndroidRuntimeFeature (internal) remains accessible from Mono.Android via that existing declaration.
|
@copilot address code review |
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Addressed all four review points in commit
|
|
@copilot let's rename |
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
Done in commit |
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
…lsVisibleTo from Mono.Android.Runtime Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
|
@copilot address this error: |
…x CS0122 build error Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
... Fixed. |
… conflict warnings (#11669) ### Summary `Microsoft.Android.Runtime.RuntimeFeature` was compiled into **two** assemblies: - `Mono.Android.Runtime.dll` (`src/Mono.Android.Runtime/Mono.Android.Runtime.csproj`) - `Mono.Android.dll` (`src/Mono.Android/Mono.Android.csproj`) Because `Mono.Android` references `Mono.Android.Runtime`, compiling `Mono.Android.dll` emitted duplicate-type warnings like: ```text warning CS0436: The type 'RuntimeFeature' in 'src/Mono.Android/Microsoft.Android.Runtime/RuntimeFeature.cs' conflicts with the imported type 'RuntimeFeature' in 'Mono.Android.Runtime'. ``` This PR makes `RuntimeFeature` live in **exactly one** assembly: `Mono.Android.Runtime.dll`. ### Changes - **`Mono.Android.csproj`** — removes the duplicate `<Compile>` of `RuntimeFeature.cs`, so the type is no longer compiled into `Mono.Android.dll`. - **`AndroidRuntime.cs`, `JNIEnv.cs`, `TypeManager.cs`** — adds `using RuntimeFeature = Microsoft.Android.Runtime.RuntimeFeature;` because once `RuntimeFeature` is imported instead of source-compiled into `Mono.Android`, the bare name conflicts with `System.Runtime.CompilerServices.RuntimeFeature`. - **`Mono.Android.Runtime.csproj` / `Mono.Android.Runtime.targets`** — migrates assembly metadata from the hand-written `Properties/AssemblyInfo.cs.in` template to MSBuild `GenerateAssemblyInfo`: - Deletes `Properties/AssemblyInfo.cs.in` and the `_BuildAssemblyInfo_cs` target (which ran `ReplaceFileContents` on the template) from `Mono.Android.Runtime.targets`. - Sets `<GenerateAssemblyInfo>true</GenerateAssemblyInfo>` with `GenerateAssemblyConfigurationAttribute`, `GenerateAssemblyFileVersionAttribute`, `GenerateAssemblyVersionAttribute`, `GenerateRepositoryUrlAttribute`, and `IncludeSourceRevisionInInformationalVersion` all set to `false` so no new attributes are introduced vs. the old template. - Moves the attribute values to properties/items: `AssemblyTitle`, `Product`, `Company`, and the `InternalsVisibleTo` items. A new `_SetGeneratedAssemblyInfoProperties` target (before `GetAssemblyAttributes`) supplies `InformationalVersion`, `TargetPlatformIdentifier`/`TargetPlatformVersion`, and `SupportedOSPlatformVersion` from the existing `GetXAVersionInfo` version info. - Adds `InternalsVisibleTo` for `Microsoft.Android.Runtime.NativeAOT` and `Mono.Android.NET-Tests`, which previously reached `RuntimeFeature` through `Mono.Android.dll`. - Marks `Mono.Android.Runtime.dll` trimmable with `<IsTrimmable>true</IsTrimmable>`. This keeps the cross-assembly `RuntimeFeature` feature switches visible to ILLink in partial-trimmed apps. ### Why `IsTrimmable` is needed Moving `RuntimeFeature` into `Mono.Android.Runtime.dll` makes `Mono.Android.dll` consume feature-switch properties across an assembly boundary. In partial trimming, ILLink only substitutes feature-switch getters in assemblies it processes. Without marking `Mono.Android.Runtime.dll` trimmable, some runtime-specific branches stayed live, causing the CoreCLR APK size-regression test to fail. Marking `Mono.Android.Runtime.dll` trimmable lets ILLink fold those `RuntimeFeature` guards again. ### Relationship to #11625 This is a focused subset of #11625. It intentionally avoids the broader class rename (`RuntimeFeature` → `AndroidRuntimeFeature`) and resolves the resulting `CS0104` ambiguity with local `using` aliases instead. ### Follow-up The `BuildHasTrimmerWarnings` diagnostic improvement was split out into #11981 so this PR remains focused on the `RuntimeFeature` single-assembly change. ### Verification Latest PR validation: [dotnet-android #1496564](https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_build/results?buildId=1496564) ✅ The produced `Mono.Android.Runtime.dll` from that build was inspected to confirm the `GenerateAssemblyInfo` migration is faithful — all attributes from the old `AssemblyInfo.cs.in` (`AssemblyTitle`, `Product`, `Company`, `AssemblyInformationalVersion`, `TargetPlatform=Android37.0`, `SupportedOSPlatform=Android24.0`, `InternalsVisibleTo` for `Mono.Android`) are present, the new `InternalsVisibleTo` (NativeAOT, NET-Tests) and `IsTrimmable` metadata are added, and no unexpected attributes (`AssemblyConfiguration`, `AssemblyFileVersion`, `AssemblyVersion`, `RepositoryUrl`) were introduced. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This separates warning cleanups from the Java.Interop work branch so they can be reviewed independently.\n\nThe changes address warnings surfaced in local build logs across runtime, installer, and test infrastructure code.