Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] missing resource error handling (#8066)
Browse files Browse the repository at this point in the history
Context: mono/SkiaSharp#2444
Context: mono/SkiaSharp#2465
Context: fcd7cf8
Context: dc3ccf2
Context: 3ec1b15

In Classic Xamarin.Android, `@(AndroidResource)` files were embedded
into assemblies as `@(EmbeddedResource)` entries.  While convenient,
this slowed down build times (as every assembly needed to be checked
to see if it had embedded resources), and this functionality was
removed for .NET 6 in fcd7cf8.  The replacement was to use `.aar`
files, typically located in the same directory as the assembly.

Unfortunately, it was easy to incorrectly migrate from the old system
to the new system, which was the case for the 
[SkiaSharp.Views (v2.88.3) NuGet Package][0], which *uses* an
[`@attr/ignorePixelScaling`][1] resource, but did not package it.

In .NET 6 and 7, this was "fine" *so long as* that resource value was
never used.  If it *was* used, then the results were undefined: a
"garbage" resource id value would be used, which would either be
invalid or refer to a *different* resource altogether!

.NET 8 introduces the Resource Designer Assembly (dc3ccf2), which
involves *rewriting non-.NET 8 assemblies* to use the Resource
Designer Assembly, which in turn requires that all resources exist.

Which brings us to mono/SkiaSharp#2444: because the `SkiaSharp.Views`
NuGet package references but did not redistribute the
`@attr/ignorePixelScaling` resource, the assembly rewriter would
*skip over* the missing resource name, resulting in an invalid
assembly.  This in turn would result in a *runtime crash*:

	System.BadImageFormatException: 'Invalid field token 0x040000a1'

We decided that instead of ignoring missing resource fields, we
should error out.

Update `FixLegacyResourceDesignerStep` to report an error if we
cannot find the required Resource Designer Assembly property which
matches a "legacy" field.

In Debug builds, this will result in an XA8000 error:

	error XA8000: Could not find Android Resource '@attr/ignorePixelScaling'.
	Please update @(AndroidResource) to add the missing resource.

For Release builds, this instead results in an IL8000 error:

	error IL8000: Could not find Android Resource '@attr/ignorePixelScaling'.
	Please update @(AndroidResource) to add the missing resource.

Note that this only moves a runtime error to a compile-time error.
The fix for mono/SkiaSharp#2444 is mono/SkiaSharp#2465, which adds
`SkiaSharp.Views.Android.aar` to the `SkiaSharp.Views` NuGet package.
This should be fixed in the (forthcoming) SkaSharp.Views 2.88.4.
[A *workaround* for mono/SkiaSharp#2444][3] is to define the
`@attr/ignorePixelScaling` resource within your app project.

Additionally, we found another problem while investigating
mono/SkiaSharp#2444 around resource name case sensitivity.  Not all
Android Resource names need to be lowercase; generally only those
that are based on *filenames* need to be lowercase, e.g.
`@layout/main`.  Resources which are defined within XML containers,
such as [string resources][2], may contain uppercase letters.

While looking at the output for the test app, it turns out the final
assembly does NOT include the expected `anim` based resources.
Looking at the IL we found that the properties were NOT being fixed
up to use the correct casing, e.g. we have `enterfromright` instead
of `EnterFromRight`.  Further investigation tracked the issue down to
the `RTxtParser`; This class uses the `case_map.txt` file for lookup.
The `case_map.txt` file contains entries such as:

	animation/enterfromleft;animation/EnterFromLeft
	animation/enterfromright;animation/EnterFromRight

these allow us to map the "android" lower cased items to the C# cased
items.   The problem was we were not mapping `anim` to `animation`.
There is a method called `ResourceParser.GetNestedTypeName()` which
is used to map the android resource type to the managed resource type.
For example it maps `anim` to `Animation` and `bool` to `Boolean`.
This was not being called when we tried to map the resource.  As a
result we never found the correct cased entry.  This would result in
the final Resource Designer Assembly containing the lower cased
"android" naming of the property and not the correctly cased one
which the C#/IL would be using; the fields were never fixed up. 

The fix is to make sure we call `ResourceParser.GetNestedTypeName()`
when calculating the final property name in the `RTxtParser`.

[0]: https://www.nuget.org/packages/SkiaSharp/2.88.3
[1]: https://github.com/mono/SkiaSharp/blob/99c2437b1055dc6eb8ee25e90e5ad4afa695aa89/source/SkiaSharp.Views/SkiaSharp.Views.Android/Resources/values/attrs.xml#L4
[2]: https://developer.android.com/guide/topics/resources/string-resource
[3]: mono/SkiaSharp#2444 (comment)
  • Loading branch information
dellis1972 authored Jun 9, 2023
1 parent e97ad2f commit 3481881
Show file tree
Hide file tree
Showing 15 changed files with 288 additions and 73 deletions.
4 changes: 4 additions & 0 deletions Documentation/guides/messages/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,10 @@ Exceptions that have not been gracefully handled yet. Ideally these will be fix
These take the form of `XACCC7NNN`, where `CCC` is a 3 character code denoting the MSBuild Task that is throwing the exception,
and `NNN` is a 3 digit number indicating the type of the unhandled `Exception`.

## XA8xxx: Linker Step Errors

+ [XA8000/IL8000](xa8000.md): Could not find Android Resource '@anim/enterfromright'. Please update @(AndroidResource) to add the missing resource.

**Tasks:**
* `A2C` - `Aapt2Compile`
* `A2L` - `Aapt2Link`
Expand Down
21 changes: 21 additions & 0 deletions Documentation/guides/messages/xa8000.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
title: Xamarin.Android warning XA8000/IL8000
description: XA8000/IL8000 error code
ms.date: 06/01/2023
---
# Xamarin.Android error XA8000/IL8000

## Issue

```
Could not find Android Resource '@anim/enterfromright'. Please update @(AndroidResource) to add the missing resource.
```

## Solution

When trying to upgrade older nuget package references to use the
more recent Resource Designer Assembly, the system might encounter
fields which cannot be upgraded because the resource is missing
from either the dependency or the app.

To fix this issue the missing `AndroidResource` needs to be added to the application. Or the Nuget should be upgraded to use .net 8 or later.
1 change: 0 additions & 1 deletion build-tools/scripts/UpdateApkSizeReference.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#!/bin/bash
./build-tools/scripts/nunit3-console bin/TestRelease/net472/Xamarin.Android.Build.Tests.dll --test=Xamarin.Android.Build.Tests.BuildTest2.BuildReleaseArm64
./dotnet-local.sh test bin/TestRelease/net7.0/Xamarin.Android.Build.Tests.dll --filter=Name~BuildReleaseArm64
cp bin/TestRelease/BuildReleaseArm64*.apkdesc src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/
25 changes: 17 additions & 8 deletions src/Microsoft.Android.Sdk.ILLink/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/Microsoft.Android.Sdk.ILLink/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,10 @@
<comment>The following are literal names and should not be translated: AppDomain.CreateDomain(), AppDomain
{0} - The name of the assembly</comment>
</data>
<data name="XA8000" xml:space="preserve">
<value>Could not find Android Resource '{0}'. Please update @(AndroidResource) to add the missing resource.</value>
<comment>
{0} - An Android Resource Identifier.
</comment>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;

using Mono.Cecil;
Expand All @@ -13,6 +14,9 @@
using Mono.Tuner;
#if ILLINK
using Microsoft.Android.Sdk.ILLink;
using Resources = Microsoft.Android.Sdk.ILLink.Properties.Resources;
#else // !ILLINK
using Resources = Xamarin.Android.Tasks.Properties.Resources;
#endif // ILLINK

namespace MonoDroid.Tuner
Expand Down Expand Up @@ -120,6 +124,17 @@ Dictionary<string, MethodDefinition> BuildResourceDesignerPropertyLookup (TypeDe
return output;
}

string GetNativeTypeNameFromManagedTypeName (string name)
{
switch (name) {
case "Animation": return "anim";
case "Attribute": return "attr";
case "Boolean": return "bool";
case "Dimension": return "dimen";
default: return name.ToLower ();
}
}

protected override void FixBody (MethodBody body, TypeDefinition designer)
{
// replace
Expand All @@ -144,6 +159,14 @@ protected override void FixBody (MethodBody body, TypeDefinition designer)
instructions.Add (i, newIn);
} else {
LogMessage ($"DEBUG! Failed to find {key}!");
// The 'key' in this case will be something like Layout::Toolbar.
// We want format this into @layout/Toolbar so its easier to understand
// for the user.
var index = key.IndexOf ("::");
var typeName = GetNativeTypeNameFromManagedTypeName (key.Substring (0, index));
var identifier = key.Substring (index + 2);
var msg = string.Format (CultureInfo.CurrentCulture, Resources.XA8000, $"@{typeName}/{identifier}");
LogError (8000, msg);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,15 @@ public virtual void LogMessage (string message)
Context.LogMessage (message);
}

public virtual void LogError (int code, string error)
{
#if ILLINK
Context.LogMessage (MessageContainer.CreateCustomErrorMessage (error, code, origin: new MessageOrigin ()));
#else // !ILLINK
Console.Error.WriteLine ($"error XA{code}: {error}");
#endif // !ILLINK
}

public virtual AssemblyDefinition Resolve (AssemblyNameReference name)
{
return Context.Resolve (name);
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/Xamarin.Android.Build.Tasks/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -933,4 +933,10 @@ To use a custom JDK path for a command line build, set the 'JavaSdkDirectory' MS
{1} - A Filename.
</comment>
</data>
<data name="XA8000" xml:space="preserve">
<value>Could not find Android Resource '{0}'. Please update @(AndroidResource) to add the missing resource.</value>
<comment>
{0} - An Android Resource Identifier.
</comment>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ public override void LogMessage (string message)
logger.LogDebugMessage ("{0}", message);
}

public override void LogError (int code, string message)
{
logger.LogCodedError ($"XA{code}", message);
}

public override AssemblyDefinition Resolve (AssemblyNameReference name)
{
return resolver.Resolve (name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,14 @@ public static class KnownPackages
Id = "Mono.AotProfiler.Android",
Version = "7.0.0-preview1",
};
public static Package SkiaSharp = new Package () {
Id = "SkiaSharp",
Version = "2.88.3",
};
public static Package SkiaSharp_Views = new Package () {
Id = "SkiaSharp.Views",
Version = "2.88.3",
};
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -8,58 +8,58 @@
"Size": 1024
},
"assemblies/Java.Interop.dll": {
"Size": 58913
"Size": 58703
},
"assemblies/Mono.Android.dll": {
"Size": 87163
"Size": 86588
},
"assemblies/Mono.Android.Runtime.dll": {
"Size": 5895
"Size": 5798
},
"assemblies/rc.bin": {
"Size": 1182
"Size": 1235
},
"assemblies/System.Console.dll": {
"Size": 6438
"Size": 6442
},
"assemblies/System.Linq.dll": {
"Size": 9122
"Size": 9123
},
"assemblies/System.Private.CoreLib.dll": {
"Size": 516811
"Size": 536436
},
"assemblies/System.Runtime.dll": {
"Size": 2620
"Size": 2623
},
"assemblies/System.Runtime.InteropServices.dll": {
"Size": 3753
"Size": 3752
},
"assemblies/UnnamedProject.dll": {
"Size": 3219
"Size": 3349
},
"classes.dex": {
"Size": 19020
"Size": 19748
},
"lib/arm64-v8a/libmono-component-marshal-ilgen.so": {
"Size": 93552
},
"lib/arm64-v8a/libmonodroid.so": {
"Size": 379320
"Size": 380832
},
"lib/arm64-v8a/libmonosgen-2.0.so": {
"Size": 3090760
"Size": 3160360
},
"lib/arm64-v8a/libSystem.IO.Compression.Native.so": {
"Size": 723840
"Size": 723560
},
"lib/arm64-v8a/libSystem.Native.so": {
"Size": 94328
"Size": 94392
},
"lib/arm64-v8a/libSystem.Security.Cryptography.Native.Android.so": {
"Size": 155056
"Size": 154904
},
"lib/arm64-v8a/libxamarin-app.so": {
"Size": 16720
"Size": 16624
},
"META-INF/BNDLTOOL.RSA": {
"Size": 1213
Expand Down Expand Up @@ -95,5 +95,5 @@
"Size": 1904
}
},
"PackageSize": 2632010
"PackageSize": 2685258
}
Loading

0 comments on commit 3481881

Please sign in to comment.