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

Pass -disable-sandbox to Swift compiler if requested #7167

Merged
merged 1 commit into from
Dec 8, 2023

Conversation

neonichu
Copy link
Contributor

@neonichu neonichu commented Dec 7, 2023

The Swift compiler supports disabling sandboxing for macros now, so we should opt-in to that if the selected toolchain supports it and the user has disabled sandboxing. We warn if macros are being used and disabling sandboxing was requested but isn't available.

rdar://118851130

@neonichu neonichu self-assigned this Dec 7, 2023
@neonichu
Copy link
Contributor Author

neonichu commented Dec 7, 2023

@swift-ci please test

@neonichu
Copy link
Contributor Author

neonichu commented Dec 7, 2023

We're still blocked on testing macros in general, so this does not come with any tests.

Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be covered with a test?

The Swift compiler supports disabling sandboxing for macros now, so we should opt-in to that if the selected toolchain supports it and the user has disabled sandboxing. We warn if macros are being used and disabling sandboxing was requested but isn't available.

rdar://118851130
@neonichu neonichu force-pushed the disable-sandbox-swiftc-if-disabled branch from 3ce6666 to 2516334 Compare December 7, 2023 20:47
@neonichu
Copy link
Contributor Author

neonichu commented Dec 7, 2023

@swift-ci please test

@neonichu
Copy link
Contributor Author

neonichu commented Dec 7, 2023

@swift-ci please test windows

3 similar comments
@neonichu
Copy link
Contributor Author

neonichu commented Dec 7, 2023

@swift-ci please test windows

@neonichu
Copy link
Contributor Author

neonichu commented Dec 7, 2023

@swift-ci please test windows

@neonichu
Copy link
Contributor Author

neonichu commented Dec 7, 2023

@swift-ci please test windows

@neonichu neonichu enabled auto-merge (squash) December 7, 2023 21:06
@neonichu
Copy link
Contributor Author

neonichu commented Dec 7, 2023

Hm, looks like #7015 broke the build maybe?

@neonichu
Copy link
Contributor Author

neonichu commented Dec 7, 2023

Yah, I can't build locally anymore, either.

@neonichu
Copy link
Contributor Author

neonichu commented Dec 8, 2023

@swift-ci please test macOS

@neonichu neonichu merged commit 23042d6 into main Dec 8, 2023
5 checks passed
@neonichu neonichu deleted the disable-sandbox-swiftc-if-disabled branch December 8, 2023 17:21
@MaxDesiatov
Copy link
Contributor

MaxDesiatov commented Jan 4, 2024

@jpsim this has to be cherry-picked to release/5.10 branch if you'd like to see this in a release sooner rather than later

@jpsim
Copy link
Contributor

jpsim commented Jan 4, 2024

I wasn't aware that 5.10 had already branched off. @neonichu could you please cherry-pick this into release/5.10?

@jpsim
Copy link
Contributor

jpsim commented Jan 4, 2024

The apple/swift change was cherry picked into the 5.10 release branch here: swiftlang/swift#70079

Same for apple/swift-driver here: swiftlang/swift-driver#1493

neonichu added a commit that referenced this pull request Jan 4, 2024
The Swift compiler supports disabling sandboxing for macros now, so we
should opt-in to that if the selected toolchain supports it and the user
has disabled sandboxing. We warn if macros are being used and disabling
sandboxing was requested but isn't available.

rdar://118851130
(cherry picked from commit 23042d6)
keith pushed a commit to bazelbuild/rules_swift that referenced this pull request May 22, 2024
…xing (#1206)

This should solve #1202
 and #1204

> There were issues with Swift compiler plugins (incl. macros) and
nested sandboxes on macOS with Swift 5.9 that have been fixed with 5.10:
> swift: [swiftlang/swift#70079](swiftlang/swift#70079)
> swift-driver:
[swiftlang/swift-driver#1493](swiftlang/swift-driver#1493)
> swift-package-manager:
[swiftlang/swift-package-manager#7167](swiftlang/swift-package-manager#7167)


I am not sure whether this flag is required on Linux too.

---------

Signed-off-by: Adin Cebic <[email protected]>
Co-authored-by: Brentley Jones <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants