Skip to content

[Xamarin.Android.Build.Tasks] SRM in <GetAdditionalResourcesFromAssemblies/> #2624

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jan 18, 2019
Merged

[Xamarin.Android.Build.Tasks] SRM in <GetAdditionalResourcesFromAssemblies/> #2624

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 58 additions & 65 deletions src/Xamarin.Android.Build.Tasks/Tasks/GetAdditionalResourcesFromAssemblies.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,19 @@
using Microsoft.Build.Framework;
using System;
using System.Collections.Generic;
using System.Linq;
using System.IO;
using System.Linq;
using System.Net;
using System.Reflection.Metadata;
using System.Reflection.PortableExecutable;
using System.Security.Cryptography;
using System.Text;
using System.Text.RegularExpressions;
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;
using Mono.Cecil;
using System.Xml;
using System.Xml.Linq;
using System.Security.Cryptography;
using System.Net;
using System.ComponentModel;
using Xamarin.Android.Tools;
using Xamarin.Tools.Zip;

using Java.Interop.Tools.Cecil;

namespace Xamarin.Android.Tasks {
namespace Xamarin.Android.Tasks
{

public class GetAdditionalResourcesFromAssemblies : AsyncTask
{
Expand Down Expand Up @@ -85,45 +81,45 @@ string SubstituteEnvVariables (string path)
return regex.Replace (path, replaceEnvVar);
}

internal static string ErrorMessage (CustomAttribute attr)
internal static string ErrorMessage (CustomAttributeValue<object> attributeValue)
{
if (!attr.HasProperties)
if (attributeValue.NamedArguments.Length == 0)
return "";
CustomAttributeNamedArgument arg = attr.Properties.FirstOrDefault (p => p.Name == "PackageName");
var arg = attributeValue.NamedArguments.FirstOrDefault (p => p.Name == "PackageName");
if (arg.Name != null) {
string packageName = arg.Argument.Value as string;
string packageName = arg.Value as string;
if (packageName != null)
return string.Format ("Please install package: '{0}' available in SDK installer", packageName);
}
arg = attr.Properties.FirstOrDefault (p => p.Name == "InstallInstructions");
arg = attributeValue.NamedArguments.FirstOrDefault (p => p.Name == "InstallInstructions");
if (arg.Name != null) {
string instInstructs = arg.Argument.Value as string;
string instInstructs = arg.Value as string;
if (instInstructs != null)
return "Installation instructions: " + instInstructs;
}
return null;
}

void AddAttributeValue (ICollection<string> items, CustomAttribute attr, string errorCode, string errorFmt,
bool isDirectory, string fullPath)
void AddAttributeValue (ICollection<string> items, CustomAttributeValue<object> attributeValue, string errorCode, string errorFmt,
bool isDirectory, string fullPath, string attributeFullName)
{
if (!attr.HasConstructorArguments || attr.ConstructorArguments.Count != 1) {
LogCodedWarning (errorCode, "Attribute {0} doesn't have expected one constructor agrument", attr.AttributeType.FullName);
if (attributeValue.NamedArguments.Length == 0 || attributeValue.FixedArguments.Length != 1) {
LogCodedWarning (errorCode, "Attribute {0} doesn't have expected one constructor agrument", attributeFullName);
return;
}

CustomAttributeArgument arg = attr.ConstructorArguments.First ();
var arg = attributeValue.FixedArguments.First ();
string path = arg.Value as string;
if (string.IsNullOrEmpty (path)) {
LogCodedWarning (errorCode, "Attribute {0} contructor argument is empty or not set to string", attr.AttributeType.FullName);
LogCodedWarning (errorCode, "Attribute {0} contructor argument is empty or not set to string", attributeFullName);
return;
}
path = SubstituteEnvVariables (path).TrimEnd (Path.DirectorySeparatorChar);
string baseDir = null;
CustomAttributeNamedArgument sourceUrl = attr.Properties.FirstOrDefault (p => p.Name == "SourceUrl");
CustomAttributeNamedArgument embeddedArchive = attr.Properties.FirstOrDefault (p => p.Name == "EmbeddedArchive");
CustomAttributeNamedArgument version = attr.Properties.FirstOrDefault (p => p.Name == "Version");
CustomAttributeNamedArgument sha1sum = attr.Properties.FirstOrDefault (p => p.Name == "Sha1sum");
var sourceUrl = attributeValue.NamedArguments.FirstOrDefault (p => p.Name == "SourceUrl");
var embeddedArchive = attributeValue.NamedArguments.FirstOrDefault (p => p.Name == "EmbeddedArchive");
var version = attributeValue.NamedArguments.FirstOrDefault (p => p.Name == "Version");
var sha1sum = attributeValue.NamedArguments.FirstOrDefault (p => p.Name == "Sha1sum");
var isXamarinAssembly = Path.GetFileName (fullPath).StartsWith (AssemblyNamePrefix, StringComparison.OrdinalIgnoreCase);
var assemblyDir = Path.Combine (CachePath, Path.GetFileNameWithoutExtension (fullPath));
// upgrade the paths to not strip off the Xamarin. prefix as it might cause assembly
Expand All @@ -144,10 +140,10 @@ void AddAttributeValue (ICollection<string> items, CustomAttribute attr, string
Directory.Delete (oldAssemblyDir, recursive: true);
}
if (sourceUrl.Name != null) {
if (new Uri (sourceUrl.Argument.Value as string).IsFile)
if (new Uri (sourceUrl.Value as string).IsFile)
assemblyDir = Path.GetDirectoryName (fullPath);
baseDir = MakeSureLibraryIsInPlace (assemblyDir, sourceUrl.Argument.Value as string,
version.Argument.Value as string, embeddedArchive.Argument.Value as string, sha1sum.Argument.Value as string);
baseDir = MakeSureLibraryIsInPlace (assemblyDir, sourceUrl.Value as string,
version.Value as string, embeddedArchive.Value as string, sha1sum.Value as string);
}
if (!string.IsNullOrEmpty (baseDir) && !Path.IsPathRooted (path))
path = Path.Combine (baseDir, path);
Expand All @@ -157,7 +153,7 @@ void AddAttributeValue (ICollection<string> items, CustomAttribute attr, string
return;
}
if (!DesignTimeBuild)
LogCodedError (errorCode, errorFmt, ErrorMessage (attr), path);
LogCodedError (errorCode, errorFmt, ErrorMessage (attributeValue), path);
}

bool ExtractArchive (string url, string file, string contentDir)
Expand Down Expand Up @@ -382,12 +378,6 @@ public override bool Execute ()

void DoExecute ()
{
LogDebugMessage ("GetAdditionalResourcesFromAssemblies Task");
LogDebugMessage (" AndroidSdkDirectory: {0}", AndroidSdkDirectory);
LogDebugMessage (" AndroidNdkDirectory: {0}", AndroidNdkDirectory);
LogDebugMessage (" CacheFile: {0}", CacheFile);
LogDebugTaskItems (" Assemblies: ", Assemblies);

if (Environment.GetEnvironmentVariable ("XA_DL_IGNORE_CERT_ERRROS") == "yesyesyes") {
ServicePointManager.ServerCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => true;
LogDebugMessage (" Disabling download certificate validation callback.");
Expand All @@ -411,35 +401,38 @@ void DoExecute ()
? CachePath
: Path.Combine (Environment.GetFolderPath (Environment.SpecialFolder.LocalApplicationData), CacheBaseDir);

using (var resolver = new DirectoryAssemblyResolver (this.CreateTaskLogger (), loadDebugSymbols: false)) {
foreach (var assemblyItem in Assemblies) {
string fullPath = Path.GetFullPath (assemblyItem.ItemSpec);
if (DesignTimeBuild && !File.Exists (fullPath)) {
LogWarning ("Failed to load '{0}'. Check the file exists or the project has been built.", fullPath);
continue;
}
if (assemblies.Contains (fullPath)) {
LogDebugMessage (" Skip assembly: {0}, it was already processed", fullPath);
continue;
}
// don't try to even load mscorlib it will fail.
if (string.Compare (Path.GetFileNameWithoutExtension (fullPath), "mscorlib", StringComparison.OrdinalIgnoreCase) == 0)
continue;
assemblies.Add (fullPath);
resolver.Load (fullPath);
foreach (var assemblyItem in Assemblies) {
string fullPath = Path.GetFullPath (assemblyItem.ItemSpec);
if (DesignTimeBuild && !File.Exists (fullPath)) {
LogWarning ("Failed to load '{0}'. Check the file exists or the project has been built.", fullPath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to make this a LogCodedWarning if possible

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indent some code, then you have to write docs! 😜

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dellis1972 it looks like @brendanzagaeski looked at these codes? #2258

They weren't "actionable"?

Copy link
Contributor

@brendanzagaeski brendanzagaeski Jan 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for this exact LogWarning() in DoExecute() I temporarily skipped further work on it when I realized that the warnings in other tasks might be a higher priority because the Xamarin-authored NuGet packages have been using the Xamarin.Build.Download mechanism for a while instead of the GetAdditionalResourcesFromAssemblies mechanism. I was going to circle back eventually to the remaining uses of LogWarning() in GetAdditionalResourcesFromAssemblies.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok let me go ahead and add the warning codes for these then, thanks.

continue;
}
if (assemblies.Contains (fullPath)) {
LogDebugMessage (" Skip assembly: {0}, it was already processed", fullPath);
continue;
}
// don't try to even load mscorlib it will fail.
if (string.Compare (Path.GetFileNameWithoutExtension (fullPath), "mscorlib", StringComparison.OrdinalIgnoreCase) == 0)
continue;
assemblies.Add (fullPath);
using (var pe = new PEReader (File.OpenRead (fullPath))) {
var reader = pe.GetMetadataReader ();
var assembly = reader.GetAssemblyDefinition ();
// Append source file name (without the Xamarin. prefix or extension) to the base folder
// This would help avoid potential collisions.
foreach (var ca in resolver.GetAssembly (assemblyItem.ItemSpec).CustomAttributes) {
switch (ca.AttributeType.FullName) {
case "Android.IncludeAndroidResourcesFromAttribute":
AddAttributeValue (androidResources, ca, "XA5206", "{0}. Android resource directory {1} doesn't exist.", true, fullPath);
break;
case "Java.Interop.JavaLibraryReferenceAttribute":
AddAttributeValue (javaLibraries, ca, "XA5207", "{0}. Java library file {1} doesn't exist.", false, fullPath);
break;
case "Android.NativeLibraryReferenceAttribute":
AddAttributeValue (nativeLibraries, ca, "XA5210", "{0}. Native library file {1} doesn't exist.", false, fullPath);
break;
foreach (var handle in assembly.GetCustomAttributes ()) {
var attribute = reader.GetCustomAttribute (handle);
var fullName = reader.GetCustomAttributeFullName (attribute);
switch (fullName) {
case "Android.IncludeAndroidResourcesFromAttribute":
AddAttributeValue (androidResources, attribute.GetCustomAttributeArguments (), "XA5206", "{0}. Android resource directory {1} doesn't exist.", true, fullPath, fullName);
break;
case "Java.Interop.JavaLibraryReferenceAttribute":
AddAttributeValue (javaLibraries, attribute.GetCustomAttributeArguments (), "XA5207", "{0}. Java library file {1} doesn't exist.", false, fullPath, fullName);
break;
case "Android.NativeLibraryReferenceAttribute":
AddAttributeValue (nativeLibraries, attribute.GetCustomAttributeArguments (), "XA5210", "{0}. Native library file {1} doesn't exist.", false, fullPath, fullName);
break;
}
}
}
Expand Down