From d0a23238294198f6702e13d117f75af89dbeac62 Mon Sep 17 00:00:00 2001 From: Seth Flynn Date: Tue, 27 May 2025 22:20:53 -0400 Subject: [PATCH 01/66] lockFlake(): Allow registry lookups for overridden inputs Fixes #13144 --- src/libflake/flake.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/libflake/flake.cc b/src/libflake/flake.cc index fc27df887..24252d710 100644 --- a/src/libflake/flake.cc +++ b/src/libflake/flake.cc @@ -570,7 +570,7 @@ LockedFlake lockFlake( /* Get the input flake, resolve 'path:./...' flakerefs relative to the parent flake. */ - auto getInputFlake = [&](const FlakeRef & ref) + auto getInputFlake = [&](const FlakeRef & ref, const fetchers::UseRegistries useRegistries) { if (auto resolvedPath = resolveRelativePath()) { return readFlake(state, ref, ref, ref, *resolvedPath, inputAttrPath); @@ -578,7 +578,7 @@ LockedFlake lockFlake( return getFlake( state, ref, - useRegistriesInputs, + useRegistries, inputAttrPath); } }; @@ -660,7 +660,7 @@ LockedFlake lockFlake( } if (mustRefetch) { - auto inputFlake = getInputFlake(oldLock->lockedRef); + auto inputFlake = getInputFlake(oldLock->lockedRef, useRegistriesInputs); nodePaths.emplace(childNode, inputFlake.path.parent()); computeLocks(inputFlake.inputs, childNode, inputAttrPath, oldLock, followsPrefix, inputFlake.path, false); @@ -685,10 +685,11 @@ LockedFlake lockFlake( nuked the next time we update the lock file. That is, overrides are sticky unless you use --no-write-lock-file. */ - auto ref = (input2.ref && explicitCliOverrides.contains(inputAttrPath)) ? *input2.ref : *input.ref; + auto inputIsOverride = explicitCliOverrides.contains(inputAttrPath); + auto ref = (input2.ref && inputIsOverride) ? *input2.ref : *input.ref; if (input.isFlake) { - auto inputFlake = getInputFlake(*input.ref); + auto inputFlake = getInputFlake(*input.ref, inputIsOverride ? fetchers::UseRegistries::All : useRegistriesInputs); auto childNode = make_ref( inputFlake.lockedRef, From 94bbaddb93988352645e9b4bb4bec6c5ed730747 Mon Sep 17 00:00:00 2001 From: Philipp Otterbein Date: Sun, 18 May 2025 03:53:50 +0200 Subject: [PATCH 02/66] symbol-table: reduce memory usage and use boost::unordered_flat_set --- src/libexpr/eval.cc | 8 +- .../include/nix/expr/print-ambiguous.hh | 1 + src/libexpr/include/nix/expr/symbol-table.hh | 240 ++++++++++++++---- src/libexpr/include/nix/expr/value.hh | 7 +- src/libexpr/nixexpr.cc | 2 +- 5 files changed, 199 insertions(+), 59 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 1a067e75c..123f0e86a 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2,6 +2,7 @@ #include "nix/expr/eval-settings.hh" #include "nix/expr/primops.hh" #include "nix/expr/print-options.hh" +#include "nix/expr/symbol-table.hh" #include "nix/util/exit.hh" #include "nix/util/types.hh" #include "nix/util/util.hh" @@ -918,6 +919,11 @@ void Value::mkStringMove(const char * s, const NixStringContext & context) mkString(s, encodeContext(context)); } +void Value::mkString(const SymbolStr & s) +{ + mkString(s.c_str(), nullptr); +} + void Value::mkPath(const SourcePath & path) { mkPath(&*path.accessor, makeImmutableString(path.path.abs())); @@ -3019,7 +3025,7 @@ void EvalState::printStatistics() // XXX: overrides earlier assignment topObj["symbols"] = json::array(); auto &list = topObj["symbols"]; - symbols.dump([&](const std::string & s) { list.emplace_back(s); }); + symbols.dump([&](std::string_view s) { list.emplace_back(s); }); } if (outPath == "-") { std::cerr << topObj.dump(2) << std::endl; diff --git a/src/libexpr/include/nix/expr/print-ambiguous.hh b/src/libexpr/include/nix/expr/print-ambiguous.hh index 09a849c49..9e5a27e6d 100644 --- a/src/libexpr/include/nix/expr/print-ambiguous.hh +++ b/src/libexpr/include/nix/expr/print-ambiguous.hh @@ -1,6 +1,7 @@ #pragma once #include "nix/expr/value.hh" +#include "nix/expr/symbol-table.hh" namespace nix { diff --git a/src/libexpr/include/nix/expr/symbol-table.hh b/src/libexpr/include/nix/expr/symbol-table.hh index c04cc041b..994bddd6c 100644 --- a/src/libexpr/include/nix/expr/symbol-table.hh +++ b/src/libexpr/include/nix/expr/symbol-table.hh @@ -1,51 +1,35 @@ #pragma once ///@file -#include -#include -#include - -#include "nix/util/types.hh" +#include +#include "nix/expr/value.hh" #include "nix/util/chunked-vector.hh" #include "nix/util/error.hh" +#include +#define USE_FLAT_SYMBOL_SET (BOOST_VERSION >= 108100) +#if USE_FLAT_SYMBOL_SET +# include +#else +# include +#endif + namespace nix { -/** - * This class mainly exists to give us an operator<< for ostreams. We could also - * return plain strings from SymbolTable, but then we'd have to wrap every - * instance of a symbol that is fmt()ed, which is inconvenient and error-prone. - */ -class SymbolStr +class SymbolValue : protected Value { + friend class SymbolStr; friend class SymbolTable; -private: - const std::string * s; + uint32_t size_; + uint32_t idx; - explicit SymbolStr(const std::string & symbol): s(&symbol) {} + SymbolValue() = default; public: - bool operator == (std::string_view s2) const + operator std::string_view() const noexcept { - return *s == s2; - } - - const char * c_str() const - { - return s->c_str(); - } - - operator const std::string_view () const - { - return *s; - } - - friend std::ostream & operator <<(std::ostream & os, const SymbolStr & symbol); - - bool empty() const - { - return s->empty(); + return {payload.string.c_str, size_}; } }; @@ -56,24 +40,155 @@ public: */ class Symbol { + friend class SymbolStr; friend class SymbolTable; private: uint32_t id; - explicit Symbol(uint32_t id): id(id) {} + explicit Symbol(uint32_t id) noexcept : id(id) {} public: - Symbol() : id(0) {} + Symbol() noexcept : id(0) {} - explicit operator bool() const { return id > 0; } + [[gnu::always_inline]] + explicit operator bool() const noexcept { return id > 0; } - auto operator<=>(const Symbol other) const { return id <=> other.id; } - bool operator==(const Symbol other) const { return id == other.id; } + auto operator<=>(const Symbol other) const noexcept { return id <=> other.id; } + bool operator==(const Symbol other) const noexcept { return id == other.id; } friend class std::hash; }; +/** + * This class mainly exists to give us an operator<< for ostreams. We could also + * return plain strings from SymbolTable, but then we'd have to wrap every + * instance of a symbol that is fmt()ed, which is inconvenient and error-prone. + */ +class SymbolStr +{ + friend class SymbolTable; + + constexpr static size_t chunkSize{8192}; + using SymbolValueStore = ChunkedVector; + + const SymbolValue * s; + + struct Key + { + using HashType = boost::hash; + + SymbolValueStore & store; + std::string_view s; + std::size_t hash; + std::pmr::polymorphic_allocator & alloc; + + Key(SymbolValueStore & store, std::string_view s, std::pmr::polymorphic_allocator & stringAlloc) + : store(store) + , s(s) + , hash(HashType{}(s)) + , alloc(stringAlloc) {} + }; + +public: + SymbolStr(const SymbolValue & s) noexcept : s(&s) {} + + SymbolStr(const Key & key) + { + auto size = key.s.size(); + if (size >= std::numeric_limits::max()) { + throw Error("Size of symbol exceeds 4GiB and cannot be stored"); + } + // for multi-threaded implementations: lock store and allocator here + const auto & [v, idx] = key.store.add(SymbolValue{}); + if (size == 0) { + v.mkString("", nullptr); + } else { + auto s = key.alloc.allocate(size + 1); + memcpy(s, key.s.data(), size); + s[size] = '\0'; + v.mkString(s, nullptr); + } + v.size_ = size; + v.idx = idx; + this->s = &v; + } + + bool operator == (std::string_view s2) const noexcept + { + return *s == s2; + } + + [[gnu::always_inline]] + const char * c_str() const noexcept + { + return s->payload.string.c_str; + } + + [[gnu::always_inline]] + operator std::string_view () const noexcept + { + return *s; + } + + friend std::ostream & operator <<(std::ostream & os, const SymbolStr & symbol); + + [[gnu::always_inline]] + bool empty() const noexcept + { + return s->size_ == 0; + } + + [[gnu::always_inline]] + size_t size() const noexcept + { + return s->size_; + } + + explicit operator Symbol() const noexcept + { + return Symbol{s->idx + 1}; + } + + struct Hash + { + using is_transparent = void; + using is_avalanching = std::true_type; + + std::size_t operator()(SymbolStr str) const + { + return Key::HashType{}(*str.s); + } + + std::size_t operator()(const Key & key) const noexcept + { + return key.hash; + } + }; + + struct Equal + { + using is_transparent = void; + + bool operator()(SymbolStr a, SymbolStr b) const noexcept + { + // strings are unique, so that a pointer comparison is OK + return a.s == b.s; + } + + bool operator()(SymbolStr a, const Key & b) const noexcept + { + return a == b.s; + } + + [[gnu::always_inline]] + bool operator()(const Key & a, SymbolStr b) const noexcept + { + return operator()(b, a); + } + }; +}; + /** * Symbol table used by the parser and evaluator to represent and look * up identifiers and attributes efficiently. @@ -82,29 +197,46 @@ class SymbolTable { private: /** - * Map from string view (backed by ChunkedVector) -> offset into the store. + * SymbolTable is an append only data structure. + * During its lifetime the monotonic buffer holds all strings and nodes, if the symbol set is node based. + */ + std::pmr::monotonic_buffer_resource buffer; + std::pmr::polymorphic_allocator stringAlloc{&buffer}; + SymbolStr::SymbolValueStore store{16}; + + /** + * Transparent lookup of string view for a pointer to a ChunkedVector entry -> return offset into the store. * ChunkedVector references are never invalidated. */ - std::unordered_map symbols; - ChunkedVector store{16}; +#if USE_FLAT_SYMBOL_SET + boost::unordered_flat_set symbols{SymbolStr::chunkSize}; +#else + using SymbolValueAlloc = std::pmr::polymorphic_allocator; + boost::unordered_set symbols{SymbolStr::chunkSize, {&buffer}}; +#endif public: /** * Converts a string into a symbol. */ - Symbol create(std::string_view s) - { + Symbol create(std::string_view s) { // Most symbols are looked up more than once, so we trade off insertion performance // for lookup performance. // FIXME: make this thread-safe. - auto it = symbols.find(s); - if (it != symbols.end()) - return Symbol(it->second + 1); + return [&](T && key) -> Symbol { + if constexpr (requires { symbols.insert(key); }) { + auto [it, _] = symbols.insert(key); + return Symbol(*it); + } else { + auto it = symbols.find(key); + if (it != symbols.end()) + return Symbol(*it); - const auto & [rawSym, idx] = store.add(s); - symbols.emplace(rawSym, idx); - return Symbol(idx + 1); + it = symbols.emplace(key).first; + return Symbol(*it); + } + }(SymbolStr::Key{store, s, stringAlloc}); } std::vector resolve(const std::vector & symbols) const @@ -118,12 +250,14 @@ public: SymbolStr operator[](Symbol s) const { - if (s.id == 0 || s.id > store.size()) + uint32_t idx = s.id - uint32_t(1); + if (idx >= store.size()) unreachable(); - return SymbolStr(store[s.id - 1]); + return store[idx]; } - size_t size() const + [[gnu::always_inline]] + size_t size() const noexcept { return store.size(); } @@ -147,3 +281,5 @@ struct std::hash return std::hash{}(s.id); } }; + +#undef USE_FLAT_SYMBOL_SET diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index e9cc1cd3f..3a96577a2 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -5,7 +5,6 @@ #include #include "nix/expr/eval-gc.hh" -#include "nix/expr/symbol-table.hh" #include "nix/expr/value/context.hh" #include "nix/util/source-path.hh" #include "nix/expr/print-options.hh" @@ -65,6 +64,7 @@ struct ExprLambda; struct ExprBlackHole; struct PrimOp; class Symbol; +class SymbolStr; class PosIdx; struct Pos; class StorePath; @@ -331,10 +331,7 @@ public: void mkStringMove(const char * s, const NixStringContext & context); - inline void mkString(const SymbolStr & s) - { - mkString(s.c_str()); - } + void mkString(const SymbolStr & s); void mkPath(const SourcePath & path); void mkPath(std::string_view path); diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 1a71096d4..92071b22d 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -606,7 +606,7 @@ void ExprLambda::setDocComment(DocComment docComment) { size_t SymbolTable::totalSize() const { size_t n = 0; - dump([&] (const std::string & s) { n += s.size(); }); + dump([&] (SymbolStr s) { n += s.size(); }); return n; } From ed4e512dcd4969836663611c57b6e9903c48c5b2 Mon Sep 17 00:00:00 2001 From: Philipp Otterbein Date: Sun, 1 Jun 2025 21:00:01 +0200 Subject: [PATCH 03/66] symbol-table: reference entries instead of allocating `Value`s --- src/libexpr/eval.cc | 10 +++++----- src/libexpr/include/nix/expr/symbol-table.hh | 6 ++++++ src/libexpr/include/nix/expr/value.hh | 7 +++++-- src/libexpr/primops.cc | 8 +++----- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 123f0e86a..d0069b321 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -90,6 +90,11 @@ std::string printValue(EvalState & state, Value & v) return out.str(); } +Value * Value::toPtr(SymbolStr str) noexcept +{ + return const_cast(str.valuePtr()); +} + void Value::print(EvalState & state, std::ostream & str, PrintOptions options) { printValue(state, str, *this, options); @@ -919,11 +924,6 @@ void Value::mkStringMove(const char * s, const NixStringContext & context) mkString(s, encodeContext(context)); } -void Value::mkString(const SymbolStr & s) -{ - mkString(s.c_str(), nullptr); -} - void Value::mkPath(const SourcePath & path) { mkPath(&*path.accessor, makeImmutableString(path.path.abs())); diff --git a/src/libexpr/include/nix/expr/symbol-table.hh b/src/libexpr/include/nix/expr/symbol-table.hh index 994bddd6c..1bf5b5543 100644 --- a/src/libexpr/include/nix/expr/symbol-table.hh +++ b/src/libexpr/include/nix/expr/symbol-table.hh @@ -145,6 +145,12 @@ public: return s->size_; } + [[gnu::always_inline]] + const Value * valuePtr() const noexcept + { + return s; + } + explicit operator Symbol() const noexcept { return Symbol{s->idx + 1}; diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index 3a96577a2..febe36f80 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -172,6 +172,11 @@ private: public: + /** + * Never modify the backing `Value` object! + */ + static Value * toPtr(SymbolStr str) noexcept; + void print(EvalState &state, std::ostream &str, PrintOptions options = PrintOptions {}); // Functions needed to distinguish the type @@ -331,8 +336,6 @@ public: void mkStringMove(const char * s, const NixStringContext & context); - void mkString(const SymbolStr & s); - void mkPath(const SourcePath & path); void mkPath(std::string_view path); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index c4e6feb28..3017d259a 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -2710,7 +2710,7 @@ static void prim_attrNames(EvalState & state, const PosIdx pos, Value * * args, auto list = state.buildList(args[0]->attrs()->size()); for (const auto & [n, i] : enumerate(*args[0]->attrs())) - (list[n] = state.allocValue())->mkString(state.symbols[i.name]); + list[n] = Value::toPtr(state.symbols[i.name]); std::sort(list.begin(), list.end(), [](Value * v1, Value * v2) { return strcmp(v1->c_str(), v2->c_str()) < 0; }); @@ -3170,9 +3170,8 @@ static void prim_mapAttrs(EvalState & state, const PosIdx pos, Value * * args, V auto attrs = state.buildBindings(args[1]->attrs()->size()); for (auto & i : *args[1]->attrs()) { - Value * vName = state.allocValue(); + Value * vName = Value::toPtr(state.symbols[i.name]); Value * vFun2 = state.allocValue(); - vName->mkString(state.symbols[i.name]); vFun2->mkApp(args[0], vName); attrs.alloc(i.name).mkApp(vFun2, i.value); } @@ -3236,8 +3235,7 @@ static void prim_zipAttrsWith(EvalState & state, const PosIdx pos, Value * * arg auto attrs = state.buildBindings(attrsSeen.size()); for (auto & [sym, elem] : attrsSeen) { - auto name = state.allocValue(); - name->mkString(state.symbols[sym]); + auto name = Value::toPtr(state.symbols[sym]); auto call1 = state.allocValue(); call1->mkApp(args[0], name); auto call2 = state.allocValue(); From 0a87ba0e392ca890111d4ed7262e65c170751cb5 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 5 Jun 2025 13:09:57 +0200 Subject: [PATCH 04/66] Prevent double copy of nixpkgs source tree --- docker.nix | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docker.nix b/docker.nix index c418a9e62..e3a0635d0 100644 --- a/docker.nix +++ b/docker.nix @@ -173,7 +173,12 @@ let channel = pkgs.runCommand "channel-nixos" { inherit bundleNixpkgs; } '' mkdir $out if [ "$bundleNixpkgs" ]; then - ln -s ${nixpkgs} $out/nixpkgs + ln -s ${ + builtins.path { + path = nixpkgs; + name = "source"; + } + } $out/nixpkgs echo "[]" > $out/manifest.nix fi ''; From 3c9b9b13af52776ca08bbbb12b3a98f45de6083b Mon Sep 17 00:00:00 2001 From: Kevin Robert Stravers Date: Sat, 21 Sep 2024 02:56:04 -0400 Subject: [PATCH 05/66] nix repl: Print which variables are just loaded When we run `nix repl nixpkgs` we get "Added 6 variables". This is not useful as it doesn't tell us which variables the flake has exported to our global repl scope. This patch prints the name of each variable that was just loaded. We currently cap printing to 20 variables in order to avoid excessive prints. https://github.com/NixOS/nix/issues/11404 --- src/libcmd/repl.cc | 19 +++++++++++++++++++ tests/functional/repl.sh | 29 ++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 07968fa43..e5164b137 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -813,6 +813,25 @@ void NixRepl::addAttrsToScope(Value & attrs) staticEnv->sort(); staticEnv->deduplicate(); notice("Added %1% variables.", attrs.attrs()->size()); + + const int max_print = 20; + int counter = 0; + std::ostringstream loaded; + for (auto & i : attrs.attrs()->lexicographicOrder(state->symbols)) { + if (counter >= max_print) + break; + + if (counter > 0) + loaded << ", "; + + printIdentifier(loaded, state->symbols[i->name]); + counter += 1; + } + + notice("%1%", loaded.str()); + + if (attrs.attrs()->size() > max_print) + notice("... and %1% more", attrs.attrs()->size() - max_print); } diff --git a/tests/functional/repl.sh b/tests/functional/repl.sh index 762636e44..7f5b26913 100755 --- a/tests/functional/repl.sh +++ b/tests/functional/repl.sh @@ -157,6 +157,32 @@ foo + baz ' "3" \ ./flake ./flake\#bar --experimental-features 'flakes' +testReplResponse $' +:a { a = 1; b = 2; longerName = 3; "with spaces" = 4; } +' 'Added 4 variables. +a, b, longerName, "with spaces" +' + +cat < attribute-set.nix +{ + a = 1; + b = 2; + longerName = 3; + "with spaces" = 4; +} +EOF +testReplResponse ' +:l ./attribute-set.nix +' 'Added 4 variables. +a, b, longerName, "with spaces" +' + +testReplResponseNoRegex $' +:a builtins.foldl\' (x: y: x // y) {} (map (x: { ${builtins.toString x} = x; }) (builtins.genList (x: x) 23)) +' 'Added 23 variables. +"0", "1", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "2", "20", "21", "22", "3", "4", "5", "6" +... and 3 more' + # Test the `:reload` mechansim with flakes: # - Eval `./flake#changingThing` # - Modify the flake @@ -322,7 +348,8 @@ runRepl () { -e "s@$testDir@/path/to/tests/functional@g" \ -e "s@$testDirNoUnderscores@/path/to/tests/functional@g" \ -e "s@$nixVersion@@g" \ - -e "s@Added [0-9]* variables@Added variables@g" \ + -e "/Added [0-9]* variables/{s@ [0-9]* @ @;n;d}" \ + -e '/\.\.\. and [0-9]* more/d' \ | grep -vF $'warning: you don\'t have Internet access; disabling some network-dependent features' \ ; } From 13e37043292df2b39c0252b1d476dad4c4aa8cce Mon Sep 17 00:00:00 2001 From: Kevin Robert Stravers Date: Sat, 21 Sep 2024 02:58:18 -0400 Subject: [PATCH 06/66] nix repl: Add `:ll` to show all recently loaded variables Invoking `:ll` will start a pager with all variables which have just been loaded by `:lf`, `:l`, or by a flake provided to `nix repl` as an argument. https://github.com/NixOS/nix/issues/11404 --- src/libcmd/repl.cc | 21 ++++++++++++++++++++- tests/functional/repl.sh | 4 ++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index e5164b137..54a002765 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -69,6 +69,7 @@ struct NixRepl const static int envSize = 32768; std::shared_ptr staticEnv; + Value lastLoaded; Env * env; int displ; StringSet varNames; @@ -95,6 +96,7 @@ struct NixRepl void loadFiles(); void loadFlakes(); void reloadFilesAndFlakes(); + void showLastLoaded(); void addAttrsToScope(Value & attrs); void addVarToScope(const Symbol name, Value & v); Expr * parseString(std::string s); @@ -378,6 +380,7 @@ ProcessLineResult NixRepl::processLine(std::string line) << " current profile\n" << " :l, :load Load Nix expression and add it to scope\n" << " :lf, :load-flake Load Nix flake and add it to scope\n" + << " :ll, :last-loaded Show most recently loaded variables added to scope\n" << " :p, :print Evaluate and print expression recursively\n" << " Strings are printed directly, without escaping.\n" << " :q, :quit Exit nix-repl\n" @@ -468,6 +471,10 @@ ProcessLineResult NixRepl::processLine(std::string line) loadFlake(arg); } + else if (command == ":ll" || command == ":last-loaded") { + showLastLoaded(); + } + else if (command == ":r" || command == ":reload") { state->resetFileCache(); reloadFilesAndFlakes(); @@ -760,6 +767,16 @@ void NixRepl::initEnv() varNames.emplace(state->symbols[i.first]); } +void NixRepl::showLastLoaded() +{ + RunPager pager; + + for (auto & i : *lastLoaded.attrs()) { + std::string_view name = state->symbols[i.name]; + logger->cout(name); + } +} + void NixRepl::reloadFilesAndFlakes() { @@ -814,6 +831,8 @@ void NixRepl::addAttrsToScope(Value & attrs) staticEnv->deduplicate(); notice("Added %1% variables.", attrs.attrs()->size()); + lastLoaded = attrs; + const int max_print = 20; int counter = 0; std::ostringstream loaded; @@ -831,7 +850,7 @@ void NixRepl::addAttrsToScope(Value & attrs) notice("%1%", loaded.str()); if (attrs.attrs()->size() > max_print) - notice("... and %1% more", attrs.attrs()->size() - max_print); + notice("... and %1% more; view with :ll", attrs.attrs()->size() - max_print); } diff --git a/tests/functional/repl.sh b/tests/functional/repl.sh index 7f5b26913..7c4bae4ba 100755 --- a/tests/functional/repl.sh +++ b/tests/functional/repl.sh @@ -181,7 +181,7 @@ testReplResponseNoRegex $' :a builtins.foldl\' (x: y: x // y) {} (map (x: { ${builtins.toString x} = x; }) (builtins.genList (x: x) 23)) ' 'Added 23 variables. "0", "1", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "2", "20", "21", "22", "3", "4", "5", "6" -... and 3 more' +... and 3 more; view with :ll' # Test the `:reload` mechansim with flakes: # - Eval `./flake#changingThing` @@ -349,7 +349,7 @@ runRepl () { -e "s@$testDirNoUnderscores@/path/to/tests/functional@g" \ -e "s@$nixVersion@@g" \ -e "/Added [0-9]* variables/{s@ [0-9]* @ @;n;d}" \ - -e '/\.\.\. and [0-9]* more/d' \ + -e '/\.\.\. and [0-9]* more; view with :ll/d' \ | grep -vF $'warning: you don\'t have Internet access; disabling some network-dependent features' \ ; } From d8b067b549f90fc57f295debe40eccb4145e795c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 6 Jun 2025 14:04:44 +0200 Subject: [PATCH 07/66] repl: Don't wait on incomplete parses from imported file Fixes #13332. --- src/libcmd/repl.cc | 24 +++++++++++++----------- tests/functional/repl.sh | 6 ++++++ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 07968fa43..75a0baebc 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -158,6 +158,8 @@ static std::ostream & showDebugTrace(std::ostream & out, const PosTable & positi return out; } +MakeError(IncompleteReplExpr, ParseError); + static bool isFirstRepl = true; ReplExitStatus NixRepl::mainLoop() @@ -205,16 +207,8 @@ ReplExitStatus NixRepl::mainLoop() default: unreachable(); } - } catch (ParseError & e) { - if (e.msg().find("unexpected end of file") != std::string::npos) { - // For parse errors on incomplete input, we continue waiting for the next line of - // input without clearing the input so far. - continue; - } else { - printMsg(lvlError, e.msg()); - } - } catch (EvalError & e) { - printMsg(lvlError, e.msg()); + } catch (IncompleteReplExpr &) { + continue; } catch (Error & e) { printMsg(lvlError, e.msg()); } catch (Interrupted & e) { @@ -837,7 +831,15 @@ Expr * NixRepl::parseString(std::string s) void NixRepl::evalString(std::string s, Value & v) { - Expr * e = parseString(s); + Expr * e; + try { + e = parseString(s); + } catch (ParseError & e) { + if (e.msg().find("unexpected end of file") != std::string::npos) + // For parse errors on incomplete input, we continue waiting for the next line of + // input without clearing the input so far. + throw IncompleteReplExpr(e.msg()); + } e->eval(*state, *env, v); state->forceValue(v, v.determinePos(noPos)); } diff --git a/tests/functional/repl.sh b/tests/functional/repl.sh index 762636e44..015d296d5 100755 --- a/tests/functional/repl.sh +++ b/tests/functional/repl.sh @@ -277,6 +277,12 @@ testReplResponseNoRegex ' } ' +# Don't prompt for more input when getting unexpected EOF in imported files. +testReplResponse " +import $testDir/lang/parse-fail-eof-pos.nix +" \ +'.*error: syntax error, unexpected end of file.*' + # TODO: move init to characterisation/framework.sh badDiff=0 badExitCode=0 From 91b3573770d4628b1219577a381f79b8cb3fafe9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 6 Jun 2025 17:09:01 +0200 Subject: [PATCH 08/66] Rethrow non-EOF errors --- src/libcmd/repl.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 75a0baebc..de8558e5f 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -839,6 +839,8 @@ void NixRepl::evalString(std::string s, Value & v) // For parse errors on incomplete input, we continue waiting for the next line of // input without clearing the input so far. throw IncompleteReplExpr(e.msg()); + else + throw; } e->eval(*state, *env, v); state->forceValue(v, v.determinePos(noPos)); From c2aaa68c2c88de686e4c1608b31e2c0aaeae205e Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Tue, 10 Jun 2025 13:19:03 +0000 Subject: [PATCH 09/66] libexpr: Use `primOp` getter --- src/libexpr/eval.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 92516db3b..752938339 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -128,7 +128,7 @@ std::string showType(const Value & v) switch (v.internalType) { case tString: return v.payload.string.context ? "a string with context" : "a string"; case tPrimOp: - return fmt("the built-in function '%s'", std::string(v.payload.primOp->name)); + return fmt("the built-in function '%s'", std::string(v.primOp()->name)); case tPrimOpApp: return fmt("the partially applied built-in function '%s'", v.primOpAppPrimOp()->name); case tExternal: return v.external()->showType(); From 77f5f50ec2378c75fddfaa33c45e3b885df465e6 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Tue, 10 Jun 2025 13:22:20 +0000 Subject: [PATCH 10/66] libexpr: Use `context` getter --- src/libexpr/eval.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 752938339..7346a7651 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -126,7 +126,7 @@ std::string showType(const Value & v) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wswitch-enum" switch (v.internalType) { - case tString: return v.payload.string.context ? "a string with context" : "a string"; + case tString: return v.context() ? "a string with context" : "a string"; case tPrimOp: return fmt("the built-in function '%s'", std::string(v.primOp()->name)); case tPrimOpApp: @@ -2297,8 +2297,8 @@ std::string_view EvalState::forceString(Value & v, const PosIdx pos, std::string void copyContext(const Value & v, NixStringContext & context, const ExperimentalFeatureSettings & xpSettings) { - if (v.payload.string.context) - for (const char * * p = v.payload.string.context; *p; ++p) + if (v.context()) + for (const char * * p = v.context(); *p; ++p) context.insert(NixStringContextElem::parse(*p, xpSettings)); } From 408873c2f7e92063d380b9ed36334de6547e887e Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Tue, 10 Jun 2025 13:23:12 +0000 Subject: [PATCH 11/66] libexpr: Use `c_str` getter --- src/libexpr/include/nix/expr/symbol-table.hh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libexpr/include/nix/expr/symbol-table.hh b/src/libexpr/include/nix/expr/symbol-table.hh index 1bf5b5543..20a05a09d 100644 --- a/src/libexpr/include/nix/expr/symbol-table.hh +++ b/src/libexpr/include/nix/expr/symbol-table.hh @@ -29,7 +29,7 @@ class SymbolValue : protected Value public: operator std::string_view() const noexcept { - return {payload.string.c_str, size_}; + return {c_str(), size_}; } }; @@ -122,7 +122,7 @@ public: [[gnu::always_inline]] const char * c_str() const noexcept { - return s->payload.string.c_str; + return s->c_str(); } [[gnu::always_inline]] From 525078c59d8391b866f723338db1ee525a4a88cb Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Wed, 11 Jun 2025 08:52:04 -0700 Subject: [PATCH 12/66] Fix broken link in configuration description --- src/libstore/include/nix/store/globals.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 5fae2be23..00d929236 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -365,7 +365,7 @@ public: To build only on remote machines and disable local builds, set [`max-jobs`](#conf-max-jobs) to 0. - If you want the remote machines to use substituters, set [`builders-use-substitutes`](#conf-builders-use-substituters) to `true`. + If you want the remote machines to use substituters, set [`builders-use-substitutes`](#conf-builders-use-substitutes) to `true`. )", {}, false}; From 9eb46e9cc030016b1f4a073474a836bac1de3615 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 11 Jun 2025 19:14:31 +0200 Subject: [PATCH 13/66] Make the repl test more robust Seen in https://github.com/DeterminateSystems/nix-src/actions/runs/15590867877/job/43909540271: nix-functional-tests> grep: repl_output: No such file or directory nix-functional-tests> +(repl.sh:174) cat repl_output This is because there is a small possibility that the `nix repl` child process hasn't created `repl_output` yet. So make sure it exists. --- tests/functional/repl.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/functional/repl.sh b/tests/functional/repl.sh index 5c6993621..6db9e2d3d 100755 --- a/tests/functional/repl.sh +++ b/tests/functional/repl.sh @@ -189,7 +189,8 @@ testReplResponseNoRegex $' # - Re-eval it # - Check that the result has changed mkfifo repl_fifo -nix repl ./flake --experimental-features 'flakes' < repl_fifo > repl_output 2>&1 & +touch repl_output +nix repl ./flake --experimental-features 'flakes' < repl_fifo >> repl_output 2>&1 & repl_pid=$! exec 3>repl_fifo # Open fifo for writing echo "changingThing" >&3 From f42eaf2c8ea3c7e53d1fa15f694144f46006eaf5 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 11 Jun 2025 16:11:38 -0400 Subject: [PATCH 14/66] Create test for #13293 It currently fails, before the fix. --- tests/functional/check.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/functional/check.sh b/tests/functional/check.sh index b21349288..a1c6decf5 100755 --- a/tests/functional/check.sh +++ b/tests/functional/check.sh @@ -22,6 +22,11 @@ clearStore nix-build dependencies.nix --no-out-link nix-build dependencies.nix --no-out-link --check +# Make sure checking just one output works (#13293) +nix-build multiple-outputs.nix -A a --no-out-link +nix-store --delete "$(nix-build multiple-outputs.nix -A a.second --no-out-link)" +nix-build multiple-outputs.nix -A a.first --no-out-link --check + # Build failure exit codes (100, 104, etc.) are from # doc/manual/source/command-ref/status-build-failure.md From 52677184728a5ba21087a7536f9ee0c8eb3bc1b2 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 28 May 2025 20:37:23 -0400 Subject: [PATCH 15/66] Fix #13293 We move the `assertPathValidity` to where we know what the wanted outputs are. --- src/libstore/build/derivation-building-goal.cc | 3 --- src/libstore/build/derivation-goal.cc | 7 +++++++ src/libstore/unix/build/derivation-builder.cc | 6 +++--- .../unix/include/nix/store/build/derivation-builder.hh | 8 -------- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 19296fac3..9a91b3592 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -613,9 +613,6 @@ Goal::Co DerivationBuildingGoal::tryToBuild() void closeLogFile() override { goal.closeLogFile(); } - SingleDrvOutputs assertPathValidity() override { - return goal.assertPathValidity(); - } void appendLogTailErrorMsg(std::string & msg) override { goal.appendLogTailErrorMsg(msg); } diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index b1f081115..3fcc376ed 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -210,6 +210,13 @@ Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) .outputs = wantedOutputs, }); + if (buildMode == bmCheck) { + /* In checking mode, the builder will not register any outputs. + So we want to make sure the ones that we wanted to check are + properly there. */ + buildResult.builtOutputs = assertPathValidity(drvPath); + } + co_return amDone(g->exitCode, g->ex); }; diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index eca017487..1f15cc9e9 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -589,10 +589,10 @@ static void replaceValidPath(const Path & storePath, const Path & tmpPath) way first. We'd better not be interrupted here, because if we're repairing (say) Glibc, we end up with a broken system. */ Path oldPath; - + if (pathExists(storePath)) { // why do we loop here? - // although makeTempPath should be unique, we can't + // although makeTempPath should be unique, we can't // guarantee that. do { oldPath = makeTempPath(storePath, ".old"); @@ -1871,7 +1871,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() also a source for non-determinism. */ if (delayedException) std::rethrow_exception(delayedException); - return miscMethods->assertPathValidity(); + return {}; } /* Apply output checks. */ diff --git a/src/libstore/unix/include/nix/store/build/derivation-builder.hh b/src/libstore/unix/include/nix/store/build/derivation-builder.hh index 01266a492..5ce38e034 100644 --- a/src/libstore/unix/include/nix/store/build/derivation-builder.hh +++ b/src/libstore/unix/include/nix/store/build/derivation-builder.hh @@ -97,14 +97,6 @@ struct DerivationBuilderCallbacks */ virtual void closeLogFile() = 0; - /** - * Aborts if any output is not valid or corrupt, and otherwise - * returns a 'SingleDrvOutputs' structure containing all outputs. - * - * @todo Probably should just be in `DerivationGoal`. - */ - virtual SingleDrvOutputs assertPathValidity() = 0; - virtual void appendLogTailErrorMsg(std::string & msg) = 0; /** From 93a42a597145a2ff0214ddc6a9828921056c3657 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 11 Jun 2025 22:08:03 +0000 Subject: [PATCH 16/66] flake: Add meson formatter This adds a meson.format file that mostly mirrors the projects meson style and a pre-commit hook to enforce this style. Some low-diff files are formatted. --- maintainers/flake-module.nix | 112 ++++++++++++++++++ meson.build | 10 +- meson.format | 7 ++ meson.options | 15 ++- .../windows-version/meson.build | 2 +- .../include/nix/expr/tests/meson.build | 8 +- src/libexpr/primops/meson.build | 2 +- .../linux/include/nix/store/meson.build | 4 +- src/libstore/linux/meson.build | 4 +- .../freebsd/include/nix/util/meson.build | 4 +- src/libutil/freebsd/meson.build | 4 +- src/libutil/include/nix/util/meson.build | 4 +- .../linux/include/nix/util/meson.build | 5 +- src/libutil/linux/meson.build | 5 +- .../windows/include/nix/util/meson.build | 6 +- src/perl/t/meson.build | 4 +- tests/functional/plugins/meson.build | 4 +- .../test-libstoreconsumer/meson.build | 4 +- 18 files changed, 152 insertions(+), 52 deletions(-) create mode 100644 meson.format diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index d2bae32b6..d443d1a74 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -37,6 +37,118 @@ fi ''}"; }; + meson-format = { + enable = true; + files = "(meson.build|meson.options)$"; + entry = "${pkgs.writeScript "format-meson" '' + #!${pkgs.runtimeShell} + for file in "$@"; do + ${lib.getExe pkgs.meson} format -ic ${../meson.format} "$file" + done + ''}"; + excludes = [ + # We haven't applied formatting to these files yet + ''^doc/manual/meson.build$'' + ''^doc/manual/source/command-ref/meson.build$'' + ''^doc/manual/source/development/meson.build$'' + ''^doc/manual/source/language/meson.build$'' + ''^doc/manual/source/meson.build$'' + ''^doc/manual/source/release-notes/meson.build$'' + ''^doc/manual/source/store/meson.build$'' + ''^misc/bash/meson.build$'' + ''^misc/fish/meson.build$'' + ''^misc/launchd/meson.build$'' + ''^misc/meson.build$'' + ''^misc/systemd/meson.build$'' + ''^misc/zsh/meson.build$'' + ''^nix-meson-build-support/$'' + ''^nix-meson-build-support/big-objs/meson.build$'' + ''^nix-meson-build-support/common/meson.build$'' + ''^nix-meson-build-support/deps-lists/meson.build$'' + ''^nix-meson-build-support/export/meson.build$'' + ''^nix-meson-build-support/export-all-symbols/meson.build$'' + ''^nix-meson-build-support/generate-header/meson.build$'' + ''^nix-meson-build-support/libatomic/meson.build$'' + ''^nix-meson-build-support/subprojects/meson.build$'' + ''^scripts/meson.build$'' + ''^src/external-api-docs/meson.build$'' + ''^src/internal-api-docs/meson.build$'' + ''^src/libcmd/include/nix/cmd/meson.build$'' + ''^src/libcmd/meson.build$'' + ''^src/libcmd/nix-meson-build-support$'' + ''^src/libexpr/include/nix/expr/meson.build$'' + ''^src/libexpr/meson.build$'' + ''^src/libexpr/nix-meson-build-support$'' + ''^src/libexpr-c/meson.build$'' + ''^src/libexpr-c/nix-meson-build-support$'' + ''^src/libexpr-test-support/meson.build$'' + ''^src/libexpr-test-support/nix-meson-build-support$'' + ''^src/libexpr-tests/meson.build$'' + ''^src/libexpr-tests/nix-meson-build-support$'' + ''^src/libfetchers/include/nix/fetchers/meson.build$'' + ''^src/libfetchers/meson.build$'' + ''^src/libfetchers/nix-meson-build-support$'' + ''^src/libfetchers-c/meson.build$'' + ''^src/libfetchers-c/nix-meson-build-support$'' + ''^src/libfetchers-tests/meson.build$'' + ''^src/libfetchers-tests/nix-meson-build-support$'' + ''^src/libflake/include/nix/flake/meson.build$'' + ''^src/libflake/meson.build$'' + ''^src/libflake/nix-meson-build-support$'' + ''^src/libflake-c/meson.build$'' + ''^src/libflake-c/nix-meson-build-support$'' + ''^src/libflake-tests/meson.build$'' + ''^src/libflake-tests/nix-meson-build-support$'' + ''^src/libmain/include/nix/main/meson.build$'' + ''^src/libmain/meson.build$'' + ''^src/libmain/nix-meson-build-support$'' + ''^src/libmain-c/meson.build$'' + ''^src/libmain-c/nix-meson-build-support$'' + ''^src/libstore/include/nix/store/meson.build$'' + ''^src/libstore/meson.build$'' + ''^src/libstore/nix-meson-build-support$'' + ''^src/libstore/unix/include/nix/store/meson.build$'' + ''^src/libstore/unix/meson.build$'' + ''^src/libstore/windows/meson.build$'' + ''^src/libstore-c/meson.build$'' + ''^src/libstore-c/nix-meson-build-support$'' + ''^src/libstore-test-support/include/nix/store/tests/meson.build$'' + ''^src/libstore-test-support/meson.build$'' + ''^src/libstore-test-support/nix-meson-build-support$'' + ''^src/libstore-tests/meson.build$'' + ''^src/libstore-tests/nix-meson-build-support$'' + ''^src/libutil/meson.build$'' + ''^src/libutil/nix-meson-build-support$'' + ''^src/libutil/unix/include/nix/util/meson.build$'' + ''^src/libutil/unix/meson.build$'' + ''^src/libutil/windows/meson.build$'' + ''^src/libutil-c/meson.build$'' + ''^src/libutil-c/nix-meson-build-support$'' + ''^src/libutil-test-support/include/nix/util/tests/meson.build$'' + ''^src/libutil-test-support/meson.build$'' + ''^src/libutil-test-support/nix-meson-build-support$'' + ''^src/libutil-tests/meson.build$'' + ''^src/libutil-tests/nix-meson-build-support$'' + ''^src/nix/meson.build$'' + ''^src/nix/nix-meson-build-support$'' + ''^src/perl/lib/Nix/meson.build$'' + ''^src/perl/meson.build$'' + ''^tests/functional/ca/meson.build$'' + ''^tests/functional/common/meson.build$'' + ''^tests/functional/dyn-drv/meson.build$'' + ''^tests/functional/flakes/meson.build$'' + ''^tests/functional/git-hashing/meson.build$'' + ''^tests/functional/local-overlay-store/meson.build$'' + ''^tests/functional/meson.build$'' + ''^src/libcmd/meson.options$'' + ''^src/libexpr/meson.options$'' + ''^src/libstore/meson.options$'' + ''^src/libutil/meson.options$'' + ''^src/libutil-c/meson.options$'' + ''^src/nix/meson.options$'' + ''^src/perl/meson.options$'' + ]; + }; nixfmt-rfc-style = { enable = true; excludes = [ diff --git a/meson.build b/meson.build index 9f7471143..023791979 100644 --- a/meson.build +++ b/meson.build @@ -1,13 +1,13 @@ # This is just a stub project to include all the others as subprojects # for development shell purposes -project('nix-dev-shell', 'cpp', +project( + 'nix-dev-shell', + 'cpp', version : files('.version'), subproject_dir : 'src', - default_options : [ - 'localstatedir=/nix/var', - ], - meson_version : '>= 1.1' + default_options : [ 'localstatedir=/nix/var' ], + meson_version : '>= 1.1', ) # Internal Libraries diff --git a/meson.format b/meson.format new file mode 100644 index 000000000..4876dd962 --- /dev/null +++ b/meson.format @@ -0,0 +1,7 @@ +indent_by = ' ' +space_array = true +kwargs_force_multiline = false +wide_colon = true +group_arg_value = true +indent_before_comments = ' ' +use_editor_config = true diff --git a/meson.options b/meson.options index 329fe06bf..30670902e 100644 --- a/meson.options +++ b/meson.options @@ -1,13 +1,22 @@ # vim: filetype=meson -option('doc-gen', type : 'boolean', value : false, +option( + 'doc-gen', + type : 'boolean', + value : false, description : 'Generate documentation', ) -option('unit-tests', type : 'boolean', value : true, +option( + 'unit-tests', + type : 'boolean', + value : true, description : 'Build unit tests', ) -option('bindings', type : 'boolean', value : true, +option( + 'bindings', + type : 'boolean', + value : true, description : 'Build language bindings (e.g. Perl)', ) diff --git a/nix-meson-build-support/windows-version/meson.build b/nix-meson-build-support/windows-version/meson.build index 3a008e5df..ed4caaa9a 100644 --- a/nix-meson-build-support/windows-version/meson.build +++ b/nix-meson-build-support/windows-version/meson.build @@ -2,5 +2,5 @@ if host_machine.system() == 'windows' # https://learn.microsoft.com/en-us/cpp/porting/modifying-winver-and-win32-winnt?view=msvc-170 # #define _WIN32_WINNT_WIN8 0x0602 # We currently don't use any API which requires higher than this. - add_project_arguments([ '-D_WIN32_WINNT=0x0602' ], language: 'cpp') + add_project_arguments([ '-D_WIN32_WINNT=0x0602' ], language : 'cpp') endif diff --git a/src/libexpr-test-support/include/nix/expr/tests/meson.build b/src/libexpr-test-support/include/nix/expr/tests/meson.build index 710bd8d4e..e44d25582 100644 --- a/src/libexpr-test-support/include/nix/expr/tests/meson.build +++ b/src/libexpr-test-support/include/nix/expr/tests/meson.build @@ -1,9 +1,5 @@ # Public headers directory -include_dirs = [include_directories('../../..')] +include_dirs = [ include_directories('../../..') ] -headers = files( - 'libexpr.hh', - 'nix_api_expr.hh', - 'value/context.hh', -) +headers = files('libexpr.hh', 'nix_api_expr.hh', 'value/context.hh') diff --git a/src/libexpr/primops/meson.build b/src/libexpr/primops/meson.build index f910fe237..b8abc6409 100644 --- a/src/libexpr/primops/meson.build +++ b/src/libexpr/primops/meson.build @@ -1,6 +1,6 @@ generated_headers += gen_header.process( 'derivation.nix', - preserve_path_from: meson.project_source_root(), + preserve_path_from : meson.project_source_root(), ) sources += files( diff --git a/src/libstore/linux/include/nix/store/meson.build b/src/libstore/linux/include/nix/store/meson.build index a664aefa9..eb2ae2583 100644 --- a/src/libstore/linux/include/nix/store/meson.build +++ b/src/libstore/linux/include/nix/store/meson.build @@ -1,5 +1,3 @@ include_dirs += include_directories('../..') -headers += files( - 'personality.hh', -) +headers += files('personality.hh') diff --git a/src/libstore/linux/meson.build b/src/libstore/linux/meson.build index 6fc193cf8..93084a8ae 100644 --- a/src/libstore/linux/meson.build +++ b/src/libstore/linux/meson.build @@ -1,5 +1,3 @@ -sources += files( - 'personality.cc', -) +sources += files('personality.cc') subdir('include/nix/store') diff --git a/src/libutil/freebsd/include/nix/util/meson.build b/src/libutil/freebsd/include/nix/util/meson.build index 561c8796c..283c49b95 100644 --- a/src/libutil/freebsd/include/nix/util/meson.build +++ b/src/libutil/freebsd/include/nix/util/meson.build @@ -2,6 +2,4 @@ include_dirs += include_directories('../..') -headers += files( - 'freebsd-jail.hh', -) +headers += files('freebsd-jail.hh') diff --git a/src/libutil/freebsd/meson.build b/src/libutil/freebsd/meson.build index 8ffdc2832..34508efd0 100644 --- a/src/libutil/freebsd/meson.build +++ b/src/libutil/freebsd/meson.build @@ -1,5 +1,3 @@ -sources += files( - 'freebsd-jail.cc', -) +sources += files('freebsd-jail.cc') subdir('include/nix/util') diff --git a/src/libutil/include/nix/util/meson.build b/src/libutil/include/nix/util/meson.build index e30b8dacd..6c95a6ced 100644 --- a/src/libutil/include/nix/util/meson.build +++ b/src/libutil/include/nix/util/meson.build @@ -1,6 +1,6 @@ # Public headers directory -include_dirs = [include_directories('../..')] +include_dirs = [ include_directories('../..') ] headers = files( 'abstract-setting-to-json.hh', @@ -63,8 +63,8 @@ headers = files( 'source-path.hh', 'split.hh', 'std-hash.hh', - 'strings.hh', 'strings-inline.hh', + 'strings.hh', 'suggestions.hh', 'sync.hh', 'tarfile.hh', diff --git a/src/libutil/linux/include/nix/util/meson.build b/src/libutil/linux/include/nix/util/meson.build index e28ad8e05..ac5f318f8 100644 --- a/src/libutil/linux/include/nix/util/meson.build +++ b/src/libutil/linux/include/nix/util/meson.build @@ -2,7 +2,4 @@ include_dirs += include_directories('../..') -headers += files( - 'cgroup.hh', - 'linux-namespaces.hh', -) +headers += files('cgroup.hh', 'linux-namespaces.hh') diff --git a/src/libutil/linux/meson.build b/src/libutil/linux/meson.build index b8053a5bb..f3fe07c9c 100644 --- a/src/libutil/linux/meson.build +++ b/src/libutil/linux/meson.build @@ -1,6 +1,3 @@ -sources += files( - 'cgroup.cc', - 'linux-namespaces.cc', -) +sources += files('cgroup.cc', 'linux-namespaces.cc') subdir('include/nix/util') diff --git a/src/libutil/windows/include/nix/util/meson.build b/src/libutil/windows/include/nix/util/meson.build index 1bd56c4bd..f0d4c37e9 100644 --- a/src/libutil/windows/include/nix/util/meson.build +++ b/src/libutil/windows/include/nix/util/meson.build @@ -2,8 +2,4 @@ include_dirs += include_directories('../..') -headers += files( - 'signals-impl.hh', - 'windows-async-pipe.hh', - 'windows-error.hh', -) +headers += files('signals-impl.hh', 'windows-async-pipe.hh', 'windows-error.hh') diff --git a/src/perl/t/meson.build b/src/perl/t/meson.build index dbd1139f3..cd98453c3 100644 --- a/src/perl/t/meson.build +++ b/src/perl/t/meson.build @@ -5,9 +5,7 @@ # src #--------------------------------------------------- -nix_perl_tests = files( - 'init.t', -) +nix_perl_tests = files('init.t') foreach f : nix_perl_tests diff --git a/tests/functional/plugins/meson.build b/tests/functional/plugins/meson.build index ae66e3036..7cbc935c9 100644 --- a/tests/functional/plugins/meson.build +++ b/tests/functional/plugins/meson.build @@ -1,8 +1,6 @@ libplugintest = shared_module( 'plugintest', 'plugintest.cc', - dependencies : [ - dependency('nix-expr'), - ], + dependencies : [ dependency('nix-expr') ], build_by_default : false, ) diff --git a/tests/functional/test-libstoreconsumer/meson.build b/tests/functional/test-libstoreconsumer/meson.build index e5a1cc182..d27647ec4 100644 --- a/tests/functional/test-libstoreconsumer/meson.build +++ b/tests/functional/test-libstoreconsumer/meson.build @@ -1,8 +1,6 @@ libstoreconsumer_tester = executable( 'test-libstoreconsumer', 'main.cc', - dependencies : [ - dependency('nix-store'), - ], + dependencies : [ dependency('nix-store') ], build_by_default : false, ) From 57c72dee9b27b50b2a3b7a738cf33329acfb7236 Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Thu, 12 Jun 2025 09:41:37 +0200 Subject: [PATCH 17/66] docker: make sure `nix config check` works --- docker.nix | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docker.nix b/docker.nix index e3a0635d0..99316fc84 100644 --- a/docker.nix +++ b/docker.nix @@ -280,7 +280,6 @@ let ln -s ${profile} $out/nix/var/nix/profiles/default-1-link ln -s /nix/var/nix/profiles/default-1-link $out/nix/var/nix/profiles/default - ln -s /nix/var/nix/profiles/default $out${userHome}/.nix-profile ln -s ${channel} $out/nix/var/nix/profiles/per-user/${uname}/channels-1-link ln -s /nix/var/nix/profiles/per-user/${uname}/channels-1-link $out/nix/var/nix/profiles/per-user/${uname}/channels @@ -332,7 +331,7 @@ pkgs.dockerTools.buildLayeredImageWithNixDb { ''; config = { - Cmd = [ "${userHome}/.nix-profile/bin/bash" ]; + Cmd = [ (lib.getExe pkgs.bashInteractive) ]; User = "${toString uid}:${toString gid}"; Env = [ "USER=${uname}" From ab10fddc6ee9c0ef205e9454d6992cdb7e7df801 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Thu, 12 Jun 2025 08:24:39 -0700 Subject: [PATCH 18/66] Rework future tense in user-facing messages --- src/libcmd/installables.cc | 2 +- src/libexpr/include/nix/expr/eval-settings.hh | 20 +-- src/libexpr/primops.cc | 54 ++++---- src/libexpr/primops/context.cc | 2 +- src/libexpr/primops/fetchClosure.cc | 2 +- src/libexpr/primops/fetchTree.cc | 16 +-- .../include/nix/fetchers/fetch-settings.hh | 6 +- src/libflake/flake.cc | 4 +- src/libmain/plugin.cc | 8 +- .../include/nix/store/filetransfer.hh | 2 +- src/libstore/include/nix/store/globals.hh | 123 +++++++++--------- .../include/nix/store/local-fs-store.hh | 4 +- src/libstore/include/nix/store/local-store.hh | 2 +- .../nix/store/s3-binary-cache-store.hh | 6 +- src/libutil/experimental-features.cc | 2 +- src/libutil/include/nix/util/logging.hh | 2 +- src/nix-build/nix-build.cc | 2 +- src/nix/unix/daemon.cc | 2 +- tests/functional/fetchGit.sh | 2 +- tests/functional/flakes/unlocked-override.sh | 2 +- tests/nixos/github-flakes.nix | 22 ++-- 21 files changed, 141 insertions(+), 144 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 836aded96..49ffd82e1 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -199,7 +199,7 @@ SourceExprCommand::SourceExprCommand() .shortName = 'f', .description = "Interpret [*installables*](@docroot@/command-ref/new-cli/nix.md#installables) as attribute paths relative to the Nix expression stored in *file*. " - "If *file* is the character -, then a Nix expression will be read from standard input. " + "If *file* is the character -, then a Nix expression is read from standard input. " "Implies `--impure`.", .category = installablesCategory, .labels = {"file"}, diff --git a/src/libexpr/include/nix/expr/eval-settings.hh b/src/libexpr/include/nix/expr/eval-settings.hh index 9ff73138a..f88f13874 100644 --- a/src/libexpr/include/nix/expr/eval-settings.hh +++ b/src/libexpr/include/nix/expr/eval-settings.hh @@ -132,7 +132,7 @@ struct EvalSettings : Config Setting restrictEval{ this, false, "restrict-eval", R"( - If set to `true`, the Nix evaluator will not allow access to any + If set to `true`, the Nix evaluator doesn't allow access to any files outside of [`builtins.nixPath`](@docroot@/language/builtins.md#builtins-nixPath), or to URIs outside of @@ -157,7 +157,7 @@ struct EvalSettings : Config R"( By default, Nix allows [Import from Derivation](@docroot@/language/import-from-derivation.md). - When this setting is `true`, Nix will log a warning indicating that it performed such an import. + When this setting is `true`, Nix logs a warning indicating that it performed such an import. This option has no effect if `allow-import-from-derivation` is disabled. )" }; @@ -167,9 +167,9 @@ struct EvalSettings : Config R"( By default, Nix allows [Import from Derivation](@docroot@/language/import-from-derivation.md). - With this option set to `false`, Nix will throw an error when evaluating an expression that uses this feature, + With this option set to `false`, Nix throws an error when evaluating an expression that uses this feature, even when the required store object is readily available. - This ensures that evaluation will not require any builds to take place, + This ensures that evaluation doesn't require any builds to take place, regardless of the state of the store. )"}; @@ -188,8 +188,8 @@ struct EvalSettings : Config Setting traceFunctionCalls{this, false, "trace-function-calls", R"( - If set to `true`, the Nix evaluator will trace every function call. - Nix will print a log message at the "vomit" level for every function + If set to `true`, the Nix evaluator traces every function call. + Nix prints a log message at the "vomit" level for every function entrance and function exit. function-trace entered undefined position at 1565795816999559622 @@ -234,8 +234,8 @@ struct EvalSettings : Config Setting ignoreExceptionsDuringTry{this, false, "ignore-try", R"( - If set to true, ignore exceptions inside 'tryEval' calls when evaluating nix expressions in - debug mode (using the --debugger flag). By default the debugger will pause on all exceptions. + If set to true, ignore exceptions inside 'tryEval' calls when evaluating Nix expressions in + debug mode (using the --debugger flag). By default the debugger pauses on all exceptions. )"}; Setting traceVerbose{this, false, "trace-verbose", @@ -247,7 +247,7 @@ struct EvalSettings : Config Setting builtinsTraceDebugger{this, false, "debugger-on-trace", R"( If set to true and the `--debugger` flag is given, the following functions - will enter the debugger like [`builtins.break`](@docroot@/language/builtins.md#builtins-break). + enter the debugger like [`builtins.break`](@docroot@/language/builtins.md#builtins-break): * [`builtins.trace`](@docroot@/language/builtins.md#builtins-trace) * [`builtins.traceVerbose`](@docroot@/language/builtins.md#builtins-traceVerbose) @@ -269,7 +269,7 @@ struct EvalSettings : Config Setting builtinsAbortOnWarn{this, false, "abort-on-warn", R"( - If set to true, [`builtins.warn`](@docroot@/language/builtins.md#builtins-warn) will throw an error when logging a warning. + If set to true, [`builtins.warn`](@docroot@/language/builtins.md#builtins-warn) throws an error when logging a warning. This will give you a stack trace that leads to the location of the warning. diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 48bc272f9..4c649e6de 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -345,7 +345,7 @@ static RegisterPrimOp primop_import({ > } > ``` > - > then the following `foo.nix` will give an error: + > then the following `foo.nix` throws an error: > > ```nix > # foo.nix @@ -915,7 +915,7 @@ static RegisterPrimOp primop_ceil({ a NixInt and if `*number* < -9007199254740992` or `*number* > 9007199254740992`. If the datatype of *number* is neither a NixInt (signed 64-bit integer) nor a NixFloat - (IEEE-754 double-precision floating-point number), an evaluation error will be thrown. + (IEEE-754 double-precision floating-point number), an evaluation error is thrown. )", .fun = prim_ceil, }); @@ -1002,15 +1002,15 @@ static RegisterPrimOp primop_tryEval({ Try to shallowly evaluate *e*. Return a set containing the attributes `success` (`true` if *e* evaluated successfully, `false` if an error was thrown) and `value`, equalling *e* if - successful and `false` otherwise. `tryEval` will only prevent + successful and `false` otherwise. `tryEval` only prevents errors created by `throw` or `assert` from being thrown. - Errors `tryEval` will not catch are for example those created + Errors `tryEval` doesn't catch are, for example, those created by `abort` and type errors generated by builtins. Also note that this doesn't evaluate *e* deeply, so `let e = { x = throw ""; }; - in (builtins.tryEval e).success` will be `true`. Using + in (builtins.tryEval e).success` is `true`. Using `builtins.deepSeq` one can get the expected result: `let e = { x = throw ""; }; in - (builtins.tryEval (builtins.deepSeq e e)).success` will be + (builtins.tryEval (builtins.deepSeq e e)).success` is `false`. `tryEval` intentionally does not return the error message, because that risks bringing non-determinism into the evaluation result, and it would become very difficult to improve error reporting without breaking existing expressions. @@ -1108,7 +1108,7 @@ static RegisterPrimOp primop_trace({ If the [`debugger-on-trace`](@docroot@/command-ref/conf-file.md#conf-debugger-on-trace) option is set to `true` and the `--debugger` flag is given, the - interactive debugger will be started when `trace` is called (like + interactive debugger is started when `trace` is called (like [`break`](@docroot@/language/builtins.md#builtins-break)). )", .fun = prim_trace, @@ -1157,7 +1157,7 @@ static RegisterPrimOp primop_warn({ If the [`abort-on-warn`](@docroot@/command-ref/conf-file.md#conf-abort-on-warn) - option is set, the evaluation will be aborted after the warning is printed. + option is set, the evaluation is aborted after the warning is printed. This is useful to reveal the stack trace of the warning, when the context is non-interactive and a debugger can not be launched. )", .fun = prim_warn, @@ -1634,7 +1634,7 @@ static RegisterPrimOp primop_placeholder({ .name = "placeholder", .args = {"output"}, .doc = R"( - Return at + Return an [output placeholder string](@docroot@/store/derivation/index.md#output-placeholder) for the specified *output* that will be substituted by the corresponding [output path](@docroot@/glossary.md#gloss-output-path) @@ -1799,7 +1799,7 @@ static RegisterPrimOp primop_baseNameOf({ After this, the *base name* is returned as previously described, assuming `/` as the directory separator. (Note that evaluation must be platform independent.) - This is somewhat similar to the [GNU `basename`](https://www.gnu.org/software/coreutils/manual/html_node/basename-invocation.html) command, but GNU `basename` will strip any number of trailing slashes. + This is somewhat similar to the [GNU `basename`](https://www.gnu.org/software/coreutils/manual/html_node/basename-invocation.html) command, but GNU `basename` strips any number of trailing slashes. )", .fun = prim_baseNameOf, }); @@ -1998,9 +1998,9 @@ static RegisterPrimOp primop_findFile(PrimOp { > ] > ``` > - > and a *lookup-path* value `"nixos-config"` will cause Nix to try `/home/eelco/Dev/nixos-config` and `/etc/nixos` in that order and return the first path that exists. + > and a *lookup-path* value `"nixos-config"` causes Nix to try `/home/eelco/Dev/nixos-config` and `/etc/nixos` in that order and return the first path that exists. - If `path` starts with `http://` or `https://`, it is interpreted as the URL of a tarball that will be downloaded and unpacked to a temporary location. + If `path` starts with `http://` or `https://`, it is interpreted as the URL of a tarball to be downloaded and unpacked to a temporary location. The tarball must consist of a single top-level directory. The URLs of the tarballs from the official `nixos.org` channels can be abbreviated as `channel:`. @@ -2147,7 +2147,7 @@ static RegisterPrimOp primop_readDir({ Return the contents of the directory *path* as a set mapping directory entries to the corresponding file type. For instance, if directory `A` contains a regular file `B` and another directory - `C`, then `builtins.readDir ./A` will return the set + `C`, then `builtins.readDir ./A` returns the set ```nix { B = "regular"; C = "directory"; } @@ -2182,8 +2182,8 @@ static RegisterPrimOp primop_outputOf({ [input placeholder string](@docroot@/store/derivation/index.md#input-placeholder) if needed. - If the derivation has a statically-known output path (i.e. the derivation output is input-addressed, or fixed content-addressed), the output path will just be returned. - But if the derivation is content-addressed or if the derivation is itself not-statically produced (i.e. is the output of another derivation), an input placeholder will be returned instead. + If the derivation has a statically-known output path (i.e. the derivation output is input-addressed, or fixed content-addressed), the output path is returned. + But if the derivation is content-addressed or if the derivation is itself not-statically produced (i.e. is the output of another derivation), an input placeholder is returned instead. *`derivation reference`* must be a string that may contain a regular store path to a derivation, or may be an input placeholder reference. If the derivation is produced by a derivation, you must explicitly select `drv.outPath`. @@ -2196,7 +2196,7 @@ static RegisterPrimOp primop_outputOf({ "out" ``` - will return a input placeholder for the output of the output of `myDrv`. + returns an input placeholder for the output of the output of `myDrv`. This primop corresponds to the `^` sigil for [deriving paths](@docroot@/glossary.md#gloss-deriving-paths), e.g. as part of installable syntax on the command line. )", @@ -2582,12 +2582,12 @@ static RegisterPrimOp primop_filterSource({ > > `filterSource` should not be used to filter store paths. Since > `filterSource` uses the name of the input directory while naming - > the output directory, doing so will produce a directory name in + > the output directory, doing so produces a directory name in > the form of `--`, where `-` is > the name of the input directory. Since `` depends on the - > unfiltered directory, the name of the output directory will - > indirectly depend on files that are filtered out by the - > function. This will trigger a rebuild even when a filtered out + > unfiltered directory, the name of the output directory + > indirectly depends on files that are filtered out by the + > function. This triggers a rebuild even when a filtered out > file is changed. Use `builtins.path` instead, which allows > specifying the name of the output directory. @@ -2602,8 +2602,8 @@ static RegisterPrimOp primop_filterSource({ } ``` - However, if `source-dir` is a Subversion working copy, then all - those annoying `.svn` subdirectories will also be copied to the + However, if `source-dir` is a Subversion working copy, then all of + those annoying `.svn` subdirectories are also copied to the store. Worse, the contents of those directories may change a lot, causing lots of spurious rebuilds. With `filterSource` you can filter out the `.svn` directories: @@ -2623,8 +2623,8 @@ static RegisterPrimOp primop_filterSource({ `"regular"`, `"directory"`, `"symlink"` or `"unknown"` (for other kinds of files such as device nodes or fifos — but note that those cannot be copied to the Nix store, so if the predicate returns - `true` for them, the copy will fail). If you exclude a directory, - the entire corresponding subtree of *e2* will be excluded. + `true` for them, the copy fails). If you exclude a directory, + the entire corresponding subtree of *e2* is excluded. )", .fun = prim_filterSource, }); @@ -2698,7 +2698,7 @@ static RegisterPrimOp primop_path({ - sha256\ When provided, this is the expected hash of the file at the - path. Evaluation will fail if the hash is incorrect, and + path. Evaluation fails if the hash is incorrect, and providing a hash allows `builtins.path` to be used even when the `pure-eval` nix config option is on. )", @@ -4806,7 +4806,7 @@ void EvalState::createBaseEnv(const EvalSettings & evalSettings) .type = nInt, .doc = R"( Return the [Unix time](https://en.wikipedia.org/wiki/Unix_time) at first evaluation. - Repeated references to that name will re-use the initially obtained value. + Repeated references to that name re-use the initially obtained value. Example: @@ -4821,7 +4821,7 @@ void EvalState::createBaseEnv(const EvalSettings & evalSettings) 1683705525 ``` - The [store path](@docroot@/store/store-path.md) of a derivation depending on `currentTime` will differ for each evaluation, unless both evaluate `builtins.currentTime` in the same second. + The [store path](@docroot@/store/store-path.md) of a derivation depending on `currentTime` differs for each evaluation, unless both evaluate `builtins.currentTime` in the same second. )", .impureOnly = true, }); diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 6a7284e05..496678884 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -240,7 +240,7 @@ static RegisterPrimOp primop_getContext({ The string context tracks references to derivations within a string. It is represented as an attribute set of [store derivation](@docroot@/glossary.md#gloss-store-derivation) paths mapping to output names. - Using [string interpolation](@docroot@/language/string-interpolation.md) on a derivation will add that derivation to the string context. + Using [string interpolation](@docroot@/language/string-interpolation.md) on a derivation adds that derivation to the string context. For example, ```nix diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 4dd8b2606..ea6145f6f 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -214,7 +214,7 @@ static RegisterPrimOp primop_fetchClosure({ .doc = R"( Fetch a store path [closure](@docroot@/glossary.md#gloss-closure) from a binary cache, and return the store path as a string with context. - This function can be invoked in three ways, that we will discuss in order of preference. + This function can be invoked in three ways that we will discuss in order of preference. **Fetch a content-addressed store path** diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index bdd65b4c6..5b6dd6531 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -191,7 +191,7 @@ static void fetchTree( input.to_string()); else state.error( - "in pure evaluation mode, '%s' will not fetch unlocked input '%s'", + "in pure evaluation mode, '%s' doesn't fetch unlocked input '%s'", fetcher, input.to_string()).atPos(pos).debugThrow(); } @@ -243,7 +243,7 @@ static RegisterPrimOp primop_fetchTree({ That is, `fetchTree` is idempotent. Downloads are cached in `$XDG_CACHE_HOME/nix`. - The remote source will be fetched from the network if both are true: + The remote source is fetched from the network if both are true: - A NAR hash is supplied and the corresponding store path is not [valid](@docroot@/glossary.md#gloss-validity), that is, not available in the store > **Note** @@ -338,7 +338,7 @@ static RegisterPrimOp primop_fetchTree({ > **Note** > - > If the URL points to a local directory, and no `ref` or `rev` is given, Nix will only consider files added to the Git index, as listed by `git ls-files` but use the *current file contents* of the Git working directory. + > If the URL points to a local directory, and no `ref` or `rev` is given, Nix only considers files added to the Git index, as listed by `git ls-files` but use the *current file contents* of the Git working directory. - `ref` (String, optional) @@ -681,7 +681,7 @@ static RegisterPrimOp primop_fetchGit({ This option has no effect once `shallow` cloning is enabled. By default, the `ref` value is prefixed with `refs/heads/`. - As of 2.3.0, Nix will not prefix `refs/heads/` if `ref` starts with `refs/`. + As of 2.3.0, Nix doesn't prefix `refs/heads/` if `ref` starts with `refs/`. - `submodules` (default: `false`) @@ -840,7 +840,7 @@ static RegisterPrimOp primop_fetchGit({ } ``` - Nix will refetch the branch according to the [`tarball-ttl`](@docroot@/command-ref/conf-file.md#conf-tarball-ttl) setting. + Nix refetches the branch according to the [`tarball-ttl`](@docroot@/command-ref/conf-file.md#conf-tarball-ttl) setting. This behavior is disabled in [pure evaluation mode](@docroot@/command-ref/conf-file.md#conf-pure-eval). @@ -851,9 +851,9 @@ static RegisterPrimOp primop_fetchGit({ ``` If the URL points to a local directory, and no `ref` or `rev` is - given, `fetchGit` will use the current content of the checked-out - files, even if they are not committed or added to Git's index. It will - only consider files added to the Git repository, as listed by `git ls-files`. + given, `fetchGit` uses the current content of the checked-out + files, even if they are not committed or added to Git's index. It + only considers files added to the Git repository, as listed by `git ls-files`. )", .fun = prim_fetchGit, }); diff --git a/src/libfetchers/include/nix/fetchers/fetch-settings.hh b/src/libfetchers/include/nix/fetchers/fetch-settings.hh index 7bd04db53..9cfd25e0b 100644 --- a/src/libfetchers/include/nix/fetchers/fetch-settings.hh +++ b/src/libfetchers/include/nix/fetchers/fetch-settings.hh @@ -28,7 +28,7 @@ struct Settings : public Config space-separated `host=token` values. The specific token used is selected by matching the `host` portion against the "host" specification of the input. The `host` portion may - contain a path element which will match against the prefix + contain a path element which matches against the prefix URL for the input. (eg: `github.com/org=token`). The actual use of the `token` value is determined by the type of resource being accessed: @@ -95,11 +95,11 @@ struct Settings : public Config Setting trustTarballsFromGitForges{ this, true, "trust-tarballs-from-git-forges", R"( - If enabled (the default), Nix will consider tarballs from + If enabled (the default), Nix considers tarballs from GitHub and similar Git forges to be locked if a Git revision is specified, e.g. `github:NixOS/patchelf/7c2f768bf9601268a4e71c2ebe91e2011918a70f`. - This requires Nix to trust that the provider will return the + This requires Nix to trust that the provider returns the correct contents for the specified Git revision. If disabled, such tarballs are only considered locked if a diff --git a/src/libflake/flake.cc b/src/libflake/flake.cc index 24252d710..c178d0760 100644 --- a/src/libflake/flake.cc +++ b/src/libflake/flake.cc @@ -794,10 +794,10 @@ LockedFlake lockFlake( if (auto unlockedInput = newLockFile.isUnlocked(state.fetchSettings)) { if (lockFlags.failOnUnlocked) throw Error( - "Will not write lock file of flake '%s' because it has an unlocked input ('%s'). " + "Not writing lock file of flake '%s' because it has an unlocked input ('%s'). " "Use '--allow-dirty-locks' to allow this anyway.", topRef, *unlockedInput); if (state.fetchSettings.warnDirty) - warn("will not write lock file of flake '%s' because it has an unlocked input ('%s')", topRef, *unlockedInput); + warn("not writing lock file of flake '%s' because it has an unlocked input ('%s')", topRef, *unlockedInput); } else { if (!lockFlags.updateLockFile) throw Error("flake '%s' requires lock file changes but they're not allowed due to '--no-update-lock-file'", topRef); diff --git a/src/libmain/plugin.cc b/src/libmain/plugin.cc index db686a251..760a096ad 100644 --- a/src/libmain/plugin.cc +++ b/src/libmain/plugin.cc @@ -43,9 +43,9 @@ struct PluginSettings : Config {}, "plugin-files", R"( - A list of plugin files to be loaded by Nix. Each of these files will - be dlopened by Nix. If they contain the symbol `nix_plugin_entry()`, - this symbol will be called. Alternatively, they can affect execution + A list of plugin files to be loaded by Nix. Each of these files is + dlopened by Nix. If they contain the symbol `nix_plugin_entry()`, + this symbol is called. Alternatively, they can affect execution through static initialization. In particular, these plugins may construct static instances of RegisterPrimOp to add new primops or constants to the expression language, RegisterStoreImplementation to add new store @@ -60,7 +60,7 @@ struct PluginSettings : Config itself, they must be DSOs compatible with the instance of Nix running at the time (i.e. compiled against the same headers, not linked to any incompatible libraries). They should not be linked to - any Nix libs directly, as those will be available already at load + any Nix libraries directly, as those are already available at load time. If an entry in the list is a directory, all files in the directory diff --git a/src/libstore/include/nix/store/filetransfer.hh b/src/libstore/include/nix/store/filetransfer.hh index 10c3ec7ef..745aeb29e 100644 --- a/src/libstore/include/nix/store/filetransfer.hh +++ b/src/libstore/include/nix/store/filetransfer.hh @@ -46,7 +46,7 @@ struct FileTransferSettings : Config )"}; Setting tries{this, 5, "download-attempts", - "How often Nix will attempt to download a file before giving up."}; + "The number of times Nix attempts to download a file before giving up."}; Setting downloadBufferSize{this, 64 * 1024 * 1024, "download-buffer-size", R"( diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 00d929236..b5157b4f4 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -109,7 +109,7 @@ public: Setting tryFallback{ this, false, "fallback", R"( - If set to `true`, Nix will fall back to building from source if a + If set to `true`, Nix falls back to building from source if a binary substitute fails. This is equivalent to the `--fallback` flag. The default is `false`. )", @@ -127,11 +127,11 @@ public: MaxBuildJobsSetting maxBuildJobs{ this, 1, "max-jobs", R"( - Maximum number of jobs that Nix will try to build locally in parallel. + Maximum number of jobs that Nix tries to build locally in parallel. The special value `auto` causes Nix to use the number of CPUs in your system. Use `0` to disable local builds and directly use the remote machines specified in [`builders`](#conf-builders). - This will not affect derivations that have [`preferLocalBuild = true`](@docroot@/language/advanced-attributes.md#adv-attr-preferLocalBuild), which are always built locally. + This doesn't affect derivations that have [`preferLocalBuild = true`](@docroot@/language/advanced-attributes.md#adv-attr-preferLocalBuild), which are always built locally. > **Note** > @@ -146,8 +146,8 @@ public: this, 16, "max-substitution-jobs", R"( This option defines the maximum number of substitution jobs that Nix - will try to run in parallel. The default is `16`. The minimum value - one can choose is `1` and lower values will be interpreted as `1`. + tries to run in parallel. The default is `16`. The minimum value + one can choose is `1` and lower values are interpreted as `1`. )", {"substitution-max-jobs"}}; @@ -164,7 +164,7 @@ public: A very generic example using `derivation` and `xargs` may be more appropriate to explain the mechanism. Using `mkDerivation` as an example requires being aware of that there are multiple independent layers that are completely opaque here. --> - For instance, in Nixpkgs, if the attribute `enableParallelBuilding` for the `mkDerivation` build helper is set to `true`, it will pass the `-j${NIX_BUILD_CORES}` flag to GNU Make. + For instance, in Nixpkgs, if the attribute `enableParallelBuilding` for the `mkDerivation` build helper is set to `true`, it passes the `-j${NIX_BUILD_CORES}` flag to GNU Make. The value `0` means that the `builder` should use all available CPU cores in the system. @@ -186,7 +186,7 @@ public: this, NIX_LOCAL_SYSTEM, "system", R"( The system type of the current Nix installation. - Nix will only build a given [store derivation](@docroot@/glossary.md#gloss-store-derivation) locally when its `system` attribute equals any of the values specified here or in [`extra-platforms`](#conf-extra-platforms). + Nix only builds a given [store derivation](@docroot@/glossary.md#gloss-store-derivation) locally when its `system` attribute equals any of the values specified here or in [`extra-platforms`](#conf-extra-platforms). The default value is set when Nix itself is compiled for the system it will run on. The following system types are widely used, as Nix is actively supported on these platforms: @@ -292,28 +292,28 @@ public: > `i686-linux,x86_64-linux` 3. The SSH identity file to be used to log in to the remote machine. - If omitted, SSH will use its regular identities. + If omitted, SSH uses its regular identities. > **Example** > > `/home/user/.ssh/id_mac` - 4. The maximum number of builds that Nix will execute in parallel on the machine. + 4. The maximum number of builds that Nix executes in parallel on the machine. Typically this should be equal to the number of CPU cores. 5. The “speed factor”, indicating the relative speed of the machine as a positive integer. - If there are multiple machines of the right type, Nix will prefer the fastest, taking load into account. + If there are multiple machines of the right type, Nix prefers the fastest, taking load into account. 6. A comma-separated list of supported [system features](#conf-system-features). - A machine will only be used to build a derivation if all the features in the derivation's [`requiredSystemFeatures`](@docroot@/language/advanced-attributes.html#adv-attr-requiredSystemFeatures) attribute are supported by that machine. + A machine is only used to build a derivation if all the features in the derivation's [`requiredSystemFeatures`](@docroot@/language/advanced-attributes.html#adv-attr-requiredSystemFeatures) attribute are supported by that machine. 7. A comma-separated list of required [system features](#conf-system-features). - A machine will only be used to build a derivation if all of the machine’s required features appear in the derivation’s [`requiredSystemFeatures`](@docroot@/language/advanced-attributes.html#adv-attr-requiredSystemFeatures) attribute. + A machine is only used to build a derivation if all of the machine’s required features appear in the derivation’s [`requiredSystemFeatures`](@docroot@/language/advanced-attributes.html#adv-attr-requiredSystemFeatures) attribute. 8. The (base64-encoded) public host key of the remote machine. - If omitted, SSH will use its regular `known_hosts` file. + If omitted, SSH uses its regular `known_hosts` file. The value for this field can be obtained via `base64 -w0`. @@ -335,7 +335,7 @@ public: > nix@poochie.labs.cs.uu.nl i686-linux /home/nix/.ssh/id_scratchy 1 2 kvm benchmark > ``` > - > However, `poochie` will only build derivations that have the attribute + > However, `poochie` only builds derivations that have the attribute > > ```nix > requiredSystemFeatures = [ "benchmark" ]; @@ -348,7 +348,7 @@ public: > ``` > > `itchy` cannot do builds that require `kvm`, but `scratchy` does support such builds. - > For regular builds, `itchy` will be preferred over `scratchy` because it has a higher speed factor. + > For regular builds, `itchy` is preferred over `scratchy` because it has a higher speed factor. For Nix to use substituters, the calling user must be in the [`trusted-users`](#conf-trusted-users) list. @@ -372,15 +372,15 @@ public: Setting alwaysAllowSubstitutes{ this, false, "always-allow-substitutes", R"( - If set to `true`, Nix will ignore the [`allowSubstitutes`](@docroot@/language/advanced-attributes.md) attribute in derivations and always attempt to use [available substituters](#conf-substituters). + If set to `true`, Nix ignores the [`allowSubstitutes`](@docroot@/language/advanced-attributes.md) attribute in derivations and always attempt to use [available substituters](#conf-substituters). )"}; Setting buildersUseSubstitutes{ this, false, "builders-use-substitutes", R"( - If set to `true`, Nix will instruct [remote build machines](#conf-builders) to use their own [`substituters`](#conf-substituters) if available. + If set to `true`, Nix instructs [remote build machines](#conf-builders) to use their own [`substituters`](#conf-substituters) if available. - It means that remote build hosts will fetch as many dependencies as possible from their own substituters (e.g, from `cache.nixos.org`) instead of waiting for the local machine to upload them all. + It means that remote build hosts fetches as many dependencies as possible from their own substituters (e.g, from `cache.nixos.org`) instead of waiting for the local machine to upload them all. This can drastically reduce build times if the network connection between the local machine and the remote build host is slow. )"}; @@ -415,7 +415,7 @@ public: Setting useSubstitutes{ this, true, "substitute", R"( - If set to `true` (default), Nix will use binary substitutes if + If set to `true` (default), Nix uses binary substitutes if available. This option can be disabled to force building from source. )", @@ -432,11 +432,11 @@ public: since that would allow him/her to influence the build result. Therefore, if this option is non-empty and specifies a valid group, - builds will be performed under the user accounts that are a member + builds are performed under the user accounts that are a member of the group specified here (as listed in `/etc/group`). Those user accounts should not be used for any other purpose\! - Nix will never run two builds under the same user account at the + Nix never runs two builds under the same user account at the same time. This is to prevent an obvious security hole: a malicious user writing a Nix expression that modifies the build result of a legitimate Nix expression being built by another user. Therefore it @@ -448,7 +448,7 @@ public: by the Nix account, its group should be the group specified here, and its mode should be `1775`. - If the build users group is empty, builds will be performed under + If the build users group is empty, builds areperformed under the uid of the Nix process (that is, the uid of the caller if `NIX_REMOTE` is empty, the uid under which the Nix daemon runs if `NIX_REMOTE` is `daemon`). Obviously, this should not be used @@ -503,7 +503,7 @@ public: Setting keepLog{ this, true, "keep-build-log", R"( - If set to `true` (the default), Nix will write the build log of a + If set to `true` (the default), Nix writes the build log of a derivation (i.e. the standard output and error of its builder) to the directory `/nix/var/log/nix/drvs`. The build log can be retrieved using the command `nix-store -l path`. @@ -514,8 +514,8 @@ public: this, true, "compress-build-log", R"( If set to `true` (the default), build logs written to - `/nix/var/log/nix/drvs` will be compressed on the fly using bzip2. - Otherwise, they will not be compressed. + `/nix/var/log/nix/drvs` are compressed on the fly using bzip2. + Otherwise, they are not compressed. )", {"build-compress-log"}}; @@ -534,14 +534,14 @@ public: Setting gcKeepOutputs{ this, false, "keep-outputs", R"( - If `true`, the garbage collector will keep the outputs of - non-garbage derivations. If `false` (default), outputs will be + If `true`, the garbage collector keeps the outputs of + non-garbage derivations. If `false` (default), outputs are deleted unless they are GC roots themselves (or reachable from other roots). In general, outputs must be registered as roots separately. However, even if the output of a derivation is registered as a root, the - collector will still delete store paths that are used only at build + collector still deletes store paths that are used only at build time (e.g., the C compiler, or source tarballs downloaded from the network). To prevent it from doing so, set this option to `true`. )", @@ -550,9 +550,9 @@ public: Setting gcKeepDerivations{ this, true, "keep-derivations", R"( - If `true` (default), the garbage collector will keep the derivations - from which non-garbage store paths were built. If `false`, they will - be deleted unless explicitly registered as a root (or reachable from + If `true` (default), the garbage collector keeps the derivations + from which non-garbage store paths were built. If `false`, they are + deleted unless explicitly registered as a root (or reachable from other roots). Keeping derivation around is useful for querying and traceability @@ -582,7 +582,7 @@ public: If `true`, when you add a Nix derivation to a user environment, the path of the derivation is stored in the user environment. Thus, the - derivation will not be garbage-collected until the user environment + derivation isn't garbage-collected until the user environment generation is deleted (`nix-env --delete-generations`). To prevent build-time-only dependencies from being collected, you should also turn on `keep-outputs`. @@ -603,9 +603,9 @@ public: #endif , "sandbox", R"( - If set to `true`, builds will be performed in a *sandboxed + If set to `true`, builds are performed in a *sandboxed environment*, i.e., they’re isolated from the normal file system - hierarchy and will only see their dependencies in the Nix store, + hierarchy and only see their dependencies in the Nix store, the temporary build directory, private versions of `/proc`, `/dev`, `/dev/shm` and `/dev/pts` (on Linux), and the paths configured with the `sandbox-paths` option. This is useful to @@ -634,13 +634,13 @@ public: R"( A list of paths bind-mounted into Nix sandbox environments. You can use the syntax `target=source` to mount a path in a different - location in the sandbox; for instance, `/bin=/nix-bin` will mount + location in the sandbox; for instance, `/bin=/nix-bin` mounts the path `/nix-bin` as `/bin` inside the sandbox. If *source* is followed by `?`, then it is not an error if *source* does not exist; - for example, `/dev/nvidiactl?` specifies that `/dev/nvidiactl` will + for example, `/dev/nvidiactl?` specifies that `/dev/nvidiactl` only be mounted in the sandbox if it exists in the host filesystem. - If the source is in the Nix store, then its closure will be added to + If the source is in the Nix store, then its closure is added to the sandbox as well. Depending on how Nix was built, the default value for this option @@ -655,15 +655,15 @@ public: Setting requireDropSupplementaryGroups{this, isRootUser(), "require-drop-supplementary-groups", R"( Following the principle of least privilege, - Nix will attempt to drop supplementary groups when building with sandboxing. + Nix attempts to drop supplementary groups when building with sandboxing. However this can fail under some circumstances. For example, if the user lacks the `CAP_SETGID` capability. Search `setgroups(2)` for `EPERM` to find more detailed information on this. - If you encounter such a failure, setting this option to `false` will let you ignore it and continue. + If you encounter such a failure, setting this option to `false` enables you to ignore it and continue. But before doing so, you should consider the security implications carefully. - Not dropping supplementary groups means the build sandbox will be less restricted than intended. + Not dropping supplementary groups means the build sandbox is less restricted than intended. This option defaults to `true` when the user is root (since `root` usually has permissions to call setgroups) @@ -699,12 +699,12 @@ public: R"( The directory on the host, in which derivations' temporary build directories are created. - If not set, Nix will use the system temporary directory indicated by the `TMPDIR` environment variable. + If not set, Nix uses the system temporary directory indicated by the `TMPDIR` environment variable. Note that builds are often performed by the Nix daemon, so its `TMPDIR` is used, and not that of the Nix command line interface. This is also the location where [`--keep-failed`](@docroot@/command-ref/opt-common.md#opt-keep-failed) leaves its files. - If Nix runs without sandbox, or if the platform does not support sandboxing with bind mounts (e.g. macOS), then the [`builder`](@docroot@/language/derivations.md#attr-builder)'s environment will contain this directory, instead of the virtual location [`sandbox-build-dir`](#conf-sandbox-build-dir). + If Nix runs without sandbox, or if the platform does not support sandboxing with bind mounts (e.g. macOS), then the [`builder`](@docroot@/language/derivations.md#attr-builder)'s environment contains this directory instead of the virtual location [`sandbox-build-dir`](#conf-sandbox-build-dir). )"}; Setting allowedImpureHostPrefixes{this, {}, "allowed-impure-host-deps", @@ -745,12 +745,11 @@ public: 3. The path to the build's derivation - 4. The path to the build's scratch directory. This directory will - exist only if the build was run with `--keep-failed`. + 4. The path to the build's scratch directory. This directory + exists only if the build was run with `--keep-failed`. - The stderr and stdout output from the diff hook will not be - displayed to the user. Instead, it will print to the nix-daemon's - log. + The stderr and stdout output from the diff hook isn't + displayed to the user. Instead, it print to the nix-daemon's log. When using the Nix daemon, `diff-hook` must be set in the `nix.conf` configuration file, and cannot be passed at the command line. @@ -788,8 +787,8 @@ public: this, 60 * 60, "tarball-ttl", R"( The number of seconds a downloaded tarball is considered fresh. If - the cached tarball is stale, Nix will check whether it is still up - to date using the ETag header. Nix will download a new version if + the cached tarball is stale, Nix checks whether it is still up + to date using the ETag header. Nix downloads a new version if the ETag header is unsupported, or the cached ETag doesn't match. Setting the TTL to `0` forces Nix to always check if the tarball is @@ -824,7 +823,7 @@ public: R"( System types of executables that can be run on this machine. - Nix will only build a given [store derivation](@docroot@/glossary.md#gloss-store-derivation) locally when its `system` attribute equals any of the values specified here or in the [`system` option](#conf-system). + Nix only builds a given [store derivation](@docroot@/glossary.md#gloss-store-derivation) locally when its `system` attribute equals any of the values specified here or in the [`system` option](#conf-system). Setting this can be useful to build derivations locally on compatible machines: - `i686-linux` executables can be run on `x86_64-linux` machines (set by default) @@ -834,7 +833,7 @@ public: - `qemu-user` may be used to support non-native platforms (though this may be slow and buggy) - Build systems will usually detect the target platform to be the current physical system and therefore produce machine code incompatible with what may be intended in the derivation. + Build systems usually detect the target platform to be the current physical system and therefore produce machine code incompatible with what may be intended in the derivation. You should design your derivation's `builder` accordingly and cross-check the results when using this option against natively-built versions of your derivation. )", {}, @@ -924,7 +923,7 @@ public: this, 3600, "narinfo-cache-negative-ttl", R"( The TTL in seconds for negative lookups. - If a store path is queried from a [substituter](#conf-substituters) but was not found, there will be a negative lookup cached in the local disk cache database for the specified duration. + If a store path is queried from a [substituter](#conf-substituters) but was not found, a negative lookup is cached in the local disk cache database for the specified duration. Set to `0` to force updating the lookup cache. @@ -940,7 +939,7 @@ public: this, 30 * 24 * 3600, "narinfo-cache-positive-ttl", R"( The TTL in seconds for positive lookups. If a store path is queried - from a substituter, the result of the query will be cached in the + from a substituter, the result of the query is cached in the local disk cache database including some of the NAR metadata. The default TTL is a month, setting a shorter TTL for positive lookups can be useful for binary caches that have frequent garbage @@ -1026,7 +1025,7 @@ public: Setting netrcFile{ this, fmt("%s/%s", nixConfDir, "netrc"), "netrc-file", R"( - If set to an absolute path to a `netrc` file, Nix will use the HTTP + If set to an absolute path to a `netrc` file, Nix uses the HTTP authentication credentials in this file when trying to download from a remote host through HTTP or HTTPS. Defaults to `$NIX_CONF_DIR/netrc`. @@ -1052,7 +1051,7 @@ public: this, getDefaultSSLCertFile(), "ssl-cert-file", R"( The path of a file containing CA certificates used to - authenticate `https://` downloads. Nix by default will use + authenticate `https://` downloads. Nix by default uses the first of the following files that exists: 1. `/etc/ssl/certs/ca-certificates.crt` @@ -1084,7 +1083,7 @@ public: (Linux-specific.) By default, builders on Linux cannot acquire new privileges by calling setuid/setgid programs or programs that have file capabilities. For example, programs such as `sudo` or `ping` - will fail. (Note that in sandbox builds, no such programs are + should fail. (Note that in sandbox builds, no such programs are available unless you bind-mount them into the sandbox via the `sandbox-paths` option.) You can allow the use of such programs by enabling this option. This is impure and usually undesirable, but @@ -1108,7 +1107,7 @@ public: this, {}, "hashed-mirrors", R"( A list of web servers used by `builtins.fetchurl` to obtain files by - hash. Given a hash algorithm *ha* and a base-16 hash *h*, Nix will try to + hash. Given a hash algorithm *ha* and a base-16 hash *h*, Nix tries to download the file from *hashed-mirror*/*ha*/*h*. This allows files to be downloaded even if they have disappeared from their original URI. For example, given an example mirror `http://tarballs.nixos.org/`, @@ -1123,7 +1122,7 @@ public: Nix will attempt to download this file from `http://tarballs.nixos.org/sha256/2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae` - first. If it is not available there, if will try the original URI. + first. If it is not available there, it tries the original URI. )"}; Setting minFree{ @@ -1155,8 +1154,8 @@ public: Setting allowSymlinkedStore{ this, false, "allow-symlinked-store", R"( - If set to `true`, Nix will stop complaining if the store directory - (typically /nix/store) contains symlink components. + If set to `true`, Nix stops complaining if the store directory + (typically `/nix/store`) contains symlink components. This risks making some builds "impure" because builders sometimes "canonicalise" paths by resolving all symlink components. Problems @@ -1168,7 +1167,7 @@ public: Setting useXDGBaseDirectories{ this, false, "use-xdg-base-directories", R"( - If set to `true`, Nix will conform to the [XDG Base Directory Specification] for files in `$HOME`. + If set to `true`, Nix conforms to the [XDG Base Directory Specification] for files in `$HOME`. The environment variables used to implement this are documented in the [Environment Variables section](@docroot@/command-ref/env-common.md). [XDG Base Directory Specification]: https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html @@ -1206,7 +1205,7 @@ public: If the user is trusted (see `trusted-users` option), when building a fixed-output derivation, environment variables set in this option - will be passed to the builder if they are listed in [`impureEnvVars`](@docroot@/language/advanced-attributes.md#adv-attr-impureEnvVars). + is passed to the builder if they are listed in [`impureEnvVars`](@docroot@/language/advanced-attributes.md#adv-attr-impureEnvVars). This option is useful for, e.g., setting `https_proxy` for fixed-output derivations and in a multi-user Nix installation, or diff --git a/src/libstore/include/nix/store/local-fs-store.hh b/src/libstore/include/nix/store/local-fs-store.hh index f9421b7fe..d5fafb0c6 100644 --- a/src/libstore/include/nix/store/local-fs-store.hh +++ b/src/libstore/include/nix/store/local-fs-store.hh @@ -27,12 +27,12 @@ struct LocalFSStoreConfig : virtual StoreConfig PathSetting stateDir{this, rootDir.get() ? *rootDir.get() + "/nix/var/nix" : settings.nixStateDir, "state", - "Directory where Nix will store state."}; + "Directory where Nix stores state."}; PathSetting logDir{this, rootDir.get() ? *rootDir.get() + "/nix/var/log/nix" : settings.nixLogDir, "log", - "directory where Nix will store log files."}; + "directory where Nix stores log files."}; PathSetting realStoreDir{this, rootDir.get() ? *rootDir.get() + "/nix/store" : storeDir, "real", diff --git a/src/libstore/include/nix/store/local-store.hh b/src/libstore/include/nix/store/local-store.hh index efc59dc8c..9a118fcc5 100644 --- a/src/libstore/include/nix/store/local-store.hh +++ b/src/libstore/include/nix/store/local-store.hh @@ -54,7 +54,7 @@ struct LocalStoreConfig : std::enable_shared_from_this, virtua R"( Allow this store to be opened when its [database](@docroot@/glossary.md#gloss-nix-database) is on a read-only filesystem. - Normally Nix will attempt to open the store database in read-write mode, even for querying (when write access is not needed), causing it to fail if the database is on a read-only filesystem. + Normally Nix attempts to open the store database in read-write mode, even for querying (when write access is not needed), causing it to fail if the database is on a read-only filesystem. Enable read-only mode to disable locking and open the SQLite database with the [`immutable` parameter](https://www.sqlite.org/c3ref/open.html) set. diff --git a/src/libstore/include/nix/store/s3-binary-cache-store.hh b/src/libstore/include/nix/store/s3-binary-cache-store.hh index 9a123602e..c38591e60 100644 --- a/src/libstore/include/nix/store/s3-binary-cache-store.hh +++ b/src/libstore/include/nix/store/s3-binary-cache-store.hh @@ -25,7 +25,7 @@ struct S3BinaryCacheStoreConfig : std::enable_shared_from_this **Note** > - > This endpoint must support HTTPS and will use path-based + > This endpoint must support HTTPS and uses path-based > addressing instead of virtual host based addressing. )"}; diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index 7dee1f5c7..88f3783f5 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -107,7 +107,7 @@ constexpr std::array xpFeatureDetails .name = "git-hashing", .description = R"( Allow creating (content-addressed) store objects which are hashed via Git's hashing algorithm. - These store objects will not be understandable by older versions of Nix. + These store objects aren't understandable by older versions of Nix. )", .trackingUrl = "https://github.com/NixOS/nix/milestone/41", }, diff --git a/src/libutil/include/nix/util/logging.hh b/src/libutil/include/nix/util/logging.hh index 765975faa..dabfac483 100644 --- a/src/libutil/include/nix/util/logging.hh +++ b/src/libutil/include/nix/util/logging.hh @@ -55,7 +55,7 @@ struct LoggerSettings : Config Setting jsonLogPath{ this, "", "json-log-path", R"( - A file or unix socket to which JSON records of Nix's log output will be + A file or unix socket to which JSON records of Nix's log output are written, in the same format as `--log-format internal-json` (without the `@nix ` prefixes on each line). Concurrent writes to the same file by multiple Nix processes are not supported and diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 80ebf6bfa..3313c02aa 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -474,7 +474,7 @@ static void main_nix_build(int argc, char * * argv) } catch (Error & e) { logError(e.info()); - notice("will use bash from your environment"); + notice("uses bash from your environment"); shell = "bash"; } } diff --git a/src/nix/unix/daemon.cc b/src/nix/unix/daemon.cc index 4132d5be2..a14632c2f 100644 --- a/src/nix/unix/daemon.cc +++ b/src/nix/unix/daemon.cc @@ -572,7 +572,7 @@ struct CmdDaemon : Command addFlag({ .longName = "force-untrusted", - .description = "Force the daemon to not trust connecting clients. The connection will be processed by the receiving daemon before forwarding commands.", + .description = "Force the daemon to not trust connecting clients. The connection is processed by the receiving daemon before forwarding commands.", .handler = {[&]() { isTrustedOpt = NotTrusted; }}, diff --git a/tests/functional/fetchGit.sh b/tests/functional/fetchGit.sh index 5832facda..a41aa35c0 100755 --- a/tests/functional/fetchGit.sh +++ b/tests/functional/fetchGit.sh @@ -81,7 +81,7 @@ path2=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \" [[ $(nix eval --raw --expr "builtins.readFile (fetchGit { url = file://$repo; rev = \"$rev2\"; } + \"/hello\")") = world ]] # But without a hash, it fails. -expectStderr 1 nix eval --expr 'builtins.fetchGit "file:///foo"' | grepQuiet "'fetchGit' will not fetch unlocked input" +expectStderr 1 nix eval --expr 'builtins.fetchGit "file:///foo"' | grepQuiet "'fetchGit' doesn't fetch unlocked input" # Fetch again. This should be cached. mv $repo ${repo}-tmp diff --git a/tests/functional/flakes/unlocked-override.sh b/tests/functional/flakes/unlocked-override.sh index 512aca401..ed05440de 100755 --- a/tests/functional/flakes/unlocked-override.sh +++ b/tests/functional/flakes/unlocked-override.sh @@ -33,7 +33,7 @@ echo 456 > "$flake1Dir"/x.nix # Dirty overrides require --allow-dirty-locks. expectStderr 1 nix flake lock "$flake2Dir" --override-input flake1 "$TEST_ROOT/flake1" | - grepQuiet "Will not write lock file.*because it has an unlocked input" + grepQuiet "Not writing lock file.*because it has an unlocked input" nix flake lock "$flake2Dir" --override-input flake1 "$TEST_ROOT/flake1" --allow-dirty-locks diff --git a/tests/nixos/github-flakes.nix b/tests/nixos/github-flakes.nix index 30ab1f333..3d7a77ec5 100644 --- a/tests/nixos/github-flakes.nix +++ b/tests/nixos/github-flakes.nix @@ -1,8 +1,7 @@ -{ - lib, - config, - nixpkgs, - ... +{ lib +, config +, nixpkgs +, ... }: let pkgs = config.nodes.client.nixpkgs.pkgs; @@ -147,12 +146,11 @@ in }; client = - { - config, - lib, - pkgs, - nodes, - ... + { config + , lib + , pkgs + , nodes + , ... }: { virtualisation.writableStore = true; @@ -227,7 +225,7 @@ in # Fetching without a narHash should succeed if trust-github is set and fail otherwise. client.succeed(f"nix eval --raw --expr 'builtins.fetchTree github:github:fancy-enterprise/private-flake/{info['revision']}'") out = client.fail(f"nix eval --no-trust-tarballs-from-git-forges --raw --expr 'builtins.fetchTree github:github:fancy-enterprise/private-flake/{info['revision']}' 2>&1") - assert "will not fetch unlocked input" in out, "--no-trust-tarballs-from-git-forges did not fail with the expected error" + assert "doesn't fetch unlocked input" in out, "--no-trust-tarballs-from-git-forges did not fail with the expected error" # Shut down the web server. The flake should be cached on the client. github.succeed("systemctl stop httpd.service") From 5abaf361a41facb2baeee05aa58b9b5e3aa08f45 Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Thu, 12 Jun 2025 19:06:13 +0200 Subject: [PATCH 19/66] docker: reduce duplicates, use `coreutils-full` --- docker.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker.nix b/docker.nix index e3a0635d0..8262d1b70 100644 --- a/docker.nix +++ b/docker.nix @@ -290,7 +290,7 @@ let echo "${channelURL} ${channelName}" > $out${userHome}/.nix-channels mkdir -p $out/bin $out/usr/bin - ln -s ${pkgs.coreutils}/bin/env $out/usr/bin/env + ln -s ${pkgs.coreutils-full}/bin/env $out/usr/bin/env ln -s ${pkgs.bashInteractive}/bin/bash $out/bin/sh '' From 5862f38d00b3d84ee9272f90983a52ba1b9c71ee Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Thu, 12 Jun 2025 19:07:20 +0200 Subject: [PATCH 20/66] docker: use `callPackage`, parametrise the image build --- docker.nix | 90 +++++++++++++++++++++++--------------- flake.nix | 3 +- tests/nixos/nix-docker.nix | 10 +---- 3 files changed, 58 insertions(+), 45 deletions(-) diff --git a/docker.nix b/docker.nix index 8262d1b70..060dcd8f0 100644 --- a/docker.nix +++ b/docker.nix @@ -1,6 +1,10 @@ { - pkgs ? import { }, - lib ? pkgs.lib, + # Core dependencies + pkgs, + lib, + runCommand, + buildPackages, + # Image configuration name ? "nix", tag ? "latest", bundleNixpkgs ? true, @@ -14,36 +18,52 @@ gid ? 0, uname ? "root", gname ? "root", + # Default Packages + nix, + bashInteractive, + coreutils-full, + gnutar, + gzip, + gnugrep, + which, + curl, + less, + wget, + man, + cacert, + findutils, + iana-etc, + git, + openssh, + # Other dependencies + shadow, }: let - defaultPkgs = - with pkgs; - [ - nix - bashInteractive - coreutils-full - gnutar - gzip - gnugrep - which - curl - less - wget - man - cacert.out - findutils - iana-etc - git - openssh - ] - ++ extraPkgs; + defaultPkgs = [ + nix + bashInteractive + coreutils-full + gnutar + gzip + gnugrep + which + curl + less + wget + man + cacert.out + findutils + iana-etc + git + openssh + ] ++ extraPkgs; users = { root = { uid = 0; - shell = "${pkgs.bashInteractive}/bin/bash"; + shell = lib.getExe bashInteractive; home = "/root"; gid = 0; groups = [ "root" ]; @@ -52,7 +72,7 @@ let nobody = { uid = 65534; - shell = "${pkgs.shadow}/bin/nologin"; + shell = lib.getExe' shadow "nologin"; home = "/var/empty"; gid = 65534; groups = [ "nobody" ]; @@ -63,7 +83,7 @@ let // lib.optionalAttrs (uid != 0) { "${uname}" = { uid = uid; - shell = "${pkgs.bashInteractive}/bin/bash"; + shell = lib.getExe bashInteractive; home = "/home/${uname}"; gid = gid; groups = [ "${gname}" ]; @@ -170,7 +190,7 @@ let baseSystem = let nixpkgs = pkgs.path; - channel = pkgs.runCommand "channel-nixos" { inherit bundleNixpkgs; } '' + channel = runCommand "channel-nixos" { inherit bundleNixpkgs; } '' mkdir $out if [ "$bundleNixpkgs" ]; then ln -s ${ @@ -182,11 +202,11 @@ let echo "[]" > $out/manifest.nix fi ''; - rootEnv = pkgs.buildPackages.buildEnv { + rootEnv = buildPackages.buildEnv { name = "root-profile-env"; paths = defaultPkgs; }; - manifest = pkgs.buildPackages.runCommand "manifest.nix" { } '' + manifest = buildPackages.runCommand "manifest.nix" { } '' cat > $out < $out${userHome}/.nix-channels mkdir -p $out/bin $out/usr/bin - ln -s ${pkgs.coreutils-full}/bin/env $out/usr/bin/env - ln -s ${pkgs.bashInteractive}/bin/bash $out/bin/sh + ln -s ${lib.getExe' coreutils-full "env"} $out/usr/bin/env + ln -s ${lib.getExe bashInteractive} $out/bin/sh '' + (lib.optionalString (flake-registry-path != null) '' @@ -300,7 +320,7 @@ let globalFlakeRegistryPath="$nixCacheDir/flake-registry.json" ln -s ${flake-registry-path} $out$globalFlakeRegistryPath mkdir -p $out/nix/var/nix/gcroots/auto - rootName=$(${pkgs.nix}/bin/nix --extra-experimental-features nix-command hash file --type sha1 --base32 <(echo -n $globalFlakeRegistryPath)) + rootName=$(${lib.getExe' nix "nix"} --extra-experimental-features nix-command hash file --type sha1 --base32 <(echo -n $globalFlakeRegistryPath)) ln -s $globalFlakeRegistryPath $out/nix/var/nix/gcroots/auto/$rootName '') ); @@ -332,7 +352,7 @@ pkgs.dockerTools.buildLayeredImageWithNixDb { ''; config = { - Cmd = [ "${userHome}/.nix-profile/bin/bash" ]; + Cmd = [ (lib.getExe bashInteractive) ]; User = "${toString uid}:${toString gid}"; Env = [ "USER=${uname}" diff --git a/flake.nix b/flake.nix index 7d7c4d4c2..69bd2a21a 100644 --- a/flake.nix +++ b/flake.nix @@ -404,8 +404,7 @@ dockerImage = let pkgs = nixpkgsFor.${system}.native; - image = import ./docker.nix { - inherit pkgs; + image = pkgs.callPackage ./docker.nix { tag = pkgs.nix.version; }; in diff --git a/tests/nixos/nix-docker.nix b/tests/nixos/nix-docker.nix index c58a00cdd..f1c218585 100644 --- a/tests/nixos/nix-docker.nix +++ b/tests/nixos/nix-docker.nix @@ -1,21 +1,15 @@ # Test the container built by ../../docker.nix. { - lib, config, - nixpkgs, - hostPkgs, ... }: let pkgs = config.nodes.machine.nixpkgs.pkgs; - nixImage = import ../../docker.nix { - inherit (config.nodes.machine.nixpkgs) pkgs; - }; - nixUserImage = import ../../docker.nix { - inherit (config.nodes.machine.nixpkgs) pkgs; + nixImage = pkgs.callPackage ../../docker.nix { }; + nixUserImage = pkgs.callPackage ../../docker.nix { name = "nix-user"; uid = 1000; gid = 1000; From 6eb4ee68556af97ab88f3f909f3158e57429f6af Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Thu, 12 Jun 2025 19:18:07 +0200 Subject: [PATCH 21/66] docker: replace `git` with `gitMinimal` --- docker.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker.nix b/docker.nix index 060dcd8f0..e97ed1752 100644 --- a/docker.nix +++ b/docker.nix @@ -33,7 +33,7 @@ cacert, findutils, iana-etc, - git, + gitMinimal, openssh, # Other dependencies shadow, @@ -54,7 +54,7 @@ let cacert.out findutils iana-etc - git + gitMinimal openssh ] ++ extraPkgs; From 6587e7bcff6bfc7718cea96e64823fc2742d3d6e Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 19:42:50 +0000 Subject: [PATCH 22/66] libexpr: Add and use `lambda` getter --- src/libcmd/repl.cc | 2 +- src/libexpr-tests/primops.cc | 4 ++-- src/libexpr/eval-profiler.cc | 2 +- src/libexpr/eval.cc | 22 +++++++++++----------- src/libexpr/include/nix/expr/value.hh | 3 +++ src/libexpr/primops.cc | 4 ++-- src/libexpr/print.cc | 8 ++++---- src/libexpr/value-to-xml.cc | 12 ++++++------ src/libflake/flake.cc | 4 ++-- src/nix-build/nix-build.cc | 4 ++-- src/nix/flake.cc | 4 ++-- 11 files changed, 36 insertions(+), 33 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index f01f12860..8170bd579 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -484,7 +484,7 @@ ProcessLineResult NixRepl::processLine(std::string line) auto path = state->coerceToPath(noPos, v, context, "while evaluating the filename to edit"); return {path, 0}; } else if (v.isLambda()) { - auto pos = state->positions[v.payload.lambda.fun->pos]; + auto pos = state->positions[v.lambda().fun->pos]; if (auto path = std::get_if(&pos.origin)) return {*path, pos.line}; else diff --git a/src/libexpr-tests/primops.cc b/src/libexpr-tests/primops.cc index 2f864f2c2..6c301f157 100644 --- a/src/libexpr-tests/primops.cc +++ b/src/libexpr-tests/primops.cc @@ -667,8 +667,8 @@ namespace nix { auto v = eval("derivation"); ASSERT_EQ(v.type(), nFunction); ASSERT_TRUE(v.isLambda()); - ASSERT_NE(v.payload.lambda.fun, nullptr); - ASSERT_TRUE(v.payload.lambda.fun->hasFormals()); + ASSERT_NE(v.lambda().fun, nullptr); + ASSERT_TRUE(v.lambda().fun->hasFormals()); } TEST_F(PrimOpTest, currentTime) { diff --git a/src/libexpr/eval-profiler.cc b/src/libexpr/eval-profiler.cc index 7053e4ec7..b65bc3a4d 100644 --- a/src/libexpr/eval-profiler.cc +++ b/src/libexpr/eval-profiler.cc @@ -204,7 +204,7 @@ FrameInfo SampleStack::getFrameInfoFromValueAndPos(const Value & v, std::spanpos; - case tLambda: return payload.lambda.fun->pos; + case tLambda: return lambda().fun->pos; case tApp: return payload.app.left->determinePos(pos); default: return pos; } @@ -610,7 +610,7 @@ std::optional EvalState::getDoc(Value & v) }; } if (v.isLambda()) { - auto exprLambda = v.payload.lambda.fun; + auto exprLambda = v.lambda().fun; std::ostringstream s; std::string name; @@ -1567,13 +1567,13 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, if (vCur.isLambda()) { - ExprLambda & lambda(*vCur.payload.lambda.fun); + ExprLambda & lambda(*vCur.lambda().fun); auto size = (!lambda.arg ? 0 : 1) + (lambda.hasFormals() ? lambda.formals->formals.size() : 0); Env & env2(allocEnv(size)); - env2.up = vCur.payload.lambda.env; + env2.up = vCur.lambda().env; Displacement displ = 0; @@ -1603,7 +1603,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, symbols[i.name]) .atPos(lambda.pos) .withTrace(pos, "from call site") - .withFrame(*fun.payload.lambda.env, lambda) + .withFrame(*fun.lambda().env, lambda) .debugThrow(); } env2.values[displ++] = i.def->maybeThunk(*this, env2); @@ -1630,7 +1630,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, .atPos(lambda.pos) .withTrace(pos, "from call site") .withSuggestions(suggestions) - .withFrame(*fun.payload.lambda.env, lambda) + .withFrame(*fun.lambda().env, lambda) .debugThrow(); } unreachable(); @@ -1825,14 +1825,14 @@ void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res } } - if (!fun.isLambda() || !fun.payload.lambda.fun->hasFormals()) { + if (!fun.isLambda() || !fun.lambda().fun->hasFormals()) { res = fun; return; } - auto attrs = buildBindings(std::max(static_cast(fun.payload.lambda.fun->formals->formals.size()), args.size())); + auto attrs = buildBindings(std::max(static_cast(fun.lambda().fun->formals->formals.size()), args.size())); - if (fun.payload.lambda.fun->formals->ellipsis) { + if (fun.lambda().fun->formals->ellipsis) { // If the formals have an ellipsis (eg the function accepts extra args) pass // all available automatic arguments (which includes arguments specified on // the command line via --arg/--argstr) @@ -1840,7 +1840,7 @@ void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res attrs.insert(v); } else { // Otherwise, only pass the arguments that the function accepts - for (auto & i : fun.payload.lambda.fun->formals->formals) { + for (auto & i : fun.lambda().fun->formals->formals) { auto j = args.get(i.name); if (j) { attrs.insert(*j); @@ -1850,7 +1850,7 @@ Nix attempted to evaluate a function as a top level expression; in this case it must have its arguments supplied either by default values, or passed explicitly with '--arg' or '--argstr'. See https://nixos.org/manual/nix/stable/language/constructs.html#functions.)", symbols[i.name]) - .atPos(i.pos).withFrame(*fun.payload.lambda.env, *fun.payload.lambda.fun).debugThrow(); + .atPos(i.pos).withFrame(*fun.lambda().env, *fun.lambda().fun).debugThrow(); } } } diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index febe36f80..26ec5ff38 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -482,6 +482,9 @@ public: NixFloat fpoint() const { return payload.fpoint; } + + Lambda lambda() const + { return payload.lambda; } }; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 48bc272f9..a24695bcd 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -3138,12 +3138,12 @@ static void prim_functionArgs(EvalState & state, const PosIdx pos, Value * * arg if (!args[0]->isLambda()) state.error("'functionArgs' requires a function").atPos(pos).debugThrow(); - if (!args[0]->payload.lambda.fun->hasFormals()) { + if (!args[0]->lambda().fun->hasFormals()) { v.mkAttrs(&state.emptyBindings); return; } - const auto &formals = args[0]->payload.lambda.fun->formals->formals; + const auto &formals = args[0]->lambda().fun->formals->formals; auto attrs = state.buildBindings(formals.size()); for (auto & i : formals) attrs.insert(i.name, state.getBool(i.def), i.pos); diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index 06bae9c5c..f0e0eed2d 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -453,13 +453,13 @@ private: if (v.isLambda()) { output << "lambda"; - if (v.payload.lambda.fun) { - if (v.payload.lambda.fun->name) { - output << " " << state.symbols[v.payload.lambda.fun->name]; + if (v.lambda().fun) { + if (v.lambda().fun->name) { + output << " " << state.symbols[v.lambda().fun->name]; } std::ostringstream s; - s << state.positions[v.payload.lambda.fun->pos]; + s << state.positions[v.lambda().fun->pos]; output << " @ " << filterANSIEscapes(toView(s)); } } else if (v.isPrimOp()) { diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index e26fff71b..54ff06f9e 100644 --- a/src/libexpr/value-to-xml.cc +++ b/src/libexpr/value-to-xml.cc @@ -126,18 +126,18 @@ static void printValueAsXML(EvalState & state, bool strict, bool location, break; } XMLAttrs xmlAttrs; - if (location) posToXML(state, xmlAttrs, state.positions[v.payload.lambda.fun->pos]); + if (location) posToXML(state, xmlAttrs, state.positions[v.lambda().fun->pos]); XMLOpenElement _(doc, "function", xmlAttrs); - if (v.payload.lambda.fun->hasFormals()) { + if (v.lambda().fun->hasFormals()) { XMLAttrs attrs; - if (v.payload.lambda.fun->arg) attrs["name"] = state.symbols[v.payload.lambda.fun->arg]; - if (v.payload.lambda.fun->formals->ellipsis) attrs["ellipsis"] = "1"; + if (v.lambda().fun->arg) attrs["name"] = state.symbols[v.lambda().fun->arg]; + if (v.lambda().fun->formals->ellipsis) attrs["ellipsis"] = "1"; XMLOpenElement _(doc, "attrspat", attrs); - for (auto & i : v.payload.lambda.fun->formals->lexicographicOrder(state.symbols)) + for (auto & i : v.lambda().fun->formals->lexicographicOrder(state.symbols)) doc.writeEmptyElement("attr", singletonAttrs("name", state.symbols[i.name])); } else - doc.writeEmptyElement("varpat", singletonAttrs("name", state.symbols[v.payload.lambda.fun->arg])); + doc.writeEmptyElement("varpat", singletonAttrs("name", state.symbols[v.lambda().fun->arg])); break; } diff --git a/src/libflake/flake.cc b/src/libflake/flake.cc index 24252d710..41141d41d 100644 --- a/src/libflake/flake.cc +++ b/src/libflake/flake.cc @@ -252,8 +252,8 @@ static Flake readFlake( if (auto outputs = vInfo.attrs()->get(sOutputs)) { expectType(state, nFunction, *outputs->value, outputs->pos); - if (outputs->value->isLambda() && outputs->value->payload.lambda.fun->hasFormals()) { - for (auto & formal : outputs->value->payload.lambda.fun->formals->formals) { + if (outputs->value->isLambda() && outputs->value->lambda().fun->hasFormals()) { + for (auto & formal : outputs->value->lambda().fun->formals->formals) { if (formal.name != state.sSelf) flake.inputs.emplace(state.symbols[formal.name], FlakeInput { .ref = parseFlakeRef(state.fetchSettings, std::string(state.symbols[formal.name])) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 80ebf6bfa..e302aca67 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -387,8 +387,8 @@ static void main_nix_build(int argc, char * * argv) return false; } bool add = false; - if (v.type() == nFunction && v.payload.lambda.fun->hasFormals()) { - for (auto & i : v.payload.lambda.fun->formals->formals) { + if (v.type() == nFunction && v.lambda().fun->hasFormals()) { + for (auto & i : v.lambda().fun->formals->formals) { if (state->symbols[i.name] == "inNixShell") { add = true; break; diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 95cf85663..13f7363fc 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -492,8 +492,8 @@ struct CmdFlakeCheck : FlakeCommand if (!v.isLambda()) { throw Error("overlay is not a function, but %s instead", showType(v)); } - if (v.payload.lambda.fun->hasFormals() - || !argHasName(v.payload.lambda.fun->arg, "final")) + if (v.lambda().fun->hasFormals() + || !argHasName(v.lambda().fun->arg, "final")) throw Error("overlay does not take an argument named 'final'"); // FIXME: if we have a 'nixpkgs' input, use it to // evaluate the overlay. From 441fa86e82da65a71e01a5c45e3459287ab4d01e Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 19:48:42 +0000 Subject: [PATCH 23/66] libexpr: Add and use `thunk` getter --- src/libexpr/eval.cc | 10 +++++----- src/libexpr/include/nix/expr/eval-inline.hh | 4 ++-- src/libexpr/include/nix/expr/value.hh | 5 ++++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 51036a223..badb271f2 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -160,10 +160,10 @@ bool Value::isTrivial() const internalType != tApp && internalType != tPrimOpApp && (internalType != tThunk - || (dynamic_cast(payload.thunk.expr) - && ((ExprAttrs *) payload.thunk.expr)->dynamicAttrs.empty()) - || dynamic_cast(payload.thunk.expr) - || dynamic_cast(payload.thunk.expr)); + || (dynamic_cast(thunk().expr) + && ((ExprAttrs *) thunk().expr)->dynamicAttrs.empty()) + || dynamic_cast(thunk().expr) + || dynamic_cast(thunk().expr)); } @@ -2163,7 +2163,7 @@ void EvalState::forceValueDeep(Value & v) try { // If the value is a thunk, we're evaling. Otherwise no trace necessary. auto dts = debugRepl && i.value->isThunk() - ? makeDebugTraceStacker(*this, *i.value->payload.thunk.expr, *i.value->payload.thunk.env, i.pos, + ? makeDebugTraceStacker(*this, *i.value->thunk().expr, *i.value->thunk().env, i.pos, "while evaluating the attribute '%1%'", symbols[i.name]) : nullptr; diff --git a/src/libexpr/include/nix/expr/eval-inline.hh b/src/libexpr/include/nix/expr/eval-inline.hh index 6e5759c0b..97bf71a6b 100644 --- a/src/libexpr/include/nix/expr/eval-inline.hh +++ b/src/libexpr/include/nix/expr/eval-inline.hh @@ -89,9 +89,9 @@ Env & EvalState::allocEnv(size_t size) void EvalState::forceValue(Value & v, const PosIdx pos) { if (v.isThunk()) { - Env * env = v.payload.thunk.env; + Env * env = v.thunk().env; assert(env || v.isBlackhole()); - Expr * expr = v.payload.thunk.expr; + Expr * expr = v.thunk().expr; try { v.mkBlackhole(); //checkInterrupt(); diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index 26ec5ff38..29f8ac379 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -485,6 +485,9 @@ public: Lambda lambda() const { return payload.lambda; } + + ClosureThunk thunk() const + { return payload.thunk; } }; @@ -492,7 +495,7 @@ extern ExprBlackHole eBlackHole; bool Value::isBlackhole() const { - return internalType == tThunk && payload.thunk.expr == (Expr*) &eBlackHole; + return internalType == tThunk && thunk().expr == (Expr*) &eBlackHole; } void Value::mkBlackhole() From f07a9f863e16bb71ec330abf6c2ec5d5bff68788 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 19:51:44 +0000 Subject: [PATCH 24/66] libexpr: Add and use `primOpApp` getter --- src/libexpr/eval.cc | 10 +++++----- src/libexpr/include/nix/expr/value.hh | 3 +++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index badb271f2..f9bff7b98 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -525,9 +525,9 @@ std::ostream & operator<<(std::ostream & output, const PrimOp & primOp) const PrimOp * Value::primOpAppPrimOp() const { - Value * left = payload.primOpApp.left; + Value * left = primOpApp().left; while (left && !left->isPrimOp()) { - left = left->payload.primOpApp.left; + left = left->primOpApp().left; } if (!left) @@ -1702,7 +1702,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, Value * primOp = &vCur; while (primOp->isPrimOpApp()) { argsDone++; - primOp = primOp->payload.primOpApp.left; + primOp = primOp->primOpApp().left; } assert(primOp->isPrimOp()); auto arity = primOp->primOp()->arity; @@ -1718,8 +1718,8 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, Value * vArgs[maxPrimOpArity]; auto n = argsDone; - for (Value * arg = &vCur; arg->isPrimOpApp(); arg = arg->payload.primOpApp.left) - vArgs[--n] = arg->payload.primOpApp.right; + for (Value * arg = &vCur; arg->isPrimOpApp(); arg = arg->primOpApp().left) + vArgs[--n] = arg->primOpApp().right; for (size_t i = 0; i < argsLeft; ++i) vArgs[argsDone + i] = args[i]; diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index 29f8ac379..797e31191 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -488,6 +488,9 @@ public: ClosureThunk thunk() const { return payload.thunk; } + + FunctionApplicationThunk primOpApp() const + { return payload.primOpApp; } }; From c041d71406f706d971ee0ad53d555450de7ad03d Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 19:53:44 +0000 Subject: [PATCH 25/66] libexpr: Add and use `app` getter --- src/libexpr/eval.cc | 2 +- src/libexpr/include/nix/expr/eval-inline.hh | 2 +- src/libexpr/include/nix/expr/value.hh | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index f9bff7b98..37018007f 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -148,7 +148,7 @@ PosIdx Value::determinePos(const PosIdx pos) const switch (internalType) { case tAttrs: return attrs()->pos; case tLambda: return lambda().fun->pos; - case tApp: return payload.app.left->determinePos(pos); + case tApp: return app().left->determinePos(pos); default: return pos; } #pragma GCC diagnostic pop diff --git a/src/libexpr/include/nix/expr/eval-inline.hh b/src/libexpr/include/nix/expr/eval-inline.hh index 97bf71a6b..7d13d7cc7 100644 --- a/src/libexpr/include/nix/expr/eval-inline.hh +++ b/src/libexpr/include/nix/expr/eval-inline.hh @@ -106,7 +106,7 @@ void EvalState::forceValue(Value & v, const PosIdx pos) } } else if (v.isApp()) - callFunction(*v.payload.app.left, *v.payload.app.right, v, pos); + callFunction(*v.app().left, *v.app().right, v, pos); } diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index 797e31191..d25511c45 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -491,6 +491,9 @@ public: FunctionApplicationThunk primOpApp() const { return payload.primOpApp; } + + FunctionApplicationThunk app() const + { return payload.app; } }; From e4df1891237b58050e7b10380f7f62551ade2783 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 19:57:46 +0000 Subject: [PATCH 26/66] libexpr: Add and use `pathStr` getter --- src/libexpr-c/nix_api_value.cc | 2 +- src/libexpr/eval.cc | 6 +++--- src/libexpr/include/nix/expr/value.hh | 5 ++++- src/libexpr/primops.cc | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index 298d94845..8afe35a4b 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/src/libexpr-c/nix_api_value.cc @@ -252,7 +252,7 @@ const char * nix_get_path_string(nix_c_context * context, const nix_value * valu // We could use v.path().to_string().c_str(), but I'm concerned this // crashes. Looks like .path() allocates a CanonPath with a copy of the // string, then it gets the underlying data from that. - return v.payload.path.path; + return v.pathStr(); } NIXC_CATCH_ERRS_NULL } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 37018007f..66ac2e9fd 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2368,7 +2368,7 @@ BackedStringView EvalState::coerceToString( !canonicalizePath && !copyToStore ? // FIXME: hack to preserve path literals that end in a // slash, as in /foo/${x}. - v.payload.path.path + v.pathStr() : copyToStore ? store->printStorePath(copyPathToStore(context, v.path())) : std::string(v.path().path.abs()); @@ -2643,7 +2643,7 @@ void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::st ValuePrinter(*this, v2, errorPrintOptions)) .debugThrow(); } - if (strcmp(v1.payload.path.path, v2.payload.path.path) != 0) { + if (strcmp(v1.pathStr(), v2.pathStr()) != 0) { error( "path '%s' is not equal to path '%s'", ValuePrinter(*this, v1, errorPrintOptions), @@ -2811,7 +2811,7 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v return // FIXME: compare accessors by their fingerprint. v1.payload.path.accessor == v2.payload.path.accessor - && strcmp(v1.payload.path.path, v2.payload.path.path) == 0; + && strcmp(v1.pathStr(), v2.pathStr()) == 0; case nNull: return true; diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index d25511c45..fc9ce1b39 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -445,7 +445,7 @@ public: assert(internalType == tPath); return SourcePath( ref(payload.path.accessor->shared_from_this()), - CanonPath(CanonPath::unchecked_t(), payload.path.path)); + CanonPath(CanonPath::unchecked_t(), pathStr())); } std::string_view string_view() const @@ -494,6 +494,9 @@ public: FunctionApplicationThunk app() const { return payload.app; } + + const char * pathStr() const + { return payload.path.path; } }; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index a24695bcd..60f44ca62 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -651,7 +651,7 @@ struct CompareValues // Note: we don't take the accessor into account // since it's not obvious how to compare them in a // reproducible way. - return strcmp(v1->payload.path.path, v2->payload.path.path) < 0; + return strcmp(v1->pathStr(), v2->pathStr()) < 0; case nList: // Lexicographic comparison for (size_t i = 0;; i++) { From bc6b52aff012592a9794df979c0617c85f46c2c3 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 20:01:38 +0000 Subject: [PATCH 27/66] libexpr: Add and use `pathAccessor` getter --- src/libexpr/eval.cc | 4 ++-- src/libexpr/include/nix/expr/value.hh | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 66ac2e9fd..ae422a3d4 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2636,7 +2636,7 @@ void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::st return; case nPath: - if (v1.payload.path.accessor != v2.payload.path.accessor) { + if (v1.pathAccessor() != v2.pathAccessor()) { error( "path '%s' is not equal to path '%s' because their accessors are different", ValuePrinter(*this, v1, errorPrintOptions), @@ -2810,7 +2810,7 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v case nPath: return // FIXME: compare accessors by their fingerprint. - v1.payload.path.accessor == v2.payload.path.accessor + v1.pathAccessor() == v2.pathAccessor() && strcmp(v1.pathStr(), v2.pathStr()) == 0; case nNull: diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index fc9ce1b39..80bee59e9 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -444,7 +444,7 @@ public: { assert(internalType == tPath); return SourcePath( - ref(payload.path.accessor->shared_from_this()), + ref(pathAccessor()->shared_from_this()), CanonPath(CanonPath::unchecked_t(), pathStr())); } @@ -497,6 +497,9 @@ public: const char * pathStr() const { return payload.path.path; } + + SourceAccessor * pathAccessor() const + { return payload.path.accessor; } }; From 7b46eb9958e7681958020b68d9ea30a52be7cf09 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 22:29:05 +0000 Subject: [PATCH 28/66] libexpr: Remove non-const overload of `listElems` This overload isn't actually necessary anywhere and doesn't make much sense. The pointers to `Value`s are themselves const, but the `Value`s are mutable. A non-const member function implies that the object itself can be modified but this doesn't make much sense considering the return type: `Value * const * `, which is a pointer to a constant array of pointers to mutable values. --- src/libexpr/include/nix/expr/value.hh | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index 80bee59e9..fcc118c7e 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -410,11 +410,6 @@ public: return internalType == tList1 || internalType == tList2 || internalType == tListN; } - Value * const * listElems() - { - return internalType == tList1 || internalType == tList2 ? payload.smallList : payload.bigList.elems; - } - std::span listItems() const { assert(isList()); From f8c1ac9515c03eedb79a7aa9e437d04428c4f736 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 11 Jun 2025 15:15:00 -0400 Subject: [PATCH 29/66] Introduce top-level `structuredAttrs` field in JSON derivation format Makes the behavoral change of #13263 without the underlying refactor. Hopefully this clearly safe from a perf and GC perspective, and will make it easier to benchmark #13263. --- .../source/language/advanced-attributes.md | 20 ++----- .../source/protocols/json/derivation.md | 4 ++ doc/manual/source/store/derivation/index.md | 11 ++++ ...-attributes-structured-attrs-defaults.json | 12 +++- .../advanced-attributes-structured-attrs.json | 58 ++++++++++++++++++- ...-attributes-structured-attrs-defaults.json | 10 +++- .../advanced-attributes-structured-attrs.json | 56 +++++++++++++++++- src/libstore/derivations.cc | 17 +++++- tests/functional/structured-attrs.sh | 2 +- 9 files changed, 169 insertions(+), 21 deletions(-) diff --git a/doc/manual/source/language/advanced-attributes.md b/doc/manual/source/language/advanced-attributes.md index a939847e1..34c3b636b 100644 --- a/doc/manual/source/language/advanced-attributes.md +++ b/doc/manual/source/language/advanced-attributes.md @@ -53,23 +53,13 @@ Derivations can declare some infrequently used optional attributes. - [`__structuredAttrs`]{#adv-attr-structuredAttrs}\ If the special attribute `__structuredAttrs` is set to `true`, the other derivation - attributes are serialised into a file in JSON format. The environment variable - `NIX_ATTRS_JSON_FILE` points to the exact location of that file both in a build - and a [`nix-shell`](../command-ref/nix-shell.md). This obviates the need for - [`passAsFile`](#adv-attr-passAsFile) since JSON files have no size restrictions, - unlike process environments. + attributes are serialised into a file in JSON format. - It also makes it possible to tweak derivation settings in a structured way; see - [`outputChecks`](#adv-attr-outputChecks) for example. + This obviates the need for [`passAsFile`](#adv-attr-passAsFile) since JSON files have no size restrictions, unlike process environments. + It also makes it possible to tweak derivation settings in a structured way; + see [`outputChecks`](#adv-attr-outputChecks) for example. - As a convenience to Bash builders, - Nix writes a script that initialises shell variables - corresponding to all attributes that are representable in Bash. The - environment variable `NIX_ATTRS_SH_FILE` points to the exact - location of the script, both in a build and a - [`nix-shell`](../command-ref/nix-shell.md). This includes non-nested - (associative) arrays. For example, the attribute `hardening.format = true` - ends up as the Bash associative array element `${hardening[format]}`. + See the [corresponding section in the derivation page](@docroot@/store/derivation/index.md#structured-attrs) for further details. > **Warning** > diff --git a/doc/manual/source/protocols/json/derivation.md b/doc/manual/source/protocols/json/derivation.md index 205b75fa5..04881776a 100644 --- a/doc/manual/source/protocols/json/derivation.md +++ b/doc/manual/source/protocols/json/derivation.md @@ -91,3 +91,7 @@ is a JSON object with the following fields: * `env`: The environment passed to the `builder`. + +* `structuredAttrs`: + [Strucutured Attributes](@docroot@/store/derivation/index.md#structured-attrs), only defined if the derivation contains them. + Structured attributes are JSON, and thus embedded as-is. diff --git a/doc/manual/source/store/derivation/index.md b/doc/manual/source/store/derivation/index.md index 16ffc0923..147330df6 100644 --- a/doc/manual/source/store/derivation/index.md +++ b/doc/manual/source/store/derivation/index.md @@ -138,6 +138,17 @@ See [Wikipedia](https://en.wikipedia.org/wiki/Argv) for details. Environment variables which will be passed to the [builder](#builder) executable. +#### Structured Attributes {#structured-attrs} + +Nix also has special support for embedding JSON in the derivations. + +The environment variable `NIX_ATTRS_JSON_FILE` points to the exact location of that file both in a build and a [`nix-shell`](@docroot@/command-ref/nix-shell.md). + +As a convenience to Bash builders, Nix writes a script that initialises shell variables corresponding to all attributes that are representable in Bash. +The environment variable `NIX_ATTRS_SH_FILE` points to the exact location of the script, both in a build and a [`nix-shell`](@docroot@/command-ref/nix-shell.md). +This includes non-nested (associative) arrays. +For example, the attribute `hardening.format = true` ends up as the Bash associative array element `${hardening[format]}`. + ### Placeholders Placeholders are opaque values used within the [process creation fields] to [store objects] for which we don't yet know [store path]s. diff --git a/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults.json b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults.json index 7d3c932b2..183148b29 100644 --- a/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults.json +++ b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs-defaults.json @@ -5,7 +5,6 @@ ], "builder": "/bin/bash", "env": { - "__json": "{\"builder\":\"/bin/bash\",\"name\":\"advanced-attributes-structured-attrs-defaults\",\"outputHashAlgo\":\"sha256\",\"outputHashMode\":\"recursive\",\"outputs\":[\"out\",\"dev\"],\"system\":\"my-system\"}", "dev": "/02qcpld1y6xhs5gz9bchpxaw0xdhmsp5dv88lh25r2ss44kh8dxz", "out": "/1rz4g4znpzjwh1xymhjpm42vipw92pr73vdgl6xs1hycac8kf2n9" }, @@ -22,5 +21,16 @@ "method": "nar" } }, + "structuredAttrs": { + "builder": "/bin/bash", + "name": "advanced-attributes-structured-attrs-defaults", + "outputHashAlgo": "sha256", + "outputHashMode": "recursive", + "outputs": [ + "out", + "dev" + ], + "system": "my-system" + }, "system": "my-system" } diff --git a/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs.json b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs.json index a421efea7..ec044d778 100644 --- a/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs.json +++ b/src/libstore-tests/data/derivation/ca/advanced-attributes-structured-attrs.json @@ -5,7 +5,6 @@ ], "builder": "/bin/bash", "env": { - "__json": "{\"__darwinAllowLocalNetworking\":true,\"__impureHostDeps\":[\"/usr/bin/ditto\"],\"__noChroot\":true,\"__sandboxProfile\":\"sandcastle\",\"allowSubstitutes\":false,\"builder\":\"/bin/bash\",\"exportReferencesGraph\":{\"refs1\":[\"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9\"],\"refs2\":[\"/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv\"]},\"impureEnvVars\":[\"UNICORN\"],\"name\":\"advanced-attributes-structured-attrs\",\"outputChecks\":{\"bin\":{\"disallowedReferences\":[\"/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g\"],\"disallowedRequisites\":[\"/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8\"]},\"dev\":{\"maxClosureSize\":5909,\"maxSize\":789},\"out\":{\"allowedReferences\":[\"/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9\"],\"allowedRequisites\":[\"/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z\"]}},\"outputHashAlgo\":\"sha256\",\"outputHashMode\":\"recursive\",\"outputs\":[\"out\",\"bin\",\"dev\"],\"preferLocalBuild\":true,\"requiredSystemFeatures\":[\"rainbow\",\"uid-range\"],\"system\":\"my-system\"}", "bin": "/04f3da1kmbr67m3gzxikmsl4vjz5zf777sv6m14ahv22r65aac9m", "dev": "/02qcpld1y6xhs5gz9bchpxaw0xdhmsp5dv88lh25r2ss44kh8dxz", "out": "/1rz4g4znpzjwh1xymhjpm42vipw92pr73vdgl6xs1hycac8kf2n9" @@ -44,5 +43,62 @@ "method": "nar" } }, + "structuredAttrs": { + "__darwinAllowLocalNetworking": true, + "__impureHostDeps": [ + "/usr/bin/ditto" + ], + "__noChroot": true, + "__sandboxProfile": "sandcastle", + "allowSubstitutes": false, + "builder": "/bin/bash", + "exportReferencesGraph": { + "refs1": [ + "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9" + ], + "refs2": [ + "/nix/store/qnml92yh97a6fbrs2m5qg5cqlc8vni58-bar.drv" + ] + }, + "impureEnvVars": [ + "UNICORN" + ], + "name": "advanced-attributes-structured-attrs", + "outputChecks": { + "bin": { + "disallowedReferences": [ + "/0nyw57wm2iicnm9rglvjmbci3ikmcp823czdqdzdcgsnnwqps71g" + ], + "disallowedRequisites": [ + "/07f301yqyz8c6wf6bbbavb2q39j4n8kmcly1s09xadyhgy6x2wr8" + ] + }, + "dev": { + "maxClosureSize": 5909, + "maxSize": 789 + }, + "out": { + "allowedReferences": [ + "/164j69y6zir9z0339n8pjigg3rckinlr77bxsavzizdaaljb7nh9" + ], + "allowedRequisites": [ + "/0nr45p69vn6izw9446wsh9bng9nndhvn19kpsm4n96a5mycw0s4z" + ] + } + }, + "outputHashAlgo": "sha256", + "outputHashMode": "recursive", + "outputs": [ + "out", + "bin", + "dev" + ], + "preferLocalBuild": true, + "requiredSystemFeatures": [ + "rainbow", + "uid-range" + ], + "system": "my-system" + }, "system": "my-system" } diff --git a/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults.json b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults.json index 473d006ac..f5349e6c3 100644 --- a/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults.json +++ b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs-defaults.json @@ -5,7 +5,6 @@ ], "builder": "/bin/bash", "env": { - "__json": "{\"builder\":\"/bin/bash\",\"name\":\"advanced-attributes-structured-attrs-defaults\",\"outputs\":[\"out\",\"dev\"],\"system\":\"my-system\"}", "dev": "/nix/store/8bazivnbipbyi569623skw5zm91z6kc2-advanced-attributes-structured-attrs-defaults-dev", "out": "/nix/store/f8f8nvnx32bxvyxyx2ff7akbvwhwd9dw-advanced-attributes-structured-attrs-defaults" }, @@ -20,5 +19,14 @@ "path": "/nix/store/f8f8nvnx32bxvyxyx2ff7akbvwhwd9dw-advanced-attributes-structured-attrs-defaults" } }, + "structuredAttrs": { + "builder": "/bin/bash", + "name": "advanced-attributes-structured-attrs-defaults", + "outputs": [ + "out", + "dev" + ], + "system": "my-system" + }, "system": "my-system" } diff --git a/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs.json b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs.json index d68502d56..b8d566462 100644 --- a/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs.json +++ b/src/libstore-tests/data/derivation/ia/advanced-attributes-structured-attrs.json @@ -5,7 +5,6 @@ ], "builder": "/bin/bash", "env": { - "__json": "{\"__darwinAllowLocalNetworking\":true,\"__impureHostDeps\":[\"/usr/bin/ditto\"],\"__noChroot\":true,\"__sandboxProfile\":\"sandcastle\",\"allowSubstitutes\":false,\"builder\":\"/bin/bash\",\"exportReferencesGraph\":{\"refs1\":[\"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo\"],\"refs2\":[\"/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv\"]},\"impureEnvVars\":[\"UNICORN\"],\"name\":\"advanced-attributes-structured-attrs\",\"outputChecks\":{\"bin\":{\"disallowedReferences\":[\"/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar\"],\"disallowedRequisites\":[\"/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev\"]},\"dev\":{\"maxClosureSize\":5909,\"maxSize\":789},\"out\":{\"allowedReferences\":[\"/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo\"],\"allowedRequisites\":[\"/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev\"]}},\"outputs\":[\"out\",\"bin\",\"dev\"],\"preferLocalBuild\":true,\"requiredSystemFeatures\":[\"rainbow\",\"uid-range\"],\"system\":\"my-system\"}", "bin": "/nix/store/33qms3h55wlaspzba3brlzlrm8m2239g-advanced-attributes-structured-attrs-bin", "dev": "/nix/store/wyfgwsdi8rs851wmy1xfzdxy7y5vrg5l-advanced-attributes-structured-attrs-dev", "out": "/nix/store/7cxy4zx1vqc885r4jl2l64pymqbdmhii-advanced-attributes-structured-attrs" @@ -41,5 +40,60 @@ "path": "/nix/store/7cxy4zx1vqc885r4jl2l64pymqbdmhii-advanced-attributes-structured-attrs" } }, + "structuredAttrs": { + "__darwinAllowLocalNetworking": true, + "__impureHostDeps": [ + "/usr/bin/ditto" + ], + "__noChroot": true, + "__sandboxProfile": "sandcastle", + "allowSubstitutes": false, + "builder": "/bin/bash", + "exportReferencesGraph": { + "refs1": [ + "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" + ], + "refs2": [ + "/nix/store/vj2i49jm2868j2fmqvxm70vlzmzvgv14-bar.drv" + ] + }, + "impureEnvVars": [ + "UNICORN" + ], + "name": "advanced-attributes-structured-attrs", + "outputChecks": { + "bin": { + "disallowedReferences": [ + "/nix/store/r5cff30838majxk5mp3ip2diffi8vpaj-bar" + ], + "disallowedRequisites": [ + "/nix/store/9b61w26b4avv870dw0ymb6rw4r1hzpws-bar-dev" + ] + }, + "dev": { + "maxClosureSize": 5909, + "maxSize": 789 + }, + "out": { + "allowedReferences": [ + "/nix/store/p0hax2lzvjpfc2gwkk62xdglz0fcqfzn-foo" + ], + "allowedRequisites": [ + "/nix/store/z0rjzy29v9k5qa4nqpykrbzirj7sd43v-foo-dev" + ] + } + }, + "outputs": [ + "out", + "bin", + "dev" + ], + "preferLocalBuild": true, + "requiredSystemFeatures": [ + "rainbow", + "uid-range" + ], + "system": "my-system" + }, "system": "my-system" } diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 7c13b4f63..0657a7499 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -1333,6 +1333,11 @@ nlohmann::json Derivation::toJSON(const StoreDirConfig & store) const res["args"] = args; res["env"] = env; + if (auto it = env.find("__json"); it != env.end()) { + res["env"].erase("__json"); + res["structuredAttrs"] = nlohmann::json::parse(it->second); + } + return res; } @@ -1396,7 +1401,17 @@ Derivation Derivation::fromJSON( res.platform = getString(valueAt(json, "system")); res.builder = getString(valueAt(json, "builder")); res.args = getStringList(valueAt(json, "args")); - res.env = getStringMap(valueAt(json, "env")); + + auto envJson = valueAt(json, "env"); + try { + res.env = getStringMap(envJson); + } catch (Error & e) { + e.addTrace({}, "while reading key 'env'"); + throw; + } + + if (auto structuredAttrs = get(json, "structuredAttrs")) + res.env.insert_or_assign("__json", structuredAttrs->dump()); return res; } diff --git a/tests/functional/structured-attrs.sh b/tests/functional/structured-attrs.sh index 465676b41..2bd9b4aaf 100755 --- a/tests/functional/structured-attrs.sh +++ b/tests/functional/structured-attrs.sh @@ -50,4 +50,4 @@ expectStderr 0 nix-instantiate --expr "$hackyExpr" --eval --strict | grepQuiet " # Check it works with the expected structured attrs hacky=$(nix-instantiate --expr "$hackyExpr") -nix derivation show "$hacky" | jq --exit-status '."'"$hacky"'".env.__json | fromjson | . == {"a": 1}' +nix derivation show "$hacky" | jq --exit-status '."'"$hacky"'".structuredAttrs | . == {"a": 1}' From 699db04df3e755b9c3411567ca332709c39ba07d Mon Sep 17 00:00:00 2001 From: jayeshv Date: Fri, 13 Jun 2025 12:28:27 +0200 Subject: [PATCH 30/66] Fix a minor typo --- doc/manual/source/installation/installing-binary.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/source/installation/installing-binary.md b/doc/manual/source/installation/installing-binary.md index 6a1a5ddca..21c156374 100644 --- a/doc/manual/source/installation/installing-binary.md +++ b/doc/manual/source/installation/installing-binary.md @@ -25,7 +25,7 @@ This performs the default type of installation for your platform: We recommend the multi-user installation if it supports your platform and you can authenticate with `sudo`. -The installer can configured with various command line arguments and environment variables. +The installer can be configured with various command line arguments and environment variables. To show available command line flags: ```console From e27a06278376a3721d06a6cc88ae711bd0937406 Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Sat, 14 Jun 2025 10:37:36 +0200 Subject: [PATCH 31/66] docker: remove last use of `pkgs.` Follow-up of https://github.com/NixOS/nix/pull/13354 --- docker.nix | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docker.nix b/docker.nix index 598d1a125..14648f0d1 100644 --- a/docker.nix +++ b/docker.nix @@ -2,6 +2,7 @@ # Core dependencies pkgs, lib, + dockerTools, runCommand, buildPackages, # Image configuration @@ -325,7 +326,7 @@ let ); in -pkgs.dockerTools.buildLayeredImageWithNixDb { +dockerTools.buildLayeredImageWithNixDb { inherit name From b2596a76158e20b07d9edac48fcb56e7cc8ec0b7 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Sun, 15 Jun 2025 16:51:38 +0000 Subject: [PATCH 32/66] libutil: Add custom PeekSort implementation Unlike std::sort and std::stable_sort, this implementation does not lead to out-of-bounds memory reads or other undefined behavior when the predicate is not strict weak ordering. This makes it possible to use this function in libexpr for builtins.sort, where an incorrectly implemented comparator in the user nix code currently can crash and burn the evaluator by invoking C++ UB. --- src/libutil-tests/meson.build | 1 + src/libutil-tests/sort.cc | 274 +++++++++++++++++++++ src/libutil/include/nix/util/meson.build | 1 + src/libutil/include/nix/util/sort.hh | 299 +++++++++++++++++++++++ 4 files changed, 575 insertions(+) create mode 100644 src/libutil-tests/sort.cc create mode 100644 src/libutil/include/nix/util/sort.hh diff --git a/src/libutil-tests/meson.build b/src/libutil-tests/meson.build index f2552550d..b3776e094 100644 --- a/src/libutil-tests/meson.build +++ b/src/libutil-tests/meson.build @@ -65,6 +65,7 @@ sources = files( 'position.cc', 'processes.cc', 'references.cc', + 'sort.cc', 'spawn.cc', 'strings.cc', 'suggestions.cc', diff --git a/src/libutil-tests/sort.cc b/src/libutil-tests/sort.cc new file mode 100644 index 000000000..8eee961c8 --- /dev/null +++ b/src/libutil-tests/sort.cc @@ -0,0 +1,274 @@ +#include +#include +#include "nix/util/sort.hh" + +#include +#include +#include +#include + +namespace nix { + +struct MonotonicSubranges : public ::testing::Test +{ + std::vector empty_; + std::vector basic_ = {1, 0, -1, -100, 10, 10, 20, 40, 5, 5, 20, 10, 10, 1, -5}; +}; + +TEST_F(MonotonicSubranges, empty) +{ + ASSERT_EQ(weaklyIncreasingPrefix(empty_.begin(), empty_.end()), empty_.begin()); + ASSERT_EQ(weaklyIncreasingSuffix(empty_.begin(), empty_.end()), empty_.begin()); + ASSERT_EQ(strictlyDecreasingPrefix(empty_.begin(), empty_.end()), empty_.begin()); + ASSERT_EQ(strictlyDecreasingSuffix(empty_.begin(), empty_.end()), empty_.begin()); +} + +TEST_F(MonotonicSubranges, basic) +{ + ASSERT_EQ(strictlyDecreasingPrefix(basic_.begin(), basic_.end()), basic_.begin() + 4); + ASSERT_EQ(strictlyDecreasingSuffix(basic_.begin(), basic_.end()), basic_.begin() + 12); + std::reverse(basic_.begin(), basic_.end()); + ASSERT_EQ(weaklyIncreasingPrefix(basic_.begin(), basic_.end()), basic_.begin() + 5); + ASSERT_EQ(weaklyIncreasingSuffix(basic_.begin(), basic_.end()), basic_.begin() + 11); +} + +template +class SortTestPermutations : public ::testing::Test +{ + std::vector initialData = {std::numeric_limits::max(), std::numeric_limits::min(), 0, 0, 42, 126, 36}; + std::vector vectorData; + std::list listData; + +public: + std::vector scratchVector; + std::list scratchList; + std::vector empty; + + void SetUp() override + { + vectorData = initialData; + std::sort(vectorData.begin(), vectorData.end()); + listData = std::list(vectorData.begin(), vectorData.end()); + } + + bool nextPermutation() + { + std::next_permutation(vectorData.begin(), vectorData.end()); + std::next_permutation(listData.begin(), listData.end()); + scratchList = listData; + scratchVector = vectorData; + return vectorData == initialData; + } +}; + +using SortPermutationsTypes = ::testing::Types; + +TYPED_TEST_SUITE(SortTestPermutations, SortPermutationsTypes); + +TYPED_TEST(SortTestPermutations, insertionsort) +{ + while (!this->nextPermutation()) { + auto & list = this->scratchList; + insertionsort(list.begin(), list.end()); + ASSERT_TRUE(std::is_sorted(list.begin(), list.end())); + auto & vector = this->scratchVector; + insertionsort(vector.begin(), vector.end()); + ASSERT_TRUE(std::is_sorted(vector.begin(), vector.end())); + } +} + +TYPED_TEST(SortTestPermutations, peeksort) +{ + while (!this->nextPermutation()) { + auto & vector = this->scratchVector; + peeksort(vector.begin(), vector.end()); + ASSERT_TRUE(std::is_sorted(vector.begin(), vector.end())); + } +} + +TEST(InsertionSort, empty) +{ + std::vector empty; + insertionsort(empty.begin(), empty.end()); +} + +struct RandomPeekSort : public ::testing::TestWithParam< + std::tuple> +{ + using ValueType = int; + std::vector data_; + std::mt19937 urng_; + std::uniform_int_distribution distribution_; + + void SetUp() override + { + auto [maxSize, min, max, iterations] = GetParam(); + urng_ = std::mt19937(GTEST_FLAG_GET(random_seed)); + distribution_ = std::uniform_int_distribution(min, max); + } + + auto regenerate() + { + auto [maxSize, min, max, iterations] = GetParam(); + std::size_t dataSize = std::uniform_int_distribution(0, maxSize)(urng_); + data_.resize(dataSize); + std::generate(data_.begin(), data_.end(), [&]() { return distribution_(urng_); }); + } +}; + +TEST_P(RandomPeekSort, defaultComparator) +{ + auto [maxSize, min, max, iterations] = GetParam(); + + for (std::size_t i = 0; i < iterations; ++i) { + regenerate(); + peeksort(data_.begin(), data_.end()); + ASSERT_TRUE(std::is_sorted(data_.begin(), data_.end())); + /* Sorting is idempotent */ + peeksort(data_.begin(), data_.end()); + ASSERT_TRUE(std::is_sorted(data_.begin(), data_.end())); + } +} + +TEST_P(RandomPeekSort, greater) +{ + auto [maxSize, min, max, iterations] = GetParam(); + + for (std::size_t i = 0; i < iterations; ++i) { + regenerate(); + peeksort(data_.begin(), data_.end(), std::greater{}); + ASSERT_TRUE(std::is_sorted(data_.begin(), data_.end(), std::greater{})); + /* Sorting is idempotent */ + peeksort(data_.begin(), data_.end(), std::greater{}); + ASSERT_TRUE(std::is_sorted(data_.begin(), data_.end(), std::greater{})); + } +} + +TEST_P(RandomPeekSort, brokenComparator) +{ + auto [maxSize, min, max, iterations] = GetParam(); + + /* This is a pretty nice way of modeling a worst-case scenario for a broken comparator. + If the sorting algorithm doesn't break in such case, then surely all deterministic + predicates won't break it. */ + auto comp = [&]([[maybe_unused]] const auto & lhs, [[maybe_unused]] const auto & rhs) -> bool { + return std::uniform_int_distribution(0, 1)(urng_); + }; + + for (std::size_t i = 0; i < iterations; ++i) { + regenerate(); + auto originalData = data_; + peeksort(data_.begin(), data_.end(), comp); + /* Check that the output is just a reordering of the input. This is the + contract of the implementation in regard to comparators that don't + define a strict weak order. */ + std::sort(data_.begin(), data_.end()); + std::sort(originalData.begin(), originalData.end()); + ASSERT_EQ(originalData, data_); + } +} + +TEST_P(RandomPeekSort, stability) +{ + auto [maxSize, min, max, iterations] = GetParam(); + + for (std::size_t i = 0; i < iterations; ++i) { + regenerate(); + std::vector> pairs; + + /* Assign sequential ids to objects. After the sort ids for equivalent + elements should be in ascending order. */ + std::transform( + data_.begin(), data_.end(), std::back_inserter(pairs), [id = std::size_t{0}](auto && val) mutable { + return std::pair{val, ++id}; + }); + + auto comp = [&]([[maybe_unused]] const auto & lhs, [[maybe_unused]] const auto & rhs) -> bool { + return lhs.first > rhs.first; + }; + + peeksort(pairs.begin(), pairs.end(), comp); + ASSERT_TRUE(std::is_sorted(pairs.begin(), pairs.end(), comp)); + + for (auto begin = pairs.begin(), end = pairs.end(); begin < end; ++begin) { + auto key = begin->first; + auto innerEnd = std::find_if_not(begin, end, [key](const auto & lhs) { return lhs.first == key; }); + ASSERT_TRUE(std::is_sorted(begin, innerEnd, [](const auto & lhs, const auto & rhs) { + return lhs.second < rhs.second; + })); + begin = innerEnd; + } + } +} + +using RandomPeekSortParamType = RandomPeekSort::ParamType; + +INSTANTIATE_TEST_SUITE_P( + PeekSort, + RandomPeekSort, + ::testing::Values( + RandomPeekSortParamType{128, std::numeric_limits::min(), std::numeric_limits::max(), 1024}, + RandomPeekSortParamType{7753, -32, 32, 128}, + RandomPeekSortParamType{11719, std::numeric_limits::min(), std::numeric_limits::max(), 64}, + RandomPeekSortParamType{4063, 0, 32, 256}, + RandomPeekSortParamType{771, -8, 8, 2048}, + RandomPeekSortParamType{433, 0, 1, 2048}, + RandomPeekSortParamType{0, 0, 0, 1}, /* empty case */ + RandomPeekSortParamType{ + 1, std::numeric_limits::min(), std::numeric_limits::max(), 1}, /* single element */ + RandomPeekSortParamType{ + 2, std::numeric_limits::min(), std::numeric_limits::max(), 2}, /* two elements */ + RandomPeekSortParamType{55425, std::numeric_limits::min(), std::numeric_limits::max(), 128})); + +template +struct SortProperty : public ::testing::Test +{}; + +using SortPropertyTypes = ::testing::Types; +TYPED_TEST_SUITE(SortProperty, SortPropertyTypes); + +RC_GTEST_TYPED_FIXTURE_PROP(SortProperty, peeksortSorted, (std::vector vec)) +{ + peeksort(vec.begin(), vec.end()); + RC_ASSERT(std::is_sorted(vec.begin(), vec.end())); +} + +RC_GTEST_TYPED_FIXTURE_PROP(SortProperty, peeksortSortedGreater, (std::vector vec)) +{ + auto comp = std::greater(); + peeksort(vec.begin(), vec.end(), comp); + RC_ASSERT(std::is_sorted(vec.begin(), vec.end(), comp)); +} + +RC_GTEST_TYPED_FIXTURE_PROP(SortProperty, insertionsortSorted, (std::vector vec)) +{ + insertionsort(vec.begin(), vec.end()); + RC_ASSERT(std::is_sorted(vec.begin(), vec.end())); +} + +RC_GTEST_PROP(SortProperty, peeksortStability, (std::vector> vec)) +{ + auto comp = [](auto lhs, auto rhs) { return lhs.first < rhs.first; }; + auto copy = vec; + std::stable_sort(copy.begin(), copy.end(), comp); + peeksort(vec.begin(), vec.end(), comp); + RC_ASSERT(copy == vec); +} + +RC_GTEST_TYPED_FIXTURE_PROP(SortProperty, peeksortSortedLinearComparisonComplexity, (std::vector vec)) +{ + peeksort(vec.begin(), vec.end()); + RC_ASSERT(std::is_sorted(vec.begin(), vec.end())); + std::size_t comparisonCount = 0; + auto countingComp = [&](auto lhs, auto rhs) { + ++comparisonCount; + return lhs < rhs; + }; + + peeksort(vec.begin(), vec.end(), countingComp); + + /* In the sorted case comparison complexify should be linear. */ + RC_ASSERT(comparisonCount <= vec.size()); +} + +} // namespace nix diff --git a/src/libutil/include/nix/util/meson.build b/src/libutil/include/nix/util/meson.build index 6c95a6ced..22438c1d0 100644 --- a/src/libutil/include/nix/util/meson.build +++ b/src/libutil/include/nix/util/meson.build @@ -59,6 +59,7 @@ headers = files( 'signals.hh', 'signature/local-keys.hh', 'signature/signer.hh', + 'sort.hh', 'source-accessor.hh', 'source-path.hh', 'split.hh', diff --git a/src/libutil/include/nix/util/sort.hh b/src/libutil/include/nix/util/sort.hh new file mode 100644 index 000000000..0affdf3ce --- /dev/null +++ b/src/libutil/include/nix/util/sort.hh @@ -0,0 +1,299 @@ +#pragma once + +#include +#include +#include +#include +#include +#include + +/** + * @file + * + * In-house implementation of sorting algorithms. Used for cases when several properties + * need to be upheld regardless of the stdlib implementation of std::sort or + * std::stable_sort. + * + * PeekSort implementation is adapted from reference implementation + * https://github.com/sebawild/powersort licensed under the MIT License. + * + */ + +/* PeekSort attribution: + * + * MIT License + * + * Copyright (c) 2022 Sebastian Wild + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +namespace nix { + +/** + * Merge sorted runs [begin, middle) with [middle, end) in-place [begin, end). + * Uses a temporary working buffer by first copying [begin, end) to it. + * + * @param begin Start of the first subrange to be sorted. + * @param middle End of the first sorted subrange and the start of the second. + * @param end End of the second sorted subrange. + * @param workingBegin Start of the working buffer. + * @param comp Comparator implementing an operator()(const ValueType& lhs, const ValueType& rhs). + * + * @pre workingBegin buffer must have at least std::distance(begin, end) elements. + * + * @note We can't use std::inplace_merge or std::merge, because their behavior + * is undefined if the comparator is not strict weak ordering. + */ +template< + std::forward_iterator Iter, + std::random_access_iterator BufIter, + typename Comparator = std::less>> +void mergeSortedRunsInPlace(Iter begin, Iter middle, Iter end, BufIter workingBegin, Comparator comp = {}) +{ + const BufIter workingMiddle = std::move(begin, middle, workingBegin); + const BufIter workingEnd = std::move(middle, end, workingMiddle); + + Iter output = begin; + BufIter workingLeft = workingBegin; + BufIter workingRight = workingMiddle; + + while (workingLeft != workingMiddle && workingRight != workingEnd) { + /* Note the inversion here !comp(...., ....). This is required for the merge to be stable. + If a == b where a if from the left part and b is the the right, then we have to pick + a. */ + *output++ = !comp(*workingRight, *workingLeft) ? std::move(*workingLeft++) : std::move(*workingRight++); + } + + std::move(workingLeft, workingMiddle, output); + std::move(workingRight, workingEnd, output); +} + +/** + * Simple insertion sort. + * + * Does not require that the std::iter_value_t is copyable. + * + * @param begin Start of the range to sort. + * @param end End of the range to sort. + * @comp Comparator the defines the ordering. Order of elements if the comp is not strict weak ordering + * is not specified. + * @throws Nothing. + * + * Note on exception safety: this function provides weak exception safety + * guarantees. To elaborate: if the comparator throws or move assignment + * throws (value type is not nothrow_move_assignable) then the range is left in + * a consistent, but unspecified state. + * + * @note This can't be implemented in terms of binary search if the strict weak ordering + * needs to be handled in a well-defined but unspecified manner. + */ +template>> +void insertionsort(Iter begin, Iter end, Comparator comp = {}) +{ + if (begin == end) + return; + for (Iter current = std::next(begin); current != end; ++current) { + for (Iter insertionPoint = current; + insertionPoint != begin && comp(*insertionPoint, *std::prev(insertionPoint)); + --insertionPoint) { + std::swap(*insertionPoint, *std::prev(insertionPoint)); + } + } +} + +/** + * Find maximal i <= end such that [begin, i) is strictly decreasing according + * to the specified comparator. + */ +template>> +Iter strictlyDecreasingPrefix(Iter begin, Iter end, Comparator && comp = {}) +{ + if (begin == end) + return begin; + while (std::next(begin) != end && /* *std::next(begin) < begin */ + comp(*std::next(begin), *begin)) + ++begin; + return std::next(begin); +} + +/** + * Find minimal i >= start such that [i, end) is strictly decreasing according + * to the specified comparator. + */ +template>> +Iter strictlyDecreasingSuffix(Iter begin, Iter end, Comparator && comp = {}) +{ + if (begin == end) + return end; + while (std::prev(end) > begin && /* *std::prev(end) < *std::prev(end, 2) */ + comp(*std::prev(end), *std::prev(end, 2))) + --end; + return std::prev(end); +} + +/** + * Find maximal i <= end such that [begin, i) is weakly increasing according + * to the specified comparator. + */ +template>> +Iter weaklyIncreasingPrefix(Iter begin, Iter end, Comparator && comp = {}) +{ + return strictlyDecreasingPrefix(begin, end, std::not_fn(std::forward(comp))); +} + +/** + * Find minimal i >= start such that [i, end) is weakly increasing according + * to the specified comparator. + */ +template>> +Iter weaklyIncreasingSuffix(Iter begin, Iter end, Comparator && comp = {}) +{ + return strictlyDecreasingSuffix(begin, end, std::not_fn(std::forward(comp))); +} + +/** + * Peeksort stable sorting algorithm. Sorts elements in-place. + * Allocates additional memory as needed. + * + * @details + * PeekSort is a stable, near-optimal natural mergesort. Most importantly, like any + * other mergesort it upholds the "Ord safety" property. Meaning that even for + * comparator predicates that don't satisfy strict weak ordering it can't result + * in infinite loops/out of bounds memory accesses or other undefined behavior. + * + * As a quick reminder, strict weak ordering relation operator< must satisfy + * the following properties. Keep in mind that in C++ an equvalence relation + * is specified in terms of operator< like so: a ~ b iff !(a < b) && !(b < a). + * + * 1. a < a === false - relation is irreflexive + * 2. a < b, b < c => a < c - transitivity + * 3. a ~ b, a ~ b, b ~ c => a ~ c, transitivity of equivalence + * + * @see https://www.wild-inter.net/publications/munro-wild-2018 + * @see https://github.com/Voultapher/sort-research-rs/blob/main/writeup/sort_safety/text.md#property-analysis + * + * The order of elements when comp is not strict weak ordering is not specified, but + * is not undefined. The output is always some permutation of the input, regardless + * of the comparator provided. + * Relying on ordering in such cases is erroneous, but this implementation + * will happily accept broken comparators and will not crash. + * + * @param begin Start of the range to be sorted. + * @param end End of the range to be sorted. + * @comp comp Comparator implementing an operator()(const ValueType& lhs, const ValueType& rhs). + * + * @throws std::bad_alloc if the temporary buffer can't be allocated. + * + * @return Nothing. + * + * Note on exception safety: this function provides weak exception safety + * guarantees. To elaborate: if the comparator throws or move assignment + * throws (value type is not nothrow_move_assignable) then the range is left in + * a consistent, but unspecified state. + * + */ +template>> +/* ValueType must be default constructible to create the temporary buffer */ + requires std::is_default_constructible_v> +void peeksort(Iter begin, Iter end, Comparator comp = {}) +{ + auto length = std::distance(begin, end); + + /* Special-case very simple inputs. This is identical to how libc++ does it. */ + switch (length) { + case 0: + [[fallthrough]]; + case 1: + return; + case 2: + if (comp(*--end, *begin)) /* [a, b], b < a */ + std::swap(*begin, *end); + return; + } + + using ValueType = std::iter_value_t; + auto workingBuffer = std::vector(length); + + /* + * sorts [begin, end), assuming that [begin, leftRunEnd) and + * [rightRunBegin, end) are sorted. + * Modified implementation from: + * https://github.com/sebawild/powersort/blob/1d078b6be9023e134c4f8f6de88e2406dc681e89/src/sorts/peeksort.h + */ + auto peeksortImpl = [&workingBuffer, + &comp](auto & peeksortImpl, Iter begin, Iter end, Iter leftRunEnd, Iter rightRunBegin) { + if (leftRunEnd == end || rightRunBegin == begin) + return; + + /* Dispatch to simpler insertion sort implementation for smaller cases + Cut-off limit is the same as in libstdc++ + https://github.com/gcc-mirror/gcc/blob/d9375e490072d1aae73a93949aa158fcd2a27018/libstdc%2B%2B-v3/include/bits/stl_algo.h#L4977 + */ + static constexpr std::size_t insertionsortThreshold = 16; + size_t length = std::distance(begin, end); + if (length <= insertionsortThreshold) + return insertionsort(begin, end, comp); + + Iter middle = std::next(begin, (length / 2)); /* Middle split between m and m - 1 */ + + if (middle <= leftRunEnd) { + /* |XXXXXXXX|XX X| */ + peeksortImpl(peeksortImpl, leftRunEnd, end, std::next(leftRunEnd), rightRunBegin); + mergeSortedRunsInPlace(begin, leftRunEnd, end, workingBuffer.begin(), comp); + return; + } else if (middle >= rightRunBegin) { + /* |XX X|XXXXXXXX| */ + peeksortImpl(peeksortImpl, begin, rightRunBegin, leftRunEnd, std::prev(rightRunBegin)); + mergeSortedRunsInPlace(begin, rightRunBegin, end, workingBuffer.begin(), comp); + return; + } + + /* Find middle run, i.e., run containing m - 1 */ + Iter i, j; + + if (!comp(*middle, *std::prev(middle)) /* *std::prev(middle) <= *middle */) { + i = weaklyIncreasingSuffix(leftRunEnd, middle, comp); + j = weaklyIncreasingPrefix(std::prev(middle), rightRunBegin, comp); + } else { + i = strictlyDecreasingSuffix(leftRunEnd, middle, comp); + j = strictlyDecreasingPrefix(std::prev(middle), rightRunBegin, comp); + std::reverse(i, j); + } + + if (i == begin && j == end) + return; /* single run */ + + if (middle - i < j - middle) { + /* |XX x|xxxx X| */ + peeksortImpl(peeksortImpl, begin, i, leftRunEnd, std::prev(i)); + peeksortImpl(peeksortImpl, i, end, j, rightRunBegin); + mergeSortedRunsInPlace(begin, i, end, workingBuffer.begin(), comp); + } else { + /* |XX xxx|x X| */ + peeksortImpl(peeksortImpl, begin, j, leftRunEnd, i); + peeksortImpl(peeksortImpl, j, end, std::next(j), rightRunBegin); + mergeSortedRunsInPlace(begin, j, end, workingBuffer.begin(), comp); + } + }; + + peeksortImpl(peeksortImpl, begin, end, /*leftRunEnd=*/begin, /*rightRunBegin=*/end); +} + +} From 351d898c43202ca9c3f94daf7d727b3b7bbd3daa Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Sun, 15 Jun 2025 16:51:40 +0000 Subject: [PATCH 33/66] libexpr: Switch builtins.sort primop to use peeksort This prevents C++ level undefined behavior from affecting the evaluator. Stdlib implementation details should not affect eval, regardless of the build platform. Even erroneous usage of `builtins.sort` should not make it possible to crash the evaluator or produce results that depend on the host platform. --- src/libexpr/primops.cc | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 60f44ca62..75d7465dd 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -14,6 +14,7 @@ #include "nix/expr/value-to-xml.hh" #include "nix/expr/primops.hh" #include "nix/fetchers/fetch-to-store.hh" +#include "nix/util/sort.hh" #include #include @@ -3695,10 +3696,14 @@ static void prim_sort(EvalState & state, const PosIdx pos, Value * * args, Value return state.forceBool(vBool, pos, "while evaluating the return value of the sorting function passed to builtins.sort"); }; - /* FIXME: std::sort can segfault if the comparator is not a strict - weak ordering. What to do? std::stable_sort() seems more - resilient, but no guarantees... */ - std::stable_sort(list.begin(), list.end(), comparator); + /* NOTE: Using custom implementation because std::sort and std::stable_sort + are not resilient to comparators that violate strict weak ordering. Diagnosing + incorrect implementations is a O(n^3) problem, so doing the checks is much more + expensive that doing the sorting. For this reason we choose to use sorting algorithms + that are can't be broken by invalid comprators. peeksort (mergesort) + doesn't misbehave when any of the strict weak order properties is + violated - output is always a reordering of the input. */ + peeksort(list.begin(), list.end(), comparator); v.mkList(list); } From ddcfc81ff1fe01e4ab74cec80709cac8f77361d5 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Sun, 15 Jun 2025 16:51:42 +0000 Subject: [PATCH 34/66] libexpr: Document requirements for comparator passed to builtins.sort --- src/libexpr/primops.cc | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 75d7465dd..ba568e38d 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -3725,6 +3725,32 @@ static RegisterPrimOp primop_sort({ This is a stable sort: it preserves the relative order of elements deemed equal by the comparator. + + *comparator* must impose a strict weak ordering on the set of values + in the *list*. This means that for any elements *a*, *b* and *c* from the + *list*, *comparator* must satisfy the following relations: + + 1. Transitivity + + ```nix + comparator a b && comparator b c -> comparator a c + ``` + + 1. Irreflexivity + + ```nix + comparator a a == false + ``` + + 1. Transitivity of equivalence + + ```nix + let equiv = a: b: (!comparator a b && !comparator b a); in + equiv a b && equiv b c -> equiv a c + ``` + + If the *comparator* violates any of these properties, then `builtins.sort` + reorders elements in an unspecified manner. )", .fun = prim_sort, }); From f9170a84f6eb6a162c800c39c17d3b34eff87dda Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Sun, 15 Jun 2025 16:51:44 +0000 Subject: [PATCH 35/66] tests/functional/lang: Add sort stability test for lists langer than 16 elements libstdc++'s std::stable_sort and new builtins.sort implementation special-case ranges with length less than or equal to 16 and delegate to insertionsort. Having a larger e2e test would allow catching sort stability issues at functional level as well. --- tests/functional/lang/eval-okay-sort.exp | 2 +- tests/functional/lang/eval-okay-sort.nix | 74 ++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/tests/functional/lang/eval-okay-sort.exp b/tests/functional/lang/eval-okay-sort.exp index 899119e20..fcb3b2224 100644 --- a/tests/functional/lang/eval-okay-sort.exp +++ b/tests/functional/lang/eval-okay-sort.exp @@ -1 +1 @@ -[ [ 42 77 147 249 483 526 ] [ 526 483 249 147 77 42 ] [ "bar" "fnord" "foo" "xyzzy" ] [ { key = 1; value = "foo"; } { key = 1; value = "fnord"; } { key = 2; value = "bar"; } ] [ [ ] [ ] [ 1 ] [ 1 4 ] [ 1 5 ] [ 1 6 ] [ 2 ] [ 2 3 ] [ 3 ] [ 3 ] ] ] +[ [ 42 77 147 249 483 526 ] [ 526 483 249 147 77 42 ] [ "bar" "fnord" "foo" "xyzzy" ] [ { key = 1; value = "foo"; } { key = 1; value = "fnord"; } { key = 2; value = "bar"; } ] [ { key = 1; value = "foo"; } { key = 1; value = "foo2"; } { key = 1; value = "foo3"; } { key = 1; value = "foo4"; } { key = 1; value = "foo5"; } { key = 1; value = "foo6"; } { key = 1; value = "foo7"; } { key = 1; value = "foo8"; } { key = 2; value = "bar"; } { key = 2; value = "bar2"; } { key = 2; value = "bar3"; } { key = 2; value = "bar4"; } { key = 2; value = "bar5"; } { key = 3; value = "baz"; } { key = 3; value = "baz2"; } { key = 3; value = "baz3"; } { key = 3; value = "baz4"; } { key = 4; value = "biz1"; } ] [ [ ] [ ] [ 1 ] [ 1 4 ] [ 1 5 ] [ 1 6 ] [ 2 ] [ 2 3 ] [ 3 ] [ 3 ] ] ] diff --git a/tests/functional/lang/eval-okay-sort.nix b/tests/functional/lang/eval-okay-sort.nix index 412bda4a0..7a3b7f71b 100644 --- a/tests/functional/lang/eval-okay-sort.nix +++ b/tests/functional/lang/eval-okay-sort.nix @@ -37,6 +37,80 @@ with builtins; value = "fnord"; } ]) + (sort (x: y: x.key < y.key) [ + { + key = 1; + value = "foo"; + } + { + key = 2; + value = "bar"; + } + { + key = 1; + value = "foo2"; + } + { + key = 2; + value = "bar2"; + } + { + key = 2; + value = "bar3"; + } + { + key = 2; + value = "bar4"; + } + { + key = 1; + value = "foo3"; + } + { + key = 3; + value = "baz"; + } + { + key = 3; + value = "baz2"; + } + { + key = 1; + value = "foo4"; + } + { + key = 3; + value = "baz3"; + } + { + key = 1; + value = "foo5"; + } + { + key = 1; + value = "foo6"; + } + { + key = 2; + value = "bar5"; + } + { + key = 3; + value = "baz4"; + } + { + key = 1; + value = "foo7"; + } + { + key = 4; + value = "biz1"; + } + { + key = 1; + value = "foo8"; + } + ]) (sort lessThan [ [ 1 From c1aaa970c7f444de14a7c03885f4f0b5a76eb980 Mon Sep 17 00:00:00 2001 From: Philipp Otterbein Date: Sun, 15 Jun 2025 00:32:36 +0200 Subject: [PATCH 36/66] libexpr: further removal of std::string copies --- src/libexpr/primops.cc | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 60f44ca62..9024f84d4 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1179,7 +1179,7 @@ static void prim_second(EvalState & state, const PosIdx pos, Value * * args, Val static void derivationStrictInternal( EvalState & state, - const std::string & name, + std::string_view name, const Bindings * attrs, Value & v); @@ -1199,7 +1199,7 @@ static void prim_derivationStrict(EvalState & state, const PosIdx pos, Value * * /* Figure out the name first (for stack backtraces). */ auto nameAttr = state.getAttr(state.sName, attrs, "in the attrset passed as argument to builtins.derivationStrict"); - std::string drvName; + std::string_view drvName; try { drvName = state.forceStringNoCtx(*nameAttr->value, pos, "while evaluating the `name` attribute passed to builtins.derivationStrict"); } catch (Error & e) { @@ -1258,7 +1258,7 @@ static void checkDerivationName(EvalState & state, std::string_view drvName) static void derivationStrictInternal( EvalState & state, - const std::string & drvName, + std::string_view drvName, const Bindings * attrs, Value & v) { @@ -1898,7 +1898,7 @@ static void prim_findFile(EvalState & state, const PosIdx pos, Value * * args, V try { auto rewrites = state.realiseContext(context); - path = rewriteStrings(path, rewrites); + path = rewriteStrings(std::move(path), rewrites); } catch (InvalidPathError & e) { state.error( "cannot find '%1%', since path '%2%' is not valid", @@ -1908,8 +1908,8 @@ static void prim_findFile(EvalState & state, const PosIdx pos, Value * * args, V } lookupPath.elements.emplace_back(LookupPath::Elem { - .prefix = LookupPath::Prefix { .s = prefix }, - .path = LookupPath::Path { .s = path }, + .prefix = LookupPath::Prefix { .s = std::move(prefix) }, + .path = LookupPath::Path { .s = std::move(path) }, }); } @@ -2374,8 +2374,8 @@ static RegisterPrimOp primop_fromJSON({ static void prim_toFile(EvalState & state, const PosIdx pos, Value * * args, Value & v) { NixStringContext context; - std::string name(state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.toFile")); - std::string contents(state.forceString(*args[1], context, pos, "while evaluating the second argument passed to builtins.toFile")); + auto name = state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.toFile"); + auto contents = state.forceString(*args[1], context, pos, "while evaluating the second argument passed to builtins.toFile"); StorePathSet refs; @@ -2632,7 +2632,7 @@ static RegisterPrimOp primop_filterSource({ static void prim_path(EvalState & state, const PosIdx pos, Value * * args, Value & v) { std::optional path; - std::string name; + std::string_view name; Value * filterFun = nullptr; auto method = ContentAddressMethod::Raw::NixArchive; std::optional expectedHash; @@ -4562,12 +4562,12 @@ static void prim_replaceStrings(EvalState & state, const PosIdx pos, Value * * a "'from' and 'to' arguments passed to builtins.replaceStrings have different lengths" ).atPos(pos).debugThrow(); - std::vector from; + std::vector from; from.reserve(args[0]->listSize()); for (auto elem : args[0]->listItems()) from.emplace_back(state.forceString(*elem, pos, "while evaluating one of the strings to replace passed to builtins.replaceStrings")); - std::unordered_map cache; + std::unordered_map cache; auto to = args[1]->listItems(); NixStringContext context; From 18dc96269d01f7fa1e2e61086444a89f5ce18890 Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Sun, 15 Jun 2025 22:58:26 +0200 Subject: [PATCH 37/66] docker: add basics OpenContainers labels --- docker.nix | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docker.nix b/docker.nix index 14648f0d1..74e970e4b 100644 --- a/docker.nix +++ b/docker.nix @@ -19,6 +19,13 @@ gid ? 0, uname ? "root", gname ? "root", + Labels ? { + "org.opencontainers.image.title" = "Nix"; + "org.opencontainers.image.source" = "https://github.com/NixOS/nix"; + "org.opencontainers.image.vendor" = "Nix project"; + "org.opencontainers.image.version" = nix.version; + "org.opencontainers.image.description" = "Nix container image"; + }, # Default Packages nix, bashInteractive, @@ -352,6 +359,7 @@ dockerTools.buildLayeredImageWithNixDb { ''; config = { + inherit Labels; Cmd = [ (lib.getExe bashInteractive) ]; User = "${toString uid}:${toString gid}"; Env = [ From bb44347fac3b97cc19f1d0ab0594478cc928cbd8 Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Sun, 15 Jun 2025 23:04:50 +0200 Subject: [PATCH 38/66] docker: expose `config.Cmd` as parameter --- docker.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker.nix b/docker.nix index 74e970e4b..6cfa7d551 100644 --- a/docker.nix +++ b/docker.nix @@ -26,6 +26,7 @@ "org.opencontainers.image.version" = nix.version; "org.opencontainers.image.description" = "Nix container image"; }, + Cmd ? [ (lib.getExe bashInteractive) ], # Default Packages nix, bashInteractive, @@ -359,8 +360,7 @@ dockerTools.buildLayeredImageWithNixDb { ''; config = { - inherit Labels; - Cmd = [ (lib.getExe bashInteractive) ]; + inherit Cmd Labels; User = "${toString uid}:${toString gid}"; Env = [ "USER=${uname}" From d64c92216409b08ca40f43e601fbdae59114e812 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Tue, 17 Jun 2025 08:45:29 +0200 Subject: [PATCH 39/66] libstore: fix race condition when creating state directories Running parallel nix in nix can lead to multiple instances trying to create the state directories and failing on the `createSymlink` step, because the link already exists. `replaceSymlink` is already idempotent, so let's use that. Resolves #2706 --- src/libstore/local-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 76fadba86..e53cab2dc 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -133,7 +133,7 @@ LocalStore::LocalStore(ref config) Path gcRootsDir = config->stateDir + "/gcroots"; if (!pathExists(gcRootsDir)) { createDirs(gcRootsDir); - createSymlink(profilesDir, gcRootsDir + "/profiles"); + replaceSymlink(profilesDir, gcRootsDir + "/profiles"); } for (auto & perUserDir : {profilesDir + "/per-user", gcRootsDir + "/per-user"}) { From 77f6b6532f582a9db2bd6317f4fd272c32a05c7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20=C4=8Cun=C3=A1t?= Date: Wed, 18 Jun 2025 10:05:02 +0200 Subject: [PATCH 40/66] tests: fixup with jq-1.8.0 --- tests/functional/flakes/flakes.sh | 2 +- tests/functional/flakes/relative-paths.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/flakes/flakes.sh b/tests/functional/flakes/flakes.sh index e8b051198..ce695a6cb 100755 --- a/tests/functional/flakes/flakes.sh +++ b/tests/functional/flakes/flakes.sh @@ -160,7 +160,7 @@ expect 1 nix build -o "$TEST_ROOT/result" "$flake2Dir#bar" --no-update-lock-file nix build -o "$TEST_ROOT/result" "$flake2Dir#bar" --commit-lock-file [[ -e "$flake2Dir/flake.lock" ]] [[ -z $(git -C "$flake2Dir" diff main || echo failed) ]] -[[ $(jq --indent 0 . < "$flake2Dir/flake.lock") =~ ^'{"nodes":{"flake1":{"locked":{"lastModified":'.*',"narHash":"sha256-'.*'","ref":"refs/heads/master","rev":"'.*'","revCount":2,"type":"git","url":"file:///'.*'"},"original":{"id":"flake1","type":"indirect"}},"root":{"inputs":{"flake1":"flake1"}}},"root":"root","version":7}'$ ]] +[[ $(jq --indent 0 --compact-output . < "$flake2Dir/flake.lock") =~ ^'{"nodes":{"flake1":{"locked":{"lastModified":'.*',"narHash":"sha256-'.*'","ref":"refs/heads/master","rev":"'.*'","revCount":2,"type":"git","url":"file:///'.*'"},"original":{"id":"flake1","type":"indirect"}},"root":{"inputs":{"flake1":"flake1"}}},"root":"root","version":7}'$ ]] # Rerunning the build should not change the lockfile. nix build -o "$TEST_ROOT/result" "$flake2Dir#bar" diff --git a/tests/functional/flakes/relative-paths.sh b/tests/functional/flakes/relative-paths.sh index 5810fdebe..7480cd504 100644 --- a/tests/functional/flakes/relative-paths.sh +++ b/tests/functional/flakes/relative-paths.sh @@ -69,7 +69,7 @@ git -C "$rootFlake" add flake.nix sub2/flake.nix git -C "$rootFlake" add sub2/flake.lock [[ $(nix eval "$subflake2#y") = 15 ]] -[[ $(jq --indent 0 . < "$subflake2/flake.lock") =~ ^'{"nodes":{"root":{"inputs":{"root":"root_2","sub1":"sub1"}},"root_2":{"inputs":{"sub0":"sub0"},"locked":{"path":"..","type":"path"},"original":{"path":"..","type":"path"},"parent":[]},"root_3":{"inputs":{"sub0":"sub0_2"},"locked":{"path":"../","type":"path"},"original":{"path":"../","type":"path"},"parent":["sub1"]},"sub0":{"locked":{"path":"sub0","type":"path"},"original":{"path":"sub0","type":"path"},"parent":["root"]},"sub0_2":{"locked":{"path":"sub0","type":"path"},"original":{"path":"sub0","type":"path"},"parent":["sub1","root"]},"sub1":{"inputs":{"root":"root_3"},"locked":{"path":"../sub1","type":"path"},"original":{"path":"../sub1","type":"path"},"parent":[]}},"root":"root","version":7}'$ ]] +[[ $(jq --indent 0 --compact-output . < "$subflake2/flake.lock") =~ ^'{"nodes":{"root":{"inputs":{"root":"root_2","sub1":"sub1"}},"root_2":{"inputs":{"sub0":"sub0"},"locked":{"path":"..","type":"path"},"original":{"path":"..","type":"path"},"parent":[]},"root_3":{"inputs":{"sub0":"sub0_2"},"locked":{"path":"../","type":"path"},"original":{"path":"../","type":"path"},"parent":["sub1"]},"sub0":{"locked":{"path":"sub0","type":"path"},"original":{"path":"sub0","type":"path"},"parent":["root"]},"sub0_2":{"locked":{"path":"sub0","type":"path"},"original":{"path":"sub0","type":"path"},"parent":["sub1","root"]},"sub1":{"inputs":{"root":"root_3"},"locked":{"path":"../sub1","type":"path"},"original":{"path":"../sub1","type":"path"},"parent":[]}},"root":"root","version":7}'$ ]] # Make sure there are no content locks for relative path flakes. (! grep "$TEST_ROOT" "$subflake2/flake.lock") From da76bc0cacd0fcaddbf3031e40f3a07d5a73ddf7 Mon Sep 17 00:00:00 2001 From: Nikita Krasnov Date: Wed, 18 Jun 2025 12:40:07 +0300 Subject: [PATCH 41/66] Fix broken link --- doc/manual/source/store/store-object.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/source/store/store-object.md b/doc/manual/source/store/store-object.md index 115e107fb..bbf4803dd 100644 --- a/doc/manual/source/store/store-object.md +++ b/doc/manual/source/store/store-object.md @@ -18,7 +18,7 @@ In particular, the edge corresponding to a reference is from the store object th References other than a self-reference must not form a cycle. The graph of references excluding self-references thus forms a [directed acyclic graph]. -[directed acyclic graph]: @docroot@/glossary.md#gloss-directed acyclic graph +[directed acyclic graph]: @docroot@/glossary.md#gloss-directed-acyclic-graph We can take the [transitive closure] of the references graph, which any pair of store objects have an edge not if there is a single reference from the first to the second, but a path of one or more references from the first to the second. The *requisites* of a store object are all store objects reachable by paths of references which start with given store object's references. From 86dda9884ae5aa8674470df051c3954d4e6665c4 Mon Sep 17 00:00:00 2001 From: Nikita Krasnov Date: Wed, 18 Jun 2025 12:46:53 +0300 Subject: [PATCH 42/66] Fix typo --- doc/manual/source/store/store-object.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/source/store/store-object.md b/doc/manual/source/store/store-object.md index 115e107fb..0cdf4ec91 100644 --- a/doc/manual/source/store/store-object.md +++ b/doc/manual/source/store/store-object.md @@ -25,7 +25,7 @@ The *requisites* of a store object are all store objects reachable by paths of r [transitive closure]: https://en.wikipedia.org/wiki/Transitive_closure -We can also take the [transpose graph] ofthe references graph, where we reverse the orientation of all edges. +We can also take the [transpose graph] of the references graph, where we reverse the orientation of all edges. The *referrers* of a store object are the store objects that reference it. [transpose graph]: https://en.wikipedia.org/wiki/Transpose_graph From d2a25fbe5123645fe2426e44918b55b5c3cccef2 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Wed, 18 Jun 2025 08:23:37 -0700 Subject: [PATCH 43/66] Fix Nix formatting changes --- tests/nixos/github-flakes.nix | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/nixos/github-flakes.nix b/tests/nixos/github-flakes.nix index 3d7a77ec5..06142c2ef 100644 --- a/tests/nixos/github-flakes.nix +++ b/tests/nixos/github-flakes.nix @@ -1,7 +1,8 @@ -{ lib -, config -, nixpkgs -, ... +{ + lib, + config, + nixpkgs, + ... }: let pkgs = config.nodes.client.nixpkgs.pkgs; @@ -146,11 +147,12 @@ in }; client = - { config - , lib - , pkgs - , nodes - , ... + { + config, + lib, + pkgs, + nodes, + ... }: { virtualisation.writableStore = true; From 9b57573baea5abd242c5f62f537c7582c0097c3b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 18 Jun 2025 18:06:24 +0200 Subject: [PATCH 44/66] Revert "Drop magic-nix-cache" This reverts commit 9cc8be26747a0206613421a1ba1c3b1f54212e8b since magic-nix-cache works again (thanks @jchv). --- .github/workflows/ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fb70fae87..29cb33f56 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,6 +40,7 @@ jobs: extra_nix_config: | sandbox = true max-jobs = 1 + - uses: DeterminateSystems/magic-nix-cache-action@main # Since ubuntu 22.30, unprivileged usernamespaces are no longer allowed to map to the root user: # https://ubuntu.com/blog/ubuntu-23-10-restricted-unprivileged-user-namespaces - run: sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 @@ -133,6 +134,7 @@ jobs: - uses: cachix/install-nix-action@v31 with: install_url: https://releases.nixos.org/nix/nix-2.20.3/install + - uses: DeterminateSystems/magic-nix-cache-action@main - run: echo NIX_VERSION="$(nix --experimental-features 'nix-command flakes' eval .\#nix.version | tr -d \")" >> $GITHUB_ENV - run: nix --experimental-features 'nix-command flakes' build .#dockerImage -L - run: docker load -i ./result/image.tar.gz @@ -174,6 +176,7 @@ jobs: steps: - uses: actions/checkout@v4 - uses: DeterminateSystems/nix-installer-action@main + - uses: DeterminateSystems/magic-nix-cache-action@main - run: | nix build -L \ .#hydraJobs.tests.functional_user \ @@ -199,4 +202,5 @@ jobs: repository: NixOS/flake-regressions-data path: flake-regressions/tests - uses: DeterminateSystems/nix-installer-action@main + - uses: DeterminateSystems/magic-nix-cache-action@main - run: nix build -L --out-link ./new-nix && PATH=$(pwd)/new-nix/bin:$PATH MAX_FLAKES=25 flake-regressions/eval-all.sh From 20ba6be7491c14978dba52991c7257342b9087e4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 19 Jun 2025 13:58:51 +0200 Subject: [PATCH 45/66] Improve the Rosetta installation hint The Nix daemon detects supported system types at start time, so it needs to be restarted to detect x86_64-darwin support. --- src/libstore/unix/build/derivation-builder.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 1f15cc9e9..85b586373 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -700,7 +700,7 @@ void DerivationBuilderImpl::startBuilder() // since aarch64-darwin has Rosetta 2, this user can actually run x86_64-darwin on their hardware - we should tell them to run the command to install Darwin 2 if (drv.platform == "x86_64-darwin" && settings.thisSystem == "aarch64-darwin") - msg += fmt("\nNote: run `%s` to run programs for x86_64-darwin", Magenta("/usr/sbin/softwareupdate --install-rosetta")); + msg += fmt("\nNote: run `%s` to run programs for x86_64-darwin", Magenta("/usr/sbin/softwareupdate --install-rosetta && launchctl stop org.nixos.nix-daemon")); throw BuildError(msg); } From 3132aba8e425628ec74de134cea02e1bc85cd7c1 Mon Sep 17 00:00:00 2001 From: Luc Perkins Date: Thu, 19 Jun 2025 15:23:10 -0700 Subject: [PATCH 46/66] Fix broken test --- tests/functional/fetchGitShallow.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/fetchGitShallow.sh b/tests/functional/fetchGitShallow.sh index cf7e5fd4f..4c21bd7ac 100644 --- a/tests/functional/fetchGitShallow.sh +++ b/tests/functional/fetchGitShallow.sh @@ -43,7 +43,7 @@ path=$(nix eval --impure --raw --expr "(builtins.fetchTree { type = \"git\"; url [[ $(nix eval --impure --expr "(builtins.fetchTree { type = \"git\"; url = \"file://$TEST_ROOT/shallow-clone\"; ref = \"dev\"; shallow = true; }).revCount or 123") == 123 ]] # Test 3: Check unlocked input error message -expectStderr 1 nix eval --expr 'builtins.fetchTree { type = "git"; url = "file:///foo"; }' | grepQuiet "'fetchTree' will not fetch unlocked input" +expectStderr 1 nix eval --expr 'builtins.fetchTree { type = "git"; url = "file:///foo"; }' | grepQuiet "'fetchTree' doesn't fetch unlocked input" # Test 4: Regression test for revCount in worktrees derived from shallow clones # Add a worktree to the shallow clone From 785f3867fd6c4f0dfa0863b215a30ae262187472 Mon Sep 17 00:00:00 2001 From: Nikita Krasnov Date: Fri, 20 Jun 2025 21:19:13 +0300 Subject: [PATCH 47/66] Update docs --- doc/manual/source/store/derivation/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/source/store/derivation/index.md b/doc/manual/source/store/derivation/index.md index 147330df6..5ebbfa7c7 100644 --- a/doc/manual/source/store/derivation/index.md +++ b/doc/manual/source/store/derivation/index.md @@ -200,7 +200,7 @@ This ensures that there is a canonical [store path] used to refer to the derivat > **Note** > > Currently, the canonical encoding for every derivation is the "ATerm" format, -> but this is subject to change for types derivations which are not yet stable. +> but this is subject to change for the types of derivations which are not yet stable. Regardless of the format used, when serializing a derivation to a store object, that store object will be content-addressed. From 6ef683cb2ae64fa9bdddf54356613b2adf6844c6 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Fri, 20 Jun 2025 23:12:36 +0300 Subject: [PATCH 48/66] Restore multiline formatting of lists in meson files Applies a workaround to enforce multiline formatting of lists to reduce code churn introduced in 93a42a597145a2ff0214ddc6a9828921056c3657. --- meson.build | 5 ++++- .../include/nix/expr/tests/meson.build | 7 ++++++- src/libstore/linux/include/nix/store/meson.build | 5 ++++- src/libstore/linux/meson.build | 5 ++++- src/libutil/freebsd/include/nix/util/meson.build | 5 ++++- src/libutil/freebsd/meson.build | 5 ++++- src/libutil/linux/include/nix/util/meson.build | 6 +++++- src/libutil/linux/meson.build | 6 +++++- src/libutil/windows/include/nix/util/meson.build | 7 ++++++- src/perl/t/meson.build | 5 ++++- tests/functional/plugins/meson.build | 5 ++++- tests/functional/test-libstoreconsumer/meson.build | 5 ++++- 12 files changed, 54 insertions(+), 12 deletions(-) diff --git a/meson.build b/meson.build index 023791979..4a3a517fb 100644 --- a/meson.build +++ b/meson.build @@ -6,7 +6,10 @@ project( 'cpp', version : files('.version'), subproject_dir : 'src', - default_options : [ 'localstatedir=/nix/var' ], + default_options : [ + 'localstatedir=/nix/var', + # hack for trailing newline + ], meson_version : '>= 1.1', ) diff --git a/src/libexpr-test-support/include/nix/expr/tests/meson.build b/src/libexpr-test-support/include/nix/expr/tests/meson.build index e44d25582..84ec401ab 100644 --- a/src/libexpr-test-support/include/nix/expr/tests/meson.build +++ b/src/libexpr-test-support/include/nix/expr/tests/meson.build @@ -2,4 +2,9 @@ include_dirs = [ include_directories('../../..') ] -headers = files('libexpr.hh', 'nix_api_expr.hh', 'value/context.hh') +headers = files( + 'libexpr.hh', + 'nix_api_expr.hh', + 'value/context.hh', + # hack for trailing newline +) diff --git a/src/libstore/linux/include/nix/store/meson.build b/src/libstore/linux/include/nix/store/meson.build index eb2ae2583..c8e6a8268 100644 --- a/src/libstore/linux/include/nix/store/meson.build +++ b/src/libstore/linux/include/nix/store/meson.build @@ -1,3 +1,6 @@ include_dirs += include_directories('../..') -headers += files('personality.hh') +headers += files( + 'personality.hh', + # hack for trailing newline +) diff --git a/src/libstore/linux/meson.build b/src/libstore/linux/meson.build index 93084a8ae..5771cead5 100644 --- a/src/libstore/linux/meson.build +++ b/src/libstore/linux/meson.build @@ -1,3 +1,6 @@ -sources += files('personality.cc') +sources += files( + 'personality.cc', + # hack for trailing newline +) subdir('include/nix/store') diff --git a/src/libutil/freebsd/include/nix/util/meson.build b/src/libutil/freebsd/include/nix/util/meson.build index 283c49b95..4b7d78624 100644 --- a/src/libutil/freebsd/include/nix/util/meson.build +++ b/src/libutil/freebsd/include/nix/util/meson.build @@ -2,4 +2,7 @@ include_dirs += include_directories('../..') -headers += files('freebsd-jail.hh') +headers += files( + 'freebsd-jail.hh', + # hack for trailing newline +) diff --git a/src/libutil/freebsd/meson.build b/src/libutil/freebsd/meson.build index 34508efd0..d9b91a03d 100644 --- a/src/libutil/freebsd/meson.build +++ b/src/libutil/freebsd/meson.build @@ -1,3 +1,6 @@ -sources += files('freebsd-jail.cc') +sources += files( + 'freebsd-jail.cc', + # hack for trailing newline +) subdir('include/nix/util') diff --git a/src/libutil/linux/include/nix/util/meson.build b/src/libutil/linux/include/nix/util/meson.build index ac5f318f8..ec7030c49 100644 --- a/src/libutil/linux/include/nix/util/meson.build +++ b/src/libutil/linux/include/nix/util/meson.build @@ -2,4 +2,8 @@ include_dirs += include_directories('../..') -headers += files('cgroup.hh', 'linux-namespaces.hh') +headers += files( + 'cgroup.hh', + 'linux-namespaces.hh', + # hack for trailing newline +) diff --git a/src/libutil/linux/meson.build b/src/libutil/linux/meson.build index f3fe07c9c..230dd46f3 100644 --- a/src/libutil/linux/meson.build +++ b/src/libutil/linux/meson.build @@ -1,3 +1,7 @@ -sources += files('cgroup.cc', 'linux-namespaces.cc') +sources += files( + 'cgroup.cc', + 'linux-namespaces.cc', + # hack for trailing newline +) subdir('include/nix/util') diff --git a/src/libutil/windows/include/nix/util/meson.build b/src/libutil/windows/include/nix/util/meson.build index f0d4c37e9..5d0ace929 100644 --- a/src/libutil/windows/include/nix/util/meson.build +++ b/src/libutil/windows/include/nix/util/meson.build @@ -2,4 +2,9 @@ include_dirs += include_directories('../..') -headers += files('signals-impl.hh', 'windows-async-pipe.hh', 'windows-error.hh') +headers += files( + 'signals-impl.hh', + 'windows-async-pipe.hh', + 'windows-error.hh', + # hack for trailing newline +) diff --git a/src/perl/t/meson.build b/src/perl/t/meson.build index cd98453c3..5e75920ac 100644 --- a/src/perl/t/meson.build +++ b/src/perl/t/meson.build @@ -5,7 +5,10 @@ # src #--------------------------------------------------- -nix_perl_tests = files('init.t') +nix_perl_tests = files( + 'init.t', + # hack for trailing newline +) foreach f : nix_perl_tests diff --git a/tests/functional/plugins/meson.build b/tests/functional/plugins/meson.build index 7cbc935c9..41050ffc1 100644 --- a/tests/functional/plugins/meson.build +++ b/tests/functional/plugins/meson.build @@ -1,6 +1,9 @@ libplugintest = shared_module( 'plugintest', 'plugintest.cc', - dependencies : [ dependency('nix-expr') ], + dependencies : [ + dependency('nix-expr'), + # hack for trailing newline + ], build_by_default : false, ) diff --git a/tests/functional/test-libstoreconsumer/meson.build b/tests/functional/test-libstoreconsumer/meson.build index d27647ec4..ce566035f 100644 --- a/tests/functional/test-libstoreconsumer/meson.build +++ b/tests/functional/test-libstoreconsumer/meson.build @@ -1,6 +1,9 @@ libstoreconsumer_tester = executable( 'test-libstoreconsumer', 'main.cc', - dependencies : [ dependency('nix-store') ], + dependencies : [ + dependency('nix-store'), + # hack for trailing newline + ], build_by_default : false, ) From 7226a116a0bbc1f304d8160615525cb0aeb46096 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Wed, 26 Mar 2025 01:04:12 +0100 Subject: [PATCH 49/66] libutil: guess or invent a path from file descriptors This is useful for certain error recovery paths (no pun intended) that does not thread through the original path name. Change-Id: I2d800740cb4f9912e64c923120d3f977c58ccb7e Signed-off-by: Raito Bezarius --- src/libutil/file-descriptor.cc | 26 +++++++++++++++++++ .../include/nix/util/file-descriptor.hh | 17 ++++++++++++ src/libutil/meson.build | 2 ++ 3 files changed, 45 insertions(+) diff --git a/src/libutil/file-descriptor.cc b/src/libutil/file-descriptor.cc index 9e0827442..9193a30bb 100644 --- a/src/libutil/file-descriptor.cc +++ b/src/libutil/file-descriptor.cc @@ -1,5 +1,8 @@ #include "nix/util/serialise.hh" #include "nix/util/util.hh" +#include "nix/util/file-system.hh" + +#include "util-config-private.hh" #include #include @@ -74,6 +77,29 @@ Descriptor AutoCloseFD::get() const return fd; } +std::string guessOrInventPathFromFD(Descriptor fd) + { + assert(fd >= 0); + /* On Linux, there's no F_GETPATH available. + * But we can read /proc/ */ + #if defined(__linux__) + try { + return readLink(fmt("/proc/self/fd/%1%", fd)); + } catch (...) { + } + #elif defined (HAVE_F_GETPATH) && HAVE_F_GETPATH + std::string fdName(PATH_MAX, '\0'); + if (fcntl(fd, F_GETPATH, fdName.data()) != -1) { + fdName.resize(strlen(fdName.c_str())); + return fdName; + } + #else + #error "No implementation for retrieving file descriptors path." + #endif + + return fmt("", fd); + } + void AutoCloseFD::close() { diff --git a/src/libutil/include/nix/util/file-descriptor.hh b/src/libutil/include/nix/util/file-descriptor.hh index e2bcce2a2..35b359fb5 100644 --- a/src/libutil/include/nix/util/file-descriptor.hh +++ b/src/libutil/include/nix/util/file-descriptor.hh @@ -106,6 +106,14 @@ void drainFD( #endif ); + /* + * Will attempt to guess *A* path associated that might lead to the same file as used by this + * file descriptor. + * + * The returned string should NEVER be used as a valid path. + */ + std::string guessOrInventPathFromFD(Descriptor fd); + /** * Get [Standard Input](https://en.wikipedia.org/wiki/Standard_streams#Standard_input_(stdin)) */ @@ -160,6 +168,15 @@ public: AutoCloseFD& operator =(const AutoCloseFD & fd) = delete; AutoCloseFD& operator =(AutoCloseFD&& fd); Descriptor get() const; + + /** + * Will attempt to guess *A* path associated that might lead to the same file as used by this + * file descriptor. + * + * The returned string should NEVER be used as a valid path. + */ + std::string guessOrInventPath() const { return guessOrInventPathFromFD(fd); } + explicit operator bool() const; Descriptor release(); void close(); diff --git a/src/libutil/meson.build b/src/libutil/meson.build index f5ad2b1f6..2203e2294 100644 --- a/src/libutil/meson.build +++ b/src/libutil/meson.build @@ -37,6 +37,8 @@ foreach funcspec : check_funcs configdata.set(define_name, define_value, description: funcspec[1]) endforeach +configdata.set('HAVE_F_GETPATH', cxx.has_header_symbol('fcntl.h', 'F_GETPATH').to_int()) + subdir('nix-meson-build-support/libatomic') if host_machine.system() == 'windows' From 6a5b6ad3b75a1032ee495cec456e8d28b7e0595e Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Wed, 26 Mar 2025 01:04:59 +0100 Subject: [PATCH 50/66] libstore: open build directory as a dirfd as well We now keep around a proper AutoCloseFD around the temporary directory which we plan to use for openat operations and avoiding the build directory being swapped out while we are doing something else. Change-Id: I18d387b0f123ebf2d20c6405cd47ebadc5505f2a Signed-off-by: Raito Bezarius --- src/libstore/unix/build/derivation-builder.cc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 85b586373..e2dfc5860 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -95,6 +95,11 @@ protected: */ Path topTmpDir; + /** + * The file descriptor of the temporary directory. + */ + AutoCloseFD tmpDirFd; + /** * The sort of derivation we are building. * @@ -710,6 +715,13 @@ void DerivationBuilderImpl::startBuilder() topTmpDir = createTempDir(settings.buildDir.get().value_or(""), "nix-build-" + std::string(drvPath.name()), 0700); setBuildTmpDir(); assert(!tmpDir.empty()); + + /* The TOCTOU between the previous mkdir call and this open call is unavoidable due to + POSIX semantics.*/ + tmpDirFd = AutoCloseFD{open(tmpDir.c_str(), O_RDONLY | O_NOFOLLOW | O_DIRECTORY)}; + if (!tmpDirFd) + throw SysError("failed to open the build temporary directory descriptor '%1%'", tmpDir); + chownToBuilder(tmpDir); for (auto & [outputName, status] : initialOutputs) { From 002d202653a2750872ed7b943363ce4029af4dec Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Wed, 26 Mar 2025 01:05:34 +0100 Subject: [PATCH 51/66] libstore: chown to builder variant for file descriptors We use it immediately for the build temporary directory. Change-Id: I180193c63a2b98721f5fb8e542c4e39c099bb947 Signed-off-by: Raito Bezarius --- src/libstore/unix/build/derivation-builder.cc | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index e2dfc5860..30468d3b2 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -305,9 +305,17 @@ protected: /** * Make a file owned by the builder. + * + * SAFETY: this function is prone to TOCTOU as it receives a path and not a descriptor. + * It's only safe to call in a child of a directory only visible to the owner. */ void chownToBuilder(const Path & path); + /** + * Make a file owned by the builder addressed by its file descriptor. + */ + void chownToBuilder(const AutoCloseFD & fd); + /** * Run the builder's process. */ @@ -722,7 +730,7 @@ void DerivationBuilderImpl::startBuilder() if (!tmpDirFd) throw SysError("failed to open the build temporary directory descriptor '%1%'", tmpDir); - chownToBuilder(tmpDir); + chownToBuilder(tmpDirFd); for (auto & [outputName, status] : initialOutputs) { /* Set scratch path we'll actually use during the build. @@ -1267,6 +1275,13 @@ void DerivationBuilderImpl::chownToBuilder(const Path & path) throw SysError("cannot change ownership of '%1%'", path); } +void DerivationBuilderImpl::chownToBuilder(const AutoCloseFD & fd) +{ + if (!buildUser) return; + if (fchown(fd.get(), buildUser->getUID(), buildUser->getGID()) == -1) + throw SysError("cannot change ownership of file '%1%'", fd.guessOrInventPath()); +} + void DerivationBuilderImpl::runChild() { /* Warning: in the child we should absolutely not make any SQLite From 034f59bbb9a8c6a5febfed042fa9119410a3f123 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Wed, 26 Mar 2025 01:06:03 +0100 Subject: [PATCH 52/66] libutil: writeFile variant for file descriptors `writeFile` lose its `sync` boolean flag to make things simpler. A new `writeFileAndSync` function is created and all call sites are converted to it. Change-Id: Ib871a5283a9c047db1e4fe48a241506e4aab9192 Signed-off-by: Raito Bezarius --- src/libstore/local-store.cc | 4 +-- src/libutil/file-system.cc | 35 ++++++++++++++++----- src/libutil/include/nix/util/file-system.hh | 13 ++++++-- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index e53cab2dc..00c1ac6dd 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -247,7 +247,7 @@ LocalStore::LocalStore(ref config) else if (curSchema == 0) { /* new store */ curSchema = nixSchemaVersion; openDB(*state, true); - writeFile(schemaPath, fmt("%1%", curSchema), 0666, true); + writeFileAndSync(schemaPath, fmt("%1%", curSchema), 0666); } else if (curSchema < nixSchemaVersion) { @@ -298,7 +298,7 @@ LocalStore::LocalStore(ref config) txn.commit(); } - writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, true); + writeFileAndSync(schemaPath, fmt("%1%", nixSchemaVersion), 0666); lockFile(globalLock.get(), ltRead, true); } diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index fee2945ff..4c3a56e65 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -304,7 +304,7 @@ void readFile(const Path & path, Sink & sink, bool memory_map) } -void writeFile(const Path & path, std::string_view s, mode_t mode, bool sync) +void writeFile(const Path & path, std::string_view s, mode_t mode) { AutoCloseFD fd = toDescriptor(open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT // TODO @@ -314,20 +314,39 @@ void writeFile(const Path & path, std::string_view s, mode_t mode, bool sync) , mode)); if (!fd) throw SysError("opening file '%1%'", path); + + writeFile(fd, s, mode); + + /* Close explicitly to propagate the exceptions. */ + fd.close(); +} + +void writeFile(AutoCloseFD & fd, std::string_view s, mode_t mode) +{ + assert(fd); try { writeFull(fd.get(), s); } catch (Error & e) { - e.addTrace({}, "writing file '%1%'", path); + e.addTrace({}, "writing file '%1%'", fd.guessOrInventPath()); throw; } - if (sync) - fd.fsync(); - // Explicitly close to make sure exceptions are propagated. - fd.close(); - if (sync) - syncParent(path); } +void writeFileAndSync(const Path & path, std::string_view s, mode_t mode) +{ + { + AutoCloseFD fd{open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, mode)}; + if (!fd) + throw SysError("opening file '%1%'", path); + + writeFile(fd, s, mode); + fd.fsync(); + /* Close explicitly to ensure that exceptions are propagated. */ + fd.close(); + } + + syncParent(path); +} void writeFile(const Path & path, Source & source, mode_t mode, bool sync) { diff --git a/src/libutil/include/nix/util/file-system.hh b/src/libutil/include/nix/util/file-system.hh index 0121745ab..77d5f2aa1 100644 --- a/src/libutil/include/nix/util/file-system.hh +++ b/src/libutil/include/nix/util/file-system.hh @@ -172,10 +172,10 @@ void readFile(const Path & path, Sink & sink, bool memory_map = true); /** * Write a string to a file. */ -void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, bool sync = false); -static inline void writeFile(const std::filesystem::path & path, std::string_view s, mode_t mode = 0666, bool sync = false) +void writeFile(const Path & path, std::string_view s, mode_t mode = 0666); +static inline void writeFile(const std::filesystem::path & path, std::string_view s, mode_t mode = 0666) { - return writeFile(path.string(), s, mode, sync); + return writeFile(path.string(), s, mode); } void writeFile(const Path & path, Source & source, mode_t mode = 0666, bool sync = false); @@ -184,6 +184,13 @@ static inline void writeFile(const std::filesystem::path & path, Source & source return writeFile(path.string(), source, mode, sync); } +void writeFile(AutoCloseFD & fd, std::string_view s, mode_t mode = 0666 ); + +/** + * Write a string to a file and flush the file and its parents direcotry to disk. + */ +void writeFileAndSync(const Path & path, std::string_view s, mode_t mode = 0666); + /** * Flush a path's parent directory to disk. */ From 4e59d3fdb25335d7ab2612e72ff191e9bfbba8f4 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Wed, 26 Mar 2025 01:07:47 +0100 Subject: [PATCH 53/66] libstore: ensure that `passAsFile` is created in the original temp dir This ensures that `passAsFile` data is created inside the expected temporary build directory by `openat()` from the parent directory file descriptor. This avoids a TOCTOU which is part of the attack chain of CVE-????. Change-Id: Ie5273446c4a19403088d0389ae8e3f473af8879a Signed-off-by: Raito Bezarius --- src/libstore/unix/build/derivation-builder.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 30468d3b2..22445d547 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -1070,8 +1070,11 @@ void DerivationBuilderImpl::initEnv() auto hash = hashString(HashAlgorithm::SHA256, i.first); std::string fn = ".attr-" + hash.to_string(HashFormat::Nix32, false); Path p = tmpDir + "/" + fn; - writeFile(p, rewriteStrings(i.second, inputRewrites)); - chownToBuilder(p); + AutoCloseFD passAsFileFd{openat(tmpDirFd.get(), fn.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC | O_EXCL | O_NOFOLLOW, 0666)}; + if (!passAsFileFd) + throw SysError("opening `passAsFile` file in the sandbox '%1%'", p); + writeFile(passAsFileFd, rewriteStrings(i.second, inputRewrites)); + chownToBuilder(passAsFileFd); env[i.first + "Path"] = tmpDirInSandbox() + "/" + fn; } } From 5ec047f348d747d079bfb5d92a10a6c8d446089b Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Wed, 26 Mar 2025 12:42:55 +0100 Subject: [PATCH 54/66] libutil: ensure that `_deletePath` does NOT use absolute paths with dirfds When calling `_deletePath` with a parent file descriptor, `openat` is made effective by using relative paths to the directory file descriptor. To avoid the problem, the signature is changed to resist misuse with an assert in the prologue of the function. Change-Id: I6b3fc766bad2afe54dc27d47d1df3873e188de96 Signed-off-by: Raito Bezarius --- src/libutil/file-system.cc | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 4c3a56e65..12ada6d5c 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -444,6 +444,8 @@ void recursiveSync(const Path & path) static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, uint64_t & bytesFreed, std::exception_ptr & ex MOUNTEDPATHS_PARAM) { #ifndef _WIN32 + /* This ensures that `name` is an immediate child of `parentfd`. */ + assert(!path.empty() && path.string().find('/') == std::string::npos && "`name` is an immediate child to `parentfd`"); checkInterrupt(); #ifdef __FreeBSD__ @@ -454,13 +456,13 @@ static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, } #endif - std::string name(baseNameOf(path.native())); + std::string name(path.native()); struct stat st; if (fstatat(parentfd, name.c_str(), &st, AT_SYMLINK_NOFOLLOW) == -1) { if (errno == ENOENT) return; - throw SysError("getting status of %1%", path); + throw SysError("getting status of '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); } if (!S_ISDIR(st.st_mode)) { @@ -491,23 +493,24 @@ static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, /* Make the directory accessible. */ const auto PERM_MASK = S_IRUSR | S_IWUSR | S_IXUSR; if ((st.st_mode & PERM_MASK) != PERM_MASK) { - if (fchmodat(parentfd, name.c_str(), st.st_mode | PERM_MASK, 0) == -1) - throw SysError("chmod %1%", path); + if (fchmodat(parentfd, name.c_str(), st.st_mode | PERM_MASK, 0) == -1) { + throw SysError("chmod '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); + } } - int fd = openat(parentfd, path.c_str(), O_RDONLY); + int fd = openat(parentfd, name.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW); if (fd == -1) - throw SysError("opening directory %1%", path); + throw SysError("opening directory '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); AutoCloseDir dir(fdopendir(fd)); if (!dir) - throw SysError("opening directory %1%", path); + throw SysError("opening directory '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); struct dirent * dirent; while (errno = 0, dirent = readdir(dir.get())) { /* sic */ checkInterrupt(); std::string childName = dirent->d_name; if (childName == "." || childName == "..") continue; - _deletePath(dirfd(dir.get()), path + "/" + childName, bytesFreed, ex MOUNTEDPATHS_ARG); + _deletePath(dirfd(dir.get()), childName, bytesFreed, ex MOUNTEDPATHS_ARG); } if (errno) throw SysError("reading directory %1%", path); } @@ -516,7 +519,7 @@ static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, if (unlinkat(parentfd, name.c_str(), flags) == -1) { if (errno == ENOENT) return; try { - throw SysError("cannot unlink %1%", path); + throw SysError("cannot unlink '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); } catch (...) { if (!ex) ex = std::current_exception(); @@ -544,7 +547,7 @@ static void _deletePath(const std::filesystem::path & path, uint64_t & bytesFree std::exception_ptr ex; - _deletePath(dirfd.get(), path, bytesFreed, ex MOUNTEDPATHS_ARG); + _deletePath(dirfd.get(), path.filename(), bytesFreed, ex MOUNTEDPATHS_ARG); if (ex) std::rethrow_exception(ex); From 4ea4813753c895118b7377495647b48f240ff4d8 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Thu, 27 Mar 2025 12:22:26 +0100 Subject: [PATCH 55/66] libstore: ensure that temporary directory is always 0o000 before deletion In the case the deletion fails, we should ensure that the temporary directory cannot be used for nefarious purposes. Change-Id: I498a2dd0999a74195d13642f44a5de1e69d46120 Signed-off-by: Raito Bezarius --- src/libstore/unix/build/derivation-builder.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 22445d547..c61fe7001 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -2093,6 +2093,15 @@ void DerivationBuilderImpl::checkOutputs(const std::map Date: Thu, 12 Jun 2025 11:15:58 +0200 Subject: [PATCH 56/66] Tweak comment --- src/libstore/unix/build/derivation-builder.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index c61fe7001..a125676e5 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -2093,8 +2093,8 @@ void DerivationBuilderImpl::checkOutputs(const std::map Date: Thu, 12 Jun 2025 11:35:28 +0200 Subject: [PATCH 57/66] Drop guessOrInventPathFromFD() No need to do hacky stuff like that when we already know the original path. --- src/libstore/unix/build/derivation-builder.cc | 14 +++---- src/libutil/file-descriptor.cc | 26 ------------- src/libutil/file-system.cc | 39 +++++++++---------- .../include/nix/util/file-descriptor.hh | 17 -------- src/libutil/include/nix/util/file-system.hh | 4 +- src/libutil/meson.build | 2 - 6 files changed, 28 insertions(+), 74 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index a125676e5..36405a8a2 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -314,7 +314,7 @@ protected: /** * Make a file owned by the builder addressed by its file descriptor. */ - void chownToBuilder(const AutoCloseFD & fd); + void chownToBuilder(int fd, const Path & path); /** * Run the builder's process. @@ -730,7 +730,7 @@ void DerivationBuilderImpl::startBuilder() if (!tmpDirFd) throw SysError("failed to open the build temporary directory descriptor '%1%'", tmpDir); - chownToBuilder(tmpDirFd); + chownToBuilder(tmpDirFd.get(), tmpDir); for (auto & [outputName, status] : initialOutputs) { /* Set scratch path we'll actually use during the build. @@ -1073,8 +1073,8 @@ void DerivationBuilderImpl::initEnv() AutoCloseFD passAsFileFd{openat(tmpDirFd.get(), fn.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC | O_EXCL | O_NOFOLLOW, 0666)}; if (!passAsFileFd) throw SysError("opening `passAsFile` file in the sandbox '%1%'", p); - writeFile(passAsFileFd, rewriteStrings(i.second, inputRewrites)); - chownToBuilder(passAsFileFd); + writeFile(passAsFileFd, p, rewriteStrings(i.second, inputRewrites)); + chownToBuilder(passAsFileFd.get(), p); env[i.first + "Path"] = tmpDirInSandbox() + "/" + fn; } } @@ -1278,11 +1278,11 @@ void DerivationBuilderImpl::chownToBuilder(const Path & path) throw SysError("cannot change ownership of '%1%'", path); } -void DerivationBuilderImpl::chownToBuilder(const AutoCloseFD & fd) +void DerivationBuilderImpl::chownToBuilder(int fd, const Path & path) { if (!buildUser) return; - if (fchown(fd.get(), buildUser->getUID(), buildUser->getGID()) == -1) - throw SysError("cannot change ownership of file '%1%'", fd.guessOrInventPath()); + if (fchown(fd, buildUser->getUID(), buildUser->getGID()) == -1) + throw SysError("cannot change ownership of file '%1%'", path); } void DerivationBuilderImpl::runChild() diff --git a/src/libutil/file-descriptor.cc b/src/libutil/file-descriptor.cc index 9193a30bb..9e0827442 100644 --- a/src/libutil/file-descriptor.cc +++ b/src/libutil/file-descriptor.cc @@ -1,8 +1,5 @@ #include "nix/util/serialise.hh" #include "nix/util/util.hh" -#include "nix/util/file-system.hh" - -#include "util-config-private.hh" #include #include @@ -77,29 +74,6 @@ Descriptor AutoCloseFD::get() const return fd; } -std::string guessOrInventPathFromFD(Descriptor fd) - { - assert(fd >= 0); - /* On Linux, there's no F_GETPATH available. - * But we can read /proc/ */ - #if defined(__linux__) - try { - return readLink(fmt("/proc/self/fd/%1%", fd)); - } catch (...) { - } - #elif defined (HAVE_F_GETPATH) && HAVE_F_GETPATH - std::string fdName(PATH_MAX, '\0'); - if (fcntl(fd, F_GETPATH, fdName.data()) != -1) { - fdName.resize(strlen(fdName.c_str())); - return fdName; - } - #else - #error "No implementation for retrieving file descriptors path." - #endif - - return fmt("", fd); - } - void AutoCloseFD::close() { diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 12ada6d5c..3fd5ae669 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -315,19 +315,19 @@ void writeFile(const Path & path, std::string_view s, mode_t mode) if (!fd) throw SysError("opening file '%1%'", path); - writeFile(fd, s, mode); + writeFile(fd, path, s, mode); /* Close explicitly to propagate the exceptions. */ fd.close(); } -void writeFile(AutoCloseFD & fd, std::string_view s, mode_t mode) +void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode) { assert(fd); try { writeFull(fd.get(), s); } catch (Error & e) { - e.addTrace({}, "writing file '%1%'", fd.guessOrInventPath()); + e.addTrace({}, "writing file '%1%'", origPath); throw; } } @@ -339,7 +339,7 @@ void writeFileAndSync(const Path & path, std::string_view s, mode_t mode) if (!fd) throw SysError("opening file '%1%'", path); - writeFile(fd, s, mode); + writeFile(fd, path, s, mode); fd.fsync(); /* Close explicitly to ensure that exceptions are propagated. */ fd.close(); @@ -444,8 +444,6 @@ void recursiveSync(const Path & path) static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, uint64_t & bytesFreed, std::exception_ptr & ex MOUNTEDPATHS_PARAM) { #ifndef _WIN32 - /* This ensures that `name` is an immediate child of `parentfd`. */ - assert(!path.empty() && path.string().find('/') == std::string::npos && "`name` is an immediate child to `parentfd`"); checkInterrupt(); #ifdef __FreeBSD__ @@ -456,13 +454,14 @@ static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, } #endif - std::string name(path.native()); + std::string name(path.filename()); + assert(name != "." && name != ".." && !name.empty()); struct stat st; if (fstatat(parentfd, name.c_str(), &st, AT_SYMLINK_NOFOLLOW) == -1) { if (errno == ENOENT) return; - throw SysError("getting status of '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); + throw SysError("getting status of %1%", path); } if (!S_ISDIR(st.st_mode)) { @@ -493,24 +492,23 @@ static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, /* Make the directory accessible. */ const auto PERM_MASK = S_IRUSR | S_IWUSR | S_IXUSR; if ((st.st_mode & PERM_MASK) != PERM_MASK) { - if (fchmodat(parentfd, name.c_str(), st.st_mode | PERM_MASK, 0) == -1) { - throw SysError("chmod '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); - } + if (fchmodat(parentfd, name.c_str(), st.st_mode | PERM_MASK, 0) == -1) + throw SysError("chmod %1%", path); } int fd = openat(parentfd, name.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW); if (fd == -1) - throw SysError("opening directory '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); + throw SysError("opening directory %1%", path); AutoCloseDir dir(fdopendir(fd)); if (!dir) - throw SysError("opening directory '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); + throw SysError("opening directory %1%", path); struct dirent * dirent; while (errno = 0, dirent = readdir(dir.get())) { /* sic */ checkInterrupt(); std::string childName = dirent->d_name; if (childName == "." || childName == "..") continue; - _deletePath(dirfd(dir.get()), childName, bytesFreed, ex MOUNTEDPATHS_ARG); + _deletePath(dirfd(dir.get()), path / childName, bytesFreed, ex MOUNTEDPATHS_ARG); } if (errno) throw SysError("reading directory %1%", path); } @@ -519,7 +517,7 @@ static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, if (unlinkat(parentfd, name.c_str(), flags) == -1) { if (errno == ENOENT) return; try { - throw SysError("cannot unlink '%1%' in directory '%2%'", name, guessOrInventPathFromFD(parentfd)); + throw SysError("cannot unlink %1%", path); } catch (...) { if (!ex) ex = std::current_exception(); @@ -535,19 +533,18 @@ static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, static void _deletePath(const std::filesystem::path & path, uint64_t & bytesFreed MOUNTEDPATHS_PARAM) { - Path dir = dirOf(path.string()); - if (dir == "") - dir = "/"; + assert(path.is_absolute()); + assert(path.parent_path() != path); - AutoCloseFD dirfd = toDescriptor(open(dir.c_str(), O_RDONLY)); + AutoCloseFD dirfd = toDescriptor(open(path.parent_path().string().c_str(), O_RDONLY)); if (!dirfd) { if (errno == ENOENT) return; - throw SysError("opening directory '%1%'", path); + throw SysError("opening directory %s", path.parent_path()); } std::exception_ptr ex; - _deletePath(dirfd.get(), path.filename(), bytesFreed, ex MOUNTEDPATHS_ARG); + _deletePath(dirfd.get(), path, bytesFreed, ex MOUNTEDPATHS_ARG); if (ex) std::rethrow_exception(ex); diff --git a/src/libutil/include/nix/util/file-descriptor.hh b/src/libutil/include/nix/util/file-descriptor.hh index 35b359fb5..e2bcce2a2 100644 --- a/src/libutil/include/nix/util/file-descriptor.hh +++ b/src/libutil/include/nix/util/file-descriptor.hh @@ -106,14 +106,6 @@ void drainFD( #endif ); - /* - * Will attempt to guess *A* path associated that might lead to the same file as used by this - * file descriptor. - * - * The returned string should NEVER be used as a valid path. - */ - std::string guessOrInventPathFromFD(Descriptor fd); - /** * Get [Standard Input](https://en.wikipedia.org/wiki/Standard_streams#Standard_input_(stdin)) */ @@ -168,15 +160,6 @@ public: AutoCloseFD& operator =(const AutoCloseFD & fd) = delete; AutoCloseFD& operator =(AutoCloseFD&& fd); Descriptor get() const; - - /** - * Will attempt to guess *A* path associated that might lead to the same file as used by this - * file descriptor. - * - * The returned string should NEVER be used as a valid path. - */ - std::string guessOrInventPath() const { return guessOrInventPathFromFD(fd); } - explicit operator bool() const; Descriptor release(); void close(); diff --git a/src/libutil/include/nix/util/file-system.hh b/src/libutil/include/nix/util/file-system.hh index 77d5f2aa1..5f062412d 100644 --- a/src/libutil/include/nix/util/file-system.hh +++ b/src/libutil/include/nix/util/file-system.hh @@ -173,18 +173,20 @@ void readFile(const Path & path, Sink & sink, bool memory_map = true); * Write a string to a file. */ void writeFile(const Path & path, std::string_view s, mode_t mode = 0666); + static inline void writeFile(const std::filesystem::path & path, std::string_view s, mode_t mode = 0666) { return writeFile(path.string(), s, mode); } void writeFile(const Path & path, Source & source, mode_t mode = 0666, bool sync = false); + static inline void writeFile(const std::filesystem::path & path, Source & source, mode_t mode = 0666, bool sync = false) { return writeFile(path.string(), source, mode, sync); } -void writeFile(AutoCloseFD & fd, std::string_view s, mode_t mode = 0666 ); +void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode = 0666); /** * Write a string to a file and flush the file and its parents direcotry to disk. diff --git a/src/libutil/meson.build b/src/libutil/meson.build index 2203e2294..f5ad2b1f6 100644 --- a/src/libutil/meson.build +++ b/src/libutil/meson.build @@ -37,8 +37,6 @@ foreach funcspec : check_funcs configdata.set(define_name, define_value, description: funcspec[1]) endforeach -configdata.set('HAVE_F_GETPATH', cxx.has_header_symbol('fcntl.h', 'F_GETPATH').to_int()) - subdir('nix-meson-build-support/libatomic') if host_machine.system() == 'windows' From a4b5584fb10e967f753a54b8c02cc5229b4cab8e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 12 Jun 2025 12:14:04 +0200 Subject: [PATCH 58/66] Replace 'bool sync' with an enum for clarity And drop writeFileAndSync(). --- src/libstore/local-store.cc | 4 +-- src/libutil/file-content-address.cc | 2 +- src/libutil/file-system.cc | 32 +++++++-------------- src/libutil/include/nix/util/file-system.hh | 19 ++++++------ 4 files changed, 21 insertions(+), 36 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 00c1ac6dd..9e1324262 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -247,7 +247,7 @@ LocalStore::LocalStore(ref config) else if (curSchema == 0) { /* new store */ curSchema = nixSchemaVersion; openDB(*state, true); - writeFileAndSync(schemaPath, fmt("%1%", curSchema), 0666); + writeFile(schemaPath, fmt("%1%", curSchema), 0666, FsSync::Yes); } else if (curSchema < nixSchemaVersion) { @@ -298,7 +298,7 @@ LocalStore::LocalStore(ref config) txn.commit(); } - writeFileAndSync(schemaPath, fmt("%1%", nixSchemaVersion), 0666); + writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, FsSync::Yes); lockFile(globalLock.get(), ltRead, true); } diff --git a/src/libutil/file-content-address.cc b/src/libutil/file-content-address.cc index 142bc70d5..d95781691 100644 --- a/src/libutil/file-content-address.cc +++ b/src/libutil/file-content-address.cc @@ -93,7 +93,7 @@ void restorePath( { switch (method) { case FileSerialisationMethod::Flat: - writeFile(path, source, 0666, startFsync); + writeFile(path, source, 0666, startFsync ? FsSync::Yes : FsSync::No); break; case FileSerialisationMethod::NixArchive: restorePath(path, source, startFsync); diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 3fd5ae669..79e6cf354 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -304,7 +304,7 @@ void readFile(const Path & path, Sink & sink, bool memory_map) } -void writeFile(const Path & path, std::string_view s, mode_t mode) +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 @@ -315,40 +315,28 @@ void writeFile(const Path & path, std::string_view s, mode_t mode) if (!fd) throw SysError("opening file '%1%'", path); - writeFile(fd, path, s, mode); + writeFile(fd, path, s, mode, sync); /* Close explicitly to propagate the exceptions. */ fd.close(); } -void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode) +void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode, FsSync sync) { assert(fd); try { writeFull(fd.get(), s); + + if (sync == FsSync::Yes) + fd.fsync(); + } catch (Error & e) { e.addTrace({}, "writing file '%1%'", origPath); throw; } } -void writeFileAndSync(const Path & path, std::string_view s, mode_t mode) -{ - { - AutoCloseFD fd{open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, mode)}; - if (!fd) - throw SysError("opening file '%1%'", path); - - writeFile(fd, path, s, mode); - fd.fsync(); - /* Close explicitly to ensure that exceptions are propagated. */ - fd.close(); - } - - syncParent(path); -} - -void writeFile(const Path & path, Source & source, mode_t mode, bool sync) +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 @@ -372,11 +360,11 @@ void writeFile(const Path & path, Source & source, mode_t mode, bool sync) e.addTrace({}, "writing file '%1%'", path); throw; } - if (sync) + if (sync == FsSync::Yes) fd.fsync(); // Explicitly close to make sure exceptions are propagated. fd.close(); - if (sync) + if (sync == FsSync::Yes) syncParent(path); } diff --git a/src/libutil/include/nix/util/file-system.hh b/src/libutil/include/nix/util/file-system.hh index 5f062412d..c45cb55aa 100644 --- a/src/libutil/include/nix/util/file-system.hh +++ b/src/libutil/include/nix/util/file-system.hh @@ -169,29 +169,26 @@ std::string readFile(const Path & path); std::string readFile(const std::filesystem::path & path); void readFile(const Path & path, Sink & sink, bool memory_map = true); +enum struct FsSync { Yes, No }; + /** * Write a string to a file. */ -void writeFile(const Path & path, std::string_view s, mode_t mode = 0666); +void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No); -static inline void writeFile(const std::filesystem::path & path, std::string_view s, mode_t mode = 0666) +static inline void writeFile(const std::filesystem::path & path, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No) { - return writeFile(path.string(), s, mode); + return writeFile(path.string(), s, mode, sync); } -void writeFile(const Path & path, Source & source, mode_t mode = 0666, bool sync = false); +void writeFile(const Path & path, Source & source, mode_t mode = 0666, FsSync sync = FsSync::No); -static inline void writeFile(const std::filesystem::path & path, Source & source, mode_t mode = 0666, bool sync = false) +static inline void writeFile(const std::filesystem::path & path, Source & source, mode_t mode = 0666, FsSync sync = FsSync::No) { return writeFile(path.string(), source, mode, sync); } -void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode = 0666); - -/** - * Write a string to a file and flush the file and its parents direcotry to disk. - */ -void writeFileAndSync(const Path & path, std::string_view s, mode_t mode = 0666); +void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No); /** * Flush a path's parent directory to disk. From 9af4c267c6cf89ccd27c4f11a32b15533d20a20e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 12 Jun 2025 12:30:32 +0200 Subject: [PATCH 59/66] Chown structured attr files safely --- src/libstore/unix/build/derivation-builder.cc | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 36405a8a2..e2a148aeb 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -316,6 +316,13 @@ protected: */ void chownToBuilder(int fd, const Path & path); + /** + * Create a file in `tmpDir` owned by the builder. + */ + void writeBuilderFile( + const std::string & name, + std::string_view contents); + /** * Run the builder's process. */ @@ -1069,16 +1076,10 @@ void DerivationBuilderImpl::initEnv() } else { auto hash = hashString(HashAlgorithm::SHA256, i.first); std::string fn = ".attr-" + hash.to_string(HashFormat::Nix32, false); - Path p = tmpDir + "/" + fn; - AutoCloseFD passAsFileFd{openat(tmpDirFd.get(), fn.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC | O_EXCL | O_NOFOLLOW, 0666)}; - if (!passAsFileFd) - throw SysError("opening `passAsFile` file in the sandbox '%1%'", p); - writeFile(passAsFileFd, p, rewriteStrings(i.second, inputRewrites)); - chownToBuilder(passAsFileFd.get(), p); + writeBuilderFile(fn, rewriteStrings(i.second, inputRewrites)); env[i.first + "Path"] = tmpDirInSandbox() + "/" + fn; } } - } /* For convenience, set an environment pointing to the top build @@ -1153,11 +1154,9 @@ void DerivationBuilderImpl::writeStructuredAttrs() auto jsonSh = StructuredAttrs::writeShell(json); - writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); - chownToBuilder(tmpDir + "/.attrs.sh"); + writeBuilderFile(".attrs.sh", rewriteStrings(jsonSh, inputRewrites)); env["NIX_ATTRS_SH_FILE"] = tmpDirInSandbox() + "/.attrs.sh"; - writeFile(tmpDir + "/.attrs.json", rewriteStrings(json.dump(), inputRewrites)); - chownToBuilder(tmpDir + "/.attrs.json"); + writeBuilderFile(".attrs.json", rewriteStrings(json.dump(), inputRewrites)); env["NIX_ATTRS_JSON_FILE"] = tmpDirInSandbox() + "/.attrs.json"; } } @@ -1285,6 +1284,18 @@ void DerivationBuilderImpl::chownToBuilder(int fd, const Path & path) throw SysError("cannot change ownership of file '%1%'", path); } +void DerivationBuilderImpl::writeBuilderFile( + const std::string & name, + std::string_view contents) +{ + auto path = std::filesystem::path(tmpDir) / name; + AutoCloseFD fd{openat(tmpDirFd.get(), name.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC | O_EXCL | O_NOFOLLOW, 0666)}; + if (!fd) + throw SysError("creating file %s", path); + writeFile(fd, path, contents); + chownToBuilder(fd.get(), path); +} + void DerivationBuilderImpl::runChild() { /* Warning: in the child we should absolutely not make any SQLite From 88b7db1ba455926868dd61f5ea39e454e5fb0433 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 30 Mar 2025 16:45:34 +0200 Subject: [PATCH 60/66] libstore: Don't default build-dir to temp-dir, store setting If a build directory is accessible to other users it is possible to smuggle data in and out of build directories. Usually this is only a build purity problem, but in combination with other issues it can be used to break out of a build sandbox. to prevent this we default to using a subdirectory of nixStateDir (which is more restrictive). (cherry picked from pennae Lix commit 55b416f6897fb0d8a9315a530a9b7f0914458ded) (store setting done by roberth) --- doc/manual/rl-next/build-dir-mandatory.md | 9 +++++ misc/systemd/nix-daemon.conf.in | 3 +- src/libstore/globals.cc | 1 + src/libstore/include/nix/store/globals.hh | 9 +---- src/libstore/include/nix/store/local-store.hh | 33 ++++++++++++++++++- src/libstore/local-store.cc | 12 +++++++ src/libstore/unix/build/derivation-builder.cc | 6 +++- .../build-remote-trustless-should-fail-0.sh | 1 - tests/functional/build-remote-trustless.sh | 1 - tests/functional/build-remote.sh | 1 - tests/functional/supplementary-groups.sh | 1 - 11 files changed, 62 insertions(+), 15 deletions(-) create mode 100644 doc/manual/rl-next/build-dir-mandatory.md diff --git a/doc/manual/rl-next/build-dir-mandatory.md b/doc/manual/rl-next/build-dir-mandatory.md new file mode 100644 index 000000000..cb45a4315 --- /dev/null +++ b/doc/manual/rl-next/build-dir-mandatory.md @@ -0,0 +1,9 @@ +--- +synopsis: "`build-dir` no longer defaults to `$TMPDIR`" +--- + +The directory in which temporary build directories are created no longer defaults +to `TMPDIR` or `/tmp`, to avoid builders making their directories +world-accessible. This behavior allowed escaping the build sandbox and can +cause build impurities even when not used maliciously. We now default to `builds` +in `NIX_STATE_DIR` (which is `/nix/var/nix/builds` in the default configuration). diff --git a/misc/systemd/nix-daemon.conf.in b/misc/systemd/nix-daemon.conf.in index e7b264234..a0ddc4019 100644 --- a/misc/systemd/nix-daemon.conf.in +++ b/misc/systemd/nix-daemon.conf.in @@ -1 +1,2 @@ -d @localstatedir@/nix/daemon-socket 0755 root root - - +d @localstatedir@/nix/daemon-socket 0755 root root - - +d @localstatedir@/nix/builds 0755 root root 7d - diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index de5128347..721491def 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -6,6 +6,7 @@ #include "nix/util/abstract-setting-to-json.hh" #include "nix/util/compute-levels.hh" #include "nix/util/signals.hh" +#include "nix/util/types.hh" #include #include diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index b5157b4f4..0ac689b55 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -697,14 +697,7 @@ public: Setting> buildDir{this, std::nullopt, "build-dir", R"( - The directory on the host, in which derivations' temporary build directories are created. - - If not set, Nix uses the system temporary directory indicated by the `TMPDIR` environment variable. - Note that builds are often performed by the Nix daemon, so its `TMPDIR` is used, and not that of the Nix command line interface. - - This is also the location where [`--keep-failed`](@docroot@/command-ref/opt-common.md#opt-keep-failed) leaves its files. - - If Nix runs without sandbox, or if the platform does not support sandboxing with bind mounts (e.g. macOS), then the [`builder`](@docroot@/language/derivations.md#attr-builder)'s environment contains this directory instead of the virtual location [`sandbox-build-dir`](#conf-sandbox-build-dir). + Override the `build-dir` store setting for all stores that have this setting. )"}; Setting allowedImpureHostPrefixes{this, {}, "allowed-impure-host-deps", diff --git a/src/libstore/include/nix/store/local-store.hh b/src/libstore/include/nix/store/local-store.hh index 9a118fcc5..e52d51f75 100644 --- a/src/libstore/include/nix/store/local-store.hh +++ b/src/libstore/include/nix/store/local-store.hh @@ -34,7 +34,38 @@ struct OptimiseStats uint64_t bytesFreed = 0; }; -struct LocalStoreConfig : std::enable_shared_from_this, virtual LocalFSStoreConfig +struct LocalBuildStoreConfig : virtual LocalFSStoreConfig { + +private: + /** + Input for computing the build directory. See `getBuildDir()`. + */ + Setting> buildDir{this, std::nullopt, "build-dir", + R"( + The directory on the host, in which derivations' temporary build directories are created. + + If not set, Nix will use the `builds` subdirectory of its configured state directory. + + Note that builds are often performed by the Nix daemon, so its `build-dir` applies. + + Nix will create this directory automatically with suitable permissions if it does not exist. + Otherwise its permissions must allow all users to traverse the directory (i.e. it must have `o+x` set, in unix parlance) for non-sandboxed builds to work correctly. + + This is also the location where [`--keep-failed`](@docroot@/command-ref/opt-common.md#opt-keep-failed) leaves its files. + + If Nix runs without sandbox, or if the platform does not support sandboxing with bind mounts (e.g. macOS), then the [`builder`](@docroot@/language/derivations.md#attr-builder)'s environment will contain this directory, instead of the virtual location [`sandbox-build-dir`](#conf-sandbox-build-dir). + + > **Warning** + > + > `build-dir` must not be set to a world-writable directory. + > Placing temporary build directories in a world-writable place allows other users to access or modify build data that is currently in use. + > This alone is merely an impurity, but combined with another factor this has allowed malicious derivations to escape the build sandbox. + )"}; +public: + Path getBuildDir() const; +}; + +struct LocalStoreConfig : std::enable_shared_from_this, virtual LocalFSStoreConfig, virtual LocalBuildStoreConfig { using LocalFSStoreConfig::LocalFSStoreConfig; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9e1324262..e25a802ec 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -77,6 +77,18 @@ std::string LocalStoreConfig::doc() ; } +Path LocalBuildStoreConfig::getBuildDir() const +{ + if (settings.buildDir.get().has_value()) { + return *settings.buildDir.get(); + } + if (buildDir.get().has_value()) { + return *buildDir.get(); + } + + return stateDir.get() + "/builds"; +} + ref LocalStore::Config::openStore() const { return make_ref(ref{shared_from_this()}); diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index e2a148aeb..a036c95f1 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -725,9 +725,13 @@ void DerivationBuilderImpl::startBuilder() throw BuildError(msg); } + auto buildDir = getLocalStore(store).config->getBuildDir(); + + createDirs(buildDir); + /* Create a temporary directory where the build will take place. */ - topTmpDir = createTempDir(settings.buildDir.get().value_or(""), "nix-build-" + std::string(drvPath.name()), 0700); + topTmpDir = createTempDir(buildDir, "nix-build-" + std::string(drvPath.name()), 0700); setBuildTmpDir(); assert(!tmpDir.empty()); diff --git a/tests/functional/build-remote-trustless-should-fail-0.sh b/tests/functional/build-remote-trustless-should-fail-0.sh index 3401de1b0..e79527d72 100755 --- a/tests/functional/build-remote-trustless-should-fail-0.sh +++ b/tests/functional/build-remote-trustless-should-fail-0.sh @@ -12,7 +12,6 @@ requiresUnprivilegedUserNamespaces [[ $busybox =~ busybox ]] || skipTest "no busybox" unset NIX_STORE_DIR -unset NIX_STATE_DIR # We first build a dependency of the derivation we eventually want to # build. diff --git a/tests/functional/build-remote-trustless.sh b/tests/functional/build-remote-trustless.sh index 9f91a91a9..6014b57bb 100644 --- a/tests/functional/build-remote-trustless.sh +++ b/tests/functional/build-remote-trustless.sh @@ -9,7 +9,6 @@ requiresUnprivilegedUserNamespaces [[ "$busybox" =~ busybox ]] || skipTest "no busybox" unset NIX_STORE_DIR -unset NIX_STATE_DIR remoteDir=$TEST_ROOT/remote diff --git a/tests/functional/build-remote.sh b/tests/functional/build-remote.sh index 765cd71b4..f396bc72e 100644 --- a/tests/functional/build-remote.sh +++ b/tests/functional/build-remote.sh @@ -8,7 +8,6 @@ requiresUnprivilegedUserNamespaces # Avoid store dir being inside sandbox build-dir unset NIX_STORE_DIR -unset NIX_STATE_DIR function join_by { local d=$1; shift; echo -n "$1"; shift; printf "%s" "${@/#/$d}"; } diff --git a/tests/functional/supplementary-groups.sh b/tests/functional/supplementary-groups.sh index 400333f7d..a667d3e99 100755 --- a/tests/functional/supplementary-groups.sh +++ b/tests/functional/supplementary-groups.sh @@ -14,7 +14,6 @@ execUnshare < Date: Thu, 12 Jun 2025 11:04:07 +0200 Subject: [PATCH 61/66] Disallow the build directory having world-writable parents --- src/libstore/unix/build/derivation-builder.cc | 15 +++++++++++++++ tests/nixos/chroot-store.nix | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index a036c95f1..b089a0169 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -698,6 +698,18 @@ static void handleChildException(bool sendException) } } +static bool checkNotWorldWritable(std::filesystem::path path) +{ + while (true) { + auto st = lstat(path); + if (st.st_mode & S_IWOTH) + return false; + if (path == path.parent_path()) break; + path = path.parent_path(); + } + return true; +} + void DerivationBuilderImpl::startBuilder() { /* Make sure that no other processes are executing under the @@ -729,6 +741,9 @@ void DerivationBuilderImpl::startBuilder() createDirs(buildDir); + if (buildUser && !checkNotWorldWritable(buildDir)) + throw Error("Path %s or a parent directory is world-writable or a symlink. That's not allowed for security.", buildDir); + /* Create a temporary directory where the build will take place. */ topTmpDir = createTempDir(buildDir, "nix-build-" + std::string(drvPath.name()), 0700); diff --git a/tests/nixos/chroot-store.nix b/tests/nixos/chroot-store.nix index f89a20bc4..0a4fff992 100644 --- a/tests/nixos/chroot-store.nix +++ b/tests/nixos/chroot-store.nix @@ -41,5 +41,9 @@ in # Test that /nix/store is available via an overlayfs mount. machine.succeed("nix shell --store /tmp/nix ${pkgA} --command cowsay foo >&2") + + # Building in /tmp should fail for security reasons. + err = machine.fail("nix build --offline --store /tmp/nix --expr 'builtins.derivation { name = \"foo\"; system = \"x86_64-linux\"; builder = \"/foo\"; }' 2>&1") + assert "is world-writable" in err ''; } From 2e2fe4cb07c81f4af4798d1f67e0f6b8d6263e44 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 12 Jun 2025 11:12:23 +0200 Subject: [PATCH 62/66] Cleanup --- src/libstore/globals.cc | 1 - src/libstore/include/nix/store/local-store.hh | 3 ++- src/libstore/local-store.cc | 14 ++++++-------- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 721491def..de5128347 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -6,7 +6,6 @@ #include "nix/util/abstract-setting-to-json.hh" #include "nix/util/compute-levels.hh" #include "nix/util/signals.hh" -#include "nix/util/types.hh" #include #include diff --git a/src/libstore/include/nix/store/local-store.hh b/src/libstore/include/nix/store/local-store.hh index e52d51f75..fd7e6fc36 100644 --- a/src/libstore/include/nix/store/local-store.hh +++ b/src/libstore/include/nix/store/local-store.hh @@ -34,7 +34,8 @@ struct OptimiseStats uint64_t bytesFreed = 0; }; -struct LocalBuildStoreConfig : virtual LocalFSStoreConfig { +struct LocalBuildStoreConfig : virtual LocalFSStoreConfig +{ private: /** diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index e25a802ec..0d2d96e61 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -79,14 +79,12 @@ std::string LocalStoreConfig::doc() Path LocalBuildStoreConfig::getBuildDir() const { - if (settings.buildDir.get().has_value()) { - return *settings.buildDir.get(); - } - if (buildDir.get().has_value()) { - return *buildDir.get(); - } - - return stateDir.get() + "/builds"; + return + settings.buildDir.get().has_value() + ? *settings.buildDir.get() + : buildDir.get().has_value() + ? *buildDir.get() + : stateDir.get() + "/builds"; } ref LocalStore::Config::openStore() const From 37685b1c9c936da8644822d6ae60242b8ffcee8e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 19 Jun 2025 13:26:10 +0200 Subject: [PATCH 63/66] Fix Darwin test failure in repl.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes error: … while processing sandbox path '/private/tmp/nix-shell.0MDgyx/nix-test/ca/repl/store/nix/var/nix/builds/nix-build-simple.drv-65916-3910734210' (/private/tmp/nix-shell.0MDgyx/nix-test/ca/repl/store) error: 'nix' is too short to be a valid store path which happened because we were now putting the build directory underneath the store directory. --- src/libstore/unix/build/derivation-builder.cc | 2 +- tests/functional/repl.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index b089a0169..fd62aa664 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -922,7 +922,7 @@ DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox() store.computeFSClosure(store.toStorePath(i.second.source).first, closure); } catch (InvalidPath & e) { } catch (Error & e) { - e.addTrace({}, "while processing 'sandbox-paths'"); + e.addTrace({}, "while processing sandbox path '%s'", i.second.source); throw; } for (auto & i : closure) { diff --git a/tests/functional/repl.sh b/tests/functional/repl.sh index 6db9e2d3d..bfe18c9e5 100755 --- a/tests/functional/repl.sh +++ b/tests/functional/repl.sh @@ -67,7 +67,7 @@ testRepl () { # Simple test, try building a drv testRepl # Same thing (kind-of), but with a remote store. -testRepl --store "$TEST_ROOT/store?real=$NIX_STORE_DIR" +testRepl --store "$TEST_ROOT/other-root?real=$NIX_STORE_DIR" # Remove ANSI escape sequences. They can prevent grep from finding a match. stripColors () { From df21f249877cead560782f5ccdd1796bc5682349 Mon Sep 17 00:00:00 2001 From: Egor Konovalov <73017521+egorkonovalov@users.noreply.github.com> Date: Mon, 23 Jun 2025 11:26:59 +0200 Subject: [PATCH 64/66] Fix link Remove extra `realise` --- doc/manual/source/store/derivation/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/source/store/derivation/index.md b/doc/manual/source/store/derivation/index.md index 5ebbfa7c7..1687ad8c0 100644 --- a/doc/manual/source/store/derivation/index.md +++ b/doc/manual/source/store/derivation/index.md @@ -173,7 +173,7 @@ There are two types of placeholder, corresponding to the two cases where this pr > **Explanation** > -> In general, we need to realise [realise] a [store object] in order to be sure to have a store object for it. +> In general, we need to [realise] a [store object] in order to be sure to have a store object for it. > But for these two cases this is either impossible or impractical: > > - In the output case this is impossible: From 55d12dfc5db53f7b2f90fef235e7115c9c17e8b2 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 25 Jun 2025 00:07:58 +0300 Subject: [PATCH 65/66] libstore-tests: Don't leak memory in tests We shouldn't leak memory in unit tests in order to make enabling ASAN easier. --- .../include/nix/store/tests/nix_api_store.hh | 8 ++++++-- src/libstore-tests/nix_api_store.cc | 9 +++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/libstore-test-support/include/nix/store/tests/nix_api_store.hh b/src/libstore-test-support/include/nix/store/tests/nix_api_store.hh index 63f80cf91..e51be3dab 100644 --- a/src/libstore-test-support/include/nix/store/tests/nix_api_store.hh +++ b/src/libstore-test-support/include/nix/store/tests/nix_api_store.hh @@ -34,6 +34,8 @@ public: Store * store; std::string nixDir; std::string nixStoreDir; + std::string nixStateDir; + std::string nixLogDir; protected: void init_local_store() @@ -53,11 +55,13 @@ protected: #endif nixStoreDir = nixDir + "/my_nix_store"; + nixStateDir = nixDir + "/my_state"; + nixLogDir = nixDir + "/my_log"; // Options documented in `nix help-stores` const char * p1[] = {"store", nixStoreDir.c_str()}; - const char * p2[] = {"state", (new std::string(nixDir + "/my_state"))->c_str()}; - const char * p3[] = {"log", (new std::string(nixDir + "/my_log"))->c_str()}; + const char * p2[] = {"state", nixStateDir.c_str()}; + const char * p3[] = {"log", nixLogDir.c_str()}; const char ** params[] = {p1, p2, p3, nullptr}; diff --git a/src/libstore-tests/nix_api_store.cc b/src/libstore-tests/nix_api_store.cc index 4eb95360a..3d9f7908b 100644 --- a/src/libstore-tests/nix_api_store.cc +++ b/src/libstore-tests/nix_api_store.cc @@ -71,17 +71,21 @@ TEST_F(nix_api_store_test, ReturnsValidStorePath) ASSERT_NE(result, nullptr); ASSERT_STREQ("name", result->path.name().data()); ASSERT_STREQ(PATH_SUFFIX.substr(1).c_str(), result->path.to_string().data()); + nix_store_path_free(result); } TEST_F(nix_api_store_test, SetsLastErrCodeToNixOk) { - nix_store_parse_path(ctx, store, (nixStoreDir + PATH_SUFFIX).c_str()); + StorePath * path = nix_store_parse_path(ctx, store, (nixStoreDir + PATH_SUFFIX).c_str()); ASSERT_EQ(ctx->last_err_code, NIX_OK); + nix_store_path_free(path); } TEST_F(nix_api_store_test, DoesNotCrashWhenContextIsNull) { - ASSERT_NO_THROW(nix_store_parse_path(ctx, store, (nixStoreDir + PATH_SUFFIX).c_str())); + StorePath * path = nullptr; + ASSERT_NO_THROW(path = nix_store_parse_path(ctx, store, (nixStoreDir + PATH_SUFFIX).c_str())); + nix_store_path_free(path); } TEST_F(nix_api_store_test, get_version) @@ -119,6 +123,7 @@ TEST_F(nix_api_store_test, nix_store_is_valid_path_not_in_store) { StorePath * path = nix_store_parse_path(ctx, store, (nixStoreDir + PATH_SUFFIX).c_str()); ASSERT_EQ(false, nix_store_is_valid_path(ctx, store, path)); + nix_store_path_free(path); } TEST_F(nix_api_store_test, nix_store_real_path) From eaced1e0d2a041668a07f34ea2c94c78466d08aa Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 26 Jun 2025 17:04:34 +0200 Subject: [PATCH 66/66] Move FlakeCommand into a header, allow separate registration of subcommands This allows us to start splitting up src/nix/flake.cc. --- .../include/nix/cmd/common-eval-args.hh | 3 +- src/nix/flake-command.hh | 27 ++++++ src/nix/flake.cc | 90 ++++++++----------- 3 files changed, 67 insertions(+), 53 deletions(-) create mode 100644 src/nix/flake-command.hh diff --git a/src/libcmd/include/nix/cmd/common-eval-args.hh b/src/libcmd/include/nix/cmd/common-eval-args.hh index 716340425..88ede1ed7 100644 --- a/src/libcmd/include/nix/cmd/common-eval-args.hh +++ b/src/libcmd/include/nix/cmd/common-eval-args.hh @@ -5,6 +5,7 @@ #include "nix/util/canon-path.hh" #include "nix/main/common-args.hh" #include "nix/expr/search-path.hh" +#include "nix/expr/eval-settings.hh" #include @@ -15,10 +16,8 @@ class Store; namespace fetchers { struct Settings; } class EvalState; -struct EvalSettings; struct CompatibilitySettings; class Bindings; -struct SourcePath; namespace flake { struct Settings; } diff --git a/src/nix/flake-command.hh b/src/nix/flake-command.hh new file mode 100644 index 000000000..36dfe44c6 --- /dev/null +++ b/src/nix/flake-command.hh @@ -0,0 +1,27 @@ +#pragma once + +#include "nix/cmd/command.hh" +#include "nix/cmd/installable-flake.hh" +#include "nix/flake/flake.hh" + +namespace nix { + +using namespace nix::flake; + +class FlakeCommand : virtual Args, public MixFlakeOptions +{ +protected: + std::string flakeUrl = "."; + +public: + + FlakeCommand(); + + FlakeRef getFlakeRef(); + + LockedFlake lockFlake(); + + std::vector getFlakeRefsForCompletion() override; +}; + +} diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 13f7363fc..1d20add02 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -1,11 +1,9 @@ -#include "nix/cmd/command.hh" -#include "nix/cmd/installable-flake.hh" +#include "flake-command.hh" #include "nix/main/common-args.hh" #include "nix/main/shared.hh" #include "nix/expr/eval.hh" #include "nix/expr/eval-inline.hh" #include "nix/expr/eval-settings.hh" -#include "nix/flake/flake.hh" #include "nix/expr/get-drvs.hh" #include "nix/util/signals.hh" #include "nix/store/store-open.hh" @@ -33,43 +31,36 @@ using namespace nix::flake; using json = nlohmann::json; struct CmdFlakeUpdate; -class FlakeCommand : virtual Args, public MixFlakeOptions + +FlakeCommand::FlakeCommand() { -protected: - std::string flakeUrl = "."; + expectArgs({ + .label = "flake-url", + .optional = true, + .handler = {&flakeUrl}, + .completer = {[&](AddCompletions & completions, size_t, std::string_view prefix) { + completeFlakeRef(completions, getStore(), prefix); + }} + }); +} -public: +FlakeRef FlakeCommand::getFlakeRef() +{ + return parseFlakeRef(fetchSettings, flakeUrl, std::filesystem::current_path().string()); //FIXME +} - FlakeCommand() - { - expectArgs({ - .label = "flake-url", - .optional = true, - .handler = {&flakeUrl}, - .completer = {[&](AddCompletions & completions, size_t, std::string_view prefix) { - completeFlakeRef(completions, getStore(), prefix); - }} - }); - } +LockedFlake FlakeCommand::lockFlake() +{ + return flake::lockFlake(flakeSettings, *getEvalState(), getFlakeRef(), lockFlags); +} - FlakeRef getFlakeRef() - { - return parseFlakeRef(fetchSettings, flakeUrl, std::filesystem::current_path().string()); //FIXME - } - - LockedFlake lockFlake() - { - return flake::lockFlake(flakeSettings, *getEvalState(), getFlakeRef(), lockFlags); - } - - std::vector getFlakeRefsForCompletion() override - { - return { - // Like getFlakeRef but with expandTilde called first - parseFlakeRef(fetchSettings, expandTilde(flakeUrl), std::filesystem::current_path().string()) - }; - } -}; +std::vector FlakeCommand::getFlakeRefsForCompletion() +{ + return { + // Like getFlakeRef but with expandTilde called first + parseFlakeRef(fetchSettings, expandTilde(flakeUrl), std::filesystem::current_path().string()) + }; +} struct CmdFlakeUpdate : FlakeCommand { @@ -1528,21 +1519,7 @@ struct CmdFlakePrefetch : FlakeCommand, MixJSON struct CmdFlake : NixMultiCommand { CmdFlake() - : NixMultiCommand( - "flake", - { - {"update", []() { return make_ref(); }}, - {"lock", []() { return make_ref(); }}, - {"metadata", []() { return make_ref(); }}, - {"info", []() { return make_ref(); }}, - {"check", []() { return make_ref(); }}, - {"init", []() { return make_ref(); }}, - {"new", []() { return make_ref(); }}, - {"clone", []() { return make_ref(); }}, - {"archive", []() { return make_ref(); }}, - {"show", []() { return make_ref(); }}, - {"prefetch", []() { return make_ref(); }}, - }) + : NixMultiCommand("flake", RegisterCommand::getCommandsFor({"flake"})) { } @@ -1566,3 +1543,14 @@ struct CmdFlake : NixMultiCommand }; static auto rCmdFlake = registerCommand("flake"); +static auto rCmdFlakeArchive = registerCommand2({"flake", "archive"}); +static auto rCmdFlakeCheck = registerCommand2({"flake", "check"}); +static auto rCmdFlakeClone = registerCommand2({"flake", "clone"}); +static auto rCmdFlakeInfo = registerCommand2({"flake", "info"}); +static auto rCmdFlakeInit = registerCommand2({"flake", "init"}); +static auto rCmdFlakeLock = registerCommand2({"flake", "lock"}); +static auto rCmdFlakeMetadata = registerCommand2({"flake", "metadata"}); +static auto rCmdFlakeNew = registerCommand2({"flake", "new"}); +static auto rCmdFlakePrefetch = registerCommand2({"flake", "prefetch"}); +static auto rCmdFlakeShow = registerCommand2({"flake", "show"}); +static auto rCmdFlakeUpdate = registerCommand2({"flake", "update"});