From 888aaca63eccef235bd04fe8af7e6435fdf2f4f1 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Wed, 3 Jan 2018 13:09:18 +0000 Subject: [PATCH] [Xamarin.Android.Build.Tasks] Rework Aapt log processing. Context #1134 Aapt is pretty inconsistent with how it reports errors and warnings. Warnings are written to stderr.. So we are reworking the output processing for aapt to hopefully handle errors are warnings in a more consistent way. Rather than process the output in realtime we will buffer it and process the output once aapt has completed. This will allow us to check to see if the required output file (R.java) was created. If it wasn't then we can assume that the process failed. We can then process the output knowing that we are expecting and error. In the case where R.java does exist, we can assume if we do find anything which looks like an error it is more than likely a warning. --- src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs | 36 +++++++++++++++---- .../AndroidRegExTests.cs | 8 +++++ .../AndroidUpdateResourcesTest.cs | 17 +++++++++ 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs b/src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs index 8e06b43b970..fad39f01259 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs @@ -95,6 +95,17 @@ public class Aapt : AsyncTask Dictionary resource_name_case_map = new Dictionary (); AssemblyIdentityMap assemblyMap = new AssemblyIdentityMap (); + struct OutputLine { + public string Line; + public bool StdError; + + public OutputLine (string line, bool stdError) + { + Line = line; + StdError = stdError; + } + } + bool ManifestIsUpToDate (string manifestFile) { return !String.IsNullOrEmpty (AndroidComponentResgenFlagFile) && @@ -102,7 +113,7 @@ bool ManifestIsUpToDate (string manifestFile) File.GetLastWriteTime (AndroidComponentResgenFlagFile) > File.GetLastWriteTime (manifestFile); } - bool RunAapt (string commandLine) + bool RunAapt (string commandLine, IList output) { var stdout_completed = new ManualResetEvent (false); var stderr_completed = new ManualResetEvent (false); @@ -119,13 +130,13 @@ bool RunAapt (string commandLine) using (var proc = new Process ()) { proc.OutputDataReceived += (sender, e) => { if (e.Data != null) - LogMessage (e.Data, MessageImportance.Normal); + output.Add (new OutputLine (e.Data, stdError: false)); else stdout_completed.Set (); }; proc.ErrorDataReceived += (sender, e) => { if (e.Data != null) - LogEventsFromTextOutput (e.Data, MessageImportance.Normal); + output.Add (new OutputLine (e.Data, stdError: true)); else stderr_completed.Set (); }; @@ -151,7 +162,16 @@ bool RunAapt (string commandLine) bool ExecuteForAbi (string cmd, string currentResourceOutputFile) { - var ret = RunAapt (cmd); + var output = new List (); + var ret = RunAapt (cmd, output); + var success = File.Exists (Path.Combine (JavaDesignerOutputDirectory, "R.java")); + foreach (var line in output) { + if (line.StdError) { + LogEventsFromTextOutput (line.Line, MessageImportance.Normal, success); + } else { + LogMessage (line.Line, MessageImportance.Normal); + } + } if (ret && !string.IsNullOrEmpty (currentResourceOutputFile)) { var tmpfile = currentResourceOutputFile + ".bk"; MonoAndroidHelper.CopyIfZipChanged (tmpfile, currentResourceOutputFile); @@ -362,7 +382,7 @@ protected string GenerateFullPathToTool () return Path.Combine (ToolPath, string.IsNullOrEmpty (ToolExe) ? ToolName : ToolExe); } - protected void LogEventsFromTextOutput (string singleLine, MessageImportance messageImportance) + protected void LogEventsFromTextOutput (string singleLine, MessageImportance messageImportance, bool apptResult) { if (string.IsNullOrEmpty (singleLine)) return; @@ -399,7 +419,11 @@ protected void LogEventsFromTextOutput (string singleLine, MessageImportance mes } } - LogError ("APT0000", string.Format("{0} \"{1}\".", singleLine.Trim(), singleLine.Substring(singleLine.LastIndexOfAny(new char[] { '\\', '/' }) + 1)), ToolName); + if (!apptResult) { + LogError ("APT0000", string.Format ("{0} \"{1}\".", singleLine.Trim (), singleLine.Substring (singleLine.LastIndexOfAny (new char [] { '\\', '/' }) + 1)), ToolName); + } else { + LogWarning (singleLine); + } } } } diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidRegExTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidRegExTests.cs index d47a3e0ccc0..f1431bc797e 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidRegExTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidRegExTests.cs @@ -74,6 +74,14 @@ public IEnumerator GetEnumerator () /*expectedLevel*/ "", /*expectedMessage*/ "max res 10, skipping values-sw600dp-land" }; + yield return new object [] { + /*message*/ "max res 10, skipping values-sw720dp-land-v13", + /*expectedToMatch*/ true, + /*expectedFile*/ "", + /*expectedLine*/ "", + /*expectedLevel*/ "", + /*expectedMessage*/ "max res 10, skipping values-sw720dp-land-v13" + }; yield return new object [] { /*message*/ "Error: unable to generate entry for resource data", /*expectedToMatch*/ true, diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs index e998357a5e8..5263bb64fb2 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs @@ -185,6 +185,23 @@ public void ReportAaptErrorsInOriginalFileName () } } + [Test] + public void ReportAaptWarningsForBlankLevel () + { + //This test should get the warning `Invalid file name: must contain only [a-z0-9_.]` + // However, still fails due to aapt failing, Resource.designer.cs is not generated + var proj = new XamarinAndroidApplicationProject (); + proj.AndroidResources.Add (new AndroidItem.AndroidResource ("Resources\\drawable\\Image (1).png") { + BinaryContent = () => XamarinAndroidCommonProject.icon_binary_mdpi + }); + using (var b = CreateApkBuilder ("temp/ReportAaptWarningsForBlankLevel")) { + b.ThrowOnBuildFailure = false; + Assert.IsFalse (b.Build (proj), "Build should have failed."); + StringAssertEx.Contains ("APT0000", b.LastBuildOutput, "An error message with a blank \"level\", should be reported as an error!"); + Assert.IsTrue (b.Clean (proj), "Clean should have succeeded."); + } + } + [Test] public void RepetiviteBuildUpdateSingleResource () {