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

sandbox: Allow UNIX sockets on macOS even when block-network is used. #3444

Closed
wants to merge 1 commit into from

Conversation

philwo
Copy link
Member

@philwo philwo commented Jul 24, 2017

No description provided.

@ittaiz
Copy link
Member

ittaiz commented Jul 25, 2017

Amazing!!! The test passes, thanks :)
There isn't a real chance this will be cherry picked right?
Asking in hope (but without real anticipation).
This is a blocker for us but we'll be willing to use the commit it goes in until 0.6.0

@ittaiz
Copy link
Member

ittaiz commented Jul 25, 2017 via email

@philwo philwo requested a review from ulfjack July 25, 2017 05:30
@philwo philwo self-assigned this Jul 25, 2017
@philwo
Copy link
Member Author

philwo commented Jul 25, 2017

@ulfjack On macOS we don't have network virtualization, so "block-network" on that platform is used to block access to external networks in order to not accidentally depend on non-hermetic resources. We do allow networking with non-virtualized localhost though.

I'd argue that just allowing UNIX sockets fits into the current philosophy and that we don't need a special flag for this. What do you think?

Copy link
Contributor

@ulfjack ulfjack left a comment

Choose a reason for hiding this comment

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

SGTM

@philwo
Copy link
Member Author

philwo commented Jul 25, 2017

Thanks! Importing and sending for internal review now.

@philwo
Copy link
Member Author

philwo commented Jul 25, 2017

Fix merged internally.

@ittaiz
Copy link
Member

ittaiz commented Jul 25, 2017 via email

@philwo
Copy link
Member Author

philwo commented Jul 25, 2017

I'll kindly ask @buchgr, but I don't know in what state we currently are with the 0.5.3 release :)

@bazel-io bazel-io closed this in cd159bc Jul 25, 2017
@philwo
Copy link
Member Author

philwo commented Jul 26, 2017

@ittaiz This was cherry-picked and will be part of the next Bazel 0.5.3 release candidate.

buchgr pushed a commit that referenced this pull request Jul 26, 2017
@buchgr
Copy link
Contributor

buchgr commented Jul 26, 2017

Just triggered the creation of the next release candidate with this change :)

@ittaiz
Copy link
Member

ittaiz commented Jul 26, 2017 via email

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

Successfully merging this pull request may close these issues.

5 participants