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

List and Review OOB packages shipping replacements of Desktop assemblies #20505

Closed
karelz opened this issue Mar 7, 2017 · 12 comments
Closed
Labels
area-Meta enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@karelz
Copy link
Member

karelz commented Mar 7, 2017

We need to review all NuGet packages which replace Desktop .NET Framework gac'd assemblies - shipping such NuGet packages is HARD (see e.g. #18280), not tested and causes trouble with binding redirects (see #17770).

Known list of packages:

  1. System.Net.Http - Desktop OOB System.Net.Http - don't ship desktop implementation #20502
  2. System.IO.Compression - Get rid of System.IO.Compression OOB - .NET Standard 2.0 should forward to Desktop Framework #20074
  3. System.Runtime.Serialization.Primitives - Stop shipping Desktop OOB package for System.Runtime.Serialization.Primitives #20501

[3] is example where we extend Desktop - we need to enumerate all such cases in CoreFX and decide for 2.0.

@karelz karelz changed the title Review OOB packages shipping replacements of Desktop assemblies List and Review OOB packages shipping replacements of Desktop assemblies Mar 7, 2017
@karelz
Copy link
Member Author

karelz commented Mar 7, 2017

I need to find someone to do the review/audit of all CoreFX packages.

@tarekgh
Copy link
Member

tarekgh commented Mar 7, 2017

CC @weshaggard @ericstj

@karelz
Copy link
Member Author

karelz commented Mar 7, 2017

cc @danmosemsft

@mellinoe
Copy link
Contributor

mellinoe commented Mar 7, 2017

We also ship System.Numerics.Vectors.dll for .NET Framework, but the version in the .NET Framework is only a facade. So there is no "replacement" going on there, just adding the Vector/Vector<T> types on top of the forwarders.

@karelz
Copy link
Member Author

karelz commented Mar 7, 2017

@mellinoe thanks, the key thing is if we use the same assembly identity with higher assembly version - if that's not the case for System.Numerics.Vectors, then we are safe.

@tarekgh
Copy link
Member

tarekgh commented Apr 12, 2017

@ericstj @safern @AlexGhiondea does this issue related to the work you are doing now? I am trying to know if there is anything else need to be done here.

@tarekgh
Copy link
Member

tarekgh commented Apr 13, 2017

@ericstj @AlexGhiondea please advise if we this work already tracked by you or there is anything need to be done here.

@ericstj
Copy link
Member

ericstj commented Apr 13, 2017

It's not clear to me what this issue is even asking to do. We can't "not ship" implementation assemblies that fill in types/members that are already part of a previous netstandard version. That'd be a breaking change. You can see the superset of these assemblies with https://github.com/dotnet/corefx/search?p=2&q=IsNetFxNetstandard&type=&utf8=%E2%9C%93.

@tarekgh
Copy link
Member

tarekgh commented Apr 13, 2017

@karelz could you please clarify the ask?

@karelz
Copy link
Member Author

karelz commented Apr 13, 2017

Since I created this issue, I learned a lot more about the topic (thanks for @ericstj and @weshaggard for their patience explaining more and more details to me).
While I still don't know everything (it is truly rocket science), I think I am comfortable with this plan:

  1. All facades and partial facades can keep shipping -- they are additive things and don't replace anything in Desktop.
  2. All packages that replace implementation in Desktop are good candidates to stop shipping entirely.

As a result we can close this issue as it is not tracking anything meaningful.

@karelz karelz closed this as completed Apr 13, 2017
@desmondgc
Copy link

@karelz can you explain in more detail why System.Runtime.Serialization.Primitives is considered a non-issue?

We just spent days fighting with binding redirects and needed to install System.Runtime.Serialization.Primitives package in dozens of projects that don't need it directly but reference a .NET Standard library that requires it. In addition, we ended up needing to add a binding redirect to Visual Studio itself in order to generate service references!

All in all, it feels like a colossal mess and I'm disappointed to see that this is considered "working as designed."

@ericstj
Copy link
Member

ericstj commented Jun 7, 2017

@desmondgc it sounds like you expected that all things available by default to a .NET Standard library were included by default in frameworks that support NETStandard. That is the direction we're going with NETStandard2.0. We're adding all the support libraries to the tooling and referencing them automatically when you bring in a netstandard library to a desktop project. A future version of desktop will include everything inbox as well so it should alleviate the dependency on bindingRedirects to load the assemblies.

We're not ignoring these issues, but we can't go back and undo the new API that was exposed by the new version of these assemblies.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Meta enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

6 participants