Remove unnecessary suppressions in APICompat files#87094
Conversation
- Remove unnecessary suppressions in APICompat files. This is in preparation for dotnet/sdk#32964 which will validate the existing suppressions going forward. - Set the required APICompat properties for the future tooling support.
|
Tagging subscribers to this area: @dotnet/area-infrastructure-libraries Issue DetailsRelated dotnet/sdk#32964
|
ViktorHofer
requested review from
MichalStrehovsky and
marek-safar
as code owners
| <ApiCompatPreserveUnnecessarySuppressions>true</ApiCompatPreserveUnnecessarySuppressions> | ||
| <ApiCompatPermitUnnecessarySuppressions>true</ApiCompatPermitUnnecessarySuppressions> |
There was a problem hiding this comment.
These two properties sound very similar. Why two?
There was a problem hiding this comment.
APICompat is used in dotnet/runtime but also in various other repositories. these two new settings are on by default but need to be disabled at least in runtime for a few projects (mentioned that in the code comment in resolveContract.targets). The feature flags are distinct as the latter allows unnecessary suppressions to exist in the suppression files and the former trims out unnecessary suppressions when re-generating a suppression file via the /p:GenerateAPICompatSuppressionFile=true switch.
|
|
||
| <PropertyGroup> | ||
| <IsRuntimeAndReferenceAssembly Condition="'$(IsRuntimeAndReferenceAssembly)' == '' and | ||
| '$(HasMatchingContract)' != 'true' and |
There was a problem hiding this comment.
For my own education: What is a "matching contract"?
There was a problem hiding this comment.
A matching contract is a reference source project assembly in the context of a source project, i.e. ref/net8.0/System.Text.Json.dll is the matching contract assembly for lib/net8.0/System.Text.Json.dll. Matching refers to the compatible TFM.
| <PropertyGroup> | ||
| <IsRuntimeAndReferenceAssembly Condition="'$(IsRuntimeAndReferenceAssembly)' == '' and | ||
| '$(HasMatchingContract)' != 'true' and | ||
| '$(IsPrivateAssembly)' != 'true'">true</IsRuntimeAndReferenceAssembly> |
There was a problem hiding this comment.
Can you give me an example of a private assembly?
| <PropertyGroup> | ||
| <BinPlaceRef Condition="'$(BinPlaceRef)' == '' and ('$(IsReferenceAssemblyProject)' == 'true' or '$(IsRuntimeAndReferenceAssembly)' == 'true')">true</BinPlaceRef> | ||
| <BinPlaceRuntime Condition="'$(BinPlaceRuntime)' == '' and ('$(IsSourceProject)' == 'true' or '$(IsRuntimeAndReferenceAssembly)' == 'true')">true</BinPlaceRuntime> | ||
| <BinPlaceRuntime Condition="'$(BinPlaceRuntime)' == '' and '$(IsSourceProject)' == 'true'">true</BinPlaceRuntime> |
There was a problem hiding this comment.
I see that you moved IsRuntimeAndReferenceAssembly to resolveContract.targets` and changed it a bit. Why did you move it there?
Can you please explain why you removed the condition from IsNETCoreAppSrc and BinPlaceRuntime?
There was a problem hiding this comment.
Because IsruntimeAndReferenceAssembly is used in a condition in resolveContract.targets and resolveContract.targets is imported before src/libraries/Directory.Build.targets.
| <Target>T:System.Runtime.CompilerServices.AsyncMethodBuilderAttribute:[T:System.AttributeUsageAttribute]</Target> | ||
| <Left>netstandard2.1/netstandard.dll</Left> | ||
| <Right>net8.0/netstandard.dll</Right> | ||
| </Suppression> |
There was a problem hiding this comment.
Can you please explain why all these suppressions got removed if both ApiCompatPreserveUnnecessarySuppressions and ApiCompatPermitUnnecessarySuppressions are set to true in resolveContract.targets?
There was a problem hiding this comment.
These two properties are conditionally set to true for IsSourceProject projects (which apicompat.proj isn't):
Line 403 in a9f3262
In general, we want to disable these new features for packable projects that also have a matching reference source project. I tried to capture that in the added comment in resolveContract.targets.
ghost
locked as resolved and limited conversation to collaborators
2 participants
Related dotnet/sdk#32964