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

chore: enable v2 sandbox on mac #15647

Merged
merged 2 commits into from Dec 13, 2018

Conversation

Projects
None yet
4 participants
@nornagon
Copy link
Contributor

commented Nov 8, 2018

Description of Change

This re-enables the v2 sandbox on mac (disabled by 878c5b6). That commit asserts that the v2 sandbox would prevent the mixed-sandbox mode from working, but in my testing with this change, it seems to work fine.

$ ps aux | grep Electron | grep type=renderer
# (abridged)
33083 .../Electron Helper --type=renderer [...] --no-sandbox
33082 .../Electron Helper --type=renderer [...] --enable-sandbox --seatbelt-client=53

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes
  • tests are changed or added
  • relevant documentation is changed or added
  • PR title follows semantic commit guidelines

Release Notes

Notes:

@nornagon nornagon requested a review from zcbenz Nov 8, 2018

@nornagon nornagon requested a review from as a code owner Nov 8, 2018

@nornagon nornagon requested a review from tarruda Nov 8, 2018

@nornagon nornagon added the wip label Nov 13, 2018

@nornagon

This comment has been minimized.

Copy link
Contributor Author

commented Nov 13, 2018

marking as WIP because this has exposed some failures on linux and some tests that expect the sandbox option to do weird things like loading the sandboxed renderer client without actually sandboxing the renderer.

@codebytere

This comment has been minimized.

Copy link
Member

commented Nov 28, 2018

ah dangit i didn't intend to approve this yet, ignore the above 🤦‍♀️

unintentional

@nornagon nornagon force-pushed the v2-sandbox branch from 0f77136 to 226cc2d Dec 3, 2018

@nornagon nornagon requested a review from as a code owner Dec 3, 2018

@nornagon

This comment has been minimized.

Copy link
Contributor Author

commented Dec 3, 2018

Rebased on top of #15894

@deepak1556

This comment has been minimized.

Copy link
Member

commented Dec 7, 2018

Upstream is trying to remove the kMacV2Sandbox feature flag soon, they tried it in 71 but due to some failures its fortunately delayed 5406108

@nornagon

This comment has been minimized.

Copy link
Contributor Author

commented Dec 7, 2018

Pretty sure I can update & land this before 72. Thanks for the info.

@nornagon nornagon force-pushed the v2-sandbox branch 2 times, most recently from dfc2db0 to a0d485e Dec 10, 2018

@nornagon

This comment has been minimized.

Copy link
Contributor Author

commented Dec 12, 2018

I believe the mac and arm failures are flakes, see #16027

@nornagon nornagon removed the wip label Dec 12, 2018

@zcbenz

zcbenz approved these changes Dec 12, 2018

@nornagon nornagon force-pushed the v2-sandbox branch from a0d485e to 56cd7eb Dec 12, 2018

@nornagon nornagon force-pushed the v2-sandbox branch from 56cd7eb to 9f90a5e Dec 12, 2018

@nornagon nornagon merged commit 4250f84 into master Dec 13, 2018

27 checks passed

Absolute Zero
Artifact Comparison No Changes
Details
Semantic Pull Request ready to be squashed
Details
WIP ready for review
Details
appveyor: win-ia32-debug AppVeyor build succeeded
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-debug AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
ci/circleci: linux-arm-debug Your tests passed on CircleCI!
Details
ci/circleci: linux-arm-testing Your tests passed on CircleCI!
Details
ci/circleci: linux-arm64-debug Your tests passed on CircleCI!
Details
ci/circleci: linux-arm64-testing Your tests passed on CircleCI!
Details
ci/circleci: linux-checkout Your tests passed on CircleCI!
Details
ci/circleci: linux-ia32-debug Your tests passed on CircleCI!
Details
ci/circleci: linux-ia32-testing Your tests passed on CircleCI!
Details
ci/circleci: linux-ia32-testing-tests Your tests passed on CircleCI!
Details
ci/circleci: linux-x64-debug Your tests passed on CircleCI!
Details
ci/circleci: linux-x64-testing Your tests passed on CircleCI!
Details
ci/circleci: linux-x64-testing-tests Your tests passed on CircleCI!
Details
electron-arm-testing Build #20181212.10 succeeded
Details
electron-arm64-testing Build #20181212.10 succeeded
Details
electron-lint Build #20181212.10 succeeded
Details
electron-mas-testing Build #20181212.15 succeeded
Details
electron-osx-testing Build #20181212.15 succeeded
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

commented Dec 13, 2018

Release Notes Persisted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.