From 6651e50fa93a2033f636b18d4bcbd66d34a8d3d2 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Mon, 9 Oct 2017 16:17:35 +0100 Subject: [PATCH 1/5] [Xamarin.Android.Build.Tasks] Only Ignore Lint Checks if its supported. Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=59861 We started to ignore the `StaticFieldLeak` lint check. But this was introduced in 26.0.2 of tools. So if we try to ignore this check in older versions it will error. Invalid id or category "StaticFieldLeak". So we need to version the checks we ignore by default. This commit puts these in a dictionary which includes the version number you can run them from. --- src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs | 26 ++++++++------ .../Tasks/ResolveSdksTask.cs | 36 +++++++++++++++++-- .../Xamarin.Android.Common.targets | 2 ++ 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs b/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs index e81b23e4471..95d7f26db38 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs @@ -5,7 +5,8 @@ using System.IO; using System.Xml.Linq; using System.Linq; -using Xamarin.Android.Tools; +using Xamarin.Android.Build.Utilities; +using System.Collections.Generic; namespace Xamarin.Android.Tasks { @@ -49,6 +50,9 @@ public class Lint : ToolTask [Required] public string IntermediateOutputPath { get; set; } + [Required] + public string LintToolVersion { get; set; } + /// /// Location of an xml config files used to /// determine whether issues are enabled or disabled @@ -152,30 +156,30 @@ public Lint () LibraryJars = new ITaskItem[0]; } - - string [] disabledIssues = new string [] { + static readonly Dictionary DisabledIssuesByVersion = new Dictionary () { // We need to hard code this test in because Lint will issue an Error // if android:debuggable appears in the manifest. We actually need that // in debug mode. It seems the android tools do some magic to // decide if its needed or not. - "HardcodedDebugMode", + { "HardcodedDebugMode", new Version(1, 0) }, // We need to hard code this test as disabled in because Lint will issue a warning // for all the resources not used. Now because we don't have any Java code // that means for EVERYTHING! Which will be a HUGE amount of warnings for a large project - "UnusedResources", + { "UnusedResources", new Version(1, 0) }, // We need to hard code this test as disabled in because Lint will issue a warning // for the MonoPackageManager.java since we have to use a static to keep track of the // application instance. - "StaticFieldLeak", - // We don't call base.Super () for onCreate so we need to ignore this too. - "MissingSuperCall", + { "StaticFieldLeak", new Version(26, 0, 2) }, }; public override bool Execute () { - foreach (var disabled in disabledIssues) { - if (string.IsNullOrEmpty (DisabledIssues) || !DisabledIssues.Contains (disabled)) - DisabledIssues = disabled + (!string.IsNullOrEmpty (DisabledIssues) ? "," + DisabledIssues : ""); + Version lintToolVersion = Version.Parse (LintToolVersion); + foreach (var issue in DisabledIssuesByVersion) { + if (lintToolVersion >= issue.Value) { + if (string.IsNullOrEmpty (DisabledIssues) || !DisabledIssues.Contains (issue.Key)) + DisabledIssues = issue.Key + (!string.IsNullOrEmpty (DisabledIssues) ? "," + DisabledIssues : ""); + } } Log.LogDebugMessage ("Lint Task"); diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs index e8227ef49f2..246b3076f57 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs @@ -110,6 +110,9 @@ public class ResolveSdks : Task [Output] public string LintToolPath { get; set; } + [Output] + public string LintToolVersion { get; set; } + static bool IsWindows = Path.DirectorySeparatorChar == '\\'; static readonly string ZipAlign = IsWindows ? "zipalign.exe" : "zipalign"; static readonly string Aapt = IsWindows ? "aapt.exe" : "aapt"; @@ -178,7 +181,13 @@ public bool RunTask () } } - foreach (var dir in MonoAndroidHelper.AndroidSdk.GetBuildToolsPaths (AndroidSdkBuildToolsVersion)) { + Version lintVersion = new Version (); + if (!string.IsNullOrEmpty (LintToolPath)) { + lintVersion = GetLintVersion (Path.Combine (LintToolPath, Lint)); + } + LintToolVersion = lintVersion.ToString (); + + foreach (var dir in AndroidSdk.GetBuildToolsPaths (AndroidSdkBuildToolsVersion)) { Log.LogDebugMessage ("Trying build-tools path: {0}", dir); if (dir == null || !Directory.Exists (dir)) continue; @@ -277,6 +286,7 @@ public bool RunTask () Log.LogDebugMessage (" SupportedApiLevel: {0}", SupportedApiLevel); Log.LogDebugMessage (" AndroidSequencePointMode: {0}", AndroidSequencePointsMode); Log.LogDebugMessage (" LintToolPath: {0}", LintToolPath); + Log.LogDebugMessage (" LintToolVersion: {0}", LintToolVersion); if (!string.IsNullOrEmpty (CacheFile)) { Directory.CreateDirectory (Path.GetDirectoryName (CacheFile)); @@ -301,7 +311,8 @@ public bool RunTask () new XElement ("MonoAndroidIncludePath", MonoAndroidIncludePath), new XElement ("SupportedApiLevel", SupportedApiLevel), new XElement ("AndroidSequencePointsMode", AndroidSequencePointsMode.ToString ()), - new XElement ("LintToolPath", LintToolPath) + new XElement ("LintToolPath", LintToolPath), + new XElement ("LintToolVersion", LintToolVersion) )); document.Save (CacheFile); } @@ -311,6 +322,7 @@ public bool RunTask () } static readonly Regex javaVersionRegex = new Regex (@"""(?[\d\.]+)_\d+"""); + static readonly Regex lintVersionRegex = new Regex (@"version[\t\s]+(?[\d\.]+)"); Version GetJavaVersionForFramework (string targetFrameworkVersion) { @@ -334,6 +346,26 @@ Version GetJavaVersionForBuildTools (string buildToolsVersion) return Version.Parse (MinimumSupportedJavaVersion); } + Version GetLintVersion (string tool) { + var sb = new StringBuilder (); + MonoAndroidHelper.RunProcess (tool, "--version", (s, e) => { + if (!string.IsNullOrEmpty (e.Data)) + sb.AppendLine (e.Data); + }, (s, e) => { + if (!string.IsNullOrEmpty (e.Data)) + sb.AppendLine (e.Data); + } + ); + var versionInfo = sb.ToString (); + // lint: version 26.0.2 + var versionNumberMatch = lintVersionRegex.Match (versionInfo); + Version versionNumber; + if (versionNumberMatch.Success && Version.TryParse (versionNumberMatch.Groups ["version"]?.Value, out versionNumber)) { + return versionNumber; + } + return new Version (); + } + bool ValidateJavaVersion (string targetFrameworkVersion, string buildToolsVersion) { Version requiredJavaForFrameworkVersion = GetJavaVersionForFramework (targetFrameworkVersion); diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index fb60b3fe313..f847038bd00 100755 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -649,6 +649,7 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved. + From 47defa0f06507ec5f28cb8e5edfc11d95720e360 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Mon, 9 Oct 2017 16:22:21 +0100 Subject: [PATCH 2/5] Fixed using clause --- src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs b/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs index 95d7f26db38..3bcb3c1f435 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs @@ -5,7 +5,7 @@ using System.IO; using System.Xml.Linq; using System.Linq; -using Xamarin.Android.Build.Utilities; +using Xamarin.Android.Tools; using System.Collections.Generic; namespace Xamarin.Android.Tasks From 1e854490397f16bcee036446250b67fa2a3ded22 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Mon, 9 Oct 2017 20:23:40 +0100 Subject: [PATCH 3/5] Updated based on feedback --- src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs | 45 +++++++++++++++---- .../Tasks/ResolveSdksTask.cs | 34 +------------- .../Xamarin.Android.Common.targets | 2 - 3 files changed, 38 insertions(+), 43 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs b/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs index 3bcb3c1f435..45f55c0f0d1 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs @@ -7,6 +7,7 @@ using System.Linq; using Xamarin.Android.Tools; using System.Collections.Generic; +using System.Text; namespace Xamarin.Android.Tasks { @@ -172,17 +173,25 @@ public Lint () { "StaticFieldLeak", new Version(26, 0, 2) }, }; + static readonly Regex lintVersionRegex = new Regex (@"version[\t\s]+(?[\d\.]+)"); + public override bool Execute () { - Version lintToolVersion = Version.Parse (LintToolVersion); + Log.LogDebugMessage ("Lint Task"); + + if (string.IsNullOrEmpty (ToolPath) || !File.Exists (GenerateFullPathToTool ())) { + Log.LogCodedError ("XA5205", $"Cannot find `{ToolName}` in the Android SDK. Please set its path via /p:LintToolPath."); + return false; + } + + Version lintToolVersion = GetLintVersion (GenerateFullPathToTool ()); foreach (var issue in DisabledIssuesByVersion) { if (lintToolVersion >= issue.Value) { if (string.IsNullOrEmpty (DisabledIssues) || !DisabledIssues.Contains (issue.Key)) DisabledIssues = issue.Key + (!string.IsNullOrEmpty (DisabledIssues) ? "," + DisabledIssues : ""); } } - - Log.LogDebugMessage ("Lint Task"); + Log.LogDebugMessage (" TargetDirectory: {0}", TargetDirectory); Log.LogDebugMessage (" EnabledChecks: {0}", EnabledIssues); Log.LogDebugMessage (" DisabledChecks: {0}", DisabledIssues); @@ -194,11 +203,6 @@ public override bool Execute () Log.LogDebugTaskItems (" LibraryDirectories:", LibraryDirectories); Log.LogDebugTaskItems (" LibraryJars:", LibraryJars); - if (string.IsNullOrEmpty (ToolPath) || !File.Exists (GenerateFullPathToTool ())) { - Log.LogCodedError ("XA5205", $"Cannot find `{ToolName}` in the Android SDK. Please set its path via /p:LintToolPath."); - return false; - } - base.Execute (); return !Log.HasLoggedErrors; @@ -308,6 +312,31 @@ XDocument MergeConfigFiles() } return config; } + + Version GetLintVersion (string tool) + { + var sb = new StringBuilder (); + var result = MonoAndroidHelper.RunProcess (tool, "--version", (s, e) => { + if (!string.IsNullOrEmpty (e.Data)) + sb.AppendLine (e.Data); + }, (s, e) => { + if (!string.IsNullOrEmpty (e.Data)) + sb.AppendLine (e.Data); + } + ); + if (result != 0) { + Log.LogWarning ($"Could not get version from '{tool}'"); + return new Version (); + } + var versionInfo = sb.ToString (); + // lint: version 26.0.2 + var versionNumberMatch = lintVersionRegex.Match (versionInfo); + Version versionNumber; + if (versionNumberMatch.Success && Version.TryParse (versionNumberMatch.Groups ["version"]?.Value, out versionNumber)) { + return versionNumber; + } + return new Version (); + } } } diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs index 246b3076f57..fc2196aec3d 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs @@ -110,9 +110,6 @@ public class ResolveSdks : Task [Output] public string LintToolPath { get; set; } - [Output] - public string LintToolVersion { get; set; } - static bool IsWindows = Path.DirectorySeparatorChar == '\\'; static readonly string ZipAlign = IsWindows ? "zipalign.exe" : "zipalign"; static readonly string Aapt = IsWindows ? "aapt.exe" : "aapt"; @@ -181,12 +178,6 @@ public bool RunTask () } } - Version lintVersion = new Version (); - if (!string.IsNullOrEmpty (LintToolPath)) { - lintVersion = GetLintVersion (Path.Combine (LintToolPath, Lint)); - } - LintToolVersion = lintVersion.ToString (); - foreach (var dir in AndroidSdk.GetBuildToolsPaths (AndroidSdkBuildToolsVersion)) { Log.LogDebugMessage ("Trying build-tools path: {0}", dir); if (dir == null || !Directory.Exists (dir)) @@ -286,7 +277,6 @@ public bool RunTask () Log.LogDebugMessage (" SupportedApiLevel: {0}", SupportedApiLevel); Log.LogDebugMessage (" AndroidSequencePointMode: {0}", AndroidSequencePointsMode); Log.LogDebugMessage (" LintToolPath: {0}", LintToolPath); - Log.LogDebugMessage (" LintToolVersion: {0}", LintToolVersion); if (!string.IsNullOrEmpty (CacheFile)) { Directory.CreateDirectory (Path.GetDirectoryName (CacheFile)); @@ -311,8 +301,7 @@ public bool RunTask () new XElement ("MonoAndroidIncludePath", MonoAndroidIncludePath), new XElement ("SupportedApiLevel", SupportedApiLevel), new XElement ("AndroidSequencePointsMode", AndroidSequencePointsMode.ToString ()), - new XElement ("LintToolPath", LintToolPath), - new XElement ("LintToolVersion", LintToolVersion) + new XElement ("LintToolPath", LintToolPath) )); document.Save (CacheFile); } @@ -322,7 +311,6 @@ public bool RunTask () } static readonly Regex javaVersionRegex = new Regex (@"""(?[\d\.]+)_\d+"""); - static readonly Regex lintVersionRegex = new Regex (@"version[\t\s]+(?[\d\.]+)"); Version GetJavaVersionForFramework (string targetFrameworkVersion) { @@ -346,26 +334,6 @@ Version GetJavaVersionForBuildTools (string buildToolsVersion) return Version.Parse (MinimumSupportedJavaVersion); } - Version GetLintVersion (string tool) { - var sb = new StringBuilder (); - MonoAndroidHelper.RunProcess (tool, "--version", (s, e) => { - if (!string.IsNullOrEmpty (e.Data)) - sb.AppendLine (e.Data); - }, (s, e) => { - if (!string.IsNullOrEmpty (e.Data)) - sb.AppendLine (e.Data); - } - ); - var versionInfo = sb.ToString (); - // lint: version 26.0.2 - var versionNumberMatch = lintVersionRegex.Match (versionInfo); - Version versionNumber; - if (versionNumberMatch.Success && Version.TryParse (versionNumberMatch.Groups ["version"]?.Value, out versionNumber)) { - return versionNumber; - } - return new Version (); - } - bool ValidateJavaVersion (string targetFrameworkVersion, string buildToolsVersion) { Version requiredJavaForFrameworkVersion = GetJavaVersionForFramework (targetFrameworkVersion); diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index f847038bd00..fb60b3fe313 100755 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -649,7 +649,6 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved. - From bd324d0931d978e4363935bcfa2e5e1edb4c724b Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Mon, 9 Oct 2017 20:41:38 +0100 Subject: [PATCH 4/5] Removed LintToolVersion [Required] property as it was not longer needed --- src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs b/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs index 45f55c0f0d1..2b0b62f6575 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/Lint.cs @@ -51,9 +51,6 @@ public class Lint : ToolTask [Required] public string IntermediateOutputPath { get; set; } - [Required] - public string LintToolVersion { get; set; } - /// /// Location of an xml config files used to /// determine whether issues are enabled or disabled From bf19bd1314c664b35be54a498d1a6c54209ec1ac Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Mon, 9 Oct 2017 20:43:16 +0100 Subject: [PATCH 5/5] Fixed compile error --- src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs index fc2196aec3d..e8227ef49f2 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveSdksTask.cs @@ -178,7 +178,7 @@ public bool RunTask () } } - foreach (var dir in AndroidSdk.GetBuildToolsPaths (AndroidSdkBuildToolsVersion)) { + foreach (var dir in MonoAndroidHelper.AndroidSdk.GetBuildToolsPaths (AndroidSdkBuildToolsVersion)) { Log.LogDebugMessage ("Trying build-tools path: {0}", dir); if (dir == null || !Directory.Exists (dir)) continue;