Skip to content
This repository was archived by the owner on May 14, 2024. It is now read-only.

Conversation

@pull
Copy link

@pull pull bot commented May 13, 2024

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

@pull pull bot added ⤵️ pull merge-conflict Resolve conflicts manually labels May 13, 2024
xokdvium and others added 27 commits January 5, 2026 20:57
This OOMs VM tests in CI and it's just wasteful.
tests/functional: Reduce max-call-depth for stack overflow tests
unstable has a patch that doesn't apply to 2.0.2
packaging: fix lowdown with overridden nixpkgs
…nlikely to do so (for the diff from 3.14.0 to 3.15.0)
Get rid of real store paths in docs/tests
Using fchmodat after a fstatat in deletePath has a slight TOCTOU
window. We can plug it by using fchmodat (the libc wrapper with
AT_SYMLINK_NOFOLLOW), but it tries fchmodat2 and falls back to the
O_PATH trick while failing when procfs isn't mounted. We can do a bit
better than that and also cache whether syscalls are unsupported to
avoid the repeated context switching that glibc would impose.

Also tests the fallback path. It's only for kernels older than 6.6 and
when procfs isn't accessible that we fall back to the racy fchmodat
without AT_SYMLINK_NOFOLLOW.

What previously used to be:

openat(AT_FDCWD, "/tmp/store-race/nix/var/nix/builds", O_RDONLY) = 11
newfstatat(11, "nix-2704212-84654554", {st_mode=S_IFDIR|000, st_size=3, ...}, AT_SYMLINK_NOFOLLOW) = 0
fchmodat(11, "nix-2704212-84654554", 040700) = 0

Is now a TOCTOU-free sequence of syscalls:

openat(AT_FDCWD, "/tmp/store-race/nix/var/nix/builds", O_RDONLY) = 11
newfstatat(11, "nix-2704953-1733606057", {st_mode=S_IFDIR|000, st_size=3, ...}, AT_SYMLINK_NOFOLLOW) = 0
fchmodat2(11, "nix-2704953-1733606057", 040700, AT_SYMLINK_NOFOLLOW) = 0

Or if the fchmodat2 is not supported:

openat(11, "nix-2705443-3010460784", O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_PATH) = 12
fstat(12, {st_mode=S_IFDIR|000, st_size=3, ...}) = 0
chmod("/proc/self/fd/12", 040700)       = 0
openat(11, "nix-2705443-3010460784", O_RDONLY|O_NOFOLLOW|O_DIRECTORY) = 12

This prevents a potentially arbitrary chmod that follows symlinks,
though the race window is very small. Also in the case that fchmodat2
isn't supported we could instead open the /proc/self/fd/N path instead
of using openat, but that's pretty much equivalent. We only care
about ensuring that the thing we chmodded wasn't a symlink since
fchmodat follows symlinks and the support for AT_SYMLINK_NOFOLLOW
in libc for that is pretty spotty on Linux. E.g. glibc fails if the
AT_SYMLINK_NOFOLLOW is specified and procfs isn't available even on
regular files. The patch also includes a test that uses a user namespace
on Linux to test this exact scenario (though it's rather exotic).
libutil: Implement unix::fchmodatTryNoFollow, use in deletePath
Fixes "error: cannot create symlink '.../tests/functional/result';
already exists".
Fix structured-attrs test failure in dev shell
BinaryCacheStoreConfig: Change localNarCache to std::filesystem::path
libutil-tests: fix openFileEnsureBeneathNoSymlinks.works on cygwin
Move {init,create}GitRepo to tests/functional/common/functions.sh
Previously builtins.readDir would return an empty attribute set
instead of barfing on non-existent paths. This is a regression from
2.32 for impure eval.
amaanq and others added 30 commits January 27, 2026 10:35
The test was checking for `$stdenv` but the `fixed` derivation doesn't
actually have stdenv, it just has `FOO`. I've updated it to check the
value of `FOO` instead.
tests/functional: fix nix-shell fixed-output derivation test
Currently, tests fail when the host system has `commit.gpgsign` or
`tag.gpgsign` enabled at the system level (in my case, a custom path
located at `/etc/git/config`), since the signing key is unavailable in
the test sandbox.

The tests set `HOME=$TEST_HOME` to isolate themselves, which bypasses
the user-level git config (`~/.gitconfig`). However, if a user sets the
system-level config via `GIT_CONFIG_GLOBAL` or `GIT_CONFIG_SYSTEM`, it
still applies, causing commits to fail when signing is enabled there.

In this PR, I explicitly set `GIT_CONFIG_GLOBAL` and `GIT_CONFIG_SYSTEM`
to `/dev/null` so that the user's system config is never read from or
written to. I've also replaced `git config --global protocol.file.allow
always` with `GIT_CONFIG_*` environment variables to avoid writing to
`/dev/null`.
The two settings `envKeepDerivations` and `upgradeNixStorePathUrl` were
only used in one command each, so it makes more sense to move them to
their own files. This commit moves them both into a small self-contained
settings struct and registers them with the global config.
tests/functional: isolate git tests from host signing config
…-settings

libstore: move command-specific settings to their own files
This cleans up the logic for checking if the worker's store is a valid
local store when we're not hooking it. If we have a local store, we then
pass that as an argument to `DerivationBuildingGoal::buildLocally`,
rather than checking inside the function itself.
libstore: decide how to build in one spot
We use a different fstat on posix and windows systems,
and not all fstat users were using the correct one.
Factor out fstat to make the change easier.

See also a13de50 for other stat
functions
Refactor fstat into a wrapper in libutil
The C++ rule of five suggests that when a custom destructor is needed
then several other functions are as well. The lack of those makes
certain operations challenging
libutil: add useful functions to Pid
build-result: Make `Failure` an alias for `BuildError`
This introduces a `PersonalityArgs` struct to pass named arguments to `setPersonality`. The `impersonateLinux26` setting is now passed from the call site rather than read from settings inside the function.
libstore: add `PersonalityArgs` struct for `setPersonality`
Due to a typo in quoteRegexChars, finding runtime garbage collection roots
was failing on paths that contained a dot, or any other regex chars that would
have to be replaced.

When fixing that error, also add tests to make sure gc continues to
work.
libstore: fix runtime gc on non-standard store paths
This test insisted on placing profiles in NIX_STATE_DIR, but all
packages were removed from the profile immediately after so they did not
act as garbage collector roots. Switch to directly calling nix-build,
allowing the test to run in VMs without NIX_STATE_DIR.
Support gc-runtime functional tests in VMs
The explicit serializer added in
bfdd124 is the right place to adjust
values for sake of wire protocol compat. The protocol-agnostic `Worker`
code where it was before is the wrong spot.

(That spot was originally chosen because the back compat logic predates
having an explicit serializer for this data type to use instead.)

Co-authored-by: John Ericson <[email protected]>
This commit consolidates the four separate boolean flags
(`permanentFailure`, `timedOut`, `hashMismatch`, & `checkMismatch`) into
a single `ExitStatusFlags` struct with methods for computing exit status
codes and updating from failure status.
libstore: introduce `ExitStatusFlags` for exit status computation
…egrity check

aws-sdk-cpp used to include a checksum for uploads (CRC64 since ~September 2025).
Content-MD5 [1] should be universally supported by all s3 compatible services, since the SDK used
to include it unconditionally too.

[1]: https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObject.html
We now support `LISTEN_FDS` values greater than 1, per the systemd
socket activation spec.

These changes are by @edolstra, taken from #5265. This is just that PR
*without* the TCP parts, which I gathered are the controversial parts.
Hopefully this remainder is not so controversial.

Review with indentation ignored, because some code has moved inside a
new loop but otherwise is mostly unchanged.
libstore/s3-binary-cache-store: Add Content-MD5 header as message int…
Support systemd socket activation with multiple sockets
nix-store: fsync generated key files
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

⤵️ pull merge-conflict Resolve conflicts manually

Projects

None yet

Development

Successfully merging this pull request may close these issues.