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

ci: Always run with sandbox, even on Darwin #8240

Merged
merged 4 commits into from
May 26, 2023

Conversation

yorickvP
Copy link
Contributor

And fix a test failure in the sandbox due to /home existing on Darwin but not being accessible in the sandbox since it's a symlink to /System/Volumes/Data/home, see
https://github.com/NixOS/nix/actions/runs/4205378453/jobs/7297384658#step:6:2127:

C++ exception with description "error: getting status of /home/schnitzel/darmstadt/pommes: Operation not permitted" thrown in the test body.

On Linux this wasn't a problem because there /home doesn't exist in the sandbox

Motivation

#7735 (comment)

Context

originally by @infinisil

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

@yorickvP yorickvP requested a review from edolstra as a code owner April 19, 2023 13:48
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Apr 19, 2023
@edolstra
Copy link
Member

This fails on macOS with:

       > libc++abi: terminating with uncaught exception of type nix::SysError: error: getting status of /nix/var/nix/profiles/default/etc/ssl/certs/ca-bundle.crt: Operation not permitted

@infinisil
Copy link
Member

Fwiw, this commit did work in CI about 2 months ago: https://github.com/NixOS/nix/actions/runs/4208121587/jobs/7303768161

I suspect that some commit to master in that time frame broke the build in the sandbox.

@yorickvP
Copy link
Contributor Author

yorickvP commented May 8, 2023

This broke in #8062
Settings::Settings calls getDefaultSSLCertFile unconditionally, which does a pathExists("/etc/ssl/certs/ca-certificates.crt"), which errors on EPERM.

@yorickvP
Copy link
Contributor Author

I worked around the two problems, by calling getDefaultSSLCertFile() only when it's not overridden or set, and by ignoring EPERM from getDefaultNixPath().

The main problem is, I think, that the settings are not set up to have dynamic defaults. It also leads to misleading documentation entries, since the outputs of these functions at build time end up in the docs, and don't even match the runtime behaviour. See also the cores reproducability error.

However, I'm just here to fix the sandbox build.

@fricklerhandwerk fricklerhandwerk added release-blocker macos Nix on macOS, aka OS X, aka darwin labels May 11, 2023
@thufschmitt thufschmitt self-assigned this May 12, 2023
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-48/28102/1

Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

Thanks, that looks good overall, and it's a nice addition :) (I didn't even know it wasn't enabled by default on GH actions).

I'm curious why these test failures didn't happen on hydra. I'm assuming that Hydra does have the sandbox on, even on Darwin, right?

src/libexpr/eval.cc Outdated Show resolved Hide resolved
Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

Thanks, that looks good overall, and it's a nice addition :) (I didn't even know it wasn't enabled by default on GH actions).

I'm curious why these test failures didn't happen on hydra. I'm assuming that Hydra does have the sandbox on, even on Darwin, right?

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-05-12-nix-team-meeting-minutes-54/28197/1

@yorickvP yorickvP requested a review from thufschmitt May 26, 2023 13:35
infinisil and others added 4 commits May 26, 2023 15:36
And fix a test failure in the sandbox due to /home
existing on Darwin but not being accessible in the sandbox since it's a
symlink to /System/Volumes/Data/home, see
https://github.com/NixOS/nix/actions/runs/4205378453/jobs/7297384658#step:6:2127:

    C++ exception with description "error: getting status of /home/schnitzel/darmstadt/pommes: Operation not permitted" thrown in the test body.

On Linux this wasn't a problem because there /home doesn't exist in the sandbox
This does pathExists on various paths, which crashes on EPERM in the
macOS sandbox.
Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

Thanks, it's great to have that!

@reckenrode
Copy link

I'm curious why these test failures didn't happen on hydra. I'm assuming that Hydra does have the sandbox on, even on Darwin, right?

I ran into this issue while working on the Darwin stdenv update in nixpkgs, so I wanted to add a comment regarding sandboxing and Hydra. Hydra does not have the sandbox enabled. There are a number of packages I’ve had to add sandbox profiles to build them after bootstrapping the Darwin stdenv. There’s also been at least one failure on Hydra due to a lack of sandbox (NixOS/nixpkgs#201095).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
macos Nix on macOS, aka OS X, aka darwin release-blocker with-tests Issues related to testing. PRs with tests have some priority
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants