From b667af56b862758ad9eeb8e39bf305db6b044f66 Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Thu, 19 Sep 2024 10:02:45 +0100 Subject: [PATCH] Fix R8 Message processing to remove false positives. (#9298) 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. --- src/Xamarin.Android.Build.Tasks/Tasks/D8.cs | 8 +++++++ .../Tasks/JavaToolTask.cs | 11 ++++++--- src/Xamarin.Android.Build.Tasks/Tasks/R8.cs | 8 +++++++ .../PackagingTest.cs | 23 +++++++++++++++++++ .../Android/KnownPackages.cs | 4 ++++ 5 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/D8.cs b/src/Xamarin.Android.Build.Tasks/Tasks/D8.cs index 4a77f4fd3b5..e2e5b9349b1 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/D8.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/D8.cs @@ -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); + } } } diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/JavaToolTask.cs b/src/Xamarin.Android.Build.Tasks/Tasks/JavaToolTask.cs index 00ade54e622..92af6652ff9 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/JavaToolTask.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/JavaToolTask.cs @@ -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; } @@ -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); + } } } diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/R8.cs b/src/Xamarin.Android.Build.Tasks/Tasks/R8.cs index d9cafeb4e3d..47ea98030a2 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/R8.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/R8.cs @@ -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); + } } } diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs index 72c37fd533f..170261f1a59 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs @@ -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) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Android/KnownPackages.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Android/KnownPackages.cs index d77f5607658..4cbecb22635 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Android/KnownPackages.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Android/KnownPackages.cs @@ -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",