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

Use separate partials for iOS&tvOS instead of UnknowUnix in System.Diagnostics.Process #61871

Merged

Conversation

MaximLipnin
Copy link
Contributor

Re-using UnknownUnix partials with UnsupportedOSPlatform annotations for iOS&tvOS in System.Diagnostics.Process doesn't seem to be the right choice (see #61659 (comment)). To fix it, this PR is going to add separate partials for iOS&tvOS and clean up UnknownUnix files from PSNE annotations.

@ghost
Copy link

ghost commented Nov 20, 2021

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

Issue Details

Re-using UnknownUnix partials with UnsupportedOSPlatform annotations for iOS&tvOS in System.Diagnostics.Process doesn't seem to be the right choice (see #61659 (comment)). To fix it, this PR is going to add separate partials for iOS&tvOS and clean up UnknownUnix files from PSNE annotations.

Author: MaximLipnin
Assignees: -
Labels:

area-System.Diagnostics.Process

Milestone: -

@MaximLipnin
Copy link
Contributor Author

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MaximLipnin
Copy link
Contributor Author

/azp run runtime-manual

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@MaximLipnin MaximLipnin merged commit 8997e86 into dotnet:main Nov 24, 2021
@MaximLipnin MaximLipnin deleted the use_separate_partials_for_ios_in_SDP branch November 24, 2021 11:36
MaximLipnin added a commit to MaximLipnin/runtime that referenced this pull request Dec 1, 2021
steveisok pushed a commit that referenced this pull request Dec 2, 2021
#62235)

* Exclude the managed code around libproc on iOS/tvOS (#61590)

Since libproc is a private Apple API, it is not available on iOS/tvOS and should be excluded (see #61265 (comment) and above for more details).  
This PR excludes $(CommonPath)Interop\OSX\Interop.libproc.cs on the iOS/tvOS as well as makes some methods in Process, ProcessManager, and ProcessThread classes calling that API throw PNSE so that for iOS/tvOS it's possible to re-use the respective *.UnknownUnix.cs parts.

* [iOS] Follow up changes for 61590 (#61670)

This is a follow up PR for #61590.

It includes:

- additional UnsupportedOSPlatform annotations for some System.Diagnostics.Process APIs throwing PNSE on iOS/tvOS (they started doing so after excluding some managed logic around librpoc )

- fixing a bit ugly workaround for CS0649 (see https://github.com/dotnet/runtime/pull/61590/files#r749525127) - used a local pragma in the ThreadInfo class.

- skipping the respective S.D.P. tests ( it will address [iOS/tvOS] System.Diagnostics.Tests.ProcessTests.TestGetProcesses fails on devices #60588 as well)

* Skip System.Diagnostics.TextWriterTraceListenerTests.XmlWriterTraceListenerTests on iOS/tvOS (#61807)

This marks System.Diagnostics.TextWriterTraceListenerTests.XmlWriterTraceListenerTests withSkipOnPlatform attribute for iOS/tvOS as those tests try to create a process info, which throws PNSE after S.D.Process API's around libproc have been excluded in #61590.

* Disable several failing tests on iOSSimulator arm64 #61826

A few tests popped up as failures on the rolling build due to parts of System.Diagnostics.Process throwing PNSE. Disabled the functional tests from running on arm64 as mlaunch can't detect the return code.

* Use separate partials for iOS&tvOS instead of UnknowUnix in System.Diagnostics.Process (#61871)

* Remove NoWarn removal
@ghost ghost locked as resolved and limited conversation to collaborators Dec 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants