Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] Emit an Error if a custom view cannot b…
Browse files Browse the repository at this point in the history
…e fixed up. (dotnet#1720)

Context: dotnet#1711

When using a custom view within a layout file we replace the
`namespace.classname` with an `md5hash.classname`.  We do this by
using the `acwmap.txt` file to map known types onto the `md5` hashed
ones.  This is done in a case sensitive manner.  We also only support
identically-cased and lower-cased namespace/package names:

	ClassLibrary1.CustomView=md5d6f7135293df7527c983d45d07471c5e.CustomTextView
	classlibrary1.CustomView=md5d6f7135293df7527c983d45d07471c5e.CustomTextView

Given that a user is able to type these manually it is possible that
typo's will occur.  If for example a user types

	Classlibrary1.CustomView

this will NOT be fixed up, due to the lowercase `l` in `library`.
Instead the user will see the following error at runtime.

	FATAL UNHANDLED EXCEPTION: Android.Views.InflateException: Binary XML file line #1: Binary XML file line #1: Error inflating class Classlibrary1.CustomTextView
	---> Android.Views.InflateException: Binary XML file line #1: Error inflating class Classlibrary1.CustomTextView
	---> Java.Lang.ClassNotFoundException: Didn't find class "Classlibrary1.CustomTextView"

If the control is used in a number of places this runtime error does
nothing to help track down the problem.

Improve this scenario by detecting these issues and emitting an
XA1002 build error.  This will not only inform the user about the
problem but also provide a link to the file causing the problem.
  • Loading branch information
dellis1972 authored and jonpryor committed Aug 2, 2018
1 parent e2967da commit 967fe94
Show file tree
Hide file tree
Showing 11 changed files with 177 additions and 24 deletions.
2 changes: 2 additions & 0 deletions Documentation/guides/messages/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
### XA1xxx Project Related

+ [XA1000](xa1000.md): There was an problem parsing {file}. This is likely due to incomplete or invalid xml.
+ [XA1001](xa1001.md): AndroidResgen: Warning while updating Resource XML '{filename}': {Message}
+ [XA1002](xa1002.md): We found a matching key '{Key}' for '{Item}'. But the casing was incorrect. Please correct the casing

### XA2xxx Linker

Expand Down
7 changes: 7 additions & 0 deletions Documentation/guides/messages/xa1001.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# Compiler Warning XA1001

AndroidResgen: Warning while updating Resource XML '{filename}': {Message}

This warning is raised if we encounter an unknown issue when processing
the layout and resources. It is a generic warning that does not describe
any specific problem. The details will be in the `{Message}`.
13 changes: 13 additions & 0 deletions Documentation/guides/messages/xa1002.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Compiler Error XA1002

This error will be emitted when we are unable to find a matching custom new in the
ResourceCaseMap string.

As part of the build process `Namespace.CustomViewFoo` items in layout files are
replaced with `{MD5Hash}.CustomViewFoo`. We attempt to replace a couple of variants
of the `Namespace.CustomViewFoo`. One is the original casing found in the C# source
files. The other is all lowercase. If we cannot find a match we will do a case
insensitive lookup to see if there are items which do match. If we find one this
error will be raised.

We found a matching key 'Namespace.CustomViewFoo' for 'NameSpace.CustomViewFoo'. But the casing was incorrect. Please correct the casing
6 changes: 2 additions & 4 deletions src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public class Aapt : AsyncTask

public string ResourceSymbolsTextFileDirectory { get; set; }

Dictionary<string,string> resource_name_case_map = new Dictionary<string,string> ();
Dictionary<string,string> resource_name_case_map;
AssemblyIdentityMap assemblyMap = new AssemblyIdentityMap ();
string resourceDirectory;

Expand Down Expand Up @@ -251,9 +251,7 @@ public override bool Execute ()

void DoExecute ()
{
if (ResourceNameCaseMap != null)
foreach (var arr in ResourceNameCaseMap.Split (';').Select (l => l.Split ('|')).Where (a => a.Length == 2))
resource_name_case_map [arr [1]] = arr [0]; // lowercase -> original
resource_name_case_map = MonoAndroidHelper.LoadResourceCaseMap (ResourceNameCaseMap);

assemblyMap.Load (Path.Combine (WorkingDirectory, AssemblyIdentityMapFile));

Expand Down
6 changes: 2 additions & 4 deletions src/Xamarin.Android.Build.Tasks/Tasks/Aapt2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace Xamarin.Android.Tasks {

public class Aapt2 : AsyncTask {

protected Dictionary<string, string> resource_name_case_map = new Dictionary<string, string> ();
protected Dictionary<string, string> resource_name_case_map;

public ITaskItem [] ResourceDirectories { get; set; }

Expand Down Expand Up @@ -174,9 +174,7 @@ protected bool LogAapt2EventsFromOutput (string singleLine, MessageImportance me

protected void LoadResourceCaseMap ()
{
if (ResourceNameCaseMap != null)
foreach (var arr in ResourceNameCaseMap.Split (';').Select (l => l.Split ('|')).Where (a => a.Length == 2))
resource_name_case_map [arr [1]] = arr [0]; // lowercase -> original
resource_name_case_map = MonoAndroidHelper.LoadResourceCaseMap (ResourceNameCaseMap);
}
}
}
28 changes: 27 additions & 1 deletion src/Xamarin.Android.Build.Tasks/Tasks/ConvertResourcesCases.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (C) 2011 Xamarin, Inc. All rights reserved.

using System;
using System.Diagnostics;
using System.Collections.Generic;
using System.IO;
using System.Linq;
Expand All @@ -20,13 +21,19 @@ public class ConvertResourcesCases : Task

public string AndroidConversionFlagFile { get; set; }

public string ResourceNameCaseMap { get; set; }

Dictionary<string,string> resource_name_case_map;

public override bool Execute ()
{
Log.LogDebugMessage ("ConvertResourcesCases Task");
Log.LogDebugMessage (" ResourceDirectories: {0}", ResourceDirectories);
Log.LogDebugMessage (" AcwMapFile: {0}", AcwMapFile);
Log.LogDebugMessage (" AndroidConversionFlagFile: {0}", AndroidConversionFlagFile);
Log.LogDebugMessage (" ResourceNameCaseMap: {0}", ResourceNameCaseMap);

resource_name_case_map = MonoAndroidHelper.LoadResourceCaseMap (ResourceNameCaseMap);
var acw_map = MonoAndroidHelper.LoadAcwMapFile (AcwMapFile);

// Look in the resource xml's for capitalized stuff and fix them
Expand Down Expand Up @@ -71,7 +78,26 @@ void FixupResources (ITaskItem item, Dictionary<string, string> acwMap)
MonoAndroidHelper.SetWriteable (tmpdest);
try {
AndroidResource.UpdateXmlResource (resdir, tmpdest, acwMap,
ResourceDirectories.Where (s => s != item).Select(s => s.ItemSpec));
ResourceDirectories.Where (s => s != item).Select(s => s.ItemSpec), (t, m) => {
string targetfile = file;
if (targetfile.StartsWith (resdir, StringComparison.InvariantCultureIgnoreCase)) {
targetfile = file.Substring (resdir.Length).TrimStart (Path.DirectorySeparatorChar);
if (resource_name_case_map.TryGetValue (targetfile, out string temp))
targetfile = temp;
targetfile = Path.Combine ("Resources", targetfile);
}
switch (t) {
case TraceLevel.Error:
Log.LogCodedError ("XA1002", file: targetfile, lineNumber: 0, message: m);
break;
case TraceLevel.Warning:
Log.LogCodedWarning ("XA1001", file: targetfile, lineNumber: 0, message: m);
break;
default:
Log.LogDebugMessage (m);
break;
}
});

// We strip away an eventual UTF-8 BOM from the XML file.
// This is a requirement for the Android designer because the desktop Java renderer
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
using System;
using System.Collections.Generic;
using NUnit.Framework;
using Xamarin.ProjectTools;
using System.IO;
using System.Linq;
using Microsoft.Build.Framework;
using System.Text;
using Xamarin.Android.Tasks;
using Microsoft.Build.Utilities;

namespace Xamarin.Android.Build.Tests {

[TestFixture]
[Parallelizable (ParallelScope.Self)]
public class ConvertResourcesCasesTests : BaseTest {
[Test]
public void CheckClassIsReplacedWithMd5 ()
{
var path = Path.Combine (Root, "temp", "CheckClassIsReplacedWithMd5");
Directory.CreateDirectory (path);
var resPath = Path.Combine (path, "res");
Directory.CreateDirectory (Path.Combine (resPath, "layout"));
File.WriteAllText (Path.Combine (resPath, "layout", "main.xml"), @"<?xml version='1.0' ?>
<LinearLayout xmlns:android='http://schemas.android.com/apk/res/android'>
<ClassLibrary1.CustomView xmlns:android='http://schemas.android.com/apk/res/android' />
<classlibrary1.CustomView xmlns:android='http://schemas.android.com/apk/res/android' />
</LinearLayout>
");
var errors = new List<BuildErrorEventArgs> ();
IBuildEngine engine = new MockBuildEngine (TestContext.Out, errors);
var task = new ConvertResourcesCases {
BuildEngine = engine
};
task.ResourceDirectories = new ITaskItem [] {
new TaskItem (resPath),
};
task.AcwMapFile = Path.Combine (path, "acwmap.txt");
File.WriteAllLines (task.AcwMapFile, new string [] {
"ClassLibrary1.CustomView;md5d6f7135293df7527c983d45d07471c5e.CustomTextView",
"classlibrary1.CustomView;md5d6f7135293df7527c983d45d07471c5e.CustomTextView",
});
Assert.IsTrue (task.Execute (), "Task should have executed successfully");
var output = File.ReadAllText (Path.Combine (resPath, "layout", "main.xml"));
StringAssert.Contains ("md5d6f7135293df7527c983d45d07471c5e.CustomTextView", output, "md5d6f7135293df7527c983d45d07471c5e.CustomTextView should exist in the main.xml");
StringAssert.DoesNotContain ("ClassLibrary1.CustomView", output, "ClassLibrary1.CustomView should have been replaced.");
StringAssert.DoesNotContain ("classlibrary1.CustomView", output, "classlibrary1.CustomView should have been replaced.");
Directory.Delete (path, recursive: true);
}

[Test]
public void CheckClassIsNotReplacedWithMd5 ()
{
var path = Path.Combine (Root, "temp", "CheckClassIsNotReplacedWithMd5");
Directory.CreateDirectory (path);
var resPath = Path.Combine (path, "res");
Directory.CreateDirectory (Path.Combine (resPath, "layout"));
File.WriteAllText (Path.Combine (resPath, "layout", "main.xml"), @"<?xml version='1.0' ?>
<LinearLayout xmlns:android='http://schemas.android.com/apk/res/android'>
<ClassLibrary1.CustomView xmlns:android='http://schemas.android.com/apk/res/android' />
<classLibrary1.CustomView xmlns:android='http://schemas.android.com/apk/res/android' />
</LinearLayout>
");
var errors = new List<BuildErrorEventArgs> ();
IBuildEngine engine = new MockBuildEngine (TestContext.Out, errors);
var task = new ConvertResourcesCases {
BuildEngine = engine
};
task.ResourceDirectories = new ITaskItem [] {
new TaskItem (resPath),
};
task.AcwMapFile = Path.Combine (path, "acwmap.txt");
File.WriteAllLines (task.AcwMapFile, new string [] {
"ClassLibrary1.CustomView;md5d6f7135293df7527c983d45d07471c5e.CustomTextView",
"classlibrary1.CustomView;md5d6f7135293df7527c983d45d07471c5e.CustomTextView",
});
Assert.IsTrue (task.Execute (), "Task should have executed successfully");
var output = File.ReadAllText (Path.Combine (resPath, "layout", "main.xml"));
StringAssert.Contains ("md5d6f7135293df7527c983d45d07471c5e.CustomTextView", output, "md5d6f7135293df7527c983d45d07471c5e.CustomTextView should exist in the main.xml");
StringAssert.DoesNotContain ("ClassLibrary1.CustomView", output, "ClassLibrary1.CustomView should have been replaced.");
StringAssert.Contains ("classLibrary1.CustomView", output, "classLibrary1.CustomView should have been replaced.");
Assert.AreEqual (1, errors.Count, "One Error should have been raised.");
Assert.AreEqual ("XA1002", errors [0].Code, "XA1002 should have been raised.");
Directory.Delete (path, recursive: true);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
<Compile Include="Aapt2Tests.cs" />
<Compile Include="Tasks\ValidateJavaVersionTests.cs" />
<Compile Include="ZipArchiveExTests.cs" />
<Compile Include="ConvertResourcesCasesTests.cs" />
</ItemGroup>
<ItemGroup>
<Content Include="Expected\GenerateDesignerFileExpected.cs">
Expand Down
40 changes: 25 additions & 15 deletions src/Xamarin.Android.Build.Tasks/Utilities/AndroidResource.cs
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Text;
using System.Text.RegularExpressions;
using System.Xml.Linq;
using System.Xml.XPath;

namespace Monodroid {
static class AndroidResource {

public static void UpdateXmlResource (string res, string filename, Dictionary<string, string> acwMap, IEnumerable<string> additionalDirectories = null)
public static void UpdateXmlResource (string res, string filename, Dictionary<string, string> acwMap, IEnumerable<string> additionalDirectories = null, Action<TraceLevel, string> logMessage = null)
{
// use a temporary file so we only update the real file if things actually changed
string tmpfile = filename + ".bk";
try {
XDocument doc = XDocument.Load (filename, LoadOptions.SetLineInfo);

UpdateXmlResource (res, doc.Root, acwMap, additionalDirectories);
UpdateXmlResource (res, doc.Root, acwMap, additionalDirectories, logMessage);
using (var stream = File.OpenWrite (tmpfile))
using (var xw = new LinePreservedXmlWriter (new StreamWriter (stream)))
xw.WriteNode (doc.CreateNavigator (), false);
Expand All @@ -27,7 +29,8 @@ public static void UpdateXmlResource (string res, string filename, Dictionary<st
if (File.Exists (tmpfile)) {
File.Delete (tmpfile);
}
Console.Error.WriteLine ("AndroidResgen: Warning while updating Resource XML '{0}': {1}", filename, e.Message);
if (logMessage != null)
logMessage (TraceLevel.Warning, $"AndroidResgen: Warning while updating Resource XML '{filename}': {e.Message}");
return;
}
}
Expand Down Expand Up @@ -59,10 +62,10 @@ static IEnumerable<T> Prepend<T> (this IEnumerable<T> l, T another) where T : XN
yield return e;
}

static void UpdateXmlResource (string resourcesBasePath, XElement e, Dictionary<string, string> acwMap, IEnumerable<string> additionalDirectories = null)
static void UpdateXmlResource (string resourcesBasePath, XElement e, Dictionary<string, string> acwMap, IEnumerable<string> additionalDirectories = null, Action<TraceLevel, string> logMessage = null)
{
foreach (var elem in GetElements (e).Prepend (e)) {
TryFixCustomView (elem, acwMap);
TryFixCustomView (elem, acwMap, logMessage);
}

foreach (var path in fixResourcesAliasPaths) {
Expand Down Expand Up @@ -184,17 +187,17 @@ private static bool TryFixFragment (XAttribute attr, Dictionary<string, string>
return false;

if (attr.Name == "class" || attr.Name == android + "name") {
if (acwMap.ContainsKey (attr.Value)) {
attr.Value = acwMap[attr.Value];
if (acwMap.TryGetValue (attr.Value, out string mappedValue)) {
attr.Value = mappedValue;

return true;
}
else if (attr.Value?.Contains (',') ?? false) {
// attr.Value could be an assembly-qualified name that isn't in acw-map.txt;
// see e5b1c92c, https://github.com/xamarin/xamarin-android/issues/1296#issuecomment-365091948
var n = attr.Value.Substring (0, attr.Value.IndexOf (','));
if (acwMap.ContainsKey (n)) {
attr.Value = acwMap [n];
if (acwMap.TryGetValue (n, out mappedValue)) {
attr.Value = mappedValue;
return true;
}
}
Expand All @@ -217,15 +220,23 @@ private static bool TryFixResAuto (XAttribute attr, Dictionary<string, string> a
return false;
}

private static bool TryFixCustomView (XElement elem, Dictionary<string, string> acwMap)
private static bool TryFixCustomView (XElement elem, Dictionary<string, string> acwMap, Action<TraceLevel, string> logMessage = null)
{
// Looks for any <My.DotNet.Class ...
// and tries to change it to the ACW name
if (acwMap.ContainsKey (elem.Name.ToString ())) {
elem.Name = acwMap[elem.Name.ToString ()];
string name = elem.Name.ToString ();
if (acwMap.TryGetValue (name, out string mappedValue)) {
elem.Name = mappedValue;
return true;
}

if (logMessage == null)
return false;
var matchingKey = acwMap.FirstOrDefault(x => String.Equals(x.Key, name, StringComparison.OrdinalIgnoreCase));
if (matchingKey.Key != null) {
// we have elements with slightly different casing.
// lets issue a error.
logMessage (TraceLevel.Error, $"We found a matching key '{matchingKey.Key}' for '{name}'. But the casing was incorrect. Please correct the casing");
}
return false;
}

Expand All @@ -238,8 +249,7 @@ private static bool TryFixCustomClassAttribute (XAttribute attr, Dictionary<stri
&& (attr.Parent.Name != "transition" || attr.Name.LocalName != "class")) // For custom transitions
return false;

string mappedValue;
if (!acwMap.TryGetValue (attr.Value, out mappedValue))
if (!acwMap.TryGetValue (attr.Value, out string mappedValue))
return false;

attr.Value = mappedValue;
Expand Down
10 changes: 10 additions & 0 deletions src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -553,5 +553,15 @@ public static string TryGetAndroidJarPath (TaskLoggingHelper log, string platfor
}
return Path.Combine (platformPath, "android.jar");
}

public static Dictionary<string, string> LoadResourceCaseMap (string resourceCaseMap)
{
var result = new Dictionary<string, string> ();
if (resourceCaseMap != null) {
foreach (var arr in resourceCaseMap.Split (';').Select (l => l.Split ('|')).Where (a => a.Length == 2))
result [arr [1]] = arr [0]; // lowercase -> original
}
return result;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2219,6 +2219,7 @@ because xbuild doesn't support framework reference assemblies.
</GenerateJavaStubs>
<ConvertResourcesCases
ResourceDirectories="$(MonoAndroidResDirIntermediate);@(LibraryResourceDirectories)"
ResourceNameCaseMap="$(_AndroidResourceNameCaseMap)"
AcwMapFile="$(_AcwMapFile)" />
</Target>

Expand Down

0 comments on commit 967fe94

Please sign in to comment.