Remove System.Buffers package #32096
Conversation
| $(PackageConfigurations); | ||
| uap; | ||
| netfx; | ||
| netstandard; |
There was a problem hiding this comment.
Should we make this netcoreapp and uap? Once this becomes part of netstandard vnext we will be forced to do that.
There was a problem hiding this comment.
I will try to make this netcoreapp and uap. There was a reference to this ref assembly that was preventing me from doing so, but I will figure it out.
There was a problem hiding this comment.
The reason why I made this netstandard was because System.IO.Pipelines references System.Buffers and now with this change IO.Pipelines ref is built for netstandard, should we wait until this becomes part of vnext to make this ref netcoreapp and uap? or should we just build Pipelines ref assembly for netcoreapp and uap?
There was a problem hiding this comment.
Lets keep it simple for now and stick with netstandard.
| <BuildConfigurations> | ||
| $(PackageConfigurations); | ||
| netstandard; |
There was a problem hiding this comment.
This now adds the netstandard2.0 configuration to the package which didn't exist before we should decide if we want to do that or not.
There was a problem hiding this comment.
The src already has a netstandard PackageConfiguration and since the ref was being built for ns1.3 it was alread ns2.0 supported. What configuration should we build it for instead?
There was a problem hiding this comment.
I think I'm OK with adding the netstandard2.0 configuration to the package. It is probably better that way anyhow because it helps eliminate some package dependencies.
safern
force-pushed
the
RemoveSBuffers
branch
from
0faf4f1
to
a5c4f14
Compare
|
@bartonjs I added commit a5c4f14 to harvest S.S.C.Cng ns1.3 and ns1.4 implementation assets. @weshaggard would you mind taking another look please? |
| @@ -30,9 +30,6 @@ | |||
| <ItemGroup Condition="'$(TargetsNETCoreApp)'=='true'"> | |||
| <Compile Include="System\IO\Pipelines\ThreadPoolScheduler.netcoreapp.cs" /> | |||
| </ItemGroup> | |||
| <ItemGroup Condition="'$(TargetGroup)'=='netstandard1.3'"> | |||
| <Compile Include="System\IO\Pipelines\ThreadPoolScheduler.netstandard1.3.cs" /> | |||
| </ItemGroup> | |||
| <ItemGroup Condition="'$(TargetGroup)'=='netstandard'"> | |||
There was a problem hiding this comment.
nit: above we are using TargetsNETCoreApp, should we use TargetNetStandard here too instead?
There was a problem hiding this comment.
The reason why we're using TargetsNETCoreApp is because it is used by both, netcoreapp2.1 and netcoreapp targetgroups, but here it is specific to netstandard. Since it is the only version of netstandard used I think it is better to leave it like is.
|
@safern Are there runtimes where the current CNG package could be pulled in and netstandard1.3 or netstandard1.4 would be the best match? I'd be concerned that we'd then not be shipping bugfixes that we thought we were shipping. I'd rather "not support" a lib than harvest it, personally. |
|
When we start harvesting it we are essentially moving it to servicing only mode so fixes will only come from the servicing branches. So as long as there aren't any live active platforms that expect fixes, which is generally the case for any NS less then 2 then I think we are good harvesting the assets. |
* Remove System.Buffers package and harvest System.IO.Pipelines for ns1.3 * Run UpdateVSConfigurations * Harvest Security.Cryptography.Cng ns1.3 and ns1.4 implementations Commit migrated from dotnet/corefx@b09c098
Fixes: https://github.com/dotnet/corefx/issues/31776
Since API is frozen because this package is inbox in both netcoreapp and uap, we no longer need to build this package anymore. If we need to make changes to the package we can do it from the release branches.
cc: @weshaggard @ericstj