Fix R8 Message processing to remove false positives. #9298
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dellis1972
force-pushed
the
r8errorhandling
branch
from
688677d to
206a782
Compare
dellis1972
changed the title
|
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. |
jonathanpeppers
pushed a commit
that referenced
this pull request
Sep 23, 2024
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.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context #7008 (comment)
When building with certain libraries we are seeing the following errors
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 callingbase.LogEventsFromTextOutput (singleLine, messageImportance);.So lets add a new meethod
CheckForErrorwhich does the actual check. By defaultLogEventsFromTextOutputwill call this andbase.LogEventsFromTextOutput. However toR8andD8we overrideLogEventsFromTextOutputand only callCheckForErrorandLog.LogMessage. This fixing this issue.Add a unit test which covers the error case.