dotnet / maintenance-packages

Repository that hosts packages from the .NET platform whose original home/branch is not building any longer.
MIT License
22 stars 10 forks source link

Fix building System.Data.SqlClient #115

Closed cheenamalhotra closed 4 weeks ago

cheenamalhotra commented 2 months ago

Reworking #84 over current 'main' after #111 was merged. Fixes https://github.com/dotnet/maintenance-packages/issues/83

Todo:

carlossanlop commented 1 month ago

Running the tests on Windows with dotnet test Functional/System.Data.SqlClient.Tests.csproj is showing me that both the -windows and the -unix tests are getting executed.

There was one failure in the -windows execution on Windows, it's a Uap tests that maybe we can just delete:

[xUnit.net 00:00:13.09]     System.Data.SqlClient.Tests.ExceptionTest.LocalDBNotSupportedOnUapTest [FAIL]
[xUnit.net 00:00:13.09]       Assert.Throws() Failure
[xUnit.net 00:00:13.09]       Expected: typeof(System.PlatformNotSupportedException)
[xUnit.net 00:00:13.09]       Actual:   (No exception was thrown)
[xUnit.net 00:00:13.09]       Stack Trace:
[xUnit.net 00:00:13.09]         C:\Users\calope\source\repos\maintenance-packages\src\System.Data.SqlClient\tests\FunctionalTests\ExceptionTest.cs(151,0): at System.Data.SqlClient.Tests.ExceptionTest.LocalDBNotSupportedOnUapTest()
  Failed System.Data.SqlClient.Tests.ExceptionTest.LocalDBNotSupportedOnUapTest [1 s]
  Error Message:
   Assert.Throws() Failure
Expected: typeof(System.PlatformNotSupportedException)
Actual:   (No exception was thrown)
  Stack Trace:
     at System.Data.SqlClient.Tests.ExceptionTest.LocalDBNotSupportedOnUapTest() in C:\Users\calope\source\repos\maintenance-packages\src\System.Data.SqlClient\tests\FunctionalTests\ExceptionTest.cs:line 151
carlossanlop commented 1 month ago

There are some API compat failures that got uncovered:

/__w/1/s/.packages/microsoft.dotnet.apicompat.task/9.0.100-preview.2.24102.10/build/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error : API compatibility errors between 'lib/net6.0/System.Data.SqlClient.dll' (left) and 'runtimes/unix/lib/net6.0/System.Data.SqlClient.dll' (right): [/__w/1/s/src/System.Data.SqlClient/src/System.Data.SqlClient.csproj]
##[error].packages/microsoft.dotnet.apicompat.task/9.0.100-preview.2.24102.10/build/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error : (NETCORE_ENGINEERING_TELEMETRY=Build) API compatibility errors between 'lib/net6.0/System.Data.SqlClient.dll' (left) and 'runtimes/unix/lib/net6.0/System.Data.SqlClient.dll' (right):
/__w/1/s/.packages/microsoft.dotnet.apicompat.task/9.0.100-preview.2.24102.10/build/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error CP0001: Type 'System.Data.SqlTypes.SqlFileStream' exists on runtimes/unix/lib/net6.0/System.Data.SqlClient.dll but not on lib/net6.0/System.Data.SqlClient.dll [/__w/1/s/src/System.Data.SqlClient/src/System.Data.SqlClient.csproj]
##[error].packages/microsoft.dotnet.apicompat.task/9.0.100-preview.2.24102.10/build/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error CP0001: (NETCORE_ENGINEERING_TELEMETRY=Build) Type 'System.Data.SqlTypes.SqlFileStream' exists on runtimes/unix/lib/net6.0/System.Data.SqlClient.dll but not on lib/net6.0/System.Data.SqlClient.dll
/__w/1/s/.packages/microsoft.dotnet.apicompat.task/9.0.100-preview.2.24102.10/build/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error : API compatibility errors between 'lib/net6.0/System.Data.SqlClient.dll' (left) and 'runtimes/win/lib/net6.0/System.Data.SqlClient.dll' (right): [/__w/1/s/src/System.Data.SqlClient/src/System.Data.SqlClient.csproj]
##[error].packages/microsoft.dotnet.apicompat.task/9.0.100-preview.2.24102.10/build/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error : (NETCORE_ENGINEERING_TELEMETRY=Build) API compatibility errors between 'lib/net6.0/System.Data.SqlClient.dll' (left) and 'runtimes/win/lib/net6.0/System.Data.SqlClient.dll' (right):
/__w/1/s/.packages/microsoft.dotnet.apicompat.task/9.0.100-preview.2.24102.10/build/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error CP0001: Type 'System.Data.SqlTypes.SqlFileStream' exists on runtimes/win/lib/net6.0/System.Data.SqlClient.dll but not on lib/net6.0/System.Data.SqlClient.dll [/__w/1/s/src/System.Data.SqlClient/src/System.Data.SqlClient.csproj]
##[error].packages/microsoft.dotnet.apicompat.task/9.0.100-preview.2.24102.10/build/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error CP0001: (NETCORE_ENGINEERING_TELEMETRY=Build) Type 'System.Data.SqlTypes.SqlFileStream' exists on runtimes/win/lib/net6.0/System.Data.SqlClient.dll but not on lib/net6.0/System.Data.SqlClient.dll
/__w/1/s/.packages/microsoft.dotnet.apicompat.task/9.0.100-preview.2.24102.10/build/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error : API breaking changes found. If those are intentional, the APICompat suppression file can be updated by rebuilding with '/p:ApiCompatGenerateSuppressionFile=true' [/__w/1/s/src/System.Data.SqlClient/src/System.Data.SqlClient.csproj]
##[error].packages/microsoft.dotnet.apicompat.task/9.0.100-preview.2.24102.10/build/Microsoft.NET.ApiCompat.ValidatePackage.targets(39,5): error : (NETCORE_ENGINEERING_TELEMETRY=Build) API breaking changes found. If those are intentional, the APICompat suppression file can be updated by rebuilding with '/p:ApiCompatGenerateSuppressionFile=true'

Edit: These are still showing up even after fixing the target platform vs OS discrepancy.

carlossanlop commented 1 month ago

There are some API compat failures that got uncovered:

@cheenamalhotra I fixed the ApiCompat failures. When packing, we were generating the SqlFileStream type in the runtimes/win and runtimes/unix folders of the nupkg, but we were not generating the platform-agnostic one in the lib/ folder. So I changed the build condition for the Unsupported file to instead of building for unix only, to build for everything except windows.

The build passed locally, regular and with packing. Hopefully it passes in the CI too.

carlossanlop commented 1 month ago

It worked! Now all that's left is fixing the test failures.

ViktorHofer commented 1 month ago

@cheenamalhotra I pushed a few commits to clean things up that would have been hard to discuss in PR reviews. Hope that works for you.

ViktorHofer commented 4 weeks ago

@cheenamalhotra please merge when this is ready from your side

carlossanlop commented 4 weeks ago

Yay! 🎉