postgresqlTestHook: add sandbox profile for Darwin#383377
postgresqlTestHook: add sandbox profile for Darwin#383377tie wants to merge 3 commits intoNixOS:masterfrom
Conversation
|
Is this related to #371242? If yes, we need to add this in more places than just the test hook. |
There was a problem hiding this comment.
Is this related to #371242?
I don’t think so. I’m not aware of this issue, but it seems to mention that builds fail on Hydra and IIRC macOS builders do not use sandbox (though things could have changed since the last time I’ve checked).
Forcing mmap implementation instead of POSIX/SysV shared memory might help as discussed here, but I don’t have time to test that.
There was a problem hiding this comment.
If it's not about that, what is it about then? What kind of error are you getting that is solved by doing this?
There was a problem hiding this comment.
Well, postgresqlTestHook does not work when sandbox = true in Nix config on macOS because default sandbox profile does not allow shared memory syscalls.
running bootstrap script ... 2025-02-19 14:07:08.362 UTC [98344] FATAL: could not create shared memory segment: Operation not permitted
2025-02-19 14:07:08.362 UTC [98344] DETAIL: Failed system call was shmget(key=80109247, size=56, 03600).
(note Operation not permitted at the end of first line)
I’ve just tried running initdb with various combinations of -c shared_memory_type=VALUE and -c dynamic_shared_memory_type=VALUE and none worked, so I’ll remove this TODO comment.
There was a problem hiding this comment.
Unfortunately, I can't reproduce FATAL: could not create shared memory segment: Operation not permitted at all.
Can you give me exact instructions, i.e. which branch, which attribute to build?
There was a problem hiding this comment.
aarch64-darwinhostsandbox = trueset in/etc/nix/nix.confsudo launchctl kickstart -k system/org.nixos.nix-daemon
to restart Nix daemon ifsandbox = truewas not there before- Nix 2.18.1
- I’ve doubled-checked with Nix version 2.24.12 and this seems to be fixed darwin: allow ipc-sysv* in sandbox nix#10878, but older Nix version still require an explicit sandbox profile.
nix build --print-build-logs --no-link --impure --expr 'with import ./. { };
stdenv.mkDerivation {
name = "postgresql-test-hook-repro";
dontUnpack = true;
doCheck = true;
nativeCheckInputs = [
postgresqlTestHook
postgresql
];
installPhase = ''
runHook preInstall
mkdir -p "$out"
runHook postInstall
'';
}'
There was a problem hiding this comment.
Thanks for those details. Unfortunately, I can't test with older nix versions, because I only have non-root access to the nix-community darwin builder. But your explanation sounds sensible.
Not being able to reproduce this makes it harder for me to come up with fixes for other cases where we need the same - and I know there are some.
One major case that we surely need to address in this PR already: postgresql itself. It also runs initdb during the check phase and since we have enabled the check phase on darwin a while ago, you would hit the same. It would be kind of pointless to fix the test hook, but not being able to build postgresql itself before even getting there.
I assume you just didn't rebuild postgresql locally, yet, but have always pulled it from the cache. You should be able to reproduce it with nix-build --check -A postgresql easily.
Could you please also extend the comment to mention the version this has been fixed in and that we need to keep it around for older versions, which we still support?
There was a problem hiding this comment.
I’ve doubled-checked with Nix version 2.24.12 and this seems to be fixed
Also, lix did not backport that change (yet), so running lix on MacOS will lead to the same problems.
fbad5ef to
a42d765
Compare
| export PGSSLMODE=${PGSSLMODE:-disable} | ||
|
|
There was a problem hiding this comment.
I am curious: Did you hit a case where that was necessary to fix a build? Or is this another kind of performance improvement?
|
Converted to draft, because currently the check phase for PostgreSQL is disabled on master and I'd like to mark the postgresqlTestHook as broken for darwin, too. See #392430. Even though newer nix versions support IPC in the sandbox, they still don't clean up properly, so it still leads to build failures eventually. Now I wonder: If this bug is fixed in Nix at some point and those segments are cleaned up - should we then change the sandbox profile as suggested here? I don't think so, because that would not give us the proper IPC object cleanup. Thus, I'd rather have this fail before those segments are created on older nix versions. We could still do the second and third commit here. WDYT @tie? |
|
Completely missed the ping here. FWIW, I think allowing SysV IPC in the sandbox is not a great idea, unless it can be restricted further, which would require more research. Is there a way to get Postgres to… not do that? |
Adds sandbox profile so that PostgreSQL can set up shared memory. Also makes it run faster by skipping sync during initdb and using immediate shutdown mode.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.