Skip to content

Improving ApiCompat tool 5859b11 #4174

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 27, 2020
Merged

Improving ApiCompat tool 5859b11 #4174

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
115 changes: 84 additions & 31 deletions ...ndroid.Tools.BootstrapTasks/Xamarin.Android.Tools.BootstrapTasks/CheckApiCompatibility.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ public sealed class CheckApiCompatibility : Task
"Mono.Android.dll",
};

static string compatApiCommand = null;

// Path where Microsoft.DotNet.ApiCompat nuget package is located
[Required]
public string ApiCompatPath { get; set; }
Expand Down Expand Up @@ -60,7 +62,7 @@ public override bool Execute ()

// Check to see if Api has a previous Api defined.
if (!api_versions.TryGetValue (ApiLevel, out string previousApiLevel)) {
Log.LogError ($"Please add ApiLevel:{ApiLevel} to the list of supported apis.");
LogError ($"Please add ApiLevel:{ApiLevel} to the list of supported apis.");
return !Log.HasLoggedErrors;
}

Expand Down Expand Up @@ -88,7 +90,7 @@ public override bool Execute ()
// Check xamarin-android-api-compatibility reference directory exists
var referenceContractPath = Path.Combine (ApiCompatibilityPath, "reference");
if (!Directory.Exists (referenceContractPath)) {
Log.LogMessage (MessageImportance.High, $"CheckApiCompatibility Warning: Skipping reference contract check.\n{referenceContractPath} does not exist.");
Log.LogWarning ($"CheckApiCompatibility Warning: Skipping reference contract check.\n{referenceContractPath} does not exist.");
return !Log.HasLoggedErrors;
}

