Change AKV Provider to target .NET Standard 2.0#4016
Change AKV Provider to target .NET Standard 2.0#4016paulmedynski wants to merge 6 commits intomainfrom
Conversation
…d up NuGet packing.
| <DocumentationFile>$(BinFolder)$(Configuration).$(Platform)\$(AssemblyName).xml</DocumentationFile> | ||
| </PropertyGroup> | ||
|
|
||
| <!-- Trim/AOT Support ================================================ --> |
There was a problem hiding this comment.
.NET Standard 2.0 isn't trimmable or AOT compatible.
| <IsTrimmable>true</IsTrimmable> | ||
| <IsAotCompatible>true</IsAotCompatible> | ||
| <ItemGroup> | ||
| <EmbeddedFiles Include="$(GeneratedAssemblyInfoFile)"/> |
There was a problem hiding this comment.
Moved here from the deleted Directory.Build.props.
|
|
||
| See: https://learn.microsoft.com/en-us/nuget/reference/msbuild-targets#pack-target | ||
| --> | ||
| <PropertyGroup> |
There was a problem hiding this comment.
Absorbed from the nuspec, thanks to Copilot.
| <!-- MDS Reference --> | ||
| <ItemGroup> | ||
| <ProjectReference Include="$(RepoRoot)src\Microsoft.Data.SqlClient\src\Microsoft.Data.SqlClient.csproj" | ||
| <ProjectReference Include="$(RepoRoot)/src/Microsoft.Data.SqlClient/ref/Microsoft.Data.SqlClient.csproj" |
There was a problem hiding this comment.
The SqlClient project doesn't target .NET Standard 2.0, but it's ref project does. We don't need a real implementation to compile against.
| <readme>README.md</readme> | ||
| <repository type="git" url="https://github.com/dotnet/sqlclient" commit="$COMMITID$" /> | ||
| <description>The current data provider for SQL Server and Azure SQL databases. This has replaced System.Data.SqlClient. These classes provide access to SQL and encapsulate database-specific protocols, including tabular data stream (TDS). | ||
| <description> |
There was a problem hiding this comment.
Formatting cleanup.
There was a problem hiding this comment.
Pull request overview
This PR updates the Always Encrypted Azure Key Vault (AKV) Provider to target .NET Standard 2.0 and modernizes its packaging flow by moving from a standalone .nuspec/custom MSBuild target to SDK-style dotnet pack driven by the AKV Provider .csproj. It also updates build/pipeline wiring to use the new BuildAkvProvider/PackAkvProvider targets.
Changes:
- Removed the legacy AKV
.nuspecandGenerateAkvPackageMSBuild target infrastructure. - Updated the AKV Provider project to
netstandard2.0and embedded NuGet metadata into the.csproj. - Updated build and pipeline steps to build/pack the AKV Provider directly via
dotnet packand newbuild.projtargets.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/targets/add-ons/GenerateAkvPackage.targets | Removes the legacy GenerateAkvPackage target. |
| tools/specs/add-ons/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.nuspec | Deletes the old AKV Provider .nuspec (packaging now via .csproj). |
| tools/specs/Microsoft.Data.SqlClient.nuspec | Adds a header comment documenting required nuget pack properties (contains a small typo). |
| src/Microsoft.Data.SqlClient/add-ons/Directory.Build.props | Removes add-ons-specific props file (AKV now uses standard props chain). |
| src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.csproj | Sets netstandard2.0 and absorbs NuGet package metadata/config into the project. |
| eng/pipelines/onebranch/variables/common-variables.yml | Removes akvNuspecPath variable (no longer applicable). |
| eng/pipelines/onebranch/steps/roslyn-analyzers-akv-step.yml | Updates build target from BuildAkv to BuildAkvProvider. |
| eng/pipelines/onebranch/steps/compound-build-akv-step.yml | Updates build target naming/usage to BuildAkvProvider. |
| eng/pipelines/onebranch/jobs/build-akv-official-job.yml | Switches AKV packaging step from nuspec-based to dotnet pack on the .csproj. |
| eng/pipelines/common/templates/steps/ci-project-build-step.yml | Replaces old AKV build targets with BuildAkvProvider (but introduces a Windows gating issue). |
| eng/pipelines/common/templates/jobs/ci-build-nugets-job.yml | Switches AKV packaging from nuspec-based step to dotnet pack on the .csproj. |
| build.proj | Removes GenerateAkvPackage.targets import; adds BuildAkvProvider/PackAkvProvider; updates BuildSqlClient deps. |
| BUILDGUIDE.md | Documents the new BuildAkvProvider/PackAkvProvider targets and removes obsolete AKV targets. |
Comments suppressed due to low confidence (1)
src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.csproj:78
- Switching AKV Provider packaging from a nuspec to SDK-style
dotnet packmeans the project’s PackageReferences/ProjectReferences now flow into the produced NuGet dependency graph. This project currently referencesMicrosoft.Data.SqlClient.Extensions.Logging, which the removed nuspec did not list, so this change will add/alter external package dependencies for consumers. If the AKV provider doesn’t actually use Logging APIs, remove that reference or mark it asPrivateAssets=allto avoid changing the published dependency set.
<!-- References ====================================================== -->
<!-- MDS Reference -->
<ItemGroup>
<ProjectReference Include="$(RepoRoot)/src/Microsoft.Data.SqlClient/ref/Microsoft.Data.SqlClient.csproj"
Condition="'$(ReferenceType)' != 'Package'" />
<PackageReference Include="Microsoft.Data.SqlClient"
Condition="'$(ReferenceType)' == 'Package'" />
</ItemGroup>
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
You can also share your feedback on Copilot code review. Take the survey.
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #4016 +/- ##
==========================================
- Coverage 72.36% 64.48% -7.88%
==========================================
Files 287 282 -5
Lines 43149 66073 +22924
==========================================
+ Hits 31223 42610 +11387
- Misses 11926 23463 +11537
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…y to inhibit NuGet.exe download when generating SqlClient package.
|
|
||
| <!-- We will download the latest NuGet executable unless otherwise inhibited. --> | ||
| <PropertyGroup> | ||
| <InstallNuget Condition="'$(InstallNuget)' != 'false'">true</InstallNuget> |
There was a problem hiding this comment.
The Docs-Build-Pack-Publish pipeline isn't permitted to download things, so it will set this to false.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
You can also share your feedback on Copilot code review. Take the survey.
Description
This PR changes the AKV Provider to target .NET Standard 2.0 since it doesn't contain any code specific to .NET or .NET Framework. Changes include:
<TargetFramework>tonetstandard2.0in the AKV Provider project.add-ons/Directory.Build.props.nuspecinto thecsproj.BuildAkvProviderandPackAkvProvidertargets tobuild.proj.GenerateAkvPackagetarget and all references to it.BuildAllPackagesandPackAllPackagestargets.BuildSqlClientandPackSqlClienttargets to help fix the Docs-Build-Pack-Publish pipeline.Testing