Skip to content
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

6.0.0-rc.23205.4 nuget packages are broken #5091

Closed
pentp opened this issue Apr 10, 2023 · 2 comments · Fixed by #5111
Closed

6.0.0-rc.23205.4 nuget packages are broken #5091

pentp opened this issue Apr 10, 2023 · 2 comments · Fixed by #5111
Assignees
Labels

Comments

@pentp
Copy link

pentp commented Apr 10, 2023

  1. ref\net6.0\System.ServiceModel.Primitives.dll assembly doesn't match lib\net6.0\System.ServiceModel.Primitives.dll. For example DuplexChannelFactory<TChannel> has only 4 constructor overloads in the ref assembly. Is there an actual need for a ref assembly at all?
  2. The dll-s are somehow built without a TargetFramework attribute which confuses tooling (in some contexts they are interpreted as targeting .NET Core 3.1, in other as net48).
  3. System.ServiceModel.Primitives has useless legacy dependencies that should be removed: Microsoft.Bcl.AsyncInterfaces, System.Numerics.Vectors.
  4. Microsoft.Extensions.ObjectPool dependency version should be updated to at least net6.0.
  5. System.Security.Principal.Windows should probably not be a direct dependency for a supposedly net6.0 targeted (multi-platform) package. It should come through a net6.0-windows target probably?
@mconnew
Copy link
Member

mconnew commented Apr 12, 2023

  1. There's some history here. These packages started life as the implementation for universal Windows Store apps with .NET Native. The api surface exposed was inherited from the Windows 8 (non-native compiled) api surface which actually ran the full .NET Framework but limited which api's could be used. The way the code was originally ported to .NET Native was to bring pretty much everything over and only add to the reference assembly those things that were intended to be made available in the contract. This meant the implementation had lots of code which wasn't exposed. We're still cleaning up that mess many years later, either by removing the unwanted code, or exposing these types publicly. In the prior 4.x releases, the entire implementation lives in a private package called System.Private.ServiceModel and all the individual packages just type forwarded to this implementation. Starting with 6.0, we've pulled the implementation into their individual packages. There's still a lot of common code in System.ServiceModel.Primitives such as event logging and exception handling helper classes etc. Rather than duplicate this code everywhere, all the other packages reference the code in the Primitives implementation library. It's going to take time to get every last piece of unwanted code removed. We'll probably always have Primitives have a reference assembly. If there's an api which is implemented in Primitives but not in the reference assembly, please open an issue for it. The main blocker for most api's is just lack of unit tests for those api's.
  2. On .NET Framework I've only seen TargetFrameworkAttribute be relevant for the launching assembly but I had a look at other nuget libraries released as part of .NET and I see they have this attribute. I'll work out what needs to be done to get that added. I think it didn't matter before as we were targetting netstandard2.0 so there was really a target framework you can specify.
  3. These are a hangover from when we targeted netstandard2.0 and were missed in removing. Thanks for raising this issue, we'll do a review and remove anything not needed any more.
  4. Agreed. It doesn't harm anything as it will get upgraded automatically by any other library needing a newer version.
  5. I disagree. The library targets netstandard2.0. If we multi-targetted to net6.0-windows and net6.0, we would nearly double the size of the System.ServiceModel.Primitives package (currently 1.97MB), and System.ServiceModel.NetFramingBase (currently 304K) just for the sake of changing if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) {...} to an #if to conditionally compile. I've just dealt with the difficulty of working with having a net6.0-windows package as part of a build system for CoreWCF and it caused enough problems that I'm going to change the new NetNamedPipe client package to net6.0 and use the [SupportedOSPlatform("windows")] attribute to inform the build that it's a Windows only package and have constructors throw if those warnings are ignored and trying to run not on Windows.

@mconnew
Copy link
Member

mconnew commented Apr 20, 2023

I discovered System.Security.Principal.Windows was not needed on Primitives so if you are using HTTP only, it won't be pulled in any more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants