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

build: fix sandboxing on darwin #3303

Merged
merged 1 commit into from Jan 6, 2020
Merged

build: fix sandboxing on darwin #3303

merged 1 commit into from Jan 6, 2020

Conversation

@LnL7
Copy link
Member

@LnL7 LnL7 commented Jan 5, 2020

Starting ba87b08 getEnv now returns an
std::optional which means these getEnv() != "" conditions no longer happen
if the variables are not defined.

Starting ba87b08 getEnv now returns an
std::optional which means these getEnv() != "" conditions no longer happen
if the variables are not defined.
@LnL7
Copy link
Member Author

@LnL7 LnL7 commented Jan 5, 2020

I also have a change that opens up the sandbox slightly, which works around the current problems it has. The current stricter approach would be nicer, but I've been using this fore quite some time now and I'd rather have this instead of nothing. LnL7@a83ab2b

@@ -3338,7 +3338,7 @@ void DerivationGoal::runChild()
;
}
#if __APPLE__
else if (getEnv("_NIX_TEST_NO_SANDBOX") == "") {
else {

This comment has been minimized.

@edolstra

edolstra Jan 5, 2020
Member

Hm, why not change it to getEnv("_NIX_TEST_NO_SANDBOX").value_or("") == ""? That way you don't have to change anything else. It seems strange to initialize sandboxProfile when sandboxing is disabled.

This comment has been minimized.

@LnL7

LnL7 Jan 5, 2020
Author Member

I noticed most of the other checks use == "1" and it doesn't hurt to make this a bit more strict since it really should only be used by the tests.

@edolstra edolstra merged commit e2988f4 into NixOS:master Jan 6, 2020
@LnL7 LnL7 deleted the LnL7:darwin-sandbox branch Jan 7, 2020
@edolstra
Copy link
Member

@edolstra edolstra commented Jan 13, 2020

@LnL7 Looks like this causes a test failure on macOS: https://hydra.nixos.org/build/109896319

@LnL7
Copy link
Member Author

@LnL7 LnL7 commented Jan 13, 2020

Hmm I don't really see how that could break it, are you sure it wasn't #3302 instead?

@edolstra
Copy link
Member

@edolstra edolstra commented Jan 14, 2020

Yes that could also be it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.