Build | Correct references to Microsoft.SqlServer.Server in Visual Studio builds

[edwardneal]

Description

This follows up my previous PR - I can't currently build via Visual Studio. Upon trying to do so, the net8.0, net9.0 and net10.0 targets for ManualTests fail with the error:

The type 'IBinarySerialize' is defined in an assembly that is not referenced. You must add a reference to assembly 'Microsoft.SqlServer.Server, Version=7.0.0.0, Culture=neutral, PublicKeyToken=null'.

The versioning for Microsoft.SqlServer.Server is also wrong because the package is versioned independently of SqlClient and its associated libraries, but this doesn't break the build - and more importantly, threading the package-specific versions into Microsoft.SqlServer.Server just changes the version in the error message:

The type 'IBinarySerialize' is defined in an assembly that is not referenced. You must add a reference to assembly 'Microsoft.SqlServer.Server, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'.

Converting netcore's unconditional PackageReference into the newer style of PackageReference/ProjectReference fixes this problem.

Just to neaten up the version numbers and dependencies, we then return to Microsoft.SqlServer.Server and bring the build configuration into the same style as the Abstractions project file, adding the various version-related properties and making sure that these are threaded into the nuspec.

Issues

None.

Testing

Visual Studio builds from scratch. build.proj continues to function properly, setting correct dependencies on Microsoft.SqlServer.Server. I can continue to execute UDT-based tests in ManualTests, so the updated DLL loads properly.

[paulmedynski]

I'll trigger the pipelines once you add the SqlServer version to the several place the YAML needs it.

[mdaigle]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.39%. Comparing base (14b96a2) to head (1d0c9bb).
⚠️ Report is 68 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (14b96a2) and HEAD (1d0c9bb). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (14b96a2)	HEAD (1d0c9bb)
CI-SqlClient	1	0

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #4003       +/-   ##
===========================================
- Coverage   74.62%   64.39%   -10.24%     
===========================================
  Files         280      271        -9     
  Lines       43814    65716    +21902     
===========================================
+ Hits        32698    42317     +9619     
- Misses      11116    23399    +12283     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	64.39% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[paulmedynski]

@edwardneal The pipeline errors appear to be caused by a missing property value on the nuget command line.

[edwardneal]

I think I see the problem, thanks @paulmedynski. Does the pipeline configuration need to define sqlServerArtifactsName and sqlServerPackageVersion variables? It looks like the root cause is that these aren't being substituted by the pipeline.

[mdaigle]

I believe those need to be defined in the variables yml file. Look for the other version variables, they should be defined in the same spot.

[edwardneal]

Thanks - I think I've found them, could you rerun CI please?

[mdaigle]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[edwardneal]

I think the remaining CI error is fixed with the most recent commit - we were never building the SqlServer package.

[paulmedynski]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[paulmedynski]

I'm going to move this out to post-7.0 GA. There are several internal pipelines that will also need to be tested and it may be easier for one of us (maintainers) do the work and iterate.

[edwardneal]

That makes sense. I've reverted the pipeline-based changes to avoid conflicts in the interim.

[github-actions]

This pull request has been marked as stale due to inactivity for more than 30 days.

If you would like to keep this pull request open, please provide an update or respond to any comments. Otherwise, it will be closed automatically in 7 days.

[paulmedynski]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).