Skip to content

Fix misleading XA4243 error when more than one JavaArtifact is specified#11785

Merged
jonathanpeppers merged 2 commits into
mainfrom
copilot/fix-finder-misleading-xa4243-error
Jun 29, 2026
Merged

Fix misleading XA4243 error when more than one JavaArtifact is specified#11785
jonathanpeppers merged 2 commits into
mainfrom
copilot/fix-finder-misleading-xa4243-error

Conversation

Copilot AI commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

MavenExtensions.TryParseJavaArtifact reused XA4243 ("Attribute '{0}' is required...") when a user supplied more than one JavaArtifact — a "missing-attribute" message for what is actually a "too many artifacts" problem. A stale TODO flagged the need for a dedicated message.

Changes

  • New error code XA4256 in Resources.resx / Resources.Designer.cs: Only one '{0}' is allowed for '{1}' item '{2}'.
  • MavenExtensions.cs: log XA4256 instead of XA4243 on artifacts.Count > 1; removed TODO.
if (artifacts.Count > 1) {
	log.LogCodedError ("XA4256", Properties.Resources.XA4256, "JavaArtifact", type, task.ItemSpec);
	artifact = null;
	return false;
}

Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix misleading XA4243 error for multiple JavaArtifacts Fix misleading XA4243 error when more than one JavaArtifact is specified Jun 29, 2026
Copilot AI requested a review from jonathanpeppers June 29, 2026 02:59
@simonrozsival simonrozsival marked this pull request as ready for review June 29, 2026 07:40
Copilot AI review requested due to automatic review settings June 29, 2026 07:40
@simonrozsival simonrozsival enabled auto-merge (squash) June 29, 2026 07:40

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.

Pull request overview

Fixes an incorrect/misleading MSBuild error code emitted when multiple JavaArtifact values are provided for a single item, by introducing a dedicated error code/message and using it in MavenExtensions.TryParseJavaArtifact.

Changes:

  • Added new localized resource string/error code XA4256: “Only one '{0}' is allowed for '{1}' item '{2}'.”
  • Updated TryParseJavaArtifact to log XA4256 (instead of XA4243) when more than one artifact is specified.
  • Updated Resources.Designer.cs to expose the new XA4256 resource.
Show a summary per file
File Description
src/Xamarin.Android.Build.Tasks/Utilities/MavenExtensions.cs Emits XA4256 when JavaArtifact metadata specifies more than one artifact.
src/Xamarin.Android.Build.Tasks/Properties/Resources.resx Adds the XA4256 localized string.
src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs Adds the generated accessor for XA4256.

Copilot's findings

Files not reviewed (1)
  • src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs: Generated file
  • Files reviewed: 2/3 changed files
  • Comments generated: 1

Comment thread src/Xamarin.Android.Build.Tasks/Properties/Resources.resx
@simonrozsival simonrozsival added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Jun 29, 2026
@simonrozsival simonrozsival disabled auto-merge June 29, 2026 13:53
@jonathanpeppers jonathanpeppers merged commit 222aecd into main Jun 29, 2026
41 checks passed
@jonathanpeppers jonathanpeppers deleted the copilot/fix-finder-misleading-xa4243-error branch June 29, 2026 14:13
simonrozsival added a commit that referenced this pull request Jun 29, 2026
)

## Summary

Follow-up to the trimmable typemap manifest-generation work being split out of #11617 (and to the XA4243 manifest fix in #11785). This brings the **trimmable typemap** `ManifestGenerator` to parity with the legacy `ManifestDocument` for NativeAOT, completing the activity attribute coverage and the library/placeholder merge behavior that #11752 didn't cover.

## Changes

* **`PropertyMapper`** — add the remaining activity manifest attributes: `banner`, `colorMode`, `enableVrMode`, `lockTaskMode`, `logo`, `maxAspectRatio`, `maxRecents`, `allowEmbedded`, `autoRemoveFromRecents`, `relinquishTaskIdentity`, `resumeWhilePausing`, `showForAllUsers`/`showOnLockScreen`/`showWhenLocked`, `singleUser`, `visibleToInstantApps`, `recreateOnConfigChanges`, `rotationAnimation`. Adds a `Number` mapping kind with invariant-culture formatting for `maxAspectRatio`/`maxRecents`.
* **`ComponentElementBuilder`** — emit `<layout>` child properties from `LayoutProperties`.
* **`ComponentInfo`** — carry `LayoutProperties`.
* **`AndroidEnumConverter`** — `RotationAnimation` and `persistableMode` conversions.
* **`ManifestGenerator`** — merge library (`.aar`) manifests for the legacy merger, resolve/normalize `$(AndroidManifestPlaceholders)` (incl. package-name resolution), warn XA1010 on invalid placeholders, sort component attributes alphabetically, dedup `tools:`-namespace nodes, and emit `uses-sdk` correctly when no min is specified.

## Context

Part of slicing the #11617 mega-PR into reviewable units. `ManifestGenerator` and friends already exist on `main` (via #10991, #11037, #11143, #11752), so this applies cleanly on its own. Companion slices: #11794 (AndroidApplicationJavaClass/multidex).

## Tests

Extends `ManifestGeneratorTests` to cover all activity attributes, layout properties, placeholder resolution/warnings, library-manifest merge, and dedup. **575 generator unit tests pass locally.**
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.

5 participants