Move System.Net.Sockets to netstandard 1.7 #12249
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,12 @@ | ||
| { | ||
| "dependencies": { | ||
| "System.IO": "4.1.0", | ||
| "System.IO.FileSystem.Primitives": "4.0.1", | ||
| "System.Net.Primitives": "4.0.11", | ||
| "System.Runtime": "4.1.0", | ||
| "System.Threading.Tasks": "4.0.11" | ||
| }, | ||
| "frameworks": { | ||
| "netstandard1.7": { } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -614,6 +614,9 @@ internal bool Connected | |
| // Returns: | ||
| // | ||
| // An IASyncResult, representing the read. | ||
| #if !netcore50 | ||
| override | ||
| #endif | ||
|
Contributor
There was a problem hiding this comment. Why are we doing this special
Contributor
Author
There was a problem hiding this comment. When we build for netcore50, we're building against the older contract where these don't exist on
Member
There was a problem hiding this comment. Instead of doing this #ifdef'ing can we just remove the netcore50 build now? If you remove the build configuration we will essentially pick-up and reship the version for the last stable package. We we do go that route and remove the netcore50 configuration from the live build then we will have the accept that we aren't going to be making in bug fixes in that configuration any longer and if we do need to fix something for it we will have to do it from the release/servicing branch or add back the configuration at that time. For that reason I will leave it up to you guys to decide whether or not to remove the live build or not.
Member
There was a problem hiding this comment. Do you think that's wise given it'll just need to come back for UAP10.1?
Member
There was a problem hiding this comment. Scratch that, I see what you're saying. This won't need to come back for UAP10.1 since UAP10.1 will have this version of stream. I had recommended that @ericeil keep the netcore50 config around and building so that it would be eaiser to add the UAP10.1 config when that work is done, however here it's making the current work harder. Another option would be to bring up a UAP10.1 config now, but I suspect that'd require some more hacks.
Contributor
There was a problem hiding this comment. I don't understand why we would drop 'netcore50' suport. I don't really care what moniker we call it. But we need to have UWP working at all times. Will I still be able to build a UWP app and use System.Net.Sockets (or any other networking library). And when/how will UWP be updated in order to use the newly added contract API surface. I've heard it is when "uap10.1" gets working. But I don't understand exactly when that is and how it works.
Contributor
Author
There was a problem hiding this comment. I've logged issue #12331 to track this potential extra work. I don't think we should hold this PR for that decision.
Member
There was a problem hiding this comment. @davidsh removing the live netcore50 build doesn't break UWP. It just implies that we are going to pull the netcore50 version of the library from the last stable shipped package and reship that in the current package. It just allows us to eliminate a bunch of the extra live build configurations.
Contributor
There was a problem hiding this comment. @weshaggard Ok. So, then when can be start to have UWP working with this expanded API set? Is this "uap10.1"? If so, does that live build work? Or is there work to do in this library to make that happen. I need to get this working in UWP for the expanded (a.k.a netstandard17.....a.k.a NET Standard 2.0).
Member
There was a problem hiding this comment. Yes most of the .NET Standard 2.0 work is targeting "uap10.1", however that ties to a new tool chain that is also under development in TFS. For some things we could add them to the netcore50 version of the library assuming it isn't part of the SharedLibrary and doesn't require any new dependent APIs that are in contracts that are part of the SharedLibrary. My expectation is that most of the .NET Standard 2.0 will require newer versions of things like System.Runtime which means we won't really be able to support them in netcore50 and we will need to use uap10.1 instead. |
||
| public IAsyncResult BeginRead(byte[] buffer, int offset, int size, AsyncCallback callback, Object state) | ||
| { | ||
| #if DEBUG | ||
|
|
@@ -727,6 +730,9 @@ internal virtual IAsyncResult UnsafeBeginRead(byte[] buffer, int offset, int siz | |
| // Returns: | ||
| // | ||
| // The number of bytes read. May throw an exception. | ||
| #if !netcore50 | ||
| override | ||
| #endif | ||
| public int EndRead(IAsyncResult asyncResult) | ||
| { | ||
| #if DEBUG | ||
|
|
@@ -785,6 +791,9 @@ public int EndRead(IAsyncResult asyncResult) | |
| // Returns: | ||
| // | ||
| // An IASyncResult, representing the write. | ||
| #if !netcore50 | ||
| override | ||
| #endif | ||
| public IAsyncResult BeginWrite(byte[] buffer, int offset, int size, AsyncCallback callback, Object state) | ||
| { | ||
| #if DEBUG | ||
|
|
@@ -908,6 +917,9 @@ internal virtual IAsyncResult UnsafeBeginWrite(byte[] buffer, int offset, int si | |
| // This method is called when an async write is completed. All we | ||
| // do is call through to the core socket EndSend functionality. | ||
| // Returns: The number of bytes read. May throw an exception. | ||
| #if !netcore50 | ||
| override | ||
| #endif | ||
| public void EndWrite(IAsyncResult asyncResult) | ||
| { | ||
| #if DEBUG | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Should we even this Common code anymore? I thought
SafeHandleZeroOrMinusOneIsInvalidand friends was being added back to the public API surface (i.e. part of .NET Standard 2.0). So, shouldn't these references just be coming from Microsoft.Win32.SafeHandles contract via a project.json reference?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.
They have already been added back, but netcore50 doesn't support netstandard1.7(aka 2.0), so the common source is still needed for that build.
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.
When will netcore50 support .NET Standard 2.0? Or perhaps the real question is when will the "UWP" version of this support these APIs? Is there going to be a 'netcore51' for example? I did see uap10.1 vs. uap10.0 so I was wondering if there was a net platform definition for a revised version for UWP, similar to what is happening for 'netcoreapp10' vs. 'netcoreapp11'.
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.
netcore50 will never support .NET Standard 2.0. We do not plan to version the netcore tfm for this as netcore50 has special meaning to various systems and so it is essentially a dead end. However UWP with uap10.1 tfm is currently the target to support .NET Standard 2.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.
So, are we changing our source code where we use "if netcore50". And we use that term in our folder names and partial class names. The documentation needs to be updated if we don't use "netcore50" TFM anymore. We have separate implementations for some of the networking libraries, so need good guidelines to keep this organized and building correctly.