dotnet / android

.NET for Android provides open-source bindings of the Android SDK for use with .NET managed languages such as C#
MIT License
1.92k stars 526 forks source link

Fix R8 Message processing to remove false positives. #9298

Closed dellis1972 closed 1 day ago

dellis1972 commented 1 week ago

Context https://github.com/dotnet/android/issues/7008#issuecomment-2343312847

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.

alexander-efremov commented 2 days ago

hey @dellis1972

We have been waiting for the fix for a long time, looks now it will for fine, TYVM. Though I left a review comments if you don't mind.