-
Notifications
You must be signed in to change notification settings - Fork 64
[release/8.0] Update Windows.Compatibility external packages. #4899
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
Conversation
waiting to pick up the updated version from dotnet/runtime#112140 |
eng/Versions.props
Outdated
@@ -63,7 +63,7 @@ | |||
<SystemThreadingAccessControlVersion>8.0.0</SystemThreadingAccessControlVersion> | |||
<VSRedistCommonNetCoreSharedFrameworkx6480PackageVersion>8.0.12-servicing.24603.5</VSRedistCommonNetCoreSharedFrameworkx6480PackageVersion> | |||
<!-- wcf --> | |||
<SystemServiceModelVersion>4.10.0</SystemServiceModelVersion> | |||
<SystemServiceModelVersion>8.1.1</SystemServiceModelVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a major version change in a servicing release. Are we really OK with this @mconnew @HongGit?
The SQL and EventLog changes are minor which shouldn't carry breaking changes, but the WCF bump here could be problematic.
Is there any chance there is a minor or servicing revision of WCF that fixes the symbols issue but doesn't carry with it the larger delta? Same goes for 9.0 - we'll follow whatever we do there. The major version bump is fine for 10.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code hasn't changed a lot between 4.10.0 and 8.1.1. Our compatibility bar is so high, we still try to get 100% identical behavior to .NET Framework whenever possible, nevermind between releases of our clients.
What exactly is the symbols issue you're facing that you're trying to fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Between 4.10.0 and 8.1.1, we did refactoring to put the implementation into their own assemblies (were in System.Private.ServiceModel). We added the new package NetFramingBase that NetTcp now depends. That refactored out the common implementation between NetNamedPipe and NetTcp so they could share the core logic. It would come in transitively by adding NetTcp so I don't know if you need to explicitly add it. We kept the public api the same, added a little extra public api which isn't in .NET Framework to enable this, and everything works the same as it did before.
Other than refactoring, the differences have been adding more features from .NET Framework, and exposing a few more api's that we had implemented but not declared in the contract. We haven't had anyone open any issues that were caused by code changes between 4.10 and 8.1. It's all been around netstandard2.0 support, or existing issues that have existed since .NET Framework, or long standing behaviors in the .NET implementation which didn't change between versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly is the symbols issue you're facing that you're trying to fix?
Symbols for older builds are absent from msdl and symweb. Folks using these packages need symbols to both debug and to feed them into compliance tools that do static analysis. I double checked that the 4.10.3 packages are also missing symbols, so it would seem we don't have a different option that fixes this.
Do you think the compat risk of upgrading the dependency from 4.10.0 to 8.1.1 in an LTS release is so low that you'd prioritize getting folks the packages with symbols over it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 2 differences. The first is there's a bug where consuming these packages in a netstandard2.0 library then doesn't work from a NetFx app because we are missing reference assembly type forwards. I plan to fix that this week and we'll release an 8.1.2 version. The second issue is we don't have type forwards for the System.ServiceModel.Syndication library in any direction. Once 8.1.2 is out, if we're okay with not having Syndication support, then I wouldn't be concerned about any compat issues going to 8.1.2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those sound like issues with 8.1.1 - which I guess are a subset of the problems moving from 4.10.0 to 8.1.1. Another way to put this - do you, the WCF team, want the 8.0 build of Microsoft.Windows.Compatibility
referencing the 8.1.x
packages? Do those match the promise of LTS support?
We should raise the risk level on this - it's updating the major version of a dependency - something we typically do not do in servicing.
@Tanya-Solyanik you don't need to wait on anything. That version should flow automatically, just like other runtime packages. |
Related to dotnet/runtime#4884
Description:
An internal partner reported that Microsoft.Windows.Compatibility NuGet v8.0.12 package carries 3 assemblies(System.Data.SqlClient.dll, System.Diagnostics.EventLog.Messages.dll, and System.ServiceModel.dll) that do not have symbols on the Microsoft symbol server. The issue is not blocking for the partner, but we want to take this opportunity to update these packages to the newer, supported versions of external packages.
Fix:
System.Data.SqlClient.dll – updated to a newer compatibility package that has symbols
System.ServiceModel.dll - updated to a newer compatible package that is in support because it ships with the PowerShell. This package does not have symbols. The only WCF package that has symbols is V8 and introduces breaking changes.
System.Diagnostics.EventLog.Messages.dll – pdbs had been already added [release/9.0-staging] Include PDB for all TfmRuntimeSpecificPackageFile by github-actions[bot] · Pull Request #112139 · dotnet/runtime
Followup bugs
Symbol package questions · Issue dotnet/runtime#15457 · dotnet/arcade
What is the expected workflow for Symbols Validation of official releases? · Issue dotnet/arcade#15537 · dotnet/runtime
Customer Impact:
Windows partner has to store pdbs locally to be accessible for the duration of windows support term, which is longer that the .NET8’s. To achieve this goal, they are down loading pdbs from the symbol server, their script breaks on our package. They can’t upgrade to NET10, but can reverence the newer versions of these packages directly in their project.
Testing:
built the windows compatibility pack and verified that the right package versions are referenced.