From 22f993fab6aaf2e77cac7ac9cc1bb3dcb35e8685 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Mon, 8 Dec 2025 01:10:14 +0300 Subject: [PATCH 1/3] libutil: Get rid of TODO comments for O_CLOEXEC By default windows doesn't allow inheriting handles anyway. These comments are just confusing at this point. --- src/libutil/file-system.cc | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 0ad6d9285..4851d8cfb 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -260,8 +260,7 @@ std::string readFile(const Path & path) AutoCloseFD fd = toDescriptor(open( path.c_str(), O_RDONLY -// TODO -#ifndef _WIN32 +#ifdef O_CLOEXEC | O_CLOEXEC #endif )); @@ -294,8 +293,7 @@ void readFile(const Path & path, Sink & sink, bool memory_map) AutoCloseFD fd = toDescriptor(open( path.c_str(), O_RDONLY -// TODO -#ifndef _WIN32 +#ifdef O_CLOEXEC | O_CLOEXEC #endif )); @@ -309,8 +307,7 @@ void writeFile(const Path & path, std::string_view s, mode_t mode, FsSync sync) AutoCloseFD fd = toDescriptor(open( path.c_str(), O_WRONLY | O_TRUNC | O_CREAT -// TODO -#ifndef _WIN32 +#ifdef O_CLOEXEC | O_CLOEXEC #endif , @@ -344,8 +341,7 @@ void writeFile(const Path & path, Source & source, mode_t mode, FsSync sync) AutoCloseFD fd = toDescriptor(open( path.c_str(), O_WRONLY | O_TRUNC | O_CREAT -// TODO -#ifndef _WIN32 +#ifdef O_CLOEXEC | O_CLOEXEC #endif , From b9b6defca693e6413668647d216e3aa3c90a7465 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Mon, 8 Dec 2025 02:48:35 +0300 Subject: [PATCH 2/3] nix nar {ls,cat}: Optimize The whole NarAccessor -> listing -> lazy NarAccessor is very weird. Source can now be seek-ed over when supported, so we can support it pretty easily. Alternatively we could also make it single-pass very easily with a custom FileSystemObjectSink. It will get removed in a follow-up commit anyway. --- src/libutil/include/nix/util/nar-accessor.hh | 8 ++++ src/libutil/nar-accessor.cc | 43 ++++++++++++++------ src/nix/cat.cc | 4 +- src/nix/ls.cc | 4 +- 4 files changed, 41 insertions(+), 18 deletions(-) diff --git a/src/libutil/include/nix/util/nar-accessor.hh b/src/libutil/include/nix/util/nar-accessor.hh index 5488f21b5..745c79f60 100644 --- a/src/libutil/include/nix/util/nar-accessor.hh +++ b/src/libutil/include/nix/util/nar-accessor.hh @@ -32,8 +32,16 @@ using GetNarBytes = std::function; */ GetNarBytes seekableGetNarBytes(const Path & path); +GetNarBytes seekableGetNarBytes(Descriptor fd); + ref makeLazyNarAccessor(const nlohmann::json & listing, GetNarBytes getNarBytes); +/** + * Creates a NAR accessor from a given stream and a GetNarBytes getter. + * @param source Consumed eagerly. References to it are not persisted in the resulting SourceAccessor. + */ +ref makeLazyNarAccessor(Source & source, GetNarBytes getNarBytes); + struct NarListingRegularFile { /** diff --git a/src/libutil/nar-accessor.cc b/src/libutil/nar-accessor.cc index 22b6abdd5..5644ca408 100644 --- a/src/libutil/nar-accessor.cc +++ b/src/libutil/nar-accessor.cc @@ -111,6 +111,7 @@ struct NarAccessor : public SourceAccessor path, NarMember{.stat = {.type = Type::tRegular, .fileSize = 0, .isExecutable = false, .narOffset = 0}}); NarMemberConstructor nmc{nm, pos}; + nmc.skipContents = true; /* Don't care about contents. */ func(nmc); } @@ -141,6 +142,13 @@ struct NarAccessor : public SourceAccessor parseDump(indexer, indexer); } + NarAccessor(Source & source, GetNarBytes getNarBytes) + : getNarBytes(std::move(getNarBytes)) + { + NarIndexer indexer(*this, source); + parseDump(indexer, indexer); + } + NarAccessor(const nlohmann::json & listing, GetNarBytes getNarBytes) : getNarBytes(getNarBytes) { @@ -249,24 +257,35 @@ ref makeLazyNarAccessor(const nlohmann::json & listing, GetNarBy return make_ref(listing, getNarBytes); } +ref makeLazyNarAccessor(Source & source, GetNarBytes getNarBytes) +{ + return make_ref(source, getNarBytes); +} + GetNarBytes seekableGetNarBytes(const Path & path) { - return [path](uint64_t offset, uint64_t length) { - AutoCloseFD fd = toDescriptor(open( - path.c_str(), - O_RDONLY -#ifndef _WIN32 - | O_CLOEXEC + AutoCloseFD fd = toDescriptor(open( + path.c_str(), + O_RDONLY +#ifdef O_CLOEXEC + | O_CLOEXEC #endif - )); - if (!fd) - throw SysError("opening NAR cache file '%s'", path); + )); + if (!fd) + throw SysError("opening NAR cache file '%s'", path); - if (lseek(fromDescriptorReadOnly(fd.get()), offset, SEEK_SET) != (off_t) offset) - throw SysError("seeking in '%s'", path); + return [inner = seekableGetNarBytes(fd.get()), fd = make_ref(std::move(fd))]( + uint64_t offset, uint64_t length) { return inner(offset, length); }; +} + +GetNarBytes seekableGetNarBytes(Descriptor fd) +{ + return [fd](uint64_t offset, uint64_t length) { + if (::lseek(fromDescriptorReadOnly(fd), offset, SEEK_SET) == -1) + throw SysError("seeking in file"); std::string buf(length, 0); - readFull(fd.get(), buf.data(), length); + readFull(fd, buf.data(), length); return buf; }; diff --git a/src/nix/cat.cc b/src/nix/cat.cc index 812dfdbcf..f0dfc3ee6 100644 --- a/src/nix/cat.cc +++ b/src/nix/cat.cc @@ -79,9 +79,7 @@ struct CmdCatNar : StoreCommand, MixCat if (!fd) throw SysError("opening NAR file '%s'", narPath); auto source = FdSource{fd.get()}; - auto narAccessor = makeNarAccessor(source); - nlohmann::json listing = listNarDeep(*narAccessor, CanonPath::root); - cat(makeLazyNarAccessor(listing, seekableGetNarBytes(narPath)), CanonPath{path}); + cat(makeLazyNarAccessor(source, seekableGetNarBytes(fd.get())), CanonPath{path}); } }; diff --git a/src/nix/ls.cc b/src/nix/ls.cc index fd4a98d7a..66f52a18a 100644 --- a/src/nix/ls.cc +++ b/src/nix/ls.cc @@ -154,9 +154,7 @@ struct CmdLsNar : Command, MixLs if (!fd) throw SysError("opening NAR file '%s'", narPath); auto source = FdSource{fd.get()}; - auto narAccessor = makeNarAccessor(source); - nlohmann::json listing = listNarDeep(*narAccessor, CanonPath::root); - list(makeLazyNarAccessor(listing, seekableGetNarBytes(narPath)), CanonPath{path}); + list(makeLazyNarAccessor(source, seekableGetNarBytes(fd.get())), CanonPath{path}); } }; From c5c05e44b315836463f2f510a0e850b1acb95511 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Mon, 8 Dec 2025 02:54:13 +0300 Subject: [PATCH 3/3] Make nix nar cat work on pipes too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was lost after 2.32 while making the accessor lazy. We can restore the support for it pretty easily. Also this is significant optimization for nix nar cat. E.g. with a NAR of a linux repo this speeds up by ~3x: Benchmark 1: nix nar cat /tmp/linux.nar README Time (mean ± σ): 737.2 ms ± 5.6 ms [User: 298.1 ms, System: 435.7 ms] Range (min … max): 728.6 ms … 746.9 ms 10 runs Benchmark 2: build/src/nix/nix nar cat /tmp/linux.nar README Time (mean ± σ): 253.5 ms ± 2.9 ms [User: 56.4 ms, System: 196.3 ms] Range (min … max): 248.1 ms … 258.7 ms 12 runs --- src/libutil/include/nix/util/fs-sink.hh | 2 +- src/nix/cat.cc | 36 ++++++++++++++++++++++++- tests/functional/nar-access.sh | 2 ++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/libutil/include/nix/util/fs-sink.hh b/src/libutil/include/nix/util/fs-sink.hh index 60e8441dd..bd9c7205f 100644 --- a/src/libutil/include/nix/util/fs-sink.hh +++ b/src/libutil/include/nix/util/fs-sink.hh @@ -12,7 +12,7 @@ namespace nix { * * See `FileSystemObjectSink::createRegularFile`. */ -struct CreateRegularFileSink : Sink +struct CreateRegularFileSink : virtual Sink { /** * If set to true, the sink will not be called with the contents diff --git a/src/nix/cat.cc b/src/nix/cat.cc index f0dfc3ee6..c1d73f2a2 100644 --- a/src/nix/cat.cc +++ b/src/nix/cat.cc @@ -1,5 +1,6 @@ #include "nix/cmd/command.hh" #include "nix/store/store-api.hh" +#include "nix/util/archive.hh" #include "nix/util/nar-accessor.hh" #include "nix/util/serialise.hh" #include "nix/util/source-accessor.hh" @@ -79,7 +80,40 @@ struct CmdCatNar : StoreCommand, MixCat if (!fd) throw SysError("opening NAR file '%s'", narPath); auto source = FdSource{fd.get()}; - cat(makeLazyNarAccessor(source, seekableGetNarBytes(fd.get())), CanonPath{path}); + + struct CatRegularFileSink : NullFileSystemObjectSink + { + CanonPath neededPath = CanonPath::root; + bool found = false; + + void createRegularFile(const CanonPath & path, std::function crf) override + { + struct : CreateRegularFileSink, FdSink + { + void isExecutable() override {} + } crfSink; + + crfSink.fd = INVALID_DESCRIPTOR; + + if (path == neededPath) { + logger->stop(); + crfSink.skipContents = false; + crfSink.fd = STDOUT_FILENO; + found = true; + } else { + crfSink.skipContents = true; + } + + crf(crfSink); + } + } sink; + + sink.neededPath = CanonPath(path); + /* NOTE: We still parse the whole file to validate that it's a correct NAR. */ + parseDump(sink, source); + + if (!sink.found) + throw Error("NAR does not contain regular file '%1%'", path); } }; diff --git a/tests/functional/nar-access.sh b/tests/functional/nar-access.sh index 2b0a6a329..cd419b4ee 100755 --- a/tests/functional/nar-access.sh +++ b/tests/functional/nar-access.sh @@ -23,6 +23,8 @@ diff -u data.cat-nar "$storePath/foo/data" # Check that file contents of baz match. nix nar cat "$narFile" /foo/baz > baz.cat-nar diff -u baz.cat-nar "$storePath/foo/baz" +nix nar cat /dev/stdin /foo/baz < "$narFile" > baz.cat-nar-pipe +expect 1 nix nar cat "$narFile" /foo/baz/doesntexist 2>&1 | grep "NAR does not contain regular file '/foo/baz/doesntexist'" nix store cat "$storePath/foo/baz" > baz.cat-nar diff -u baz.cat-nar "$storePath/foo/baz"