-
Notifications
You must be signed in to change notification settings - Fork 58
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
[release/8.0] Update Windows.Compatibility external packages. #4899
base: release/8.0
Are you sure you want to change the base?
Conversation
* Update Windows.Compatibility external packages ServiceModel is updated to 8.1.1, in this version System.ServiceModel.Security and System.ServiceModel.Duplex no longer exist, their types (type forwards) are in the primitives package. Microsoft.Data.SqlClient NuGet package to 4.9.0 --------- Co-authored-by: Tanya Solyanik <[email protected]>
waiting to pick up the updated version from dotnet/runtime#112140 |
@@ -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:
Microsoft.Windows.Compatibility NuGet v8.0.12 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.
Fix:
System.Data.SqlClient.dll and System.ServiceModel.dll – updated to a newer compatibility packages that have symbols
System.Diagnostics.EventLog.Messages.dll – pdbs were added to the package in the above mentioned PR, and will flow to Microsoft.Windows.Compatibility as a servicing update
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:
Partner can’t debug their tests. They can’t upgrade. 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.
Testing:
built the windows compatibility pack and verified that the right package versions are referenced.
Verified that the new partner packages have pdbs
Risk:
Low