Package testing skips Microsoft.Windows.Compatibility

[ericstj]

This is done because the package report doesn't list any supported frameworks.

That's because package reports only consider files and not dependencies. We should update the package reports to also consider dependencies (in the absence of files) to mirror the NuGet algo.

[ericstj]

As a result of this bug we missed this problem:

error NU1603: Microsoft.Windows.Compatibility 2.1.0-preview1-26928-03 depends on System.Data.SqlClient (>= 4.6.0-preview1-26928-03) but System.Data.SqlClient 4.6.0-preview1-26928-03 was not found. An approximate best match of System.Data.SqlClient 4.6.0-preview2-26905-02 was resolved.

Related: https://github.com/dotnet/corefx/issues/32391

[safern]

From the date of the package version it seems it is depending on a live version of the package. Was SqlClient package deleted or something?

[ericstj]

It was bumped to 4.7.0

[ericstj]

Let's see what we can do to make sure this gets tested. @joperezr we also may be missing updating the version of this in our index when a new one ships in servicing. I wonder if we need to expand the logic for index updates to account for dependencies in addition to harvesting.

[joperezr]

We could do that. If we make it so that every package (not only the ones that have dlls on them) has an entry on the package index, then we can call our task to update all stable versions listed in NuGet for all of the package index so that would cover these cases. Alternatively, we could also do what you suggest and expand the logic so that it also does it for dependencies.

[ViktorHofer]

Triage: keep this in 5.0 and at least manually test the meta-package.

[safern]

Getting to this today or tomorrow.

[safern]

I just did this validation locally. Published it for different RIDs and ran VerifyClosure in it and everything looks good.

Also looked at the set of dependencies and everything looks reasonable. So I'm going to remove blocking-release and move to 6.0.0 to complete the infrastructure work.

[ViktorHofer]

@Anipik @ericstj do you think this will "just work" when migrating the Compatibility pkgproj to a csproj (and use ProjectReferences)?

[ericstj]

Conversion to CSProj won't help. The Csproj tooling is meant to test the contents of the package for API compat. The main thing we want to test for this is the validity of the package. For that it should leverage the package-testing we do after pack where we generate projects and restore them. That could be made to work today, but it needs a change here to also consider dependency groups in the nuspec: https://github.com/dotnet/arcade/blob/28a6403ee97077256fcdc60f599f0ad9e38e3cfa/src/Microsoft.DotNet.PackageTesting/GetCompatiblePackageTargetFrameworks.cs#L37

[ViktorHofer]

But the migration to csprojs would have caught other issues like just the recent one where the Compatibility pack tried to restore a dead-ended package, right?

[ericstj]

It may, but we should still test it in package testing. That’s what this issue is meant to track.

[ViktorHofer]

Sure, absolutely. @safern as you are assigned, are you currently working on this or have a branch somewhere that we can pick up?

[safern]

Nope, I never got a chance to look into this. And definitely with the API Compat work, don't have bandwidth to pick it up. I will remove my assignment.