Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Xamarin.Android.Build.Tasks] fix potential AsyncTask.Log calls #4187

Merged
merged 1 commit into from Jan 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .external
@@ -1,2 +1,2 @@
xamarin/monodroid:master@78d565884f7dd2cf76e748125485f7ed0e03eec4
xamarin/monodroid:master@017b3618bd33689f766e82b5fd640360e4c19919
mono/mono:2019-12@2edccc52a78d90fea7bcbd37844164663e712397
10 changes: 5 additions & 5 deletions src/Xamarin.Android.Build.Tasks/Tasks/Aapt.cs
Expand Up @@ -267,25 +267,25 @@ protected string GenerateCommandLineCommands (string ManifestFile, string curren

Directory.CreateDirectory (manifestDir);
manifestFile = Path.Combine (manifestDir, Path.GetFileName (ManifestFile));
ManifestDocument manifest = new ManifestDocument (ManifestFile, this.Log);
ManifestDocument manifest = new ManifestDocument (ManifestFile);
manifest.SdkVersion = AndroidSdkPlatform;
if (!string.IsNullOrEmpty (VersionCodePattern)) {
try {
manifest.CalculateVersionCode (currentAbi, VersionCodePattern, VersionCodeProperties);
} catch (ArgumentOutOfRangeException ex) {
Log.LogCodedError ("XA0003", ManifestFile, 0, ex.Message);
LogCodedError ("XA0003", ManifestFile, 0, ex.Message);
return string.Empty;
}
}
if (currentAbi != null && string.IsNullOrEmpty (VersionCodePattern)) {
manifest.SetAbi (currentAbi);
}
if (!manifest.ValidateVersionCode (out string error, out string errorCode)) {
Log.LogCodedError (errorCode, ManifestFile, 0, error);
LogCodedError (errorCode, ManifestFile, 0, error);
return string.Empty;
}
manifest.ApplicationName = ApplicationName;
manifest.Save (manifestFile);
manifest.Save (LogWarning, manifestFile);

cmd.AppendSwitchIfNotNull ("-M ", manifestFile);
var designerDirectory = Path.IsPathRooted (JavaDesignerOutputDirectory) ? JavaDesignerOutputDirectory : Path.Combine (WorkingDirectory, JavaDesignerOutputDirectory);
Expand Down Expand Up @@ -348,7 +348,7 @@ protected string GenerateCommandLineCommands (string ManifestFile, string curren

var extraArgsExpanded = ExpandString (ExtraArgs);
if (extraArgsExpanded != ExtraArgs)
Log.LogDebugMessage (" ExtraArgs expanded: {0}", extraArgsExpanded);
LogDebugMessage (" ExtraArgs expanded: {0}", extraArgsExpanded);

if (!string.IsNullOrWhiteSpace (extraArgsExpanded))
cmd.AppendSwitch (extraArgsExpanded);
Expand Down
14 changes: 7 additions & 7 deletions src/Xamarin.Android.Build.Tasks/Tasks/Aapt2Link.cs
Expand Up @@ -107,25 +107,25 @@ string GenerateCommandLineCommands (string ManifestFile, string currentAbi, stri
string manifestDir = Path.Combine (Path.GetDirectoryName (ManifestFile), currentAbi != null ? currentAbi : "manifest");
Directory.CreateDirectory (manifestDir);
string manifestFile = Path.Combine (manifestDir, Path.GetFileName (ManifestFile));
ManifestDocument manifest = new ManifestDocument (ManifestFile, this.Log);
ManifestDocument manifest = new ManifestDocument (ManifestFile);
manifest.SdkVersion = AndroidSdkPlatform;
if (!string.IsNullOrEmpty (VersionCodePattern)) {
try {
manifest.CalculateVersionCode (currentAbi, VersionCodePattern, VersionCodeProperties);
} catch (ArgumentOutOfRangeException ex) {
Log.LogCodedError ("XA0003", ManifestFile, 0, ex.Message);
LogCodedError ("XA0003", ManifestFile, 0, ex.Message);
return string.Empty;
}
}
if (currentAbi != null && string.IsNullOrEmpty (VersionCodePattern)) {
manifest.SetAbi (currentAbi);
}
if (!manifest.ValidateVersionCode (out string error, out string errorCode)) {
Log.LogCodedError (errorCode, ManifestFile, 0, error);
LogCodedError (errorCode, ManifestFile, 0, error);
return string.Empty;
}
manifest.ApplicationName = ApplicationName;
manifest.Save (manifestFile);
manifest.Save (LogWarning, manifestFile);

cmd.AppendSwitchIfNotNull ("--manifest ", manifestFile);
if (!string.IsNullOrEmpty (JavaDesignerOutputDirectory)) {
Expand All @@ -142,7 +142,7 @@ string GenerateCommandLineCommands (string ManifestFile, string currentAbi, stri
if (File.Exists (flata)) {
cmd.AppendSwitchIfNotNull ("-R ", flata);
} else {
Log.LogDebugMessage ("Archive does not exist: " + flata);
LogDebugMessage ("Archive does not exist: " + flata);
}
}
}
Expand All @@ -152,7 +152,7 @@ string GenerateCommandLineCommands (string ManifestFile, string currentAbi, stri
if (File.Exists (flata)) {
cmd.AppendSwitchIfNotNull ("-R ", flata);
} else {
Log.LogDebugMessage ("Archive does not exist: " + flata);
LogDebugMessage ("Archive does not exist: " + flata);
}
}

Expand All @@ -175,7 +175,7 @@ string GenerateCommandLineCommands (string ManifestFile, string currentAbi, stri

var extraArgsExpanded = ExpandString (ExtraArgs);
if (extraArgsExpanded != ExtraArgs)
Log.LogDebugMessage (" ExtraArgs expanded: {0}", extraArgsExpanded);
LogDebugMessage (" ExtraArgs expanded: {0}", extraArgsExpanded);

if (!string.IsNullOrWhiteSpace (extraArgsExpanded))
cmd.AppendSwitch (extraArgsExpanded);
Expand Down
7 changes: 6 additions & 1 deletion src/Xamarin.Android.Build.Tasks/Tasks/AndroidTask.cs
Expand Up @@ -34,12 +34,17 @@ public abstract class AndroidAsyncTask : AsyncTask

public abstract string TaskPrefix { get; }

[Obsolete ("You should not use the 'Log' property directly for AsyncTask. Use the 'Log*' methods instead.", error: true)]
public new TaskLoggingHelper Log {
get => base.Log;
}

public override bool Execute ()
{
try {
return RunTask ();
} catch (Exception ex) {
Log.LogUnhandledException (TaskPrefix, ex);
this.LogUnhandledException (TaskPrefix, ex);
return false;
}
}
Expand Down
14 changes: 2 additions & 12 deletions src/Xamarin.Android.Build.Tasks/Tasks/Aot.cs
Expand Up @@ -84,7 +84,7 @@ public class Aot : AndroidAsyncTask

public override bool RunTask ()
{
if (EnableLLVM && !NdkUtil.Init (Log, AndroidNdkDirectory))
if (EnableLLVM && !NdkUtil.Init (LogCodedError, AndroidNdkDirectory))
return false;

return base.RunTask ();
Expand Down Expand Up @@ -176,11 +176,6 @@ static string QuoteFileName(string fileName)
return builder.ToString();
}

static bool ValidateAotConfiguration (TaskLoggingHelper log, AndroidTargetArch arch, bool enableLLVM)
{
return true;
}

int GetNdkApiLevel(string androidNdkPath, string androidApiLevel, AndroidTargetArch arch)
{
var manifest = AndroidAppManifest.Load (ManifestFile.ItemSpec, MonoAndroidHelper.SupportedVersions);
Expand Down Expand Up @@ -312,12 +307,7 @@ IEnumerable<Config> GetAotConfigs ()
throw new Exception ("Unsupported Android target architecture ABI: " + abi);
}

if (EnableLLVM && !NdkUtil.ValidateNdkPlatform (Log, AndroidNdkDirectory, arch, enableLLVM:EnableLLVM)) {
yield return Config.Invalid;
yield break;
}

if (!ValidateAotConfiguration(Log, arch, EnableLLVM)) {
if (EnableLLVM && !NdkUtil.ValidateNdkPlatform (LogMessage, LogCodedError, AndroidNdkDirectory, arch, enableLLVM:EnableLLVM)) {
yield return Config.Invalid;
yield break;
}
Expand Down
Expand Up @@ -119,7 +119,7 @@ public async override System.Threading.Tasks.Task RunTaskAsync ()

GenerateLayoutBindings.BindingGeneratorLanguage gen;
if (!GenerateLayoutBindings.KnownBindingGenerators.TryGetValue (OutputLanguage, out gen) || gen == null) {
Log.LogDebugMessage ($"Language {OutputLanguage} isn't supported, will use {GenerateLayoutBindings.DefaultOutputGenerator.Name} instead");
LogDebugMessage ($"Language {OutputLanguage} isn't supported, will use {GenerateLayoutBindings.DefaultOutputGenerator.Name} instead");
sourceFileExtension = GenerateLayoutBindings.DefaultOutputGenerator.Extension;
} else
sourceFileExtension = OutputFileExtension;
Expand All @@ -138,7 +138,7 @@ public async override System.Threading.Tasks.Task RunTaskAsync ()
if (layoutsByName.Count >= ParallelGenerationThreshold) {
// NOTE: Update the tests in $TOP_DIR/tests/CodeBehind/UnitTests/BuildTests.cs if this message
// is changed!
Log.LogDebugMessage ($"Parsing layouts in parallel (threshold of {ParallelGenerationThreshold} layouts met)");
LogDebugMessage ($"Parsing layouts in parallel (threshold of {ParallelGenerationThreshold} layouts met)");

await this.WhenAll (layoutsByName, kvp =>
ParseAndLoadGroup (layoutsByName, kvp.Key, kvp.Value.InputItems, ref kvp.Value.LayoutBindingItems, ref kvp.Value.LayoutPartialClassItems));
Expand All @@ -160,11 +160,11 @@ public async override System.Threading.Tasks.Task RunTaskAsync ()

LayoutBindingFiles = layoutBindingFiles.ToArray ();
if (LayoutBindingFiles.Length == 0)
Log.LogDebugMessage (" No layout file qualifies for code-behind generation");
LogDebugMessage (" No layout file qualifies for code-behind generation");
LayoutPartialClassFiles = layoutPartialClassFiles.ToArray ();

Log.LogDebugTaskItems (" LayoutBindingFiles:", LayoutBindingFiles, true);
Log.LogDebugTaskItems (" LayoutPartialClassFiles:", LayoutPartialClassFiles, true);
LogDebugTaskItems (" LayoutBindingFiles:", LayoutBindingFiles);
LogDebugTaskItems (" LayoutPartialClassFiles:", LayoutPartialClassFiles);
}

void ParseAndLoadGroup (Dictionary <string, LayoutGroup> groupIndex, string groupName, List<ITaskItem> items, ref List<ITaskItem> layoutBindingFiles, ref List<ITaskItem> layoutPartialClassFiles)
Expand Down
Expand Up @@ -22,7 +22,7 @@ public class CheckGoogleSdkRequirements : AndroidTask

public override bool RunTask ()
{
ManifestDocument manifest = new ManifestDocument (ManifestFile, this.Log);
ManifestDocument manifest = new ManifestDocument (ManifestFile);

var compileSdk = MonoAndroidHelper.SupportedVersions.GetApiLevelFromFrameworkVersion (TargetFrameworkVersion);

Expand Down
6 changes: 3 additions & 3 deletions src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs
Expand Up @@ -236,7 +236,7 @@ void Run (DirectoryAssemblyResolver res)
}

// Step 3 - Merge [Activity] and friends into AndroidManifest.xml
var manifest = new ManifestDocument (ManifestTemplate, this.Log);
var manifest = new ManifestDocument (ManifestTemplate);

manifest.PackageName = PackageName;
manifest.ApplicationName = ApplicationName ?? PackageName;
Expand All @@ -250,10 +250,10 @@ void Run (DirectoryAssemblyResolver res)
manifest.NeedsInternet = NeedsInternet;
manifest.InstantRunEnabled = InstantRunEnabled;

var additionalProviders = manifest.Merge (all_java_types, ApplicationJavaClass, EmbedAssemblies, BundledWearApplicationName, MergedManifestDocuments);
var additionalProviders = manifest.Merge (Log, all_java_types, ApplicationJavaClass, EmbedAssemblies, BundledWearApplicationName, MergedManifestDocuments);

using (var stream = new MemoryStream ()) {
manifest.Save (stream);
manifest.Save (Log, stream);

// Only write the new manifest if it actually changed
MonoAndroidHelper.CopyIfStreamChanged (stream, MergedAndroidManifestOutput);
Expand Down
14 changes: 7 additions & 7 deletions src/Xamarin.Android.Build.Tasks/Tasks/GenerateLayoutBindings.cs
Expand Up @@ -96,17 +96,17 @@ public async override System.Threading.Tasks.Task RunTaskAsync ()
BindingGenerator generator = GetBindingGenerator (OutputLanguage);

if (generator == null) {
Log.LogMessage ($"Unknown binding output language '{OutputLanguage}', will use {DefaultOutputGenerator.Name} instead");
LogMessage ($"Unknown binding output language '{OutputLanguage}', will use {DefaultOutputGenerator.Name} instead");
generator = DefaultOutputGenerator.Creator ();
}

if (generator == null) {
// Should "never" happen
Log.LogError ($"Cannot find binding generator for language {OutputLanguage} or {DefaultOutputGenerator.Name}");
LogError ($"Cannot find binding generator for language {OutputLanguage} or {DefaultOutputGenerator.Name}");
return;
}

Log.LogDebugMessage ($"Generating {generator.LanguageName} binding sources");
LogDebugMessage ($"Generating {generator.LanguageName} binding sources");

var layoutGroups = new Dictionary <string, LayoutGroup> (StringComparer.Ordinal);
string layoutGroupName;
Expand All @@ -133,7 +133,7 @@ public async override System.Threading.Tasks.Task RunTaskAsync ()
if (!GetRequiredMetadata (item, CalculateLayoutCodeBehind.LayoutGroupMetadata, out layoutGroupName))
return;
if (!layoutGroups.TryGetValue (layoutGroupName, out group) || group == null) {
Log.LogError ($"Partial class item without associated binding for layout group '{layoutGroupName}'");
LogError ($"Partial class item without associated binding for layout group '{layoutGroupName}'");
return;
}

Expand Down Expand Up @@ -177,7 +177,7 @@ public async override System.Threading.Tasks.Task RunTaskAsync ()
if (ResourceFiles.Length >= CalculateLayoutCodeBehind.ParallelGenerationThreshold) {
// NOTE: Update the tests in $TOP_DIR/tests/CodeBehind/UnitTests/BuildTests.cs if this message
// is changed!
Log.LogDebugMessage ($"Generating binding code in parallel (threshold of {CalculateLayoutCodeBehind.ParallelGenerationThreshold} layouts met)");
LogDebugMessage ($"Generating binding code in parallel (threshold of {CalculateLayoutCodeBehind.ParallelGenerationThreshold} layouts met)");
var fileSet = new ConcurrentBag <string> ();
await this.WhenAll (layoutGroups, kvp => GenerateSourceForLayoutGroup (generator, kvp.Value, rpath => fileSet.Add (rpath)));
generatedFilePaths = fileSet;
Expand All @@ -190,8 +190,8 @@ public async override System.Threading.Tasks.Task RunTaskAsync ()

GeneratedFiles = generatedFilePaths.Where (gfp => !String.IsNullOrEmpty (gfp)).Select (gfp => new TaskItem (gfp)).ToArray ();
if (GeneratedFiles.Length == 0)
Log.LogWarning ("No layout binding source files generated");
Log.LogDebugTaskItems (" GeneratedFiles:", GeneratedFiles);
LogWarning ("No layout binding source files generated");
LogDebugTaskItems (" GeneratedFiles:", GeneratedFiles);
}

void GenerateSourceForLayoutGroup (BindingGenerator generator, LayoutGroup group, Action <string> pathAdder)
Expand Down
4 changes: 2 additions & 2 deletions src/Xamarin.Android.Build.Tasks/Tasks/ManifestMerger.cs
Expand Up @@ -39,9 +39,9 @@ public override bool Execute ()
bool result = base.Execute ();
if (!result)
return result;
var m = new ManifestDocument (tempFile, Log);
var m = new ManifestDocument (tempFile);
using (var ms = new MemoryStream ()) {
m.Save (ms);
m.Save (Log, ms);
MonoAndroidHelper.CopyIfStreamChanged (ms, OutputManifestFile);
return result;
}
Expand Down
26 changes: 17 additions & 9 deletions src/Xamarin.Android.Build.Tasks/Tasks/NdkUtils.cs
Expand Up @@ -25,18 +25,21 @@ public static class NdkUtil

public static bool Init (string ndkPath)
{
return Init (null, ndkPath); // For tests which don't have access to a TaskLoggingHelper
return Init (delegate { }, ndkPath); // For tests which don't have access to a TaskLoggingHelper
}

public static bool Init (TaskLoggingHelper log, string ndkPath)
public static bool Init (TaskLoggingHelper log, string ndkPath) =>
jonathanpeppers marked this conversation as resolved.
Show resolved Hide resolved
Init ((c, m) => log.LogCodedError (c, m), ndkPath);

public static bool Init (Action<string, string> logError, string ndkPath)
{
Version ndkVersion;
bool hasNdkVersion = GetNdkToolchainRelease (ndkPath ?? "", out ndkVersion);

if (!hasNdkVersion) {
log?.LogCodedError ("XA5101",
"Could not locate the Android NDK. Please make sure the Android NDK is installed in the Android SDK Manager, " +
"or if using a custom NDK path, please ensure the $(AndroidNdkDirectory) MSBuild property is set to the custom path.");
logError ("XA5101",
"Could not locate the Android NDK. Please make sure the Android NDK is installed in the Android SDK Manager, " +
"or if using a custom NDK path, please ensure the $(AndroidNdkDirectory) MSBuild property is set to the custom path.");
return false;
}

Expand All @@ -46,18 +49,23 @@ public static bool Init (TaskLoggingHelper log, string ndkPath)
}

public static bool ValidateNdkPlatform (TaskLoggingHelper log, string ndkPath, AndroidTargetArch arch, bool enableLLVM)
{
return ValidateNdkPlatform ((m) => log.LogMessage (m), (c, m) => log.LogCodedError (c, m), ndkPath, arch, enableLLVM);
}

public static bool ValidateNdkPlatform (Action<string> logMessage, Action<string, string> logError, string ndkPath, AndroidTargetArch arch, bool enableLLVM)
{
if (!UsingClangNDK)
return NdkUtilOld.ValidateNdkPlatform (log, ndkPath, arch, enableLLVM);
return NdkUtilOld.ValidateNdkPlatform (logMessage, logError, ndkPath, arch, enableLLVM);

// Check that we have a compatible NDK version for the targeted ABIs.
Version ndkVersion;
bool hasNdkVersion = GetNdkToolchainRelease (ndkPath, out ndkVersion);

if (hasNdkVersion && ndkVersion.Major < 19) {
log.LogMessage (MessageImportance.High,
"The detected Android NDK version is incompatible with this version of Xamarin.Android, " +
"please upgrade to NDK r19 or newer.");
logMessage (
"The detected Android NDK version is incompatible with this version of Xamarin.Android, " +
"please upgrade to NDK r19 or newer.");
}

return true;
Expand Down
18 changes: 9 additions & 9 deletions src/Xamarin.Android.Build.Tasks/Tasks/NdkUtilsOld.cs
Expand Up @@ -16,30 +16,30 @@ namespace Xamarin.Android.Tasks
{
public static class NdkUtilOld {

public static bool ValidateNdkPlatform (TaskLoggingHelper log, string ndkPath, AndroidTargetArch arch, bool enableLLVM)
public static bool ValidateNdkPlatform (Action<string> logMessage, Action<string, string> logError, string ndkPath, AndroidTargetArch arch, bool enableLLVM)
{
// Check that we have a compatible NDK version for the targeted ABIs.
NdkVersion ndkVersion;
bool hasNdkVersion = GetNdkToolchainRelease (ndkPath, out ndkVersion);

if (IsNdk64BitArch(arch) && hasNdkVersion && ndkVersion.Version < 10) {
log.LogMessage (MessageImportance.High,
"The detected Android NDK version is incompatible with the targeted 64-bit architecture, " +
"please upgrade to NDK r10 or newer.");
logMessage (
"The detected Android NDK version is incompatible with the targeted 64-bit architecture, " +
"please upgrade to NDK r10 or newer.");
}

// NDK r10d is buggy and cannot link x86_64 ABI shared libraries because they are 32-bits.
// See https://code.google.com/p/android/issues/detail?id=161421
if (enableLLVM && ndkVersion.Version == 10 && ndkVersion.Revision == "d" && arch == AndroidTargetArch.X86_64) {
log.LogCodedError ("XA3004", "Android NDK r10d is buggy and provides an incompatible x86_64 libm.so. " +
"See https://code.google.com/p/android/issues/detail?id=161422.");
logError ("XA3004", "Android NDK r10d is buggy and provides an incompatible x86_64 libm.so. " +
"See https://code.google.com/p/android/issues/detail?id=161422.");
return false;
}

if (enableLLVM && (ndkVersion.Version < 10 || (ndkVersion.Version == 10 && ndkVersion.Revision[0] < 'd'))) {
log.LogCodedError ("XA3005",
"The detected Android NDK version is incompatible with the targeted LLVM configuration, " +
"please upgrade to NDK r10d or newer.");
logError ("XA3005",
"The detected Android NDK version is incompatible with the targeted LLVM configuration, " +
"please upgrade to NDK r10d or newer.");
}

return true;
Expand Down