Expand Down Expand Up @@ -126,13 +128,13 @@ void ValidateApiCompat (string contractPath, bool validateAgainstReference)
foreach (var assemblyToValidate in assemblies) {
var contractAssembly = Path.Combine (contractPath, assemblyToValidate);
if (!File.Exists (contractAssembly)) {
Log.LogMessage ($"Contract assembly {assemblyToValidate} does not exists in the contract path.");
Log.LogWarning ($"Contract assembly {assemblyToValidate} does not exists in the contract path.");
continue;
}

var implementationAssembly = Path.Combine (TargetImplementationPath, assemblyToValidate);
if (!File.Exists (implementationAssembly)) {
Log.LogError ($"Implementation assembly {assemblyToValidate} exists in the contract path but not on the implementation folder.");
LogError ($"Implementation assembly {assemblyToValidate} exists in the contract path but not on the implementation folder.");
return;
}

Expand All @@ -154,13 +156,60 @@ void ValidateApiCompat (string contractPath, bool validateAgainstReference)
genApiProcess.StartInfo.UseShellExecute = false;
genApiProcess.StartInfo.CreateNoWindow = true;
genApiProcess.StartInfo.RedirectStandardOutput = true;
genApiProcess.StartInfo.RedirectStandardError = true;
genApiProcess.EnableRaisingEvents = true;

var lines = new List<string> ();
var processHasCrashed = false;
void dataReceived (object sender, DataReceivedEventArgs args)
{
if (!string.IsNullOrWhiteSpace (args.Data)) {
lines.Add (args.Data.Trim ());

if (args.Data.IndexOf ("Native Crash Reporting") != -1) {
processHasCrashed = true;
}
}
}

Log.LogMessage (MessageImportance.High, $"CompatApi command: {genApiProcess.StartInfo.FileName} {genApiProcess.StartInfo.Arguments}");
genApiProcess.OutputDataReceived += dataReceived;
genApiProcess.ErrorDataReceived += dataReceived;

// Get api definition for previous Api
genApiProcess.Start ();
ValidateIssues (genApiProcess.StandardOutput, validateAgainstReference);
genApiProcess.WaitForExit ();
for (int i = 0; i < 3; i++) {
lines.Clear ();
processHasCrashed = false;

compatApiCommand = $"CompatApi command: {genApiProcess.StartInfo.FileName} {genApiProcess.StartInfo.Arguments}";
Log.LogMessage (MessageImportance.High, compatApiCommand);

genApiProcess.Start ();
genApiProcess.BeginOutputReadLine ();
genApiProcess.BeginErrorReadLine ();

genApiProcess.WaitForExit ();

genApiProcess.CancelOutputRead ();
genApiProcess.CancelErrorRead ();

if (lines.Count == 0) {
return;
}

if (processHasCrashed) {
if (i + 1 < 3) {
Log.LogWarning ($"Process has crashed.'{Environment.NewLine}Crash report:{Environment.NewLine}{String.Join (Environment.NewLine, lines)}");
Log.LogWarning ($"We will retry.");
continue;
} else {
LogError ($"Unable to get a valid report. Process has crashed.'{Environment.NewLine}Crash report:{Environment.NewLine}{String.Join (Environment.NewLine, lines)}");
return;
}
}

ValidateIssues (lines, validateAgainstReference);
break;
}
}
} finally {
if (Directory.Exists (contractPathDirectory)) {
Expand All @@ -174,7 +223,7 @@ void ValidateApiCompat (string contractPath, bool validateAgainstReference)
}

// Validates there is no issue or issues found are acceptable
void ValidateIssues (StreamReader content, bool validateAgainstReference)
void ValidateIssues (IEnumerable<string> content, bool validateAgainstReference)
{
// Load issues into a dictionary
var issuesFound = LoadIssues (content);
Expand All @@ -195,31 +244,28 @@ void ValidateIssues (StreamReader content, bool validateAgainstReference)
} else {

// Read and Convert the acceptable issues into a dictionary
using (var streamReader = new StreamReader (acceptableIssuesFile)) {
acceptableIssues = LoadIssues (streamReader);
if (Log.HasLoggedErrors) {
return;
}
var lines = File.ReadAllLines (acceptableIssuesFile);
acceptableIssues = LoadIssues (lines);
if (Log.HasLoggedErrors) {
return;
}
}

// Now remove all acceptable issues form the dictionary of issues found.
var count = 0;
var errors = new List<string> ();
if (acceptableIssues != null) {
foreach (var item in acceptableIssues) {
if (!issuesFound.TryGetValue (item.Key, out HashSet<string> issues)) {
// we should always be able to find the assembly that is reporting the issues
Log.LogMessage (MessageImportance.High, $"There is an invalid assembly listed on the acceptable breakages file: {item.Key}");
count++;
errors.Add ($"There is an invalid assembly listed on the acceptable breakages file: {item.Key}");
continue;
}

foreach (var issue in item.Value) {
// we should always be able to remove the issue, if we try to remove an issue that does not exist,
// it means the acceptable list is incorrect and should be reported.
if (!issues.Remove (issue)) {
Log.LogMessage (MessageImportance.High, $"There is an invalid issue listed on the acceptable breakages file: {issue}");
count++;
errors.Add ($"There is an invalid issue listed on the acceptable breakages file: {issue}");
}
}
}
Expand All @@ -231,34 +277,30 @@ void ValidateIssues (StreamReader content, bool validateAgainstReference)
continue;
}

Log.LogMessage (MessageImportance.High, item.Key);
errors.Add (item.Key);
foreach (var issue in item.Value) {
Log.LogMessage (MessageImportance.High, issue);
count++;
errors.Add (issue);
}
}

if (count > 0) {
Log.LogMessage (MessageImportance.High, $"Total Issues: {count}");
Log.LogError ($"CheckApiCompatibility found nonacceptable Api breakages for ApiLevel: {ApiLevel}.");
if (errors.Count > 0) {
errors.Add ($"Total Issues: {errors.Count}");
LogError ($"CheckApiCompatibility found nonacceptable Api breakages for ApiLevel: {ApiLevel}.{Environment.NewLine}{String.Join (Environment.NewLine, errors)}");
}
}

// Converts list of issue into a dictionary
Dictionary<string, HashSet<string>> LoadIssues (StreamReader content)
Dictionary<string, HashSet<string>> LoadIssues (IEnumerable<string> content)
{
var issues = new Dictionary<string, HashSet<string>> ();
HashSet<string> currentSet = null;

while (!content.EndOfStream) {
var line = content.ReadLine ();
foreach (var line in content) {

if (string.IsNullOrWhiteSpace (line) || line.StartsWith ("#")) {
continue;
}

line = line.Trim ();

// Create hashset per assembly
if (line.StartsWith ("Compat issues with assembly", StringComparison.InvariantCultureIgnoreCase)) {
currentSet = new HashSet<string> ();
Expand All @@ -273,7 +315,9 @@ Dictionary<string, HashSet<string>> LoadIssues (StreamReader content)

if (currentSet == null) {
// Hashset should never be null, unless exception file is not defining assembly line.
Log.LogError ($"Exception report/file should start with: 'Compat issues with assembly'; was: '{line}'");
// Finish reading stream
var reportContent = Environment.NewLine + "Current content:" + Environment.NewLine + String.Join (Environment.NewLine, content);
LogError ($"Exception report/file should start with: 'Compat issues with assembly ...'{reportContent}");
return null;
}

Expand All @@ -283,5 +327,14 @@ Dictionary<string, HashSet<string>> LoadIssues (StreamReader content)

return issues;
}

void LogError(string errorMessage)
{
if (!string.IsNullOrWhiteSpace (compatApiCommand)) {
Log.LogError ($"{compatApiCommand}{Environment.NewLine}{errorMessage}");
} else {
Log.LogError (errorMessage);
}
}
}
}