ci: Always run with sandbox, even on Darwin by yorickvP · Pull Request #8240 · NixOS/nix

yorickvP · 2023-04-19T13:48:44Z

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.

edolstra · 2023-04-24T11:41:37Z

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 · 2023-04-24T16:21:35Z

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 · 2023-05-08T16:27:37Z

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

yorickvP · 2023-05-11T11:51:00Z

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.

nixos-discourse · 2023-05-12T12:34:22Z

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

thufschmitt

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?

thufschmitt

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 · 2023-05-15T13:46:29Z

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

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.

thufschmitt

Thanks, it's great to have that!

reckenrode · 2023-07-12T16:59:10Z

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).

…fix to catch (BaseError&) (#2021) Follow-through on the stopped NixOS/nix upstreaming wave. ## What changed **`lib/fork-packages.nix` — nix `upstreamPolicy`** - `aiPrsAllowed = "false"`, citing [NixOS/nix#15984](NixOS/nix#15984) and its three operative constraints: human-authored PR communication, no unreviewed automated submissions, `Assisted-by:` disclosure trailer. `upstream-sync` now refuses to open any nix PR at the repo level. **Patch intent — all nix patches flipped `attempt` → `hold`** - `0001` (GC-roots daemon crash): reworked to `catch (BaseError&)` (the narrowing proposed in [#15963](NixOS/nix#15963) after xokdvium objected to `catch (...)`); a human resubmits. - `0002` (lookup-path EPERM eval fix): cleanest candidate, framed as restoring the EPERM-tolerant path probing of [#8240](NixOS/nix#8240) lost in the `std::filesystem` migration (see #5884, still-open #8485); a human submits. - `0003-0009` (build-status series): engage on edolstra's active [#15979](NixOS/nix#15979) (`nix ps`) instead of filing a competing series. **Patch rework + disclosure** - `packages/nix/nix/patches/0001` reworked (`catch (BaseError&)`, concise comment addressing xokdvium's "too verbose" note). - `Assisted-by: Claude <noreply@anthropic.com>` trailers added to `0001` and `0002` commit messages, regenerated deterministically via `git format-patch --zero-commit --no-signature --no-stat -N`. Build-status series bytes unchanged. - `upstream-status.json` regenerated (`nix run .#upstream-sync -- nix`): all nine now `hold`. **Incidental unblock** - `packages/nu-py/python/nu/__init__.py`: `os.getcwd()` → `Path.cwd()` — a pre-existing ruff PTH109 failure on main (from #1999) that blocked the pre-commit lint for every subsequent commit. ## Validation - `checks.aarch64-darwin.patched-src-nix` — green (all 9 patches apply, incl. reworked 0001). - `checks.aarch64-darwin.patch-dag-nix` — `OK (9 patches, 0 edges)`. - `nix run .#upstream-sync -- nix --dry-run` — plans **nothing** outward: all 9 patches `[skip]`, repo-level block reported, no `would-open`/`attempt-ready`. - Full `nix-ix` package build (validates the reworked C++ compiles) — running. ## Machinery gaps filed as issues - #2016 — semantic duplicate detection (title-token search missed #15979 vs our build-status subjects). - #2017 — series grouping (empty DAG deps ⇒ closure grouping never engages). - #2018 — `Assisted-by` commit-trailer support as policy-driven disclosure. ## Human handoff Ready-to-send PR drafts (0001, 0002) and the #15979 comment are in `~/Projects/indexable-inc/nix-upstream-handoff.html` for Andrew as the accountable submitter. **No PRs are filed on NixOS/nix by this change** — the kit is for a human to submit. Prepared with AI assistance (Claude); directed and reviewed by a human maintainer.    > [!NOTE] > ### Hold all NixOS/nix upstreaming and rework GC-roots patch to catch `BaseError&` > - Sets `aiPrsAllowed` to `false` in the upstream policy for NixOS/nix, blocking automated PR creation against that repo per [NixOS/nix#15984](NixOS/nix#15984). > - Changes all nine patch entries in [fork-packages.nix](https://github.com/indexable-inc/index/pull/2021/files#diff-35e5ff5b02c2dcd32c93fd770ad2194f9fcbfc9f35fdcbd56821891b6e7452f4) and [upstream-status.json](https://github.com/indexable-inc/index/pull/2021/files#diff-9407ed440a363a47ae2dbcca49731c8db3d7dfe38943d75473dd0f5c4fec28eb) from `attempt` to `hold`, deferring upstreaming to humans. > - Reworks [0001-fix-libstore-don-t-crash-the-daemon-when-a-GC-roots-.patch](https://github.com/indexable-inc/index/pull/2021/files#diff-2a703db65303f7f7d269ae802072e8f5e4a984d141b3ab175452f2051dd5d197) to catch `BaseError&` instead of `Error&`, so `Interrupted` exceptions no longer escape the GC client-root thread and crash the daemon; removes the earlier blanket `catch(...)` block. > >  > > <sup><a href="https://app.macroscope.com">Macroscope</a> summarized 00a7768.</sup> >  >

yorickvP requested a review from edolstra as a code owner April 19, 2023 13:48

github-actions Bot added the with-tests Issues related to testing. PRs with tests have some priority label Apr 19, 2023

fricklerhandwerk approved these changes Apr 19, 2023

View reviewed changes

yorickvP force-pushed the macos-sandbox branch from d5a6279 to c6dc98b Compare May 8, 2023 14:33

yorickvP requested a review from thufschmitt as a code owner May 11, 2023 11:09

fricklerhandwerk added release-blocker macos Nix on macOS, aka OS X, aka darwin labels May 11, 2023

thufschmitt self-assigned this May 12, 2023

Ericson2314 reviewed May 12, 2023

View reviewed changes

Comment thread src/libstore/globals.hh Outdated

thufschmitt suggested changes May 12, 2023

View reviewed changes

Comment thread src/libexpr/eval.cc Outdated

thufschmitt suggested changes May 12, 2023

View reviewed changes

yorickvP requested a review from thufschmitt May 26, 2023 13:35

infinisil and others added 4 commits May 26, 2023 15:36

Call getDefaultSSLCertFile() only when none is specified

b7cde90

This does pathExists on various paths, which crashes on EPERM in the macOS sandbox.

getDefaultNixPath: ignore EPERM

a6c78ba

create pathAccessible, use it to infer default dirs

2c46248

yorickvP force-pushed the macos-sandbox branch from ce0c31a to 2c46248 Compare May 26, 2023 13:36

thufschmitt approved these changes May 26, 2023

View reviewed changes

thufschmitt merged commit 940e9eb into NixOS:master May 26, 2023

purcell mentioned this pull request Jun 14, 2023

Nix terminates with uncaught exception of type nix::SysError: error: getting status of /etc/ssl/certs/ca-certificates.crt on Darwin with sandbox enabled #8485

Closed

emilazy mentioned this pull request Jun 18, 2023

[Backport 2.15-maintenance] ci: Always run with sandbox, even on Darwin #8535

Merged

andrewgazelka mentioned this pull request Jul 6, 2026

fork(nix): hold all upstreaming per NixOS/nix#15984; rework GC-roots fix to catch (BaseError&) indexable-inc/index-bak#2021

Merged

Navigation Menu

Uh oh!

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

ci: Always run with sandbox, even on Darwin#8240
thufschmitt merged 4 commits into
NixOS:masterfrom
tweag:macos-sandbox

yorickvP commented Apr 19, 2023

Uh oh!

edolstra commented Apr 24, 2023

Uh oh!

infinisil commented Apr 24, 2023

Uh oh!

yorickvP commented May 8, 2023

Uh oh!

yorickvP commented May 11, 2023

Uh oh!

Uh oh!

nixos-discourse commented May 12, 2023

Uh oh!

thufschmitt left a comment

Uh oh!

Uh oh!

thufschmitt left a comment

Uh oh!

nixos-discourse commented May 15, 2023

Uh oh!

thufschmitt left a comment

Uh oh!

reckenrode commented Jul 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

Conversation

yorickvP commented Apr 19, 2023

Motivation

Context

Checklist for maintainers

Priorities

Uh oh!

edolstra commented Apr 24, 2023

Uh oh!

infinisil commented Apr 24, 2023

Uh oh!

yorickvP commented May 8, 2023

Uh oh!

yorickvP commented May 11, 2023

Uh oh!

Uh oh!

nixos-discourse commented May 12, 2023

Uh oh!

thufschmitt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thufschmitt left a comment

Choose a reason for hiding this comment

Uh oh!

nixos-discourse commented May 15, 2023

Uh oh!

thufschmitt left a comment

Choose a reason for hiding this comment

Uh oh!

reckenrode commented Jul 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants