Skip to content

[release/6.0] Add osx-arm64.runtime.native.System.IO.Ports #60084

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

Merged

Conversation

SeanMollet
Copy link
Contributor

@SeanMollet SeanMollet commented Oct 6, 2021

Fixes #59972

This change adds a package project to build/package it, which also adds it as a dependency to the meta runtime.native.System.IO.Ports package.

Customer Impact

.NET 6 advertises Apple Silicon support. Not having this means that part of that support is missing. In particular, myself and many others develop for .NET on Apple hw, which is moving fairly quickly to Apple Silicon. Not having this means that I have to use a Linux VM or a remote system if I want to communicate with external devices.

In my specific case, I built electronics that interface with .NET servers and clients using USB, which shows up as a virtual serial port. Not having that functionality on my development machine is quite annoying.

Testing

Tested on actual Apple Silicon hardware with various serial devices. Since no current M1 device offers native serial ports, all testing was conducted with various USB devices that convert to or present as serial ports. Works correctly.

Risk

Low. The alternative is that there is no support at all.

…ckaging.

Tested on actual osx-arm64 hardware, resulting package works correctly.
@ghost ghost added area-System.IO community-contribution Indicates that the PR has been added by a community member labels Oct 6, 2021
@ghost
Copy link

ghost commented Oct 6, 2021

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Added the .proj file so this will get packaged. The automation works for all the other parts.
Tested resulting nupkg on actual osx-arm64 hardware, works correctly.

Author: SeanMollet
Assignees: -
Labels:

area-System.IO

Milestone: -

@SeanMollet
Copy link
Contributor Author

Closes #59972

…tive.System.IO.Ports.proj

Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>
@ViktorHofer
Copy link
Member

@dotnet/area-system-io can you please take a look as well?

@jozkee
Copy link
Member

jozkee commented Oct 12, 2021

@ViktorHofer can you please merge? The PR says that I am not authorized to do so.

@ViktorHofer
Copy link
Member

I didn't see that this change actually targets release/6.0. I think for that we would need Tactics approval, wouldn't we? cc @danmoseley for awareness.

@jozkee
Copy link
Member

jozkee commented Oct 12, 2021

Ah, didn't see that either, is it ok to do this for 6.0? It may be risky.
I agree this should be done for 7.0, though.

IMO this PR should target 7.0. @SeanMollet thoughts?

@SeanMollet
Copy link
Contributor Author

@jozkee .net 6 advertises Apple Silicon support, this is part of supporting Apple Silicon. Further, the sooner it gets in, the sooner the workaround for comparison to previous packages can be dropped. As you might gather, I'd very much like to see this in the 6 release.

It's the same code as the osx-x64 version, it compiles clean on M1 and works correctly. The included tests and my own pass on both platforms (x64 and m1).

The alternative is not having support for serial ports on M1 at all. Even if it did turn out there's a bug, that's still a better situation than have no support at all.

For clarification, the note above about "this should be removed when .net 7 ships" is not about the entire PR, just the single line that bypasses previous version checks (since there isn't a previous version of this).

@jozkee
Copy link
Member

jozkee commented Oct 12, 2021

For clarification, the note above about "this should be removed when .net 7 ships" is not about the entire PR, just the single line that bypasses previous version checks (since there isn't a previous version of this).

Yes, I was asking for DisablePackageBaselineValidation :)

The alternative is not having support for serial ports on M1 at all. Even if it did turn out there's a bug, that's still a better situation than have no support at all.

Ok got it, the usual process we do is send the PR to main, which is the code that will ship for the next release and once merged there, we send a backport PR to release/6.0 where is discussed if it meets the bar.
this PR also needs a template like #60280. I don't know if for nuget packages is a different process, though.

Let's wait for @danmoseley or @jeffhandley to chime in.

@jozkee jozkee added this to the 6.0.0 milestone Oct 12, 2021
@jozkee jozkee added the Servicing-consider Issue for next servicing release review label Oct 12, 2021
@SeanMollet
Copy link
Contributor Author

@jozkee I copied your template. I'm happy to do this against main if that's the approach that's required.

@jozkee
Copy link
Member

jozkee commented Oct 12, 2021

@SeanMollet at this point, and assuming this gets approved, I think it may be better to merge this first and then backport to main. EDIT: Viktor approach sounds better.

@ViktorHofer
Copy link
Member

@SeanMollet that's all that is required, thanks very much for providing a template. Can you meanwhile please send another PR targeting main? That one we can get in quickly and we usually get changes first into main before we backport them into a release branch.

I will make sure that this gets discussed for 6.0 asap.

@ViktorHofer
Copy link
Member

this PR also needs a template like #60280. I don't know if for nuget packages is a different process, though.

It's not different. We ship nuget packages together with the product and the same bar applies here.

@ViktorHofer ViktorHofer changed the title Add osx-arm64.runtime.native.System.IO.Ports [release/6.0] Add osx-arm64.runtime.native.System.IO.Ports Oct 12, 2021
@jozkee jozkee requested a review from ericstj October 12, 2021 22:58
@jeffhandley
Copy link
Member

@SeanMollet After you got the local build, were you able to consume these APIs and verify the functionality on the Apple silicon hardware? I'd like to confirm that once the packaging fix is merged into release/6.0, you'll be unblocked.

@jeffhandley
Copy link
Member

@SeanMollet After you got the local build, were you able to consume these APIs and verify the functionality on the Apple silicon hardware? I'd like to confirm that once the packaging fix is merged into release/6.0, you'll be unblocked.

@SeanMollet And now I see you included that info but I overlooked it.

Tested on actual Apple Silicon hardware with various serial devices. Since no current M1 device offers native serial ports, all testing was conducted with various USB devices that convert to or present as serial ports. Works correctly.

Thanks!

@danmoseley
Copy link
Member

failures - #60151 already fixed.
System.Net.Sockets timeouts are clearly unrelated - I won't log an issue as this run is a bit old now.

I will rerun the timed out build out of an excess of caution.

@jeffhandley jeffhandley removed the Servicing-consider Issue for next servicing release review label Oct 13, 2021
@jeffhandley jeffhandley added the Servicing-approved Approved for servicing release label Oct 13, 2021
@danmoseley danmoseley closed this Oct 13, 2021
@danmoseley danmoseley reopened this Oct 13, 2021
@Anipik Anipik merged commit 2e71df6 into dotnet:release/6.0 Oct 13, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Nov 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO.Ports community-contribution Indicates that the PR has been added by a community member Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants