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

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

Merged
merged 3 commits into from
Nov 15, 2021

Conversation

MaximLipnin
Copy link
Contributor

@MaximLipnin MaximLipnin commented Nov 15, 2021

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.

@ghost
Copy link

ghost commented Nov 15, 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

See #61265 (comment) and above for more details

Author: MaximLipnin
Assignees: -
Labels:

area-System.Diagnostics.Process

Milestone: -

@marek-safar marek-safar added the os-ios Apple iOS label Nov 15, 2021
@ghost
Copy link

ghost commented Nov 15, 2021

Tagging subscribers to 'os-ios': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

See #61265 (comment) and above for more details

Author: MaximLipnin
Assignees: -
Labels:

area-System.Diagnostics.Process, os-ios

Milestone: -

@MaximLipnin MaximLipnin marked this pull request as ready for review November 15, 2021 15:49
@MaximLipnin
Copy link
Contributor Author

I've marked it as ready-to-review but it might still require some PNSE-throwing APIs to be marked with UnsupportedOSPlatform() on iOS/tvOS

@mdh1418
Copy link
Member

mdh1418 commented Nov 15, 2021

Nit: Make the PR description be tad bit more descriptive. The issue referenced and title of this PR suggests that just $(CommonPath)Interop\OSX\Interop.libproc.cs will be excluded. What is related and the change looks to instead have iOS and tvOS behave like UnknownUnix to throw PNSE.
Nit: libroc -> libproc in title

@MaximLipnin MaximLipnin changed the title Exclude libroc and related on iOS/tvOS Exclude the managed code around libproc on iOS/tvOS Nov 15, 2021
@steveisok
Copy link
Member

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1464098919

@github-actions
Copy link
Contributor

@steveisok an error occurred while backporting to release/6.0, please check the run log for details!

Error: @steveisok is not a repo collaborator, backporting is not allowed.

@steveisok
Copy link
Member

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1464211394

@github-actions
Copy link
Contributor

@steveisok backporting to release/6.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Exclude libroc and related on iOS/tvOS
Using index info to reconstruct a base tree...
M	src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj
CONFLICT (content): Merge conflict in src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Exclude libroc and related on iOS/tvOS
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

MaximLipnin added a commit that referenced this pull request Nov 18, 2021
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)
steveisok pushed a commit that referenced this pull request Nov 19, 2021
…stenerTests 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.
MaximLipnin added a commit to MaximLipnin/runtime that referenced this pull request Nov 19, 2021
…stenerTests on iOS/tvOS (dotnet#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 dotnet#61590.
MaximLipnin added a commit to MaximLipnin/runtime that referenced this pull request Dec 1, 2021
Since libproc is a private Apple API, it is not available on iOS/tvOS and should be excluded (see dotnet#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.
MaximLipnin added a commit to MaximLipnin/runtime that referenced this pull request Dec 1, 2021
This is a follow up PR for dotnet#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 dotnet#60588 as well)
MaximLipnin added a commit to MaximLipnin/runtime that referenced this pull request Dec 1, 2021
…stenerTests on iOS/tvOS (dotnet#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 dotnet#61590.
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 16, 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.

7 participants