Skip to content

Commit

Permalink
Fix R8 Message processing to remove false positives. (#9298)
Browse files Browse the repository at this point in the history
Context #7008 (comment)

When building with certain libraries we are seeing the following errors

4>  Invalid signature '(TT;)TV;' for method java.lang.Object kotlin.jvm.internal.PropertyReference1.get(java.lang.Object). (TaskId:792)
4>R8 : Validation error : A type variable is not in scope.
4>  Signature is ignored and will not be present in the output. (TaskId:792)
4>  Info in ...\.nuget\packages\xamarin.kotlin.stdlib\1.6.20.1\buildTransitive\monoandroid12.0\..\..\jar\org.jetbrains.kotlin.kotlin-stdlib-1.6.20.jar:kotlin/jvm/internal/PropertyReference0.class: (TaskId:792)
4>  Invalid signature '()TV;' for method java.lang.Object kotlin.jvm.internal.PropertyReference0.get(). (TaskId:792)
4>R8 : Validation error : A type variable is not in scope.
If you look carefully these are Info messages not errors. It turns out the call to base.LogEventsFromTextOutput (singleLine, messageImportance); is also including the default MSBuild error processing. This is mistaking this output for actual errors. So lets get around this by NOT calling base.LogEventsFromTextOutput (singleLine, messageImportance);.
So lets add a new meethod CheckForError which does the actual check. By default LogEventsFromTextOutput will call this and base.LogEventsFromTextOutput . However to R8 and D8 we override LogEventsFromTextOutput and only call CheckForError  and Log.LogMessage. This fixing this issue.

Add a unit test which covers the error case.
  • Loading branch information
dellis1972 authored Sep 19, 2024
1 parent 295f898 commit b667af5
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 3 deletions.
8 changes: 8 additions & 0 deletions src/Xamarin.Android.Build.Tasks/Tasks/D8.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,5 +124,13 @@ protected virtual CommandLineBuilder GetCommandLineBuilder ()

return cmd;
}

// Note: We do not want to call the base.LogEventsFromTextOutput as it will incorrectly identify
// Warnings and Info messages as errors.
protected override void LogEventsFromTextOutput (string singleLine, MessageImportance messageImportance)
{
CheckForError (singleLine);
Log.LogMessage (messageImportance, singleLine);
}
}
}
11 changes: 8 additions & 3 deletions src/Xamarin.Android.Build.Tasks/Tasks/JavaToolTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,10 @@ protected void AppendTextToErrorText (string text)
errorText.AppendLine (text);
}

protected override void LogEventsFromTextOutput (string singleLine, MessageImportance messageImportance)
protected virtual void CheckForError (string singleLine)
{
errorLines.Add (singleLine);
base.LogEventsFromTextOutput (singleLine, messageImportance); // not sure why/when we would skip this?


if (foundError) {
return;
}
Expand All @@ -229,6 +228,12 @@ protected override void LogEventsFromTextOutput (string singleLine, MessageImpor
}
foundError = foundError || match.Success || exceptionMatch.Success || customMatch;
}

protected override void LogEventsFromTextOutput (string singleLine, MessageImportance messageImportance)
{
CheckForError (singleLine);
base.LogEventsFromTextOutput (singleLine, messageImportance);
}
}
}

8 changes: 8 additions & 0 deletions src/Xamarin.Android.Build.Tasks/Tasks/R8.cs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,14 @@ protected override CommandLineBuilder GetCommandLineBuilder ()

return cmd;
}

// Note: We do not want to call the base.LogEventsFromTextOutput as it will incorrectly identify
// Warnings and Info messages as errors.
protected override void LogEventsFromTextOutput (string singleLine, MessageImportance messageImportance)
{
CheckForError (singleLine);
Log.LogMessage (messageImportance, singleLine);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,29 @@ public void CheckProguardMappingFileExists ()
}
}

[Test]
public void CheckR8InfoMessagesToNotBreakTheBuild ()
{
var proj = new XamarinAndroidApplicationProject {
IsRelease = true,
};
proj.SetProperty (proj.ReleaseProperties, KnownProperties.AndroidLinkTool, "r8");
proj.SetProperty (proj.ReleaseProperties, "AndroidCreateProguardMappingFile", true);
var packages = proj.PackageReferences;
packages.Add (KnownPackages.Xamarin_KotlinX_Coroutines_Android);
proj.OtherBuildItems.Add (new BuildItem ("ProguardConfiguration", "proguard.cfg") {
TextContent = () => @"-keepattributes Signature
-keep class kotlinx.coroutines.channels.** { *; }
"
});

using (var b = CreateApkBuilder ()) {
string mappingFile = Path.Combine (Root, b.ProjectDirectory, proj.OutputPath, "mapping.txt");
Assert.IsTrue (b.Build (proj), "build should have succeeded.");
FileAssert.Exists (mappingFile, $"'{mappingFile}' should have been generated.");
}
}

[Test]
[NonParallelizable] // Commonly fails NuGet restore
public void CheckIncludedAssemblies ([Values (false, true)] bool usesAssemblyStores)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ public static class KnownPackages
Id = "Xamarin.Kotlin.Reflect",
Version = "1.9.10.2"
};
public static Package Xamarin_KotlinX_Coroutines_Android = new Package {
Id = "Xamarin.KotlinX.Coroutines.Android",
Version = "1.8.1.1"
};
public static Package Acr_UserDialogs = new Package {
Id = "Acr.UserDialogs",
Version = "8.0.1",
Expand Down

0 comments on commit b667af5

Please sign in to comment.