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

Support Process.Start() on MacCatalyst #61507

Conversation

simonrozsival
Copy link
Member

@simonrozsival simonrozsival commented Nov 12, 2021

This PR adds support for Process.Start() and Process.Kill() for Mac Catalyst. I also added a way to run Mac Catalyst tests in App Sandbox and I resolved the problems with libproc in App Sandbox (the app is unable to obtain a list of running processes).

Closes #61295, closes #61504.

@ghost
Copy link

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

null

Author: simonrozsival
Assignees: -
Labels:

area-System.Diagnostics.Process

Milestone: -

@jkotas
Copy link
Member

jkotas commented Dec 6, 2021

simonrozsival requested a review from jkotas 4 hours ago

The changes in .cs files LGTM ... once the CI is green.

@steveisok
Copy link
Member

@safern The failures seem odd and the Supported/UnSupported attributes in the change look correct to me. Could this be an issue with ApiCompat?

@safern
Copy link
Member

safern commented Dec 6, 2021

@steveisok this is an isntance of: dotnet/arcade#7585

I'd suggest baselining those errors with:

dotnet build src\libraries\shims\ApiCompat.proj /p:UpdatePreviousNetCoreAppBaseline=true

And then committing the change.

@simonrozsival
Copy link
Member Author

It seems that the problem was related to #61437. I pulled updates from main and baselined the errors and everything seems to be ✅

There's a JIT.HardwareIntrinsics.X86.Sse3 test failing in runtime (CoreCLR Pri0 Runtime Tests Run windows arm64 checked) which is unrelated to the changes in this PR.

@simonrozsival simonrozsival merged commit 6cf785d into dotnet:main Dec 7, 2021
@simonrozsival simonrozsival deleted the simonrozsival/61295-support-process-start-on-mac-catalyst branch December 7, 2021 19:33
@steveisok
Copy link
Member

/backport to release/6.0-maui

@github-actions
Copy link
Contributor

github-actions bot commented Dec 7, 2021

Started backporting to release/6.0-maui: https://github.com/dotnet/runtime/actions/runs/1551016187

@github-actions
Copy link
Contributor

github-actions bot commented Dec 7, 2021

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

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

Applying: Modify existing tests
Applying: Enable process start and kill on MacCatalyst
Applying: Typo
Applying: Temporarily enable ProcessTests for MacCatalyst
Using index info to reconstruct a base tree...
M	src/libraries/tests.proj
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/tests.proj
CONFLICT (content): Merge conflict in src/libraries/tests.proj
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0004 Temporarily enable ProcessTests for MacCatalyst
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!

simonrozsival added a commit to simonrozsival/runtime that referenced this pull request Dec 8, 2021
* Modify existing tests

* Enable process start and kill on MacCatalyst

* Typo

* Temporarily enable ProcessTests for MacCatalyst

* Temporarily enable ProcessTests for MacCatalyst - attempt 2

* Allow running Mac Catalyst builds in App Sandbox

* Allow enabling app sandbox for the other Mac Catalyst sample

* Add missing parameter to XCode project generator

* Remove unnecessary MacCatalyst detection

* Enable App Sandbox for Mac Catalyst tests

* Create a separate test branch for App Sandbox

* Remove the restriction to enable app sandbox just for Mac Catalyst apps

* Do not throw PNSE for Mac Catalyst

* Clean-up platform-specific conditions

* Build correct Process implementation for MacCatalyst

* Try to get more information from CI for further investigation of the failing build

* Revert "Try to get more information from CI for further investigation of the failing build"

This reverts commit fc63a37.

* Add MacCatalyst target framework for System.Diagnostics.Process

* Add supported platform annotations for Mac Catalyst

* Fix annotations

* Remove incorrectly placed attributes

* Update attributes including the reference file

* Change platform attributes

* Generate app.entitlements in the BuildAppleAppBundles test build target

* Revert "Generate app.entitlements in the BuildAppleAppBundles test build target"

This reverts commit abbe224.

* Enable AppSandbox when generating CMakeLists.txt for libraries tests

* Try implementing a workaround for app sandbox mode

* Fix app sandbox detection bug

* Add explanation comment

* Enable more tests for MacCatalyst

* Add apple app builder input validation

* Add Mac Catalyst w/ App Sandbox enabled to runtime-manual test pipeline

* Enable networking in App Sandbox mode

* Skip test which won't work on Mac Catalyst

* Skip some tests in app sandbox mode

* Update src/tasks/AppleAppBuilder/AppleAppBuilder.cs

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>

* Add a clarification comment for a networking entitlement

* Simplify supported platform condition

* Remove temporarily enabled test

* Remove unnecessary attributes

* Replace checking env variable with checking errno in libproc

* Update docs

* Remove unnecessary changes

* Temporarily enable running System.Diagnostics.Process.Tests for this PR

This reverts commit 02d370c.

* Revert "Temporarily enable running System.Diagnostics.Process.Tests for this PR"

This reverts commit dc72f0f.

* Fix job suffix in runtime-manual

* Remove attributes

* Revert "Remove attributes"

This reverts commit 704e9fa.

* Try changing the order of attributes to please CI

* Update ApiCompat baseline

* ApiCompat step 1: remove all attributes for MaxWorkingSet setter

* Revert "Update ApiCompat baseline"

This reverts commit a2ad032.

* Revert "ApiCompat step 1: remove all attributes for MaxWorkingSet setter"

This reverts commit be72a3d.

* Update ApiCompat baseline

* Update ApiCompat baseline after pulling upstream main

* Remove trailing whitespace

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
@ghost ghost locked as resolved and limited conversation to collaborators Jan 7, 2022
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.

[macOS] Enable Process support on MacCatalyst Process.Start() should be supported on MacCatalyst
7 participants