-
Notifications
You must be signed in to change notification settings - Fork 6.1k
fix: Update seatbelt policy for java on macOS #3987
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
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
| env.remove("JAVA_HOME"); | ||
| env.remove(CODEX_SANDBOX_ENV_VAR); | ||
|
|
||
| let mut child = spawn_command_under_seatbelt( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think cargo clippy is asking you to drop the mut here
| let mut child = spawn_command_under_seatbelt( | |
| let child = spawn_command_under_seatbelt( |
|
Overall looks good, just a minor lint issue. |
|
I pushed a fix for the test. Please confirm you want to keep the test, btw. |
|
Looking for this to make it in soon! Nearly all of my codex responses contain: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving after syncing with @nornagon-openai
|
@bn3t, thanks for the contribution. I think this will make a lot of Codex users happy! |
Summary
This PR is related to the Issue #3978 and contains a fix to the seatbelt profile for macOS that allows to run java/jdk tooling from the sandbox. I have found that the included change is the minimum change to make it run on my machine.
There is a unit test added by codex when making this fix. I wonder if it is useful since you need java installed on the target machine for it to be relevant. I can remove it it is better.
Fixes #3978