-
Notifications
You must be signed in to change notification settings - Fork 557
Remove ApplyPreserveAttribute, warn when PreserveAttribute is referenced in assembly #10449
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.
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
base: main
Are you sure you want to change the base?
Conversation
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
<_TrimmerCustomSteps | ||
Include="$(_AndroidLinkerCustomStepAssembly)" | ||
BeforeStep="MarkStep" | ||
Type="Microsoft.Android.Sdk.ILLink.WarnOnPreserveAttribute" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, maybe you forgot to commit 1 file.
If this is for .NET 11, I think we could also put error:true
here:
[Obsolete ("Please use [System.Diagnostics.CodeAnalysis.DynamicDependencyAttribute]")] |
It has been marked [Obsolete]
a long time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, forgot to re-add the file after renaming it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like adding the error=true causes and API compat issue and we'll need to add [Obsolete] to the definition in Java.Interop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the one in java-interop is just for tests:
Or did you find it when viewing the assembly in ILSpy?
c3fa811
to
1056379
Compare
/azp run |
Commenter does not have sufficient privileges for PR 10449 in repo dotnet/android |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
We can look how to fix this later:
I think there is just a text file to update, so it can see the attribute changed. |
/azp run |
Commenter does not have sufficient privileges for PR 10449 in repo dotnet/android |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to hold this until we've branched and main
is .NET 11.
I'll come back to this and merge then. 👍
PreserveAttribute has been obsolete for some time, so the functionality of the trimmer step which marks the types based on the can be removed. Instead, we can warn if we see any references to the type within an assembly.
Context: dotnet/runtime#107211