More file system function improvements by Ericson2314 · Pull Request #15244 · NixOS/nix

Ericson2314 · 2026-02-15T17:29:55Z

It keeps growing bigger and bigger! See the commit message

dpulls · 2026-02-15T17:33:00Z

🎉 All dependencies have been resolved !

xokdvium · 2026-02-27T16:47:44Z

    if (std::filesystem::exists(destDir))
        throw Error("cannot clone into existing path %s", PathFmt(destDir));


Doesn't this prevent the case where the destination exists and is a symlink?

Ah right, it follows symlinks. We should add a test for this I suppose

xokdvium · 2026-02-27T16:56:35Z

-            /* O_EXCL together with O_CREAT ensures symbolic links in the last
-               component are not followed. */


Can we leave this comment in place?

xokdvium · 2026-04-12T08:25:57Z

+outcome::unchecked<AutoCloseFD, std::error_code>
+openDirectoryAt(Descriptor dirFd, const std::filesystem::path & path, bool create, mode_t mode)
+{
+    assert(path.is_relative());


There's a slight regression there that the path can now contain ... That shouldn't regress I think?

xokdvium · 2026-04-12T08:27:22Z

 }

-OsString readLinkAt(Descriptor dirFd, const CanonPath & path)
+OsString readLinkAt(Descriptor dirFd, const std::filesystem::path & path)


I'm not sure we want to use std::filesystem::path for these functions. Maybe OsCanonPath or something?

Both are newtype around `std::filesystem::path`s. `OsFilename` is a single filename that is not something special like `.` or `..`, and `OsCanonPath` is for canonical relative paths suitable for `openat` and Windows NT equivalents. `OsCanonPath` provides an iterator yielding `OsFilename` references for each component, and four `operator/` overloads covering all combinations of `OsCanonPath` and `OsFilename`. Convert `readLinkAt`, `openFileEnsureBeneathNoSymlinks`, and `unix::fchmodatTryNoFollow` from `const CanonPath &` to `const OsCanonPath &`. Existing call sites work via implicit `CanonPath -> OsCanonPath` conversion. Change `SymlinkNotAllowed::path` from `CanonPath` to `std::variant<CanonPath, std::filesystem::path>`, with constructor overloads for both types including template variants for custom error messages. `writeBuilderFile` now takes `OsFilename` instead of `const std::string &`, which implicitly converts to `OsCanonPath` for `openFileEnsureBeneathNoSymlinks`, replacing the manual `std::filesystem::path` normalization assertion.

Use `OsCanonPath` and `OsFilename` throughout the `*At` functions and their callers, replacing both `CanonPath` and `std::filesystem::path` parameters. - FD-based symlink creation on Unix and Windows with wrappers (`createFileSymlinkAt`, `createDirectorySymlinkAt`, `createUnknownSymlinkAt`), all taking `OsCanonPath`. Windows distinguishes between file and directory symlinks. Directory symlinks can be traversed as path components, file symlinks cannot. `createUnknownSymlinkAt` opens the symlink's parent directory and tries to open the target with `FILE_DIRECTORY_FILE` to determine if it's a directory, with `fstat` as defense in depth. This avoids relying on `descriptorToPath` which doesn't work correctly in Wine. `createUnknownSymlinkAt` is a poor solution, and a replacement for NARs should be careful to store enough information to avoid needing it at unpack time, but it is the best we can do for now. - Make `RestoreSink` more cross-platform: use `openDirectoryAt` for directory creation, `createUnknownSymlinkAt` for symlinks, and `openFileEnsureBeneathNoSymlinks` for intermediate paths on both Unix and Windows. Remove the Windows-only `std::filesystem` / `CreateFileW` / `nix::createSymlink` code paths that bypassed fd-based symlink safety. `getParentFdAndName` now returns `OsFilename` instead of `CanonPath`. - Validate `OsCanonPath` uses preferred separators on Windows (no forward slashes). Remove `lexically_normal().make_preferred()` calls from Windows `*At` implementations, since `OsCanonPath` now guarantees native separators. - Fix Wine `openFileEnsureBeneathNoSymlinks` final-component handling: check for symlinks on `ERROR_DIRECTORY` (suspected Wine-specific behavior when opening directory symlinks with `FILE_OPEN_REPARSE_POINT | FILE_DIRECTORY_FILE`). - Fix Windows error propagation in `openFileEnsureBeneathNoSymlinks`: call `SetLastError(lastError)` before returning an invalid handle, so that callers can inspect the Win32 error code. Previously the NTSTATUS was converted via `RtlNtStatusToDosError` but never stored, so `GetLastError()` returned 0. - Add `ThrowsWinError` gmock matcher. Use `NativeSysError` in test helpers so the right exception type is thrown per platform. - Update tests to use `OsFilename`/`CanonPath` instead of bare `std::filesystem::path` for `*At` function calls. Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>

Ericson2314 requested a review from edolstra as a code owner February 15, 2026 17:29

Ericson2314 force-pushed the windows-fs-sink-fixes branch from 13edb5e to 35e7906 Compare February 15, 2026 17:32

Ericson2314 force-pushed the windows-fs-sink-fixes branch 7 times, most recently from c3e9b54 to eaf0a71 Compare February 16, 2026 23:12

amaanq mentioned this pull request Feb 17, 2026

tests: quote PATH in external-builders test heredoc #15267

Merged