Skip to content
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

Delete the deprecated APICompat tooling from Arcade #14328

Merged
merged 2 commits into from
Jan 4, 2024

Conversation

ViktorHofer
Copy link
Member

@ViktorHofer ViktorHofer commented Dec 28, 2023

APICompat got re-implemented on top of Microsoft.CodeAnalysis and now lives in dotnet/sdk as v2: https://github.com/dotnet/sdk/tree/main/src/Compatibility/ApiCompat. It also became a shipping product with .NET 6.

The source code was kept around for some time so that consumers could move to the new tooling. Now seems the right time to finally delete the code-base.

This tool was already excluded in source build so the deletion won't result in a build break. If there are still some repos that depend on the live version of APICompat v1, they will just not get a new dependency update for it. The code base has been frozen anyway so consumers shouldn't be impacted by this.

To double check:

APICompat got re-implemented on top of Microsoft.CodeAnalysis and lives
in dotnet/sdk. It also became a shipping product with .NET 6.

The source code was kept around for some time so that consumers could
move to the new tooling. Now seems the right time to finally delete the
code-base.

This tool was already excluded in source build so the deletion won't
result in a build break. If there are still some repos that depend on
the live version of APICompat v1, they will just not get a new
dependency update for it. The code base has been frozen anyway so
consumers shouldn't be impacted by this.
@@ -6,7 +6,7 @@
<PropertyGroup Condition="'$(GeneratePlatformNotSupportedAssemblyMessage)' != ''">
<CoreCompileDependsOn>AddGenFacadeNotSupportedCompileItem;$(CoreCompileDependsOn)</CoreCompileDependsOn>
<!-- Not supported sources are created from the ref assembly, we currently don't produce finalizers in dummy assemblies, so we disable ApiCompat to not fail. -->
<RunApiCompat>false</RunApiCompat>
<ApiCompatValidateAssemblies Condition="'$(IsCrossTargetingBuild)' != 'true'">false</ApiCompatValidateAssemblies >
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need the condition?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comes from here: https://github.com/dotnet/runtime/blob/baeb2983ca526d3bfcda091000ecd8e48dc3aa5d/eng/resolveContract.targets#L83-L88

APICompat runs in the outer build for multi-targeting projects and without that condition, validation for all inner builds would be skipped, not just for the one that produces a PNSE assembly.

@ViktorHofer ViktorHofer merged commit 2d9f237 into main Jan 4, 2024
11 checks passed
@ViktorHofer ViktorHofer deleted the DeleteApiCompatV1 branch January 4, 2024 16:22
weshaggard added a commit to Azure/azure-sdk-for-net that referenced this pull request Apr 11, 2024
Just updated the dead link to point to the PR that remove the tool dotnet/arcade#14328.

At some point we will want to move to the new tool which is tracked at #33433
weshaggard added a commit to Azure/azure-sdk-for-net that referenced this pull request Apr 12, 2024
Just updated the dead link to point to the PR that remove the tool dotnet/arcade#14328.

At some point we will want to move to the new tool which is tracked at #33433
lmolkova pushed a commit to lmolkova/azure-sdk-for-net that referenced this pull request Apr 12, 2024
Just updated the dead link to point to the PR that remove the tool dotnet/arcade#14328.

At some point we will want to move to the new tool which is tracked at Azure#33433
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants