From 04023360edbeb3477286783b3101a5b53f78021b Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Tue, 3 Aug 2021 15:25:15 -0500 Subject: [PATCH 01/54] Evaluate nix-shell -i args relative to script When writing a shebang script, you expect your path to be relative to the script, not the cwd. We previously handled this correctly for relative file paths, but not for expressions. This handles both -p & -E args. My understanding is this should be what we want in any cases I can think of - people run scripts from many different working directories. @edolstra is there any reason to handle -p args differently in this case? Fixes #4232 --- src/nix-build/nix-build.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 75ce12a8c3e..4120ca3cf82 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -298,7 +298,9 @@ static void main_nix_build(int argc, char * * argv) else for (auto i : left) { if (fromArgs) - exprs.push_back(state->parseExprFromString(std::move(i), state->rootPath(CanonPath::fromCwd()))); + exprs.push_back(state->parseExprFromString( + std::move(i), + state->rootPath(CanonPath::fromCwd(inShebang ? dirOf(script) : ".")))); else { auto absolute = i; try { @@ -311,7 +313,7 @@ static void main_nix_build(int argc, char * * argv) /* If we're in a #! script, interpret filenames relative to the script. */ exprs.push_back(state->parseExprFromFile(resolveExprPath(state->checkSourcePath(lookupFileArg(*state, - inShebang && !packages ? absPath(i, absPath(dirOf(script))) : i))))); + inShebang ? absPath(i, absPath(dirOf(script))) : i))))); } } From 9a4641146f79d631eb0825c7313dd335715de0d6 Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Sat, 25 Nov 2023 19:06:45 -0500 Subject: [PATCH 02/54] tests: ensure nix-shell uses relative paths for expressions --- tests/functional/nix-shell.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell.sh index 13403fadb82..702d3a6b566 100644 --- a/tests/functional/nix-shell.sh +++ b/tests/functional/nix-shell.sh @@ -59,6 +59,16 @@ chmod a+rx $TEST_ROOT/shell.shebang.sh output=$($TEST_ROOT/shell.shebang.sh abc def) [ "$output" = "foo bar abc def" ] +# Test nix-shell shebang mode with an alternate working directory +sed -e "s|@ENV_PROG@|$(type -P env)|" shell.shebang.expr > $TEST_ROOT/shell.shebang.expr +chmod a+rx $TEST_ROOT/shell.shebang.expr +# Should fail due to expressions using relative path +! $TEST_ROOT/shell.shebang.expr bar +cp shell.nix config.nix $TEST_ROOT +# Should succeed +output=$($TEST_ROOT/shell.shebang.expr bar) +[ "$output" = '-e load(ARGV.shift) -- '"$TEST_ROOT"'/shell.shebang.expr bar' ] + # Test nix-shell shebang mode again with metacharacters in the filename. # First word of filename is chosen to not match any file in the test root. sed -e "s|@ENV_PROG@|$(type -P env)|" shell.shebang.sh > $TEST_ROOT/spaced\ \\\'\"shell.shebang.sh From f66f498bd43efaa6883f12ca5988a282eef09697 Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Sat, 25 Nov 2023 19:07:29 -0500 Subject: [PATCH 03/54] notes: document change in nix-shell behavior --- doc/manual/rl-next/shebang-relative.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 doc/manual/rl-next/shebang-relative.md diff --git a/doc/manual/rl-next/shebang-relative.md b/doc/manual/rl-next/shebang-relative.md new file mode 100644 index 00000000000..dbda0db4c88 --- /dev/null +++ b/doc/manual/rl-next/shebang-relative.md @@ -0,0 +1,8 @@ +synopsis: ensure nix-shell shebang uses relative path +prs: #5088 +description: { + +`nix-shell` shebangs use the script file's relative location to resolve relative paths to files passed as command line arguments, but expression arguments were still evaluated using the current working directory as a base path. +The new behavior is that evalutations are performed relative to the script. + +} From 68b8a28bc4b09cdd88f28bf0ba102a274a461b0c Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 10 Jun 2024 11:39:06 +0200 Subject: [PATCH 04/54] tests/run.sh: Check that env is mostly unmodified --- tests/functional/flakes/run.sh | 21 +++++++++++++++++++++ tests/functional/shell-hello.nix | 22 ++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/tests/functional/flakes/run.sh b/tests/functional/flakes/run.sh index 4d8b512b9b1..7fc78b3aa50 100755 --- a/tests/functional/flakes/run.sh +++ b/tests/functional/flakes/run.sh @@ -27,5 +27,26 @@ nix run --no-write-lock-file .#pkgAsPkg ! nix run --no-write-lock-file .#pkgAsApp || fail "'nix run' shouldn’t accept an 'app' defined under 'packages'" ! nix run --no-write-lock-file .#appAsPkg || fail "elements of 'apps' should be of type 'app'" +# Test that we're not setting any more environment variables than necessary. +# For instance, we might set an environment variable temporarily to affect some +# initialization or whatnot, but this must not leak into the environment of the +# command being run. +env > $TEST_ROOT/expected-env +nix run -f shell-hello.nix env > $TEST_ROOT/actual-env +# Remove/reset variables we expect to be different. +# - PATH is modified by nix shell +# - _ is set by bash and is expected to differ because it contains the original command +# - __CF_USER_TEXT_ENCODING is set by macOS and is beyond our control +sed -i \ + -e 's/PATH=.*/PATH=.../' \ + -e 's/_=.*/_=.../' \ + -e '/^__CF_USER_TEXT_ENCODING=.*$/d' \ + $TEST_ROOT/expected-env $TEST_ROOT/actual-env +sort $TEST_ROOT/expected-env | uniq > $TEST_ROOT/expected-env.sorted +# nix run appears to clear _. I don't understand why. Is this ok? +echo "_=..." >> $TEST_ROOT/actual-env +sort $TEST_ROOT/actual-env | uniq > $TEST_ROOT/actual-env.sorted +diff $TEST_ROOT/expected-env.sorted $TEST_ROOT/actual-env.sorted + clearStore diff --git a/tests/functional/shell-hello.nix b/tests/functional/shell-hello.nix index c46fdec8a8c..c920d7cb459 100644 --- a/tests/functional/shell-hello.nix +++ b/tests/functional/shell-hello.nix @@ -55,4 +55,26 @@ rec { chmod +x $out/bin/hello ''; }; + + # execs env from PATH, so that we can probe the environment + # does not allow arguments, because we don't need them + env = mkDerivation { + name = "env"; + outputs = [ "out" ]; + buildCommand = + '' + mkdir -p $out/bin + + cat > $out/bin/env <&2 + exit 1 + fi + exec env + EOF + chmod +x $out/bin/env + ''; + }; + } From 316b58dd5fcf6df5931c53c85a30018fe9d7d2ff Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 10 Jun 2024 11:39:33 +0200 Subject: [PATCH 05/54] tests/shell.sh: Check that env is mostly unmodified --- tests/functional/shell.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/functional/shell.sh b/tests/functional/shell.sh index 1760eefff31..fd0020a0fbb 100755 --- a/tests/functional/shell.sh +++ b/tests/functional/shell.sh @@ -21,6 +21,25 @@ nix shell -f shell-hello.nix hello-symlink -c hello | grep 'Hello World' # Test that symlinks outside of the store don't work. expect 1 nix shell -f shell-hello.nix forbidden-symlink -c hello 2>&1 | grepQuiet "is not in the Nix store" +# Test that we're not setting any more environment variables than necessary. +# For instance, we might set an environment variable temporarily to affect some +# initialization or whatnot, but this must not leak into the environment of the +# command being run. +env > $TEST_ROOT/expected-env +nix shell -f shell-hello.nix hello -c env > $TEST_ROOT/actual-env +# Remove/reset variables we expect to be different. +# - PATH is modified by nix shell +# - _ is set by bash and is expectedf to differ because it contains the original command +# - __CF_USER_TEXT_ENCODING is set by macOS and is beyond our control +sed -i \ + -e 's/PATH=.*/PATH=.../' \ + -e 's/_=.*/_=.../' \ + -e '/^__CF_USER_TEXT_ENCODING=.*$/d' \ + $TEST_ROOT/expected-env $TEST_ROOT/actual-env +sort $TEST_ROOT/expected-env > $TEST_ROOT/expected-env.sorted +sort $TEST_ROOT/actual-env > $TEST_ROOT/actual-env.sorted +diff $TEST_ROOT/expected-env.sorted $TEST_ROOT/actual-env.sorted + if isDaemonNewer "2.20.0pre20231220"; then # Test that command line attribute ordering is reflected in the PATH # https://github.com/NixOS/nix/issues/7905 From 13181356fc6fc3fa7e5f8ac98da6a2f28cb50003 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 6 Jul 2024 20:01:46 +0200 Subject: [PATCH 06/54] Refactor: rename runEnv -> isNixShell --- src/nix-build/nix-build.cc | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 57630c8c34b..d0b3b4f9f74 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -93,7 +93,7 @@ static std::vector shellwords(const std::string & s) static void main_nix_build(int argc, char * * argv) { auto dryRun = false; - auto runEnv = std::regex_search(argv[0], std::regex("nix-shell$")); + auto isNixShell = std::regex_search(argv[0], std::regex("nix-shell$")); auto pure = false; auto fromArgs = false; auto packages = false; @@ -107,7 +107,7 @@ static void main_nix_build(int argc, char * * argv) std::string envCommand; // interactive shell Strings envExclude; - auto myName = runEnv ? "nix-shell" : "nix-build"; + auto myName = isNixShell ? "nix-shell" : "nix-build"; auto inShebang = false; std::string script; @@ -132,7 +132,7 @@ static void main_nix_build(int argc, char * * argv) // Heuristic to see if we're invoked as a shebang script, namely, // if we have at least one argument, it's the name of an // executable file, and it starts with "#!". - if (runEnv && argc > 1) { + if (isNixShell && argc > 1) { script = argv[1]; try { auto lines = tokenizeString(readFile(script), "\n"); @@ -186,9 +186,9 @@ static void main_nix_build(int argc, char * * argv) dryRun = true; else if (*arg == "--run-env") // obsolete - runEnv = true; + isNixShell = true; - else if (runEnv && (*arg == "--command" || *arg == "--run")) { + else if (isNixShell && (*arg == "--command" || *arg == "--run")) { if (*arg == "--run") interactive = false; envCommand = getArg(*arg, arg, end) + "\nexit"; @@ -206,7 +206,7 @@ static void main_nix_build(int argc, char * * argv) else if (*arg == "--pure") pure = true; else if (*arg == "--impure") pure = false; - else if (runEnv && (*arg == "--packages" || *arg == "-p")) + else if (isNixShell && (*arg == "--packages" || *arg == "-p")) packages = true; else if (inShebang && *arg == "-i") { @@ -266,7 +266,7 @@ static void main_nix_build(int argc, char * * argv) auto autoArgs = myArgs.getAutoArgs(*state); auto autoArgsWithInNixShell = autoArgs; - if (runEnv) { + if (isNixShell) { auto newArgs = state->buildBindings(autoArgsWithInNixShell->size() + 1); newArgs.alloc("inNixShell").mkBool(true); for (auto & i : *autoArgs) newArgs.insert(i); @@ -282,13 +282,13 @@ static void main_nix_build(int argc, char * * argv) fromArgs = true; left = {joined.str()}; } else if (!fromArgs) { - if (left.empty() && runEnv && pathExists("shell.nix")) + if (left.empty() && isNixShell && pathExists("shell.nix")) left = {"shell.nix"}; if (left.empty()) left = {"default.nix"}; } - if (runEnv) + if (isNixShell) setEnv("IN_NIX_SHELL", pure ? "pure" : "impure"); PackageInfos drvs; @@ -330,7 +330,7 @@ static void main_nix_build(int argc, char * * argv) std::function takesNixShellAttr; takesNixShellAttr = [&](const Value & v) { - if (!runEnv) { + if (!isNixShell) { return false; } bool add = false; @@ -381,7 +381,7 @@ static void main_nix_build(int argc, char * * argv) store->buildPaths(paths, buildMode, evalStore); }; - if (runEnv) { + if (isNixShell) { if (drvs.size() != 1) throw UsageError("nix-shell requires a single derivation"); From 5c367ece895601a90ef4f38547e7cd84ce5d83d5 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 6 Jul 2024 20:03:30 +0200 Subject: [PATCH 07/54] Refactor: rename left -> remainingArgs --- src/nix-build/nix-build.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index d0b3b4f9f74..faa9e5fae4f 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -100,7 +100,7 @@ static void main_nix_build(int argc, char * * argv) // Same condition as bash uses for interactive shells auto interactive = isatty(STDIN_FILENO) && isatty(STDERR_FILENO); Strings attrPaths; - Strings left; + Strings remainingArgs; BuildMode buildMode = bmNormal; bool readStdin = false; @@ -246,7 +246,7 @@ static void main_nix_build(int argc, char * * argv) return false; else - left.push_back(*arg); + remainingArgs.push_back(*arg); return true; }); @@ -276,16 +276,16 @@ static void main_nix_build(int argc, char * * argv) if (packages) { std::ostringstream joined; joined << "{...}@args: with import args; (pkgs.runCommandCC or pkgs.runCommand) \"shell\" { buildInputs = [ "; - for (const auto & i : left) + for (const auto & i : remainingArgs) joined << '(' << i << ") "; joined << "]; } \"\""; fromArgs = true; - left = {joined.str()}; + remainingArgs = {joined.str()}; } else if (!fromArgs) { - if (left.empty() && isNixShell && pathExists("shell.nix")) - left = {"shell.nix"}; - if (left.empty()) - left = {"default.nix"}; + if (remainingArgs.empty() && isNixShell && pathExists("shell.nix")) + remainingArgs = {"shell.nix"}; + if (remainingArgs.empty()) + remainingArgs = {"default.nix"}; } if (isNixShell) @@ -299,7 +299,7 @@ static void main_nix_build(int argc, char * * argv) if (readStdin) exprs = {state->parseStdin()}; else - for (auto i : left) { + for (auto i : remainingArgs) { if (fromArgs) exprs.push_back(state->parseExprFromString(std::move(i), state->rootPath("."))); else { From e9479b272faaf00068348e7df8de7f50dce58113 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 6 Jul 2024 20:51:45 +0200 Subject: [PATCH 08/54] nix-build.cc: Refactor: extract baseDir variable --- src/nix-build/nix-build.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index faa9e5fae4f..648917444da 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -310,14 +310,17 @@ static void main_nix_build(int argc, char * * argv) auto [path, outputNames] = parsePathWithOutputs(absolute); if (evalStore->isStorePath(path) && hasSuffix(path, ".drv")) drvs.push_back(PackageInfo(*state, evalStore, absolute)); - else + else { /* If we're in a #! script, interpret filenames relative to the script. */ + auto baseDir = inShebang && !packages ? absPath(i, absPath(dirOf(script))) : i; + exprs.push_back( state->parseExprFromFile( resolveExprPath( lookupFileArg(*state, - inShebang && !packages ? absPath(i, absPath(dirOf(script))) : i)))); + baseDir)))); + } } } From 76245ffbebc8466ac17d241fceda6dcb9ec7c23e Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 6 Jul 2024 20:55:27 +0200 Subject: [PATCH 09/54] nix-build.cc: Refactor: extract sourcePath, resolvedPath variables --- src/nix-build/nix-build.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 648917444da..e873f712bf7 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -315,11 +315,11 @@ static void main_nix_build(int argc, char * * argv) relative to the script. */ auto baseDir = inShebang && !packages ? absPath(i, absPath(dirOf(script))) : i; - exprs.push_back( - state->parseExprFromFile( - resolveExprPath( - lookupFileArg(*state, - baseDir)))); + auto sourcePath = lookupFileArg(*state, + baseDir); + auto resolvedPath = resolveExprPath(sourcePath); + + exprs.push_back(state->parseExprFromFile(resolvedPath)); } } } From 32fb127b9cbaf833027e646de5ee5198a62b6995 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 6 Jul 2024 22:58:15 +0200 Subject: [PATCH 10/54] Add legacy setting: nix-shell-always-looks-for-shell-nix --- src/libcmd/common-eval-args.cc | 7 +++++++ src/libcmd/common-eval-args.hh | 6 ++++++ src/libcmd/meson.build | 1 + src/nix-build/nix-build.cc | 25 +++++++++++++++++++------ tests/functional/nix-shell.sh | 9 +++++++++ 5 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc index 01546f9a0bd..62745b6815f 100644 --- a/src/libcmd/common-eval-args.cc +++ b/src/libcmd/common-eval-args.cc @@ -11,6 +11,8 @@ #include "command.hh" #include "tarball.hh" #include "fetch-to-store.hh" +#include "compatibility-settings.hh" +#include "eval-settings.hh" namespace nix { @@ -33,6 +35,11 @@ EvalSettings evalSettings { static GlobalConfig::Register rEvalSettings(&evalSettings); +CompatibilitySettings compatibilitySettings {}; + +static GlobalConfig::Register rCompatibilitySettings(&compatibilitySettings); + + MixEvalArgs::MixEvalArgs() { addFlag({ diff --git a/src/libcmd/common-eval-args.hh b/src/libcmd/common-eval-args.hh index 189abf0ed52..8d303ee7c13 100644 --- a/src/libcmd/common-eval-args.hh +++ b/src/libcmd/common-eval-args.hh @@ -13,6 +13,7 @@ namespace nix { class Store; class EvalState; struct EvalSettings; +struct CompatibilitySettings; class Bindings; struct SourcePath; @@ -21,6 +22,11 @@ struct SourcePath; */ extern EvalSettings evalSettings; +/** + * Settings that control behaviors that have changed since Nix 2.3. + */ +extern CompatibilitySettings compatibilitySettings; + struct MixEvalArgs : virtual Args, virtual MixRepair { static constexpr auto category = "Common evaluation options"; diff --git a/src/libcmd/meson.build b/src/libcmd/meson.build index d9a90508a05..2c8a9fa3374 100644 --- a/src/libcmd/meson.build +++ b/src/libcmd/meson.build @@ -97,6 +97,7 @@ headers = [config_h] + files( 'command-installable-value.hh', 'command.hh', 'common-eval-args.hh', + 'compatibility-settings.hh', 'editor-for.hh', 'installable-attr-path.hh', 'installable-derived-path.hh', diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 30cc864568d..d37b16bdca0 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -26,6 +26,7 @@ #include "legacy.hh" #include "users.hh" #include "network-proxy.hh" +#include "compatibility-settings.hh" using namespace nix; using namespace std::string_literals; @@ -100,7 +101,13 @@ static SourcePath resolveShellExprPath(SourcePath path) auto resolvedOrDir = resolveExprPath(path, false); if (resolvedOrDir.resolveSymlinks().lstat().type == SourceAccessor::tDirectory) { if ((resolvedOrDir / "shell.nix").pathExists()) { - return resolvedOrDir / "shell.nix"; + if (compatibilitySettings.nixShellAlwaysLooksForShellNix) { + return resolvedOrDir / "shell.nix"; + } else { + warn("Skipping '%1%', because the setting '%2%' is disabled. This is a deprecated behavior. Consider enabling '%2%'.", + resolvedOrDir / "shell.nix", + "nix-shell-always-looks-for-shell-nix"); + } } if ((resolvedOrDir / "default.nix").pathExists()) { return resolvedOrDir / "default.nix"; @@ -302,11 +309,17 @@ static void main_nix_build(int argc, char * * argv) fromArgs = true; remainingArgs = {joined.str()}; } else if (!fromArgs && remainingArgs.empty()) { - remainingArgs = {"."}; - - // Instead of letting it throw later, we throw here to give a more relevant error message - if (isNixShell && !std::filesystem::exists("shell.nix") && !std::filesystem::exists("default.nix")) - throw Error("no argument specified and no '%s' or '%s' file found in the working directory", "shell.nix", "default.nix"); + if (isNixShell && !compatibilitySettings.nixShellAlwaysLooksForShellNix && std::filesystem::exists("shell.nix")) { + // If we're in 2.3 compatibility mode, we need to look for shell.nix + // now, because it won't be done later. + remainingArgs = {"shell.nix"}; + } else { + remainingArgs = {"."}; + + // Instead of letting it throw later, we throw here to give a more relevant error message + if (isNixShell && !std::filesystem::exists("shell.nix") && !std::filesystem::exists("default.nix")) + throw Error("no argument specified and no '%s' or '%s' file found in the working directory", "shell.nix", "default.nix"); + } } if (isNixShell) diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell.sh index b7a7db27ca3..2a1d556ddff 100755 --- a/tests/functional/nix-shell.sh +++ b/tests/functional/nix-shell.sh @@ -21,6 +21,10 @@ output=$(nix-shell --pure "$shellDotNix" -A shellDrv --run \ [ "$output" = " - foo - bar - true" ] +output=$(nix-shell --pure "$shellDotNix" -A shellDrv --option nix-shell-always-looks-for-shell-nix false --run \ + 'echo "$IMPURE_VAR - $VAR_FROM_STDENV_SETUP - $VAR_FROM_NIX - $TEST_inNixShell"') +[ "$output" = " - foo - bar - true" ] + # Test --keep output=$(nix-shell --pure --keep SELECTED_IMPURE_VAR "$shellDotNix" -A shellDrv --run \ 'echo "$IMPURE_VAR - $VAR_FROM_STDENV_SETUP - $VAR_FROM_NIX - $SELECTED_IMPURE_VAR"') @@ -101,6 +105,11 @@ nix-shell $TEST_ROOT/lookup-test -A shellDrv --run 'echo "it works"' | grepQuiet # https://github.com/NixOS/nix/issues/4529 nix-shell -I "testRoot=$TEST_ROOT" '' -A shellDrv --run 'echo "it works"' | grepQuiet "it works" +expectStderr 1 nix-shell $TEST_ROOT/lookup-test -A shellDrv --run 'echo "it works"' --option nix-shell-always-looks-for-shell-nix false \ + | grepQuiet -F "do not load default.nix!" # we did, because we chose to enable legacy behavior +expectStderr 1 nix-shell $TEST_ROOT/lookup-test -A shellDrv --run 'echo "it works"' --option nix-shell-always-looks-for-shell-nix false \ + | grepQuiet "Skipping .*lookup-test/shell\.nix.*, because the setting .*nix-shell-always-looks-for-shell-nix.* is disabled. This is a deprecated behavior\. Consider enabling .*nix-shell-always-looks-for-shell-nix.*" + ( cd $TEST_ROOT/empty; expectStderr 1 nix-shell | \ From a22f8b5276162a87072eee7f0febc0a216f4fa9b Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 6 Jul 2024 23:02:32 +0200 Subject: [PATCH 11/54] rl-next: Add note about shell.nix lookups --- .../rl-next/nix-shell-looks-for-shell-nix.md | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 doc/manual/rl-next/nix-shell-looks-for-shell-nix.md diff --git a/doc/manual/rl-next/nix-shell-looks-for-shell-nix.md b/doc/manual/rl-next/nix-shell-looks-for-shell-nix.md new file mode 100644 index 00000000000..1f44ba33c9a --- /dev/null +++ b/doc/manual/rl-next/nix-shell-looks-for-shell-nix.md @@ -0,0 +1,26 @@ +--- +synopsis: "`nix-shell ` looks for `shell.nix`" +significance: significant +issues: +- 496 +- 2279 +- 4529 +- 5431 +- 11053 +--- + +`nix-shell $x` now looks for `$x/shell.nix` when `$x` resolves to a directory. + +Although this might be seen as a breaking change, its primarily interactive usage makes it a minor issue. +This adjustment addresses a commonly reported problem. + +This also applies to `nix-shell` shebang scripts. Consider the following example: + +```shell +#!/usr/bin/env nix-shell +#!nix-shell -i bash +``` + +This will now load `shell.nix` from the script's directory, if it exists; `default.nix` otherwise. + +The old behavior can be opted into by setting the option [`nix-shell-always-looks-for-shell-nix`](@docroot@/command-ref/conf-file.md#conf-nix-shell-always-looks-for-shell-nix) to `false`. From b865625a8eab8382e1643605594e9db23271c2e5 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 6 Jul 2024 21:31:24 +0200 Subject: [PATCH 12/54] nix-shell: Look for shell.nix when directory is specified --- src/libexpr/eval.cc | 4 ++-- src/libexpr/eval.hh | 4 +++- src/nix-build/nix-build.cc | 34 ++++++++++++++++++++++----- tests/functional/nix-shell.sh | 44 +++++++++++++++++++++++++++++++++++ tests/functional/shell.nix | 11 +++++++++ 5 files changed, 88 insertions(+), 9 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 48ed66883b8..2a08621231f 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2650,7 +2650,7 @@ void EvalState::printStatistics() } -SourcePath resolveExprPath(SourcePath path) +SourcePath resolveExprPath(SourcePath path, bool addDefaultNix) { unsigned int followCount = 0, maxFollow = 1024; @@ -2666,7 +2666,7 @@ SourcePath resolveExprPath(SourcePath path) } /* If `path' refers to a directory, append `/default.nix'. */ - if (path.resolveSymlinks().lstat().type == SourceAccessor::tDirectory) + if (addDefaultNix && path.resolveSymlinks().lstat().type == SourceAccessor::tDirectory) return path / "default.nix"; return path; diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index b84bc9907c8..e45358055ed 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -850,8 +850,10 @@ std::string showType(const Value & v); /** * If `path` refers to a directory, then append "/default.nix". + * + * @param addDefaultNix Whether to append "/default.nix" after resolving symlinks. */ -SourcePath resolveExprPath(SourcePath path); +SourcePath resolveExprPath(SourcePath path, bool addDefaultNix = true); /** * Whether a URI is allowed, assuming restrictEval is enabled diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index e873f712bf7..30cc864568d 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -90,6 +90,26 @@ static std::vector shellwords(const std::string & s) return res; } +/** + * Like `resolveExprPath`, but prefers `shell.nix` instead of `default.nix`, + * and if `path` was a directory, it checks eagerly whether `shell.nix` or + * `default.nix` exist, throwing an error if they don't. + */ +static SourcePath resolveShellExprPath(SourcePath path) +{ + auto resolvedOrDir = resolveExprPath(path, false); + if (resolvedOrDir.resolveSymlinks().lstat().type == SourceAccessor::tDirectory) { + if ((resolvedOrDir / "shell.nix").pathExists()) { + return resolvedOrDir / "shell.nix"; + } + if ((resolvedOrDir / "default.nix").pathExists()) { + return resolvedOrDir / "default.nix"; + } + throw Error("neither '%s' nor '%s' found in '%s'", "shell.nix", "default.nix", resolvedOrDir); + } + return resolvedOrDir; +} + static void main_nix_build(int argc, char * * argv) { auto dryRun = false; @@ -281,11 +301,12 @@ static void main_nix_build(int argc, char * * argv) joined << "]; } \"\""; fromArgs = true; remainingArgs = {joined.str()}; - } else if (!fromArgs) { - if (remainingArgs.empty() && isNixShell && pathExists("shell.nix")) - remainingArgs = {"shell.nix"}; - if (remainingArgs.empty()) - remainingArgs = {"default.nix"}; + } else if (!fromArgs && remainingArgs.empty()) { + remainingArgs = {"."}; + + // Instead of letting it throw later, we throw here to give a more relevant error message + if (isNixShell && !std::filesystem::exists("shell.nix") && !std::filesystem::exists("default.nix")) + throw Error("no argument specified and no '%s' or '%s' file found in the working directory", "shell.nix", "default.nix"); } if (isNixShell) @@ -317,7 +338,8 @@ static void main_nix_build(int argc, char * * argv) auto sourcePath = lookupFileArg(*state, baseDir); - auto resolvedPath = resolveExprPath(sourcePath); + auto resolvedPath = + isNixShell ? resolveShellExprPath(sourcePath) : resolveExprPath(sourcePath); exprs.push_back(state->parseExprFromFile(resolvedPath)); } diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell.sh index 2c94705dec0..b7a7db27ca3 100755 --- a/tests/functional/nix-shell.sh +++ b/tests/functional/nix-shell.sh @@ -91,6 +91,50 @@ sed -e "s|@ENV_PROG@|$(type -P env)|" shell.shebang.nix > $TEST_ROOT/shell.sheba chmod a+rx $TEST_ROOT/shell.shebang.nix $TEST_ROOT/shell.shebang.nix +mkdir $TEST_ROOT/lookup-test $TEST_ROOT/empty + +cp $shellDotNix $TEST_ROOT/lookup-test/shell.nix +cp config.nix $TEST_ROOT/lookup-test/ +echo 'abort "do not load default.nix!"' > $TEST_ROOT/lookup-test/default.nix + +nix-shell $TEST_ROOT/lookup-test -A shellDrv --run 'echo "it works"' | grepQuiet "it works" +# https://github.com/NixOS/nix/issues/4529 +nix-shell -I "testRoot=$TEST_ROOT" '' -A shellDrv --run 'echo "it works"' | grepQuiet "it works" + +( + cd $TEST_ROOT/empty; + expectStderr 1 nix-shell | \ + grepQuiet "error.*no argument specified and no .*shell\.nix.* or .*default\.nix.* file found in the working directory" +) + +expectStderr 1 nix-shell -I "testRoot=$TEST_ROOT" '' | + grepQuiet "error.*neither .*shell\.nix.* nor .*default\.nix.* found in .*/empty" + +cat >$TEST_ROOT/lookup-test/shebangscript <<"EOF" +#!/usr/bin/env nix-shell +#!nix-shell -A shellDrv -i bash +[[ $VAR_FROM_NIX == bar ]] +echo "script works" +EOF +chmod +x $TEST_ROOT/lookup-test/shebangscript + +$TEST_ROOT/lookup-test/shebangscript | grepQuiet "script works" + +# https://github.com/NixOS/nix/issues/5431 +mkdir $TEST_ROOT/marco{,/polo} +echo 'abort "marco/shell.nix must not be used, but its mere existence used to cause #5431"' > $TEST_ROOT/marco/shell.nix +cat >$TEST_ROOT/marco/polo/default.nix < Date: Sat, 6 Jul 2024 23:15:01 +0200 Subject: [PATCH 13/54] rl-next: Enter PR --- doc/manual/rl-next/nix-shell-looks-for-shell-nix.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/manual/rl-next/nix-shell-looks-for-shell-nix.md b/doc/manual/rl-next/nix-shell-looks-for-shell-nix.md index 1f44ba33c9a..99be4148bf1 100644 --- a/doc/manual/rl-next/nix-shell-looks-for-shell-nix.md +++ b/doc/manual/rl-next/nix-shell-looks-for-shell-nix.md @@ -7,6 +7,8 @@ issues: - 4529 - 5431 - 11053 +prs: +- 11057 --- `nix-shell $x` now looks for `$x/shell.nix` when `$x` resolves to a directory. From d5854f33e2872b583f00d35321405c022858b9ce Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 7 Jul 2024 00:18:26 +0200 Subject: [PATCH 14/54] rl-next: Typo --- doc/manual/rl-next/shebang-relative.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/rl-next/shebang-relative.md b/doc/manual/rl-next/shebang-relative.md index dbda0db4c88..e6ab9346fe4 100644 --- a/doc/manual/rl-next/shebang-relative.md +++ b/doc/manual/rl-next/shebang-relative.md @@ -3,6 +3,6 @@ prs: #5088 description: { `nix-shell` shebangs use the script file's relative location to resolve relative paths to files passed as command line arguments, but expression arguments were still evaluated using the current working directory as a base path. -The new behavior is that evalutations are performed relative to the script. +The new behavior is that evaluations are performed relative to the script. } From f5b59fbc6478ab944e75213e5ec711c0066ad43d Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 7 Jul 2024 00:22:21 +0200 Subject: [PATCH 15/54] Fix and extend nix-shell baseDir test --- src/libcmd/common-eval-args.cc | 2 +- src/libutil/args/root.hh | 1 + src/nix-build/nix-build.cc | 6 ++++++ tests/functional/nix-shell.sh | 3 ++- tests/functional/shell.nix | 1 + tests/functional/shell.shebang.expr | 9 +++++++++ 6 files changed, 20 insertions(+), 2 deletions(-) create mode 100755 tests/functional/shell.shebang.expr diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc index 62745b6815f..ffc1ebd59da 100644 --- a/src/libcmd/common-eval-args.cc +++ b/src/libcmd/common-eval-args.cc @@ -202,7 +202,7 @@ Bindings * MixEvalArgs::getAutoArgs(EvalState & state) auto v = state.allocValue(); std::visit(overloaded { [&](const AutoArgExpr & arg) { - state.mkThunk_(*v, state.parseExprFromString(arg.expr, state.rootPath("."))); + state.mkThunk_(*v, state.parseExprFromString(arg.expr, true ? state.rootPath(absPath(getCommandBaseDir())) : state.rootPath("."))); }, [&](const AutoArgString & arg) { v->mkString(arg.s); diff --git a/src/libutil/args/root.hh b/src/libutil/args/root.hh index 5c55c37a55d..34a43b53835 100644 --- a/src/libutil/args/root.hh +++ b/src/libutil/args/root.hh @@ -29,6 +29,7 @@ struct Completions final : AddCompletions */ class RootArgs : virtual public Args { +protected: /** * @brief The command's "working directory", but only set when top level. * diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 8722950457b..cfe183888ab 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -183,6 +183,9 @@ static void main_nix_build(int argc, char * * argv) struct MyArgs : LegacyArgs, MixEvalArgs { using LegacyArgs::LegacyArgs; + void setBaseDir(Path baseDir) { + commandBaseDir = baseDir; + } }; MyArgs myArgs(myName, [&](Strings::iterator & arg, const Strings::iterator & end) { @@ -290,6 +293,9 @@ static void main_nix_build(int argc, char * * argv) state->repair = myArgs.repair; if (myArgs.repair) buildMode = bmRepair; + if (inShebang) { + myArgs.setBaseDir(absPath(dirOf(script))); + } auto autoArgs = myArgs.getAutoArgs(*state); auto autoArgsWithInNixShell = autoArgs; diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell.sh index f881acd0322..596ac595166 100755 --- a/tests/functional/nix-shell.sh +++ b/tests/functional/nix-shell.sh @@ -72,8 +72,9 @@ chmod a+rx $TEST_ROOT/shell.shebang.expr ! $TEST_ROOT/shell.shebang.expr bar cp shell.nix config.nix $TEST_ROOT # Should succeed +echo "cwd: $PWD" output=$($TEST_ROOT/shell.shebang.expr bar) -[ "$output" = '-e load(ARGV.shift) -- '"$TEST_ROOT"'/shell.shebang.expr bar' ] +[ "$output" = foo ] # Test nix-shell shebang mode again with metacharacters in the filename. # First word of filename is chosen to not match any file in the test root. diff --git a/tests/functional/shell.nix b/tests/functional/shell.nix index 75e3845ea54..a7577ff6320 100644 --- a/tests/functional/shell.nix +++ b/tests/functional/shell.nix @@ -43,6 +43,7 @@ let pkgs = rec { ASCII_PERCENT = "%"; ASCII_AT = "@"; TEST_inNixShell = if inNixShell then "true" else "false"; + FOO = fooContents; inherit stdenv; outputs = ["dev" "out"]; } // { diff --git a/tests/functional/shell.shebang.expr b/tests/functional/shell.shebang.expr new file mode 100755 index 00000000000..c602dedbf03 --- /dev/null +++ b/tests/functional/shell.shebang.expr @@ -0,0 +1,9 @@ +#! @ENV_PROG@ nix-shell +#! nix-shell "{ script, path, ... }: assert path == ./shell.nix; script { }" +#! nix-shell --no-substitute +#! nix-shell --expr +#! nix-shell --arg script "import ./shell.nix" +#! nix-shell --arg path "./shell.nix" +#! nix-shell -A shellDrv +#! nix-shell -i bash +echo "$FOO" From 6c6d5263e26bc463aee97e49a5e9b8d867a4731a Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 6 Jul 2024 22:58:15 +0200 Subject: [PATCH 16/54] Add legacy setting: nix-shell-always-looks-for-shell-nix --- src/libcmd/common-eval-args.cc | 7 +++++++ src/libcmd/common-eval-args.hh | 6 ++++++ src/libcmd/compatibility-settings.hh | 19 +++++++++++++++++++ src/libcmd/meson.build | 1 + src/nix-build/nix-build.cc | 25 +++++++++++++++++++------ tests/functional/nix-shell.sh | 9 +++++++++ 6 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 src/libcmd/compatibility-settings.hh diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc index 01546f9a0bd..62745b6815f 100644 --- a/src/libcmd/common-eval-args.cc +++ b/src/libcmd/common-eval-args.cc @@ -11,6 +11,8 @@ #include "command.hh" #include "tarball.hh" #include "fetch-to-store.hh" +#include "compatibility-settings.hh" +#include "eval-settings.hh" namespace nix { @@ -33,6 +35,11 @@ EvalSettings evalSettings { static GlobalConfig::Register rEvalSettings(&evalSettings); +CompatibilitySettings compatibilitySettings {}; + +static GlobalConfig::Register rCompatibilitySettings(&compatibilitySettings); + + MixEvalArgs::MixEvalArgs() { addFlag({ diff --git a/src/libcmd/common-eval-args.hh b/src/libcmd/common-eval-args.hh index 189abf0ed52..8d303ee7c13 100644 --- a/src/libcmd/common-eval-args.hh +++ b/src/libcmd/common-eval-args.hh @@ -13,6 +13,7 @@ namespace nix { class Store; class EvalState; struct EvalSettings; +struct CompatibilitySettings; class Bindings; struct SourcePath; @@ -21,6 +22,11 @@ struct SourcePath; */ extern EvalSettings evalSettings; +/** + * Settings that control behaviors that have changed since Nix 2.3. + */ +extern CompatibilitySettings compatibilitySettings; + struct MixEvalArgs : virtual Args, virtual MixRepair { static constexpr auto category = "Common evaluation options"; diff --git a/src/libcmd/compatibility-settings.hh b/src/libcmd/compatibility-settings.hh new file mode 100644 index 00000000000..5dc0eaf2b38 --- /dev/null +++ b/src/libcmd/compatibility-settings.hh @@ -0,0 +1,19 @@ +#pragma once +#include "config.hh" + +namespace nix { +struct CompatibilitySettings : public Config +{ + + CompatibilitySettings() = default; + + Setting nixShellAlwaysLooksForShellNix{this, true, "nix-shell-always-looks-for-shell-nix", R"( + Before Nix 2.24, [`nix-shell`](@docroot@/command-ref/nix-shell.md) would only look at `shell.nix` if it was in the working directory - when no file was specified. + + Since Nix 2.24, `nix-shell` always looks for a `shell.nix`, whether that's in the working directory, or in a directory that was passed as an argument. + + You may set this to `false` to revert to the Nix 2.3 behavior. + )"}; +}; + +}; diff --git a/src/libcmd/meson.build b/src/libcmd/meson.build index d9a90508a05..2c8a9fa3374 100644 --- a/src/libcmd/meson.build +++ b/src/libcmd/meson.build @@ -97,6 +97,7 @@ headers = [config_h] + files( 'command-installable-value.hh', 'command.hh', 'common-eval-args.hh', + 'compatibility-settings.hh', 'editor-for.hh', 'installable-attr-path.hh', 'installable-derived-path.hh', diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 30cc864568d..d37b16bdca0 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -26,6 +26,7 @@ #include "legacy.hh" #include "users.hh" #include "network-proxy.hh" +#include "compatibility-settings.hh" using namespace nix; using namespace std::string_literals; @@ -100,7 +101,13 @@ static SourcePath resolveShellExprPath(SourcePath path) auto resolvedOrDir = resolveExprPath(path, false); if (resolvedOrDir.resolveSymlinks().lstat().type == SourceAccessor::tDirectory) { if ((resolvedOrDir / "shell.nix").pathExists()) { - return resolvedOrDir / "shell.nix"; + if (compatibilitySettings.nixShellAlwaysLooksForShellNix) { + return resolvedOrDir / "shell.nix"; + } else { + warn("Skipping '%1%', because the setting '%2%' is disabled. This is a deprecated behavior. Consider enabling '%2%'.", + resolvedOrDir / "shell.nix", + "nix-shell-always-looks-for-shell-nix"); + } } if ((resolvedOrDir / "default.nix").pathExists()) { return resolvedOrDir / "default.nix"; @@ -302,11 +309,17 @@ static void main_nix_build(int argc, char * * argv) fromArgs = true; remainingArgs = {joined.str()}; } else if (!fromArgs && remainingArgs.empty()) { - remainingArgs = {"."}; - - // Instead of letting it throw later, we throw here to give a more relevant error message - if (isNixShell && !std::filesystem::exists("shell.nix") && !std::filesystem::exists("default.nix")) - throw Error("no argument specified and no '%s' or '%s' file found in the working directory", "shell.nix", "default.nix"); + if (isNixShell && !compatibilitySettings.nixShellAlwaysLooksForShellNix && std::filesystem::exists("shell.nix")) { + // If we're in 2.3 compatibility mode, we need to look for shell.nix + // now, because it won't be done later. + remainingArgs = {"shell.nix"}; + } else { + remainingArgs = {"."}; + + // Instead of letting it throw later, we throw here to give a more relevant error message + if (isNixShell && !std::filesystem::exists("shell.nix") && !std::filesystem::exists("default.nix")) + throw Error("no argument specified and no '%s' or '%s' file found in the working directory", "shell.nix", "default.nix"); + } } if (isNixShell) diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell.sh index b7a7db27ca3..2a1d556ddff 100755 --- a/tests/functional/nix-shell.sh +++ b/tests/functional/nix-shell.sh @@ -21,6 +21,10 @@ output=$(nix-shell --pure "$shellDotNix" -A shellDrv --run \ [ "$output" = " - foo - bar - true" ] +output=$(nix-shell --pure "$shellDotNix" -A shellDrv --option nix-shell-always-looks-for-shell-nix false --run \ + 'echo "$IMPURE_VAR - $VAR_FROM_STDENV_SETUP - $VAR_FROM_NIX - $TEST_inNixShell"') +[ "$output" = " - foo - bar - true" ] + # Test --keep output=$(nix-shell --pure --keep SELECTED_IMPURE_VAR "$shellDotNix" -A shellDrv --run \ 'echo "$IMPURE_VAR - $VAR_FROM_STDENV_SETUP - $VAR_FROM_NIX - $SELECTED_IMPURE_VAR"') @@ -101,6 +105,11 @@ nix-shell $TEST_ROOT/lookup-test -A shellDrv --run 'echo "it works"' | grepQuiet # https://github.com/NixOS/nix/issues/4529 nix-shell -I "testRoot=$TEST_ROOT" '' -A shellDrv --run 'echo "it works"' | grepQuiet "it works" +expectStderr 1 nix-shell $TEST_ROOT/lookup-test -A shellDrv --run 'echo "it works"' --option nix-shell-always-looks-for-shell-nix false \ + | grepQuiet -F "do not load default.nix!" # we did, because we chose to enable legacy behavior +expectStderr 1 nix-shell $TEST_ROOT/lookup-test -A shellDrv --run 'echo "it works"' --option nix-shell-always-looks-for-shell-nix false \ + | grepQuiet "Skipping .*lookup-test/shell\.nix.*, because the setting .*nix-shell-always-looks-for-shell-nix.* is disabled. This is a deprecated behavior\. Consider enabling .*nix-shell-always-looks-for-shell-nix.*" + ( cd $TEST_ROOT/empty; expectStderr 1 nix-shell | \ From 6959ac157bf4e9ff8cbd30033cf8de07f5849ab7 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 6 Jul 2024 23:02:32 +0200 Subject: [PATCH 17/54] rl-next: Add note about shell.nix lookups --- .../rl-next/nix-shell-looks-for-shell-nix.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 doc/manual/rl-next/nix-shell-looks-for-shell-nix.md diff --git a/doc/manual/rl-next/nix-shell-looks-for-shell-nix.md b/doc/manual/rl-next/nix-shell-looks-for-shell-nix.md new file mode 100644 index 00000000000..99be4148bf1 --- /dev/null +++ b/doc/manual/rl-next/nix-shell-looks-for-shell-nix.md @@ -0,0 +1,28 @@ +--- +synopsis: "`nix-shell ` looks for `shell.nix`" +significance: significant +issues: +- 496 +- 2279 +- 4529 +- 5431 +- 11053 +prs: +- 11057 +--- + +`nix-shell $x` now looks for `$x/shell.nix` when `$x` resolves to a directory. + +Although this might be seen as a breaking change, its primarily interactive usage makes it a minor issue. +This adjustment addresses a commonly reported problem. + +This also applies to `nix-shell` shebang scripts. Consider the following example: + +```shell +#!/usr/bin/env nix-shell +#!nix-shell -i bash +``` + +This will now load `shell.nix` from the script's directory, if it exists; `default.nix` otherwise. + +The old behavior can be opted into by setting the option [`nix-shell-always-looks-for-shell-nix`](@docroot@/command-ref/conf-file.md#conf-nix-shell-always-looks-for-shell-nix) to `false`. From 63262e78c7fd281b813e858640adbaa6f6b3d826 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 7 Jul 2024 00:55:33 +0200 Subject: [PATCH 18/54] Add opt-out: nix-shell-shebang-arguments-relative-to-script --- doc/manual/rl-next/shebang-relative.md | 2 ++ src/libcmd/common-eval-args.cc | 2 +- src/libcmd/compatibility-settings.hh | 9 +++++++++ src/nix-build/nix-build.cc | 4 ++-- tests/functional/nix-shell.sh | 8 ++++++++ 5 files changed, 22 insertions(+), 3 deletions(-) diff --git a/doc/manual/rl-next/shebang-relative.md b/doc/manual/rl-next/shebang-relative.md index e6ab9346fe4..ab39a359c79 100644 --- a/doc/manual/rl-next/shebang-relative.md +++ b/doc/manual/rl-next/shebang-relative.md @@ -5,4 +5,6 @@ description: { `nix-shell` shebangs use the script file's relative location to resolve relative paths to files passed as command line arguments, but expression arguments were still evaluated using the current working directory as a base path. The new behavior is that evaluations are performed relative to the script. +The old behavior can be opted into by setting the option [`nix-shell-shebang-arguments-relative-to-script`](@docroot@/command-ref/conf-file.md#conf-nix-shell-shebang-arguments-relative-to-script) to `false`. + } diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc index ffc1ebd59da..a243b8c49ad 100644 --- a/src/libcmd/common-eval-args.cc +++ b/src/libcmd/common-eval-args.cc @@ -202,7 +202,7 @@ Bindings * MixEvalArgs::getAutoArgs(EvalState & state) auto v = state.allocValue(); std::visit(overloaded { [&](const AutoArgExpr & arg) { - state.mkThunk_(*v, state.parseExprFromString(arg.expr, true ? state.rootPath(absPath(getCommandBaseDir())) : state.rootPath("."))); + state.mkThunk_(*v, state.parseExprFromString(arg.expr, compatibilitySettings.nixShellShebangArgumentsRelativeToScript ? state.rootPath(absPath(getCommandBaseDir())) : state.rootPath("."))); }, [&](const AutoArgString & arg) { v->mkString(arg.s); diff --git a/src/libcmd/compatibility-settings.hh b/src/libcmd/compatibility-settings.hh index 5dc0eaf2b38..961001080db 100644 --- a/src/libcmd/compatibility-settings.hh +++ b/src/libcmd/compatibility-settings.hh @@ -14,6 +14,15 @@ struct CompatibilitySettings : public Config You may set this to `false` to revert to the Nix 2.3 behavior. )"}; + + Setting nixShellShebangArgumentsRelativeToScript{ + this, true, "nix-shell-shebang-arguments-relative-to-script", R"( + Before Nix 2.24, the arguments in a `nix-shell` shebang - as well as `--arg` - were relative to working directory. + + Since Nix 2.24, the arguments are relative to the [base directory](@docroot@/glossary.md#gloss-base-directory) defined as the script's directory. + + You may set this to `false` to revert to the Nix 2.3 behavior. + )"}; }; }; diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index cfe183888ab..f4af3fd040a 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -293,7 +293,7 @@ static void main_nix_build(int argc, char * * argv) state->repair = myArgs.repair; if (myArgs.repair) buildMode = bmRepair; - if (inShebang) { + if (inShebang && compatibilitySettings.nixShellShebangArgumentsRelativeToScript) { myArgs.setBaseDir(absPath(dirOf(script))); } auto autoArgs = myArgs.getAutoArgs(*state); @@ -345,7 +345,7 @@ static void main_nix_build(int argc, char * * argv) if (fromArgs) exprs.push_back(state->parseExprFromString( std::move(i), - inShebang ? lookupFileArg(*state, baseDir) : state->rootPath(".") + (inShebang && compatibilitySettings.nixShellShebangArgumentsRelativeToScript) ? lookupFileArg(*state, baseDir) : state->rootPath(".") )); else { auto absolute = i; diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell.sh index 596ac595166..fd3edf81ae3 100755 --- a/tests/functional/nix-shell.sh +++ b/tests/functional/nix-shell.sh @@ -76,6 +76,14 @@ echo "cwd: $PWD" output=$($TEST_ROOT/shell.shebang.expr bar) [ "$output" = foo ] +# Test nix-shell shebang mode with an alternate working directory +sed -e "s|@ENV_PROG@|$(type -P env)|" shell.shebang.legacy.expr > $TEST_ROOT/shell.shebang.legacy.expr +chmod a+rx $TEST_ROOT/shell.shebang.legacy.expr +# Should fail due to expressions using relative path +mkdir -p "$TEST_ROOT/somewhere-unrelated" +output="$(cd "$TEST_ROOT/somewhere-unrelated"; $TEST_ROOT/shell.shebang.legacy.expr bar;)" +[[ $(realpath "$output") = $(realpath "$TEST_ROOT/somewhere-unrelated") ]] + # Test nix-shell shebang mode again with metacharacters in the filename. # First word of filename is chosen to not match any file in the test root. sed -e "s|@ENV_PROG@|$(type -P env)|" shell.shebang.sh > $TEST_ROOT/spaced\ \\\'\"shell.shebang.sh From 73602a7c6f4dc6f4f4bea8368a8564403b7b5604 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 6 Jul 2024 21:31:24 +0200 Subject: [PATCH 19/54] nix-shell: Look for shell.nix when directory is specified --- src/libexpr/eval.cc | 4 ++-- src/libexpr/eval.hh | 4 +++- src/nix-build/nix-build.cc | 34 ++++++++++++++++++++++----- tests/functional/nix-shell.sh | 44 +++++++++++++++++++++++++++++++++++ tests/functional/shell.nix | 11 +++++++++ 5 files changed, 88 insertions(+), 9 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 48ed66883b8..2a08621231f 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2650,7 +2650,7 @@ void EvalState::printStatistics() } -SourcePath resolveExprPath(SourcePath path) +SourcePath resolveExprPath(SourcePath path, bool addDefaultNix) { unsigned int followCount = 0, maxFollow = 1024; @@ -2666,7 +2666,7 @@ SourcePath resolveExprPath(SourcePath path) } /* If `path' refers to a directory, append `/default.nix'. */ - if (path.resolveSymlinks().lstat().type == SourceAccessor::tDirectory) + if (addDefaultNix && path.resolveSymlinks().lstat().type == SourceAccessor::tDirectory) return path / "default.nix"; return path; diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index b84bc9907c8..e45358055ed 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -850,8 +850,10 @@ std::string showType(const Value & v); /** * If `path` refers to a directory, then append "/default.nix". + * + * @param addDefaultNix Whether to append "/default.nix" after resolving symlinks. */ -SourcePath resolveExprPath(SourcePath path); +SourcePath resolveExprPath(SourcePath path, bool addDefaultNix = true); /** * Whether a URI is allowed, assuming restrictEval is enabled diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index e873f712bf7..30cc864568d 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -90,6 +90,26 @@ static std::vector shellwords(const std::string & s) return res; } +/** + * Like `resolveExprPath`, but prefers `shell.nix` instead of `default.nix`, + * and if `path` was a directory, it checks eagerly whether `shell.nix` or + * `default.nix` exist, throwing an error if they don't. + */ +static SourcePath resolveShellExprPath(SourcePath path) +{ + auto resolvedOrDir = resolveExprPath(path, false); + if (resolvedOrDir.resolveSymlinks().lstat().type == SourceAccessor::tDirectory) { + if ((resolvedOrDir / "shell.nix").pathExists()) { + return resolvedOrDir / "shell.nix"; + } + if ((resolvedOrDir / "default.nix").pathExists()) { + return resolvedOrDir / "default.nix"; + } + throw Error("neither '%s' nor '%s' found in '%s'", "shell.nix", "default.nix", resolvedOrDir); + } + return resolvedOrDir; +} + static void main_nix_build(int argc, char * * argv) { auto dryRun = false; @@ -281,11 +301,12 @@ static void main_nix_build(int argc, char * * argv) joined << "]; } \"\""; fromArgs = true; remainingArgs = {joined.str()}; - } else if (!fromArgs) { - if (remainingArgs.empty() && isNixShell && pathExists("shell.nix")) - remainingArgs = {"shell.nix"}; - if (remainingArgs.empty()) - remainingArgs = {"default.nix"}; + } else if (!fromArgs && remainingArgs.empty()) { + remainingArgs = {"."}; + + // Instead of letting it throw later, we throw here to give a more relevant error message + if (isNixShell && !std::filesystem::exists("shell.nix") && !std::filesystem::exists("default.nix")) + throw Error("no argument specified and no '%s' or '%s' file found in the working directory", "shell.nix", "default.nix"); } if (isNixShell) @@ -317,7 +338,8 @@ static void main_nix_build(int argc, char * * argv) auto sourcePath = lookupFileArg(*state, baseDir); - auto resolvedPath = resolveExprPath(sourcePath); + auto resolvedPath = + isNixShell ? resolveShellExprPath(sourcePath) : resolveExprPath(sourcePath); exprs.push_back(state->parseExprFromFile(resolvedPath)); } diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell.sh index 2c94705dec0..f54e3621cc8 100755 --- a/tests/functional/nix-shell.sh +++ b/tests/functional/nix-shell.sh @@ -91,6 +91,50 @@ sed -e "s|@ENV_PROG@|$(type -P env)|" shell.shebang.nix > $TEST_ROOT/shell.sheba chmod a+rx $TEST_ROOT/shell.shebang.nix $TEST_ROOT/shell.shebang.nix +mkdir $TEST_ROOT/lookup-test $TEST_ROOT/empty + +echo "import $shellDotNix" > $TEST_ROOT/lookup-test/shell.nix +cp config.nix $TEST_ROOT/lookup-test/ +echo 'abort "do not load default.nix!"' > $TEST_ROOT/lookup-test/default.nix + +nix-shell $TEST_ROOT/lookup-test -A shellDrv --run 'echo "it works"' | grepQuiet "it works" +# https://github.com/NixOS/nix/issues/4529 +nix-shell -I "testRoot=$TEST_ROOT" '' -A shellDrv --run 'echo "it works"' | grepQuiet "it works" + +( + cd $TEST_ROOT/empty; + expectStderr 1 nix-shell | \ + grepQuiet "error.*no argument specified and no .*shell\.nix.* or .*default\.nix.* file found in the working directory" +) + +expectStderr 1 nix-shell -I "testRoot=$TEST_ROOT" '' | + grepQuiet "error.*neither .*shell\.nix.* nor .*default\.nix.* found in .*/empty" + +cat >$TEST_ROOT/lookup-test/shebangscript < $TEST_ROOT/marco/shell.nix +cat >$TEST_ROOT/marco/polo/default.nix < Date: Sat, 6 Jul 2024 22:58:15 +0200 Subject: [PATCH 20/54] Add legacy setting: nix-shell-always-looks-for-shell-nix --- src/libcmd/common-eval-args.cc | 7 +++++++ src/libcmd/common-eval-args.hh | 6 ++++++ src/libcmd/compatibility-settings.hh | 19 +++++++++++++++++++ src/libcmd/meson.build | 1 + src/nix-build/nix-build.cc | 25 +++++++++++++++++++------ tests/functional/nix-shell.sh | 9 +++++++++ 6 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 src/libcmd/compatibility-settings.hh diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc index 01546f9a0bd..62745b6815f 100644 --- a/src/libcmd/common-eval-args.cc +++ b/src/libcmd/common-eval-args.cc @@ -11,6 +11,8 @@ #include "command.hh" #include "tarball.hh" #include "fetch-to-store.hh" +#include "compatibility-settings.hh" +#include "eval-settings.hh" namespace nix { @@ -33,6 +35,11 @@ EvalSettings evalSettings { static GlobalConfig::Register rEvalSettings(&evalSettings); +CompatibilitySettings compatibilitySettings {}; + +static GlobalConfig::Register rCompatibilitySettings(&compatibilitySettings); + + MixEvalArgs::MixEvalArgs() { addFlag({ diff --git a/src/libcmd/common-eval-args.hh b/src/libcmd/common-eval-args.hh index 189abf0ed52..8d303ee7c13 100644 --- a/src/libcmd/common-eval-args.hh +++ b/src/libcmd/common-eval-args.hh @@ -13,6 +13,7 @@ namespace nix { class Store; class EvalState; struct EvalSettings; +struct CompatibilitySettings; class Bindings; struct SourcePath; @@ -21,6 +22,11 @@ struct SourcePath; */ extern EvalSettings evalSettings; +/** + * Settings that control behaviors that have changed since Nix 2.3. + */ +extern CompatibilitySettings compatibilitySettings; + struct MixEvalArgs : virtual Args, virtual MixRepair { static constexpr auto category = "Common evaluation options"; diff --git a/src/libcmd/compatibility-settings.hh b/src/libcmd/compatibility-settings.hh new file mode 100644 index 00000000000..5dc0eaf2b38 --- /dev/null +++ b/src/libcmd/compatibility-settings.hh @@ -0,0 +1,19 @@ +#pragma once +#include "config.hh" + +namespace nix { +struct CompatibilitySettings : public Config +{ + + CompatibilitySettings() = default; + + Setting nixShellAlwaysLooksForShellNix{this, true, "nix-shell-always-looks-for-shell-nix", R"( + Before Nix 2.24, [`nix-shell`](@docroot@/command-ref/nix-shell.md) would only look at `shell.nix` if it was in the working directory - when no file was specified. + + Since Nix 2.24, `nix-shell` always looks for a `shell.nix`, whether that's in the working directory, or in a directory that was passed as an argument. + + You may set this to `false` to revert to the Nix 2.3 behavior. + )"}; +}; + +}; diff --git a/src/libcmd/meson.build b/src/libcmd/meson.build index d9a90508a05..2c8a9fa3374 100644 --- a/src/libcmd/meson.build +++ b/src/libcmd/meson.build @@ -97,6 +97,7 @@ headers = [config_h] + files( 'command-installable-value.hh', 'command.hh', 'common-eval-args.hh', + 'compatibility-settings.hh', 'editor-for.hh', 'installable-attr-path.hh', 'installable-derived-path.hh', diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 30cc864568d..d37b16bdca0 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -26,6 +26,7 @@ #include "legacy.hh" #include "users.hh" #include "network-proxy.hh" +#include "compatibility-settings.hh" using namespace nix; using namespace std::string_literals; @@ -100,7 +101,13 @@ static SourcePath resolveShellExprPath(SourcePath path) auto resolvedOrDir = resolveExprPath(path, false); if (resolvedOrDir.resolveSymlinks().lstat().type == SourceAccessor::tDirectory) { if ((resolvedOrDir / "shell.nix").pathExists()) { - return resolvedOrDir / "shell.nix"; + if (compatibilitySettings.nixShellAlwaysLooksForShellNix) { + return resolvedOrDir / "shell.nix"; + } else { + warn("Skipping '%1%', because the setting '%2%' is disabled. This is a deprecated behavior. Consider enabling '%2%'.", + resolvedOrDir / "shell.nix", + "nix-shell-always-looks-for-shell-nix"); + } } if ((resolvedOrDir / "default.nix").pathExists()) { return resolvedOrDir / "default.nix"; @@ -302,11 +309,17 @@ static void main_nix_build(int argc, char * * argv) fromArgs = true; remainingArgs = {joined.str()}; } else if (!fromArgs && remainingArgs.empty()) { - remainingArgs = {"."}; - - // Instead of letting it throw later, we throw here to give a more relevant error message - if (isNixShell && !std::filesystem::exists("shell.nix") && !std::filesystem::exists("default.nix")) - throw Error("no argument specified and no '%s' or '%s' file found in the working directory", "shell.nix", "default.nix"); + if (isNixShell && !compatibilitySettings.nixShellAlwaysLooksForShellNix && std::filesystem::exists("shell.nix")) { + // If we're in 2.3 compatibility mode, we need to look for shell.nix + // now, because it won't be done later. + remainingArgs = {"shell.nix"}; + } else { + remainingArgs = {"."}; + + // Instead of letting it throw later, we throw here to give a more relevant error message + if (isNixShell && !std::filesystem::exists("shell.nix") && !std::filesystem::exists("default.nix")) + throw Error("no argument specified and no '%s' or '%s' file found in the working directory", "shell.nix", "default.nix"); + } } if (isNixShell) diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell.sh index f54e3621cc8..65ff279f868 100755 --- a/tests/functional/nix-shell.sh +++ b/tests/functional/nix-shell.sh @@ -21,6 +21,10 @@ output=$(nix-shell --pure "$shellDotNix" -A shellDrv --run \ [ "$output" = " - foo - bar - true" ] +output=$(nix-shell --pure "$shellDotNix" -A shellDrv --option nix-shell-always-looks-for-shell-nix false --run \ + 'echo "$IMPURE_VAR - $VAR_FROM_STDENV_SETUP - $VAR_FROM_NIX - $TEST_inNixShell"') +[ "$output" = " - foo - bar - true" ] + # Test --keep output=$(nix-shell --pure --keep SELECTED_IMPURE_VAR "$shellDotNix" -A shellDrv --run \ 'echo "$IMPURE_VAR - $VAR_FROM_STDENV_SETUP - $VAR_FROM_NIX - $SELECTED_IMPURE_VAR"') @@ -101,6 +105,11 @@ nix-shell $TEST_ROOT/lookup-test -A shellDrv --run 'echo "it works"' | grepQuiet # https://github.com/NixOS/nix/issues/4529 nix-shell -I "testRoot=$TEST_ROOT" '' -A shellDrv --run 'echo "it works"' | grepQuiet "it works" +expectStderr 1 nix-shell $TEST_ROOT/lookup-test -A shellDrv --run 'echo "it works"' --option nix-shell-always-looks-for-shell-nix false \ + | grepQuiet -F "do not load default.nix!" # we did, because we chose to enable legacy behavior +expectStderr 1 nix-shell $TEST_ROOT/lookup-test -A shellDrv --run 'echo "it works"' --option nix-shell-always-looks-for-shell-nix false \ + | grepQuiet "Skipping .*lookup-test/shell\.nix.*, because the setting .*nix-shell-always-looks-for-shell-nix.* is disabled. This is a deprecated behavior\. Consider enabling .*nix-shell-always-looks-for-shell-nix.*" + ( cd $TEST_ROOT/empty; expectStderr 1 nix-shell | \ From c4a20a41019ef3cd806059102cbe1d45fcbdd2b8 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 6 Jul 2024 23:02:32 +0200 Subject: [PATCH 21/54] rl-next: Add note about shell.nix lookups --- .../rl-next/nix-shell-looks-for-shell-nix.md | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 doc/manual/rl-next/nix-shell-looks-for-shell-nix.md diff --git a/doc/manual/rl-next/nix-shell-looks-for-shell-nix.md b/doc/manual/rl-next/nix-shell-looks-for-shell-nix.md new file mode 100644 index 00000000000..99be4148bf1 --- /dev/null +++ b/doc/manual/rl-next/nix-shell-looks-for-shell-nix.md @@ -0,0 +1,28 @@ +--- +synopsis: "`nix-shell ` looks for `shell.nix`" +significance: significant +issues: +- 496 +- 2279 +- 4529 +- 5431 +- 11053 +prs: +- 11057 +--- + +`nix-shell $x` now looks for `$x/shell.nix` when `$x` resolves to a directory. + +Although this might be seen as a breaking change, its primarily interactive usage makes it a minor issue. +This adjustment addresses a commonly reported problem. + +This also applies to `nix-shell` shebang scripts. Consider the following example: + +```shell +#!/usr/bin/env nix-shell +#!nix-shell -i bash +``` + +This will now load `shell.nix` from the script's directory, if it exists; `default.nix` otherwise. + +The old behavior can be opted into by setting the option [`nix-shell-always-looks-for-shell-nix`](@docroot@/command-ref/conf-file.md#conf-nix-shell-always-looks-for-shell-nix) to `false`. From 0f8a655023be204499c6360e072b36f58f6f194c Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 7 Jul 2024 13:02:21 +0200 Subject: [PATCH 22/54] tests/functional/shell.nix: Implement runHook for dummy stdenv --- tests/functional/shell.nix | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/functional/shell.nix b/tests/functional/shell.nix index 75e3845ea54..1fb00c5a37a 100644 --- a/tests/functional/shell.nix +++ b/tests/functional/shell.nix @@ -26,6 +26,9 @@ let pkgs = rec { fun() { echo blabla } + runHook() { + eval "''${!1}" + } ''; stdenv = mkDerivation { From e1106b45a31228c6f5fe8be0bd5fbde08e7c3255 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 7 Jul 2024 13:03:19 +0200 Subject: [PATCH 23/54] tests/functional/nix-shell.sh: Fix Polo test for VM test It is unclear to me why this worked when not in a VM test, but the explanation would be in the part of nix-shell we're getting rid of with the devShell attribute. --- tests/functional/shell.nix | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/shell.nix b/tests/functional/shell.nix index 1fb00c5a37a..750cdf0bcaa 100644 --- a/tests/functional/shell.nix +++ b/tests/functional/shell.nix @@ -56,6 +56,7 @@ let pkgs = rec { # See nix-shell.sh polo = mkDerivation { name = "polo"; + inherit stdenv; shellHook = '' echo Polo ''; From 193dd5d9342e4ee7892b391d32240bbc431f16c8 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 7 Jul 2024 14:49:52 +0200 Subject: [PATCH 24/54] Fixup: add missing test file --- tests/functional/shell.shebang.legacy.expr | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100755 tests/functional/shell.shebang.legacy.expr diff --git a/tests/functional/shell.shebang.legacy.expr b/tests/functional/shell.shebang.legacy.expr new file mode 100755 index 00000000000..490542f430b --- /dev/null +++ b/tests/functional/shell.shebang.legacy.expr @@ -0,0 +1,10 @@ +#! @ENV_PROG@ nix-shell +#! nix-shell "{ script, path, ... }: assert path == ./shell.nix; script { fooContents = toString ./.; }" +#! nix-shell --no-substitute +#! nix-shell --expr +#! nix-shell --arg script "import ((builtins.getEnv ''TEST_ROOT'')+''/shell.nix'')" +#! nix-shell --arg path "./shell.nix" +#! nix-shell -A shellDrv +#! nix-shell -i bash +#! nix-shell --option nix-shell-shebang-arguments-relative-to-script false +echo "$FOO" From 48804cffbf0663ac8cecd603973032377a0cab07 Mon Sep 17 00:00:00 2001 From: Ryan Hendrickson Date: Mon, 8 Jul 2024 00:41:19 -0400 Subject: [PATCH 25/54] docs: fill out language/types.md#type-path --- doc/manual/src/language/syntax.md | 18 ++---------------- doc/manual/src/language/types.md | 31 ++++++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/doc/manual/src/language/syntax.md b/doc/manual/src/language/syntax.md index 238c502f979..b0779ea95b4 100644 --- a/doc/manual/src/language/syntax.md +++ b/doc/manual/src/language/syntax.md @@ -190,18 +190,13 @@ This section covers syntax and semantics of the Nix language. ### Path {#path-literal} - *Paths* are distinct from strings and can be expressed by path literals such as `./builder.sh`. - - Paths are suitable for referring to local files, and are often preferable over strings. - - Path values do not contain trailing slashes, `.` and `..`, as they are resolved when evaluating a path literal. - - Path literals are automatically resolved relative to their [base directory](@docroot@/glossary.md#gloss-base-directory). - - The files referred to by path values are automatically copied into the Nix store when used in a string interpolation or concatenation. - - Tooling can recognize path literals and provide additional features, such as autocompletion, refactoring automation and jump-to-file. + *Paths* can be expressed by path literals such as `./builder.sh`. A path literal must contain at least one slash to be recognised as such. For instance, `builder.sh` is not a path: it's parsed as an expression that selects the attribute `sh` from the variable `builder`. + Path literals are resolved relative to their [base directory](@docroot@/glossary.md#gloss-base-directory). Path literals may also refer to absolute paths by starting with a slash. > **Note** @@ -215,15 +210,6 @@ This section covers syntax and semantics of the Nix language. For example, `~/foo` would be equivalent to `/home/edolstra/foo` for a user whose home directory is `/home/edolstra`. Path literals that start with `~` are not allowed in [pure](@docroot@/command-ref/conf-file.md#conf-pure-eval) evaluation. - Paths can be used in [string interpolation] and string concatenation. - For instance, evaluating `"${./foo.txt}"` will cause `foo.txt` from the same directory to be copied into the Nix store and result in the string `"/nix/store/-foo.txt"`. - - Note that the Nix language assumes that all input files will remain _unchanged_ while evaluating a Nix expression. - For example, assume you used a file path in an interpolated string during a `nix repl` session. - Later in the same session, after having changed the file contents, evaluating the interpolated string with the file path again might not return a new [store path], since Nix might not re-read the file contents. Use `:r` to reset the repl as needed. - - [store path]: @docroot@/store/store-path.md - Path literals can also include [string interpolation], besides being [interpolated into other expressions]. [interpolated into other expressions]: ./string-interpolation.md#interpolated-expressions diff --git a/doc/manual/src/language/types.md b/doc/manual/src/language/types.md index c6cfb3c6909..229756e6be2 100644 --- a/doc/manual/src/language/types.md +++ b/doc/manual/src/language/types.md @@ -50,8 +50,37 @@ The function [`builtins.isString`](builtins.md#builtins-isString) can be used to ### Path {#type-path} - +A _path_ in the Nix language is an immutable, finite-length sequence of bytes starting with `/`, representing a POSIX-style, canonical file system path. +Path values are distinct from string values, even if they contain the same sequence of bytes. +Operations that produce paths will simplify the result as the standard C function [`realpath`] would, except that there is no symbolic link resolution. +[`realpath`]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/realpath.html + +Paths are suitable for referring to local files, and are often preferable over strings. +- Path values do not contain trailing or duplicate slashes, `.`, or `..`. +- Relative path literals are automatically resolved relative to their [base directory]. +- Tooling can recognize path literals and provide additional features, such as autocompletion, refactoring automation and jump-to-file. + +[base directory]: @docroot@/glossary.md#gloss-base-directory + +A file is not required to exist at a given path in order for that path value to be valid, but a path that is converted to a string with [string interpolation] or [string-and-path concatenation] must resolve to a readable file or directory which will be copied into the Nix store. +For instance, evaluating `"${./foo.txt}"` will cause `foo.txt` from the same directory to be copied into the Nix store and result in the string `"/nix/store/-foo.txt"`. +Operations such as [`import`] can also expect a path to resolve to a readable file or directory. + +[string interpolation]: string-interpolation.md#interpolated-expression +[string-and-path concatenation]: operators.md#string-and-path-concatenation +[`import`]: builtins.md#builtins-import + +> **Note** +> +> The Nix language assumes that all input files will remain _unchanged_ while evaluating a Nix expression. +> For example, assume you used a file path in an interpolated string during a `nix repl` session. +> Later in the same session, after having changed the file contents, evaluating the interpolated string with the file path again might not return a new [store path], since Nix might not re-read the file contents. +> Use `:r` to reset the repl as needed. + +[store path]: @docroot@/store/store-path.md + +Path values can be expressed as [path literals](syntax.md#path-literal). The function [`builtins.isPath`](builtins.md#builtins-isPath) can be used to determine if a value is a path. ### Null {#type-null} From c4e3e2dc27da93cae22cd35a11ee1ef87e23eb57 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 10 Jul 2024 16:24:31 +0200 Subject: [PATCH 26/54] Soft-deprecate the compatibility settings --- src/libcmd/compatibility-settings.hh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libcmd/compatibility-settings.hh b/src/libcmd/compatibility-settings.hh index 961001080db..0506743f3b5 100644 --- a/src/libcmd/compatibility-settings.hh +++ b/src/libcmd/compatibility-settings.hh @@ -7,14 +7,18 @@ struct CompatibilitySettings : public Config CompatibilitySettings() = default; + // Added in Nix 2.24, July 2024. Setting nixShellAlwaysLooksForShellNix{this, true, "nix-shell-always-looks-for-shell-nix", R"( Before Nix 2.24, [`nix-shell`](@docroot@/command-ref/nix-shell.md) would only look at `shell.nix` if it was in the working directory - when no file was specified. Since Nix 2.24, `nix-shell` always looks for a `shell.nix`, whether that's in the working directory, or in a directory that was passed as an argument. You may set this to `false` to revert to the Nix 2.3 behavior. + + This setting is not recommended, and will be deprecated and later removed in the future. )"}; + // Added in Nix 2.24, July 2024. Setting nixShellShebangArgumentsRelativeToScript{ this, true, "nix-shell-shebang-arguments-relative-to-script", R"( Before Nix 2.24, the arguments in a `nix-shell` shebang - as well as `--arg` - were relative to working directory. @@ -22,6 +26,8 @@ struct CompatibilitySettings : public Config Since Nix 2.24, the arguments are relative to the [base directory](@docroot@/glossary.md#gloss-base-directory) defined as the script's directory. You may set this to `false` to revert to the Nix 2.3 behavior. + + This setting is not recommended, and will be deprecated and later removed in the future. )"}; }; From 6f5f741157ff14e8a67608be9bee2bfc8d5778a8 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 11 Jul 2024 13:51:03 +0200 Subject: [PATCH 27/54] doc/rl-next/shebang-relative: Update with example --- doc/manual/rl-next/shebang-relative.md | 64 +++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 6 deletions(-) diff --git a/doc/manual/rl-next/shebang-relative.md b/doc/manual/rl-next/shebang-relative.md index ab39a359c79..c887a598a05 100644 --- a/doc/manual/rl-next/shebang-relative.md +++ b/doc/manual/rl-next/shebang-relative.md @@ -1,10 +1,62 @@ -synopsis: ensure nix-shell shebang uses relative path -prs: #5088 -description: { +--- +synopsis: "`nix-shell` shebang uses relative path" +prs: +- 5088 +- 11058 +issues: +- 4232 +--- -`nix-shell` shebangs use the script file's relative location to resolve relative paths to files passed as command line arguments, but expression arguments were still evaluated using the current working directory as a base path. -The new behavior is that evaluations are performed relative to the script. + +Relative [path](@docroot@/language/values.md#type-path) literals in `nix-shell` shebang scripts' options are now resolved relative to the [script's location](@docroot@/glossary?highlight=base%20directory#gloss-base-directory). +Previously they were resolved relative to the current working directory. +For example, consider the following script in `~/myproject/say-hi`: + +```shell +#!/usr/bin/env nix-shell +#!nix-shell --expr 'import ./shell.nix' +#!nix-shell --arg toolset './greeting-tools.nix' +#!nix-shell -i bash +hello +``` + +Older versions of `nix-shell` would resolve `shell.nix` relative to the current working directory; home in this example: + +```console +[hostname:~]$ ./myproject/say-hi +error: + … while calling the 'import' builtin + at «string»:1:2: + 1| (import ./shell.nix) + | ^ + + error: path '/home/user/shell.nix' does not exist +``` + +Since this release, `nix-shell` resolves `shell.nix` relative to the script's location, and `~/myproject/shell.nix` is used. + +```console +$ ./myproject/say-hi +Hello, world! +``` + +**Opt-out** + +This is technically a breaking change, so we have added an option so you can adapt independently of your Nix update. The old behavior can be opted into by setting the option [`nix-shell-shebang-arguments-relative-to-script`](@docroot@/command-ref/conf-file.md#conf-nix-shell-shebang-arguments-relative-to-script) to `false`. +This option will be removed in a future release. + +**`nix` command shebang** + +The experimental [`nix` command shebang](@docroot@/command-ref/new-cli/nix.md?highlight=shebang#shebang-interpreter) already behaves in this script-relative manner. + +Example: -} +```shell +#!/usr/bin/env nix +#!nix develop +#!nix --expr ``import ./shell.nix`` +#!nix -c bash +hello +``` From bb312a717451fc88f1220e1ce56700eaaf15e3de Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 11 Jul 2024 13:53:03 +0200 Subject: [PATCH 28/54] Edit CompatibilitySettings --- src/libcmd/compatibility-settings.hh | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/libcmd/compatibility-settings.hh b/src/libcmd/compatibility-settings.hh index 0506743f3b5..a129a957a64 100644 --- a/src/libcmd/compatibility-settings.hh +++ b/src/libcmd/compatibility-settings.hh @@ -13,21 +13,23 @@ struct CompatibilitySettings : public Config Since Nix 2.24, `nix-shell` always looks for a `shell.nix`, whether that's in the working directory, or in a directory that was passed as an argument. - You may set this to `false` to revert to the Nix 2.3 behavior. + You may set this to `false` to temporarily revert to the behavior of Nix 2.23 and older. - This setting is not recommended, and will be deprecated and later removed in the future. + Using this setting is not recommended. + It will be deprecated and removed. )"}; // Added in Nix 2.24, July 2024. Setting nixShellShebangArgumentsRelativeToScript{ this, true, "nix-shell-shebang-arguments-relative-to-script", R"( - Before Nix 2.24, the arguments in a `nix-shell` shebang - as well as `--arg` - were relative to working directory. + Before Nix 2.24, relative file path expressions in arguments in a `nix-shell` shebang were resolved relative to the working directory. - Since Nix 2.24, the arguments are relative to the [base directory](@docroot@/glossary.md#gloss-base-directory) defined as the script's directory. + Since Nix 2.24, `nix-shell` resolves these paths in a manner that is relative to the [base directory](@docroot@/glossary.md#gloss-base-directory), defined as the script's directory. - You may set this to `false` to revert to the Nix 2.3 behavior. + You may set this to `false` to temporarily revert to the behavior of Nix 2.23 and older. - This setting is not recommended, and will be deprecated and later removed in the future. + Using this setting is not recommended. + It will be deprecated and removed. )"}; }; From bc801e2c593c79cf234e6980d026d4e7de1e4f5f Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Sun, 14 Jul 2024 19:55:02 -0700 Subject: [PATCH 29/54] lint: fix shellcheck for misc/systemv/nix-daemon Got shellcheck passing for misc/systemv/nix-daemon Not sure how to test this since it's not running on my NixOS machine and I see no references to it in the directory otherwise. See #10795 --- flake.nix | 1 + misc/systemv/nix-daemon | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/flake.nix b/flake.nix index d83c2ecad36..51dbc309108 100644 --- a/flake.nix +++ b/flake.nix @@ -324,6 +324,7 @@ ++ pkgs.nixComponents.nix-external-api-docs.nativeBuildInputs ++ [ pkgs.buildPackages.cmake + pkgs.shellcheck modular.pre-commit.settings.package (pkgs.writeScriptBin "pre-commit-hooks-install" modular.pre-commit.settings.installationScript) diff --git a/misc/systemv/nix-daemon b/misc/systemv/nix-daemon index fea53716721..e8326f9472b 100755 --- a/misc/systemv/nix-daemon +++ b/misc/systemv/nix-daemon @@ -34,6 +34,7 @@ else fi # Source function library. +# shellcheck source=/dev/null . /etc/init.d/functions LOCKFILE=/var/lock/subsys/nix-daemon @@ -41,14 +42,20 @@ RUNDIR=/var/run/nix PIDFILE=${RUNDIR}/nix-daemon.pid RETVAL=0 -base=${0##*/} +# https://www.shellcheck.net/wiki/SC3004 +# Check if gettext exists +if ! type gettext > /dev/null 2>&1 +then + # If not, create a dummy function that returns the input verbatim + gettext() { printf '%s' "$1"; } +fi start() { mkdir -p ${RUNDIR} chown ${NIX_DAEMON_USER}:${NIX_DAEMON_USER} ${RUNDIR} - echo -n $"Starting nix daemon... " + printf '%s' "$(gettext 'Starting nix daemon... ')" daemonize -u $NIX_DAEMON_USER -p ${PIDFILE} $NIX_DAEMON_BIN $NIX_DAEMON_OPTS RETVAL=$? @@ -58,7 +65,7 @@ start() { } stop() { - echo -n $"Shutting down nix daemon: " + printf '%s' "$(gettext 'Shutting down nix daemon: ')" killproc -p ${PIDFILE} $NIX_DAEMON_BIN RETVAL=$? [ $RETVAL -eq 0 ] && rm -f ${LOCKFILE} ${PIDFILE} @@ -67,7 +74,7 @@ stop() { } reload() { - echo -n $"Reloading nix daemon... " + printf '%s' "$(gettext 'Reloading nix daemon... ')" killproc -p ${PIDFILE} $NIX_DAEMON_BIN -HUP RETVAL=$? echo @@ -105,7 +112,7 @@ case "$1" in fi ;; *) - echo $"Usage: $0 {start|stop|status|restart|condrestart}" + printf '%s' "$(gettext "Usage: $0 {start|stop|status|restart|condrestart}")" exit 2 ;; esac From 104aba0fad44c0cf6f2b65bbd753c09226ca1d0c Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Sun, 14 Jul 2024 19:57:55 -0700 Subject: [PATCH 30/54] Remove nix-daemon from exclusion --- maintainers/flake-module.nix | 1 - 1 file changed, 1 deletion(-) diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 46b3e136324..7fbc2d2d289 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -495,7 +495,6 @@ excludes = [ # We haven't linted these files yet ''^config/install-sh$'' - ''^misc/systemv/nix-daemon$'' ''^misc/bash/completion\.sh$'' ''^misc/fish/completion\.fish$'' ''^misc/zsh/completion\.zsh$'' From 1a273a623f4ecaabafe56cba50d014ceb2beb4a3 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 2 Jul 2024 15:00:39 -0400 Subject: [PATCH 31/54] Inline `settings.pluginFiles.name` In theory the warning is more noisy now, but in practice this will not happen unless the client is older than 2.14 (highly unlikely). --- src/libstore/daemon.cc | 7 +++---- src/libstore/remote-store.cc | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 40163a6214d..5c5080f8aa9 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -246,10 +246,9 @@ struct ClientSettings // the daemon, as that could cause some pretty weird stuff if (parseFeatures(tokenizeString(value)) != experimentalFeatureSettings.experimentalFeatures.get()) debug("Ignoring the client-specified experimental features"); - } else if (name == settings.pluginFiles.name) { - if (tokenizeString(value) != settings.pluginFiles.get()) - warn("Ignoring the client-specified plugin-files.\n" - "The client specifying plugins to the daemon never made sense, and was removed in Nix >=2.14."); + } else if (name == "plugin-files") { + warn("Ignoring the client-specified plugin-files.\n" + "The client specifying plugins to the daemon never made sense, and was removed in Nix >=2.14."); } else if (trusted || name == settings.buildTimeout.name diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index d749ccd0a35..6e8931ca2e4 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -128,7 +128,7 @@ void RemoteStore::setOptions(Connection & conn) overrides.erase(settings.useSubstitutes.name); overrides.erase(loggerSettings.showTrace.name); overrides.erase(experimentalFeatureSettings.experimentalFeatures.name); - overrides.erase(settings.pluginFiles.name); + overrides.erase("plugin-files"); conn.to << overrides.size(); for (auto & i : overrides) conn.to << i.first << i.second.value; From 0feeab755a19acd426cdae6887019f9886b016b4 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 2 Jul 2024 15:15:17 -0400 Subject: [PATCH 32/54] Move plugins infra to `libnixmain` They are not actually part of the store layer, but instead part of the Nix executable infra (libraries don't need plugins, executables do). This is part of a larger project of moving all of our legacy settings infra to libmain, and having the underlying libraries just have plain configuration structs detached from any settings infra / UI layer. Progress on #5638 --- meson.build | 1 + packaging/components.nix | 1 + packaging/hydra.nix | 1 + src/build-remote/build-remote.cc | 1 + src/libmain-c/.version | 1 + src/libmain-c/build-utils-meson | 1 + src/libmain-c/meson.build | 84 ++++++++++++++++++++++ src/libmain-c/nix_api_main.cc | 16 +++++ src/libmain-c/nix_api_main.h | 40 +++++++++++ src/libmain-c/package.nix | 83 ++++++++++++++++++++++ src/libmain/common-args.cc | 1 + src/libmain/meson.build | 2 + src/libmain/plugin.cc | 117 +++++++++++++++++++++++++++++++ src/libmain/plugin.hh | 12 ++++ src/libstore-c/nix_api_store.cc | 10 --- src/libstore-c/nix_api_store.h | 11 --- src/libstore/globals.cc | 55 --------------- src/libstore/globals.hh | 50 ------------- 18 files changed, 361 insertions(+), 126 deletions(-) create mode 120000 src/libmain-c/.version create mode 120000 src/libmain-c/build-utils-meson create mode 100644 src/libmain-c/meson.build create mode 100644 src/libmain-c/nix_api_main.cc create mode 100644 src/libmain-c/nix_api_main.h create mode 100644 src/libmain-c/package.nix create mode 100644 src/libmain/plugin.cc create mode 100644 src/libmain/plugin.hh diff --git a/meson.build b/meson.build index e6bdc2eac30..1c46c5c2881 100644 --- a/meson.build +++ b/meson.build @@ -26,6 +26,7 @@ subproject('external-api-docs') subproject('libutil-c') subproject('libstore-c') subproject('libexpr-c') +subproject('libmain-c') # Language Bindings subproject('perl') diff --git a/packaging/components.nix b/packaging/components.nix index 0e369a055cd..870e9ae615f 100644 --- a/packaging/components.nix +++ b/packaging/components.nix @@ -29,6 +29,7 @@ in nix-flake-tests = callPackage ../tests/unit/libflake/package.nix { }; nix-main = callPackage ../src/libmain/package.nix { }; + nix-main-c = callPackage ../src/libmain-c/package.nix { }; nix-cmd = callPackage ../src/libcmd/package.nix { }; diff --git a/packaging/hydra.nix b/packaging/hydra.nix index 4dfaf9bbfaa..dbe99247675 100644 --- a/packaging/hydra.nix +++ b/packaging/hydra.nix @@ -52,6 +52,7 @@ let "nix-flake" "nix-flake-tests" "nix-main" + "nix-main-c" "nix-cmd" "nix-ng" ]; diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 600fc7ee2b7..a0a404e575a 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -11,6 +11,7 @@ #include "machines.hh" #include "shared.hh" +#include "plugin.hh" #include "pathlocks.hh" #include "globals.hh" #include "serialise.hh" diff --git a/src/libmain-c/.version b/src/libmain-c/.version new file mode 120000 index 00000000000..b7badcd0cc8 --- /dev/null +++ b/src/libmain-c/.version @@ -0,0 +1 @@ +../../.version \ No newline at end of file diff --git a/src/libmain-c/build-utils-meson b/src/libmain-c/build-utils-meson new file mode 120000 index 00000000000..5fff21bab55 --- /dev/null +++ b/src/libmain-c/build-utils-meson @@ -0,0 +1 @@ +../../build-utils-meson \ No newline at end of file diff --git a/src/libmain-c/meson.build b/src/libmain-c/meson.build new file mode 100644 index 00000000000..1d6b2f959ff --- /dev/null +++ b/src/libmain-c/meson.build @@ -0,0 +1,84 @@ +project('nix-main-c', 'cpp', + version : files('.version'), + default_options : [ + 'cpp_std=c++2a', + # TODO(Qyriad): increase the warning level + 'warning_level=1', + 'debug=true', + 'optimization=2', + 'errorlogs=true', # Please print logs for tests that fail + ], + meson_version : '>= 1.1', + license : 'LGPL-2.1-or-later', +) + +cxx = meson.get_compiler('cpp') + +subdir('build-utils-meson/deps-lists') + +configdata = configuration_data() + +deps_private_maybe_subproject = [ + dependency('nix-util'), + dependency('nix-store'), + dependency('nix-main'), +] +deps_public_maybe_subproject = [ + dependency('nix-util-c'), + dependency('nix-store-c'), +] +subdir('build-utils-meson/subprojects') + +# TODO rename, because it will conflict with downstream projects +configdata.set_quoted('PACKAGE_VERSION', meson.project_version()) + +config_h = configure_file( + configuration : configdata, + output : 'config-main.h', +) + +add_project_arguments( + # TODO(Qyriad): Yes this is how the autoconf+Make system did it. + # It would be nice for our headers to be idempotent instead. + + # From C++ libraries, only for internals + '-include', 'config-util.hh', + '-include', 'config-store.hh', + '-include', 'config-main.hh', + + # From C libraries, for our public, installed headers too + '-include', 'config-util.h', + '-include', 'config-store.h', + '-include', 'config-main.h', + language : 'cpp', +) + +subdir('build-utils-meson/diagnostics') + +sources = files( + 'nix_api_main.cc', +) + +include_dirs = [include_directories('.')] + +headers = [config_h] + files( + 'nix_api_main.h', +) + +subdir('build-utils-meson/export-all-symbols') + +this_library = library( + 'nixmainc', + sources, + dependencies : deps_public + deps_private + deps_other, + include_directories : include_dirs, + link_args: linker_export_flags, + prelink : true, # For C++ static initializers + install : true, +) + +install_headers(headers, subdir : 'nix', preserve_path : true) + +libraries_private = [] + +subdir('build-utils-meson/export') diff --git a/src/libmain-c/nix_api_main.cc b/src/libmain-c/nix_api_main.cc new file mode 100644 index 00000000000..692d53f47e0 --- /dev/null +++ b/src/libmain-c/nix_api_main.cc @@ -0,0 +1,16 @@ +#include "nix_api_store.h" +#include "nix_api_store_internal.h" +#include "nix_api_util.h" +#include "nix_api_util_internal.h" + +#include "plugin.hh" + +nix_err nix_init_plugins(nix_c_context * context) +{ + if (context) + context->last_err_code = NIX_OK; + try { + nix::initPlugins(); + } + NIXC_CATCH_ERRS +} diff --git a/src/libmain-c/nix_api_main.h b/src/libmain-c/nix_api_main.h new file mode 100644 index 00000000000..3957b992fd3 --- /dev/null +++ b/src/libmain-c/nix_api_main.h @@ -0,0 +1,40 @@ +#ifndef NIX_API_MAIN_H +#define NIX_API_MAIN_H +/** + * @defgroup libmain libmain + * @brief C bindings for nix libmain + * + * libmain has misc utilities for CLI commands + * @{ + */ +/** @file + * @brief Main entry for the libmain C bindings + */ + +#include "nix_api_util.h" +#include + +#ifdef __cplusplus +extern "C" { +#endif +// cffi start + +/** + * @brief Loads the plugins specified in Nix's plugin-files setting. + * + * Call this once, after calling your desired init functions and setting + * relevant settings. + * + * @param[out] context Optional, stores error information + * @return NIX_OK if the initialization was successful, an error code otherwise. + */ +nix_err nix_init_plugins(nix_c_context * context); + +// cffi end +#ifdef __cplusplus +} +#endif +/** + * @} + */ +#endif // NIX_API_MAIN_H diff --git a/src/libmain-c/package.nix b/src/libmain-c/package.nix new file mode 100644 index 00000000000..478e34a856a --- /dev/null +++ b/src/libmain-c/package.nix @@ -0,0 +1,83 @@ +{ lib +, stdenv +, mkMesonDerivation +, releaseTools + +, meson +, ninja +, pkg-config + +, nix-util-c +, nix-store +, nix-store-c +, nix-main + +# Configuration Options + +, version +}: + +let + inherit (lib) fileset; +in + +mkMesonDerivation (finalAttrs: { + pname = "nix-main-c"; + inherit version; + + workDir = ./.; + fileset = fileset.unions [ + ../../build-utils-meson + ./build-utils-meson + ../../.version + ./.version + ./meson.build + # ./meson.options + (fileset.fileFilter (file: file.hasExt "cc") ./.) + (fileset.fileFilter (file: file.hasExt "hh") ./.) + (fileset.fileFilter (file: file.hasExt "h") ./.) + ]; + + outputs = [ "out" "dev" ]; + + nativeBuildInputs = [ + meson + ninja + pkg-config + ]; + + propagatedBuildInputs = [ + nix-util-c + nix-store + nix-store-c + nix-main + ]; + + preConfigure = + # "Inline" .version so it's not a symlink, and includes the suffix. + # Do the meson utils, without modification. + '' + chmod u+w ./.version + echo ${version} > ../../.version + ''; + + mesonFlags = [ + ]; + + env = lib.optionalAttrs (stdenv.isLinux && !(stdenv.hostPlatform.isStatic && stdenv.system == "aarch64-linux")) { + LDFLAGS = "-fuse-ld=gold"; + }; + + enableParallelBuilding = true; + + separateDebugInfo = !stdenv.hostPlatform.isStatic; + + strictDeps = true; + + hardeningDisable = lib.optional stdenv.hostPlatform.isStatic "pie"; + + meta = { + platforms = lib.platforms.unix ++ lib.platforms.windows; + }; + +}) diff --git a/src/libmain/common-args.cc b/src/libmain/common-args.cc index a94845ab8f3..768b2177c94 100644 --- a/src/libmain/common-args.cc +++ b/src/libmain/common-args.cc @@ -5,6 +5,7 @@ #include "logging.hh" #include "loggers.hh" #include "util.hh" +#include "plugin.hh" namespace nix { diff --git a/src/libmain/meson.build b/src/libmain/meson.build index 859ce22f8ba..fe6133596a9 100644 --- a/src/libmain/meson.build +++ b/src/libmain/meson.build @@ -64,6 +64,7 @@ subdir('build-utils-meson/diagnostics') sources = files( 'common-args.cc', 'loggers.cc', + 'plugin.cc', 'progress-bar.cc', 'shared.cc', ) @@ -79,6 +80,7 @@ include_dirs = [include_directories('.')] headers = [config_h] + files( 'common-args.hh', 'loggers.hh', + 'plugin.hh', 'progress-bar.hh', 'shared.hh', ) diff --git a/src/libmain/plugin.cc b/src/libmain/plugin.cc new file mode 100644 index 00000000000..52b5b60e415 --- /dev/null +++ b/src/libmain/plugin.cc @@ -0,0 +1,117 @@ +#ifndef _WIN32 +# include +#endif + +#include "config-global.hh" +#include "signals.hh" + +namespace nix { + +struct PluginFilesSetting : public BaseSetting +{ + bool pluginsLoaded = false; + + PluginFilesSetting( + Config * options, + const Paths & def, + const std::string & name, + const std::string & description, + const std::set & aliases = {}) + : BaseSetting(def, true, name, description, aliases) + { + options->addSetting(this); + } + + Paths parse(const std::string & str) const override; +}; + +Paths PluginFilesSetting::parse(const std::string & str) const +{ + if (pluginsLoaded) + throw UsageError( + "plugin-files set after plugins were loaded, you may need to move the flag before the subcommand"); + return BaseSetting::parse(str); +} + +struct PluginSettings : Config +{ + PluginFilesSetting pluginFiles{ + this, + {}, + "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 + 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 + implementations, RegisterCommand to add new subcommands to the `nix` + command, and RegisterSetting to add new nix config settings. See the + constructors for those types for more details. + + Warning! These APIs are inherently unstable and may change from + release to release. + + Since these files are loaded into the same address space as Nix + 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 + time. + + If an entry in the list is a directory, all files in the directory + are loaded as plugins (non-recursively). + )"}; +}; + +static PluginSettings pluginSettings; + +static GlobalConfig::Register rPluginSettings(&pluginSettings); + +void initPlugins() +{ + assert(!pluginSettings.pluginFiles.pluginsLoaded); + for (const auto & pluginFile : pluginSettings.pluginFiles.get()) { + std::vector pluginFiles; + try { + auto ents = std::filesystem::directory_iterator{pluginFile}; + for (const auto & ent : ents) { + checkInterrupt(); + pluginFiles.emplace_back(ent.path()); + } + } catch (std::filesystem::filesystem_error & e) { + if (e.code() != std::errc::not_a_directory) + throw; + pluginFiles.emplace_back(pluginFile); + } + for (const auto & file : pluginFiles) { + checkInterrupt(); + /* handle is purposefully leaked as there may be state in the + DSO needed by the action of the plugin. */ +#ifndef _WIN32 // TODO implement via DLL loading on Windows + void * handle = dlopen(file.c_str(), RTLD_LAZY | RTLD_LOCAL); + if (!handle) + throw Error("could not dynamically open plugin file '%s': %s", file, dlerror()); + + /* Older plugins use a statically initialized object to run their code. + Newer plugins can also export nix_plugin_entry() */ + void (*nix_plugin_entry)() = (void (*)()) dlsym(handle, "nix_plugin_entry"); + if (nix_plugin_entry) + nix_plugin_entry(); +#else + throw Error("could not dynamically open plugin file '%s'", file); +#endif + } + } + + /* Since plugins can add settings, try to re-apply previously + unknown settings. */ + globalConfig.reapplyUnknownSettings(); + globalConfig.warnUnknownSettings(); + + /* Tell the user if they try to set plugin-files after we've already loaded */ + pluginSettings.pluginFiles.pluginsLoaded = true; +} + +} diff --git a/src/libmain/plugin.hh b/src/libmain/plugin.hh new file mode 100644 index 00000000000..4221c1b1713 --- /dev/null +++ b/src/libmain/plugin.hh @@ -0,0 +1,12 @@ +#pragma once +///@file + +namespace nix { + +/** + * This should be called after settings are initialized, but before + * anything else + */ +void initPlugins(); + +} diff --git a/src/libstore-c/nix_api_store.cc b/src/libstore-c/nix_api_store.cc index 4fe25c7d4ee..79841ca49a1 100644 --- a/src/libstore-c/nix_api_store.cc +++ b/src/libstore-c/nix_api_store.cc @@ -29,16 +29,6 @@ nix_err nix_libstore_init_no_load_config(nix_c_context * context) NIXC_CATCH_ERRS } -nix_err nix_init_plugins(nix_c_context * context) -{ - if (context) - context->last_err_code = NIX_OK; - try { - nix::initPlugins(); - } - NIXC_CATCH_ERRS -} - Store * nix_store_open(nix_c_context * context, const char * uri, const char *** params) { if (context) diff --git a/src/libstore-c/nix_api_store.h b/src/libstore-c/nix_api_store.h index d3cb8fab84e..4b213445778 100644 --- a/src/libstore-c/nix_api_store.h +++ b/src/libstore-c/nix_api_store.h @@ -42,17 +42,6 @@ nix_err nix_libstore_init(nix_c_context * context); */ nix_err nix_libstore_init_no_load_config(nix_c_context * context); -/** - * @brief Loads the plugins specified in Nix's plugin-files setting. - * - * Call this once, after calling your desired init functions and setting - * relevant settings. - * - * @param[out] context Optional, stores error information - * @return NIX_OK if the initialization was successful, an error code otherwise. - */ -nix_err nix_init_plugins(nix_c_context * context); - /** * @brief Open a nix store. * diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 4eabf6054dc..fa4c0ba7fad 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -15,7 +15,6 @@ #include #ifndef _WIN32 -# include # include #endif @@ -335,60 +334,6 @@ unsigned int MaxBuildJobsSetting::parse(const std::string & str) const } -Paths PluginFilesSetting::parse(const std::string & str) const -{ - if (pluginsLoaded) - throw UsageError("plugin-files set after plugins were loaded, you may need to move the flag before the subcommand"); - return BaseSetting::parse(str); -} - - -void initPlugins() -{ - assert(!settings.pluginFiles.pluginsLoaded); - for (const auto & pluginFile : settings.pluginFiles.get()) { - std::vector pluginFiles; - try { - auto ents = std::filesystem::directory_iterator{pluginFile}; - for (const auto & ent : ents) { - checkInterrupt(); - pluginFiles.emplace_back(ent.path()); - } - } catch (std::filesystem::filesystem_error & e) { - if (e.code() != std::errc::not_a_directory) - throw; - pluginFiles.emplace_back(pluginFile); - } - for (const auto & file : pluginFiles) { - checkInterrupt(); - /* handle is purposefully leaked as there may be state in the - DSO needed by the action of the plugin. */ -#ifndef _WIN32 // TODO implement via DLL loading on Windows - void *handle = - dlopen(file.c_str(), RTLD_LAZY | RTLD_LOCAL); - if (!handle) - throw Error("could not dynamically open plugin file '%s': %s", file, dlerror()); - - /* Older plugins use a statically initialized object to run their code. - Newer plugins can also export nix_plugin_entry() */ - void (*nix_plugin_entry)() = (void (*)())dlsym(handle, "nix_plugin_entry"); - if (nix_plugin_entry) - nix_plugin_entry(); -#else - throw Error("could not dynamically open plugin file '%s'", file); -#endif - } - } - - /* Since plugins can add settings, try to re-apply previously - unknown settings. */ - globalConfig.reapplyUnknownSettings(); - globalConfig.warnUnknownSettings(); - - /* Tell the user if they try to set plugin-files after we've already loaded */ - settings.pluginFiles.pluginsLoaded = true; -} - static void preloadNSS() { /* builtin:fetchurl can trigger a DNS lookup, which with glibc can trigger a dynamic library load of diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index dfe25f31726..30d7537bd52 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -31,23 +31,6 @@ struct MaxBuildJobsSetting : public BaseSetting unsigned int parse(const std::string & str) const override; }; -struct PluginFilesSetting : public BaseSetting -{ - bool pluginsLoaded = false; - - PluginFilesSetting(Config * options, - const Paths & def, - const std::string & name, - const std::string & description, - const std::set & aliases = {}) - : BaseSetting(def, true, name, description, aliases) - { - options->addSetting(this); - } - - Paths parse(const std::string & str) const override; -}; - const uint32_t maxIdsPerBuild = #if __linux__ 1 << 16 @@ -1158,33 +1141,6 @@ public: Setting minFreeCheckInterval{this, 5, "min-free-check-interval", "Number of seconds between checking free disk space."}; - PluginFilesSetting pluginFiles{ - this, {}, "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 - 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 - implementations, RegisterCommand to add new subcommands to the `nix` - command, and RegisterSetting to add new nix config settings. See the - constructors for those types for more details. - - Warning! These APIs are inherently unstable and may change from - release to release. - - Since these files are loaded into the same address space as Nix - 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 - time. - - If an entry in the list is a directory, all files in the directory - are loaded as plugins (non-recursively). - )"}; - Setting narBufferSize{this, 32 * 1024 * 1024, "nar-buffer-size", "Maximum size of NARs before spilling them to disk."}; @@ -1278,12 +1234,6 @@ public: // FIXME: don't use a global variable. extern Settings settings; -/** - * This should be called after settings are initialized, but before - * anything else - */ -void initPlugins(); - /** * Load the configuration (from `nix.conf`, `NIX_CONFIG`, etc.) into the * given configuration object. From 6c9d62dcebbb042e24cb7e07c5cf5369f1db6ba0 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 15 Jul 2024 19:04:37 +0200 Subject: [PATCH 33/54] Doc comments: use std::unordered_map Co-authored-by: Eelco Dolstra --- src/libexpr/eval.hh | 4 ++-- src/libexpr/parser-state.hh | 2 +- src/libexpr/parser.y | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index d376046ae30..f09e6223a77 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -130,7 +130,7 @@ struct Constant typedef std::map ValMap; #endif -typedef std::map DocCommentMap; +typedef std::unordered_map DocCommentMap; struct Env { @@ -335,7 +335,7 @@ private: * Associate source positions of certain AST nodes with their preceding doc comment, if they have one. * Grouped by file. */ - std::map positionToDocComment; + std::unordered_map positionToDocComment; LookupPath lookupPath; diff --git a/src/libexpr/parser-state.hh b/src/libexpr/parser-state.hh index 983a17a2e98..5a7bcb7175d 100644 --- a/src/libexpr/parser-state.hh +++ b/src/libexpr/parser-state.hh @@ -64,7 +64,7 @@ struct LexerState /** * @brief Maps some positions to a DocComment, where the comment is relevant to the location. */ - std::map & positionToDocComment; + std::unordered_map & positionToDocComment; PosTable & positions; PosTable::Origin origin; diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 452d265bcbf..8ea176b2461 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -48,7 +48,7 @@ namespace nix { -typedef std::map DocCommentMap; +typedef std::unordered_map DocCommentMap; Expr * parseExprFromBuf( char * text, From 64b46000ad92e772cd30f691bd75d937a2b84158 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 16 Jul 2024 16:46:41 +0200 Subject: [PATCH 34/54] Add std::hash --- src/libexpr/pos-idx.hh | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/libexpr/pos-idx.hh b/src/libexpr/pos-idx.hh index e1349156022..1d711681fba 100644 --- a/src/libexpr/pos-idx.hh +++ b/src/libexpr/pos-idx.hh @@ -2,12 +2,15 @@ #include +#include "util.hh" + namespace nix { class PosIdx { friend struct LazyPosAcessors; friend class PosTable; + friend class std::hash; private: uint32_t id; @@ -37,8 +40,28 @@ public: { return id == other.id; } + + size_t hash() const noexcept + { + size_t h = 854125; + hash_combine(h, id); + return h; + } }; inline PosIdx noPos = {}; } + +namespace std { + +template<> +struct hash +{ + std::size_t operator()(nix::PosIdx pos) const noexcept + { + return pos.hash(); + } +}; + +} // namespace std From d0e9878389cc00caff84d0bbaa37fe008af638ee Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 16 Jul 2024 22:22:15 +0200 Subject: [PATCH 35/54] Remove unused boost include and split out std-hash.hh Splitting it out immediately answers questions like [this], without increasing the number of compilation units. I did consider using boost::hash_combine instead, but it doesn't seem to be quite as capable, accepting only two arguments. [this]: https://github.com/NixOS/nix/pull/11113#discussion_r1679991573 --- src/libexpr/pos-idx.hh | 2 +- src/libutil/meson.build | 1 + src/libutil/source-path.hh | 3 +-- src/libutil/std-hash.hh | 24 ++++++++++++++++++++++++ src/libutil/util.hh | 14 -------------- 5 files changed, 27 insertions(+), 17 deletions(-) create mode 100644 src/libutil/std-hash.hh diff --git a/src/libexpr/pos-idx.hh b/src/libexpr/pos-idx.hh index 1d711681fba..f3ea3a2e5c6 100644 --- a/src/libexpr/pos-idx.hh +++ b/src/libexpr/pos-idx.hh @@ -2,7 +2,7 @@ #include -#include "util.hh" +#include "std-hash.hh" namespace nix { diff --git a/src/libutil/meson.build b/src/libutil/meson.build index fbfcbe67c42..04c778c3114 100644 --- a/src/libutil/meson.build +++ b/src/libutil/meson.build @@ -216,6 +216,7 @@ headers = [config_h] + files( 'source-accessor.hh', 'source-path.hh', 'split.hh', + 'std-hash.hh', 'strings.hh', 'strings-inline.hh', 'suggestions.hh', diff --git a/src/libutil/source-path.hh b/src/libutil/source-path.hh index 1e96b72e52d..fc2288f747a 100644 --- a/src/libutil/source-path.hh +++ b/src/libutil/source-path.hh @@ -8,8 +8,7 @@ #include "ref.hh" #include "canon-path.hh" #include "source-accessor.hh" - -#include // for boost::hash_combine +#include "std-hash.hh" namespace nix { diff --git a/src/libutil/std-hash.hh b/src/libutil/std-hash.hh new file mode 100644 index 00000000000..c359d11ca63 --- /dev/null +++ b/src/libutil/std-hash.hh @@ -0,0 +1,24 @@ +#pragma once + +//!@file Hashing utilities for use with unordered_map, etc. (ie low level implementation logic, not domain logic like +//! Nix hashing) + +#include + +namespace nix { + +/** + * hash_combine() from Boost. Hash several hashable values together + * into a single hash. + */ +inline void hash_combine(std::size_t & seed) {} + +template +inline void hash_combine(std::size_t & seed, const T & v, Rest... rest) +{ + std::hash hasher; + seed ^= hasher(v) + 0x9e3779b9 + (seed << 6) + (seed >> 2); + hash_combine(seed, rest...); +} + +} // namespace nix diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 83b42a5283e..877d1527945 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -375,18 +375,4 @@ inline std::string operator + (std::string_view s1, const char * s2) return s; } -/** - * hash_combine() from Boost. Hash several hashable values together - * into a single hash. - */ -inline void hash_combine(std::size_t & seed) { } - -template -inline void hash_combine(std::size_t & seed, const T & v, Rest... rest) -{ - std::hash hasher; - seed ^= hasher(v) + 0x9e3779b9 + (seed<<6) + (seed>>2); - hash_combine(seed, rest...); -} - } From 0a1a116f4b2fd6840a61d8f85494cade6fd6a306 Mon Sep 17 00:00:00 2001 From: Philip Taron Date: Tue, 16 Jul 2024 13:51:52 -0700 Subject: [PATCH 36/54] builtins.genericClosure: fix documentation typo --- src/libexpr/primops.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 127823d4944..5a373a43bda 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -719,7 +719,7 @@ static RegisterPrimOp primop_genericClosure(PrimOp { .doc = R"( `builtins.genericClosure` iteratively computes the transitive closure over an arbitrary relation defined by a function. - It takes *attrset* with two attributes named `startSet` and `operator`, and returns a list of attrbute sets: + It takes *attrset* with two attributes named `startSet` and `operator`, and returns a list of attribute sets: - `startSet`: The initial list of attribute sets. From 5b6a21acc5a4c14e4d5f79bd256e667f8e01ae30 Mon Sep 17 00:00:00 2001 From: Las Safin Date: Tue, 16 Jul 2024 21:05:16 +0000 Subject: [PATCH 37/54] Avoid casting function pointer in libutil test support Casting function pointers seems to be almost always UB. See https://stackoverflow.com/questions/559581/casting-a-function-pointer-to-another-type Fixed by doing the casting of `void*` to `std::string*` inside the function instead. Caught by UBSan. --- tests/unit/libutil-support/tests/string_callback.cc | 5 +++-- tests/unit/libutil-support/tests/string_callback.hh | 5 ++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/libutil-support/tests/string_callback.cc b/tests/unit/libutil-support/tests/string_callback.cc index 2d0e0dad08f..7a13bd4ff9c 100644 --- a/tests/unit/libutil-support/tests/string_callback.cc +++ b/tests/unit/libutil-support/tests/string_callback.cc @@ -2,9 +2,10 @@ namespace nix::testing { -void observe_string_cb(const char * start, unsigned int n, std::string * user_data) +void observe_string_cb(const char * start, unsigned int n, void * user_data) { - *user_data = std::string(start); + auto user_data_casted = reinterpret_cast(user_data); + *user_data_casted = std::string(start); } } diff --git a/tests/unit/libutil-support/tests/string_callback.hh b/tests/unit/libutil-support/tests/string_callback.hh index a02ea3a1b69..9a7e8d85dab 100644 --- a/tests/unit/libutil-support/tests/string_callback.hh +++ b/tests/unit/libutil-support/tests/string_callback.hh @@ -3,14 +3,13 @@ namespace nix::testing { -void observe_string_cb(const char * start, unsigned int n, std::string * user_data); +void observe_string_cb(const char * start, unsigned int n, void * user_data); inline void * observe_string_cb_data(std::string & out) { return (void *) &out; }; -#define OBSERVE_STRING(str) \ - (nix_get_string_callback) nix::testing::observe_string_cb, nix::testing::observe_string_cb_data(str) +#define OBSERVE_STRING(str) nix::testing::observe_string_cb, nix::testing::observe_string_cb_data(str) } From a1f3f103bc1a618a83b51a97dd0fb0c2f66a6b41 Mon Sep 17 00:00:00 2001 From: Las Safin Date: Tue, 16 Jul 2024 21:38:19 +0000 Subject: [PATCH 38/54] Check if drv is initialized in DerivationGoal::waiteeDone It might not be set, in which case we shouldn't do anything. Surprisingly, this somehow did not cause segfaults before? Caught by UBSan. --- src/libstore/build/derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 010f905d640..b809e3ffe3f 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1569,7 +1569,7 @@ void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result) { Goal::waiteeDone(waitee, result); - if (!useDerivation) return; + if (!useDerivation || !drv) return; auto & fullDrv = *dynamic_cast(drv.get()); auto * dg = dynamic_cast(&*waitee); From f5ebaea2775da9991c5f7258bf8059b1b5770bab Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 17 Jul 2024 13:31:31 +0200 Subject: [PATCH 39/54] Simplify PosIdx::hash() In C++ we don't need to salt the hash. --- src/libexpr/pos-idx.hh | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/libexpr/pos-idx.hh b/src/libexpr/pos-idx.hh index f3ea3a2e5c6..2faa6b7fe4f 100644 --- a/src/libexpr/pos-idx.hh +++ b/src/libexpr/pos-idx.hh @@ -1,8 +1,7 @@ #pragma once #include - -#include "std-hash.hh" +#include namespace nix { @@ -43,9 +42,7 @@ public: size_t hash() const noexcept { - size_t h = 854125; - hash_combine(h, id); - return h; + return std::hash{}(id); } }; From ece334b53284c2718515d35a81862712c9df06df Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 14 Jul 2024 13:42:12 +0200 Subject: [PATCH 40/54] tests/functional/repl: Characterize side effecting print behavior Reported on matrix by aleksana: https://matrix.to/#/!VRULIdgoKmKPzJZzjj:nixos.org/$7wZp5lUDTd-_u6MYo8kWWcysjtqTiQqP8dLI0RDNVVM?via=nixos.org&via=matrix.org&via=nixos.dev --- .../repl/pretty-print-idempotent.expected | 33 +++++++++++++++++++ .../repl/pretty-print-idempotent.in | 9 +++++ .../repl/pretty-print-idempotent.nix | 19 +++++++++++ 3 files changed, 61 insertions(+) create mode 100644 tests/functional/repl/pretty-print-idempotent.expected create mode 100644 tests/functional/repl/pretty-print-idempotent.in create mode 100644 tests/functional/repl/pretty-print-idempotent.nix diff --git a/tests/functional/repl/pretty-print-idempotent.expected b/tests/functional/repl/pretty-print-idempotent.expected new file mode 100644 index 00000000000..e74239564f3 --- /dev/null +++ b/tests/functional/repl/pretty-print-idempotent.expected @@ -0,0 +1,33 @@ +Nix +Type :? for help. +Added variables. + +{ + homepage = "https://example.com"; +} + +{ homepage = "https://example.com"; } + +{ + layerOne = { ... }; +} + +{ + layerOne = { ... }; +} + +[ + "https://example.com" +] + +[ "https://example.com" ] + +[ + [ ... ] +] + +[ + [ ... ] +] + + diff --git a/tests/functional/repl/pretty-print-idempotent.in b/tests/functional/repl/pretty-print-idempotent.in new file mode 100644 index 00000000000..5f865316fdf --- /dev/null +++ b/tests/functional/repl/pretty-print-idempotent.in @@ -0,0 +1,9 @@ +:l pretty-print-idempotent.nix +oneDeep +oneDeep +twoDeep +twoDeep +oneDeepList +oneDeepList +twoDeepList +twoDeepList diff --git a/tests/functional/repl/pretty-print-idempotent.nix b/tests/functional/repl/pretty-print-idempotent.nix new file mode 100644 index 00000000000..68929f387e4 --- /dev/null +++ b/tests/functional/repl/pretty-print-idempotent.nix @@ -0,0 +1,19 @@ +{ + oneDeep = { + homepage = "https://" + "example.com"; + }; + twoDeep = { + layerOne = { + homepage = "https://" + "example.com"; + }; + }; + + oneDeepList = [ + ("https://" + "example.com") + ]; + twoDeepList = [ + [ + ("https://" + "example.com") + ] + ]; +} From a0635a80b2c3bc89baffd64b1d91d7efa2d2fd3a Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 14 Jul 2024 14:28:21 +0200 Subject: [PATCH 41/54] printAttrs: Force item before determining whether to print multi-line --- src/libexpr/print.cc | 6 ++++++ tests/functional/repl/pretty-print-idempotent.expected | 4 +--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index 2f377e5886e..6167abee81f 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -299,6 +299,9 @@ class Printer output << ANSI_NORMAL; } + /** + * @note This may force items. + */ bool shouldPrettyPrintAttrs(AttrVec & v) { if (!options.shouldPrettyPrint() || v.empty()) { @@ -315,6 +318,9 @@ class Printer return true; } + // It is ok to force the item(s) here, because they will be printed anyway. + state.forceValue(*item, item->determinePos(noPos)); + // Pretty-print single-item attrsets only if they contain nested // structures. auto itemType = item->type(); diff --git a/tests/functional/repl/pretty-print-idempotent.expected b/tests/functional/repl/pretty-print-idempotent.expected index e74239564f3..949d7a0e6b1 100644 --- a/tests/functional/repl/pretty-print-idempotent.expected +++ b/tests/functional/repl/pretty-print-idempotent.expected @@ -2,9 +2,7 @@ Nix Type :? for help. Added variables. -{ - homepage = "https://example.com"; -} +{ homepage = "https://example.com"; } { homepage = "https://example.com"; } From da3eff60bc35763270f57c3cc17e613a19513709 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 14 Jul 2024 14:31:20 +0200 Subject: [PATCH 42/54] printList: Force item before determining whether to print multi-line --- src/libexpr/print.cc | 6 ++++++ tests/functional/repl/pretty-print-idempotent.expected | 4 +--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index 6167abee81f..bc17d6bfe26 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -377,6 +377,9 @@ class Printer } } + /** + * @note This may force items. + */ bool shouldPrettyPrintList(std::span list) { if (!options.shouldPrettyPrint() || list.empty()) { @@ -393,6 +396,9 @@ class Printer return true; } + // It is ok to force the item(s) here, because they will be printed anyway. + state.forceValue(*item, item->determinePos(noPos)); + // Pretty-print single-item lists only if they contain nested // structures. auto itemType = item->type(); diff --git a/tests/functional/repl/pretty-print-idempotent.expected b/tests/functional/repl/pretty-print-idempotent.expected index 949d7a0e6b1..f38b9b56969 100644 --- a/tests/functional/repl/pretty-print-idempotent.expected +++ b/tests/functional/repl/pretty-print-idempotent.expected @@ -14,9 +14,7 @@ Added variables. layerOne = { ... }; } -[ - "https://example.com" -] +[ "https://example.com" ] [ "https://example.com" ] From 464e5925cb21150e3c94f31224efabd3c1e74237 Mon Sep 17 00:00:00 2001 From: Las Safin Date: Wed, 17 Jul 2024 13:10:01 +0100 Subject: [PATCH 43/54] Avoid accessing uninitialized settings in own init (#11117) The default value for the setting was evaluated by calling a method on the object _being currently constructed_, so we were using it before all fields were initialized. This has been fixed by making the called method static, and not using the previously used fields at all. But functionality hasn't changed! The fields were usually always zero (by chance?) anyway, meaning the conditional path was always taken. Thus the current logic has been kept, the code simplified, and UB removed. This was found with the helper of UBSan. --- src/libexpr/eval-settings.cc | 10 ++++------ src/libexpr/eval-settings.hh | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/libexpr/eval-settings.cc b/src/libexpr/eval-settings.cc index e2151aa7fc1..eb5761638a7 100644 --- a/src/libexpr/eval-settings.cc +++ b/src/libexpr/eval-settings.cc @@ -56,7 +56,7 @@ EvalSettings::EvalSettings(bool & readOnlyMode, EvalSettings::LookupPathHooks lo builtinsAbortOnWarn = true; } -Strings EvalSettings::getDefaultNixPath() const +Strings EvalSettings::getDefaultNixPath() { Strings res; auto add = [&](const Path & p, const std::string & s = std::string()) { @@ -69,11 +69,9 @@ Strings EvalSettings::getDefaultNixPath() const } }; - if (!restrictEval && !pureEval) { - add(getNixDefExpr() + "/channels"); - add(rootChannelsDir() + "/nixpkgs", "nixpkgs"); - add(rootChannelsDir()); - } + add(getNixDefExpr() + "/channels"); + add(rootChannelsDir() + "/nixpkgs", "nixpkgs"); + add(rootChannelsDir()); return res; } diff --git a/src/libexpr/eval-settings.hh b/src/libexpr/eval-settings.hh index b915ba530a0..89a42caba5d 100644 --- a/src/libexpr/eval-settings.hh +++ b/src/libexpr/eval-settings.hh @@ -43,7 +43,7 @@ struct EvalSettings : Config bool & readOnlyMode; - Strings getDefaultNixPath() const; + static Strings getDefaultNixPath(); static bool isPseudoUrl(std::string_view s); From 87f8ff23fe68c597f2090d2c024e8336ba0d2f2d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 17 Jul 2024 16:44:34 +0200 Subject: [PATCH 44/54] BasicClientConnection::handshake(): Don't send our version twice This was accidentally introduced in f71b4da0b3ed994f2bfc3764df6f524ebe72c4da. We didn't notice this because the version got interpreted by the daemon as the obsolete "CPU affinity will follow" field, and being non-zero, it would then read another integer for the ignored CPU affinity. --- src/libstore/worker-protocol-connection.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libstore/worker-protocol-connection.cc b/src/libstore/worker-protocol-connection.cc index 072bae8da82..3a640051ebc 100644 --- a/src/libstore/worker-protocol-connection.cc +++ b/src/libstore/worker-protocol-connection.cc @@ -152,7 +152,6 @@ WorkerProto::BasicClientConnection::handshake(BufferedSink & to, Source & from, throw Error("Nix daemon protocol version not supported"); if (GET_PROTOCOL_MINOR(daemonVersion) < 10) throw Error("the Nix daemon version is too old"); - to << localVersion; return std::min(daemonVersion, localVersion); } From d095fa32395b133907b65df4647bfd48787cd010 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 17 Jul 2024 14:51:05 -0400 Subject: [PATCH 45/54] WIP --- src/libstore/config-parse-impl.hh | 24 +++++ src/libstore/config-parse.hh | 29 +++-- src/libstore/path.cc | 2 +- src/libstore/store-api.cc | 168 ++++++++++++----------------- src/libstore/store-api.hh | 128 ++++------------------ src/libstore/store-dir-config.cc | 19 ++-- src/libstore/store-dir-config.hh | 14 +-- src/libstore/store-open.hh | 38 +++++++ src/libstore/store-reference.hh | 3 +- src/libstore/store-registration.cc | 95 ++++++++++++++++ src/libstore/store-registration.hh | 55 ++++++++++ src/libutil/config-abstract.hh | 9 +- 12 files changed, 340 insertions(+), 244 deletions(-) create mode 100644 src/libstore/config-parse-impl.hh create mode 100644 src/libstore/store-open.hh create mode 100644 src/libstore/store-registration.cc create mode 100644 src/libstore/store-registration.hh diff --git a/src/libstore/config-parse-impl.hh b/src/libstore/config-parse-impl.hh new file mode 100644 index 00000000000..ebb65305ae3 --- /dev/null +++ b/src/libstore/config-parse-impl.hh @@ -0,0 +1,24 @@ +#pragma once +//@file + +#include + +#include "config-parse.hh" +#include "util.hh" + +namespace nix::config { + +template +std::optional> SettingInfo::parseConfig(const nlohmann::json::object_t & map) const +{ + auto * p = get(map, name); + return p ? (JustValue{.value = p->get()}) : (std::optional>{}); +} + +/** + * Look up the setting's name in a map, falling back on the default if + * it does not exist. + */ +#define CONFIG_ROW(FIELD) .FIELD = descriptions.FIELD.parseConfig(params).value_or(defaults.FIELD) + +} diff --git a/src/libstore/config-parse.hh b/src/libstore/config-parse.hh index e0cdaaeaccd..5d17f9f8d6a 100644 --- a/src/libstore/config-parse.hh +++ b/src/libstore/config-parse.hh @@ -1,14 +1,21 @@ #pragma once +//@file -/** - * Look up the setting's name in a map, falling back on the default if - * it does not exist. - */ -#define CONFIG_ROW(FIELD) \ - .FIELD = { \ - .value = ({ \ - auto p = get(params, descriptions.FIELD.name); \ - p ? *p : defaults.FIELD.value; \ - }) \ - } +#include +#include "config-abstract.hh" +#include "util.hh" + +namespace nix::config { + +template +struct SettingInfo +{ + std::string name; + std::string description; + bool documentDefault = true; + + std::optional> parseConfig(const nlohmann::json::object_t & map) const; +}; + +} diff --git a/src/libstore/path.cc b/src/libstore/path.cc index f2fa91cb2e2..3e9d054778c 100644 --- a/src/libstore/path.cc +++ b/src/libstore/path.cc @@ -89,7 +89,7 @@ StorePath StoreDirConfig::parseStorePath(std::string_view path) const canonPath(std::string(path)) #endif ; - if (dirOf(p) != storeDir.get()) + if (dirOf(p) != storeDir) throw BadStorePath("path '%s' is not in the Nix store", p); return StorePath(baseNameOf(p)); } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 2c4dee518b4..bf47026ed47 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -5,6 +5,7 @@ #include "realisation.hh" #include "derivations.hh" #include "store-api.hh" +#include "store-open.hh" #include "util.hh" #include "nar-info-disk-cache.hh" #include "thread-pool.hh" @@ -18,6 +19,7 @@ #include "worker-protocol.hh" #include "signals.hh" #include "users.hh" +#include "config-parse-impl.hh" #include #include @@ -28,7 +30,6 @@ using json = nlohmann::json; namespace nix { - bool StoreDirConfig::isInStore(PathView path) const { return isInDir(path, storeDir); @@ -188,6 +189,70 @@ std::pair StoreDirConfig::computeStorePath( } +const StoreConfigT storeConfigDefaults = { + .pathInfoCacheSize = { .value = 65536 }, + .isTrusted = { .value = false }, + .priority = { .value = 0 }, + .wantMassQuery = { .value = false }, + .systemFeatures = { .value = StoreConfig::getDefaultSystemFeatures() }, +}; + +const StoreConfigT storeConfigDescriptions = { + .pathInfoCacheSize = { + .name = "path-info-cache-size", + .description = "Size of the in-memory store path metadata cache.", + }, + .isTrusted = { + .name = "trusted", + .description = R"( + Whether paths from this store can be used as substitutes + even if they are not signed by a key listed in the + [`trusted-public-keys`](@docroot@/command-ref/conf-file.md#conf-trusted-public-keys) + setting. + )", + }, + .priority = { + .name = "priority", + .description = R"( + Priority of this store when used as a [substituter](@docroot@/command-ref/conf-file.md#conf-substituters). + A lower value means a higher priority. + )", + }, + .wantMassQuery = { + .name = "want-mass-query", + .description = R"( + Whether this store can be queried efficiently for path validity when used as a [substituter](@docroot@/command-ref/conf-file.md#conf-substituters). + )", + }, + + .systemFeatures = { + .name = "system-features", + .description = R"( + Optional [system features](@docroot@/command-ref/conf-file.md#conf-system-features) available on the system this store uses to build derivations. + + Example: `"kvm"` + )", + // The default value is CPU- and OS-specific, and thus + // unsuitable to be rendered in the documentation. + .documentDefault = false, + }, +}; + +StoreConfigT parseStoreConfig(const StoreReference::Params & params) +{ + constexpr auto & defaults = storeConfigDefaults; + constexpr auto & descriptions = storeConfigDescriptions; + + return { + CONFIG_ROW(pathInfoCacheSize), + CONFIG_ROW(isTrusted), + CONFIG_ROW(priority), + CONFIG_ROW(wantMassQuery), + CONFIG_ROW(systemFeatures), + }; +} + + StorePath Store::addToStore( std::string_view name, const SourcePath & path, @@ -432,7 +497,7 @@ StringSet StoreConfig::getDefaultSystemFeatures() return res; } -Store::Store(const Params & params) +Store::Store(const StoreReference::Params & params) : StoreConfig(params) , state({(size_t) pathInfoCacheSize}) { @@ -1267,102 +1332,3 @@ Derivation Store::readInvalidDerivation(const StorePath & drvPath) { return readDerivationCommon(*this, drvPath, false); } } - - -#include "local-store.hh" -#include "uds-remote-store.hh" - - -namespace nix { - -ref openStore(const std::string & uri, - const Store::Params & extraParams) -{ - return openStore(StoreReference::parse(uri, extraParams)); -} - -ref openStore(StoreReference && storeURI) -{ - auto & params = storeURI.params; - - auto store = std::visit(overloaded { - [&](const StoreReference::Auto &) -> std::shared_ptr { - auto stateDir = getOr(params, "state", settings.nixStateDir); - if (access(stateDir.c_str(), R_OK | W_OK) == 0) - return std::make_shared(params); - else if (pathExists(settings.nixDaemonSocketFile)) - return std::make_shared(params); - #if __linux__ - else if (!pathExists(stateDir) - && params.empty() - && !isRootUser() - && !getEnv("NIX_STORE_DIR").has_value() - && !getEnv("NIX_STATE_DIR").has_value()) - { - /* If /nix doesn't exist, there is no daemon socket, and - we're not root, then automatically set up a chroot - store in ~/.local/share/nix/root. */ - auto chrootStore = getDataDir() + "/nix/root"; - if (!pathExists(chrootStore)) { - try { - createDirs(chrootStore); - } catch (SystemError & e) { - return std::make_shared(params); - } - warn("'%s' does not exist, so Nix will use '%s' as a chroot store", stateDir, chrootStore); - } else - debug("'%s' does not exist, so Nix will use '%s' as a chroot store", stateDir, chrootStore); - return std::make_shared("local", chrootStore, params); - } - #endif - else - return std::make_shared(params); - }, - [&](const StoreReference::Specified & g) { - for (auto implem : *Implementations::registered) - if (implem.uriSchemes.count(g.scheme)) - return implem.create(g.scheme, g.authority, params); - - throw Error("don't know how to open Nix store with scheme '%s'", g.scheme); - }, - }, storeURI.variant); - - experimentalFeatureSettings.require(store->experimentalFeature()); - store->warnUnknownSettings(); - store->init(); - - return ref { store }; -} - -std::list> getDefaultSubstituters() -{ - static auto stores([]() { - std::list> stores; - - StringSet done; - - auto addStore = [&](const std::string & uri) { - if (!done.insert(uri).second) return; - try { - stores.push_back(openStore(uri)); - } catch (Error & e) { - logWarning(e.info()); - } - }; - - for (auto uri : settings.substituters.get()) - addStore(uri); - - stores.sort([](ref & a, ref & b) { - return a->priority < b->priority; - }); - - return stores; - } ()); - - return stores; -} - -std::vector * Implementations::registered = 0; - -} diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index a4d2138f66d..11d5d1c61f6 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -9,7 +9,6 @@ #include "lru-cache.hh" #include "sync.hh" #include "globals.hh" -#include "config.hh" #include "path-info.hh" #include "repair-flag.hh" #include "store-dir-config.hh" @@ -41,7 +40,7 @@ namespace nix { * 2. A class `Foo : virtual Store, virtual FooConfig` that contains the * implementation of the store. * - * This class is expected to have a constructor `Foo(const Params & params)` + * This class is expected to have a constructor `Foo(const StoreReference::Params & params)` * that calls `StoreConfig(params)` (otherwise you're gonna encounter an * `assertion failure` when trying to instantiate it). * @@ -97,7 +96,27 @@ struct KeyedBuildResult; typedef std::map> StorePathCAMap; -struct StoreConfig : public StoreDirConfig +template class F> +struct StoreConfigT +{ + F pathInfoCacheSize; + + F isTrusted; + + F priority; + + F wantMassQuery; + + F systemFeatures; +}; + +extern const StoreConfigT storeConfigDefaults; + +extern const StoreConfigT storeConfigDescriptions; + +StoreConfigT parseStoreConfig(const StoreReference::Params &); + +struct StoreConfig : StoreConfigT, StoreDirConfig { using StoreDirConfig::StoreDirConfig; @@ -128,39 +147,6 @@ struct StoreConfig : public StoreDirConfig { return std::nullopt; } - - const Setting pathInfoCacheSize{this, 65536, "path-info-cache-size", - "Size of the in-memory store path metadata cache."}; - - const Setting isTrusted{this, false, "trusted", - R"( - Whether paths from this store can be used as substitutes - even if they are not signed by a key listed in the - [`trusted-public-keys`](@docroot@/command-ref/conf-file.md#conf-trusted-public-keys) - setting. - )"}; - - Setting priority{this, 0, "priority", - R"( - Priority of this store when used as a [substituter](@docroot@/command-ref/conf-file.md#conf-substituters). - A lower value means a higher priority. - )"}; - - Setting wantMassQuery{this, false, "want-mass-query", - R"( - Whether this store can be queried efficiently for path validity when used as a [substituter](@docroot@/command-ref/conf-file.md#conf-substituters). - )"}; - - Setting systemFeatures{this, getDefaultSystemFeatures(), - "system-features", - R"( - Optional [system features](@docroot@/command-ref/conf-file.md#conf-system-features) available on the system this store uses to build derivations. - - Example: `"kvm"` - )", - {}, - // Don't document the machine-specific default value - false}; }; class Store : public std::enable_shared_from_this, public virtual StoreConfig @@ -203,7 +189,7 @@ protected: std::shared_ptr diskCache; - Store(const Params & params); + Store(const StoreReference::Params & params); public: /** @@ -855,74 +841,6 @@ StorePath resolveDerivedPath(Store &, const SingleDerivedPath &, Store * evalSto OutputPathMap resolveDerivedPath(Store &, const DerivedPath::Built &, Store * evalStore = nullptr); -/** - * @return a Store object to access the Nix store denoted by - * ‘uri’ (slight misnomer...). - */ -ref openStore(StoreReference && storeURI); - - -/** - * Opens the store at `uri`, where `uri` is in the format expected by `StoreReference::parse` - - */ -ref openStore(const std::string & uri = settings.storeUri.get(), - const Store::Params & extraParams = Store::Params()); - - -/** - * @return the default substituter stores, defined by the - * ‘substituters’ option and various legacy options. - */ -std::list> getDefaultSubstituters(); - -struct StoreFactory -{ - std::set uriSchemes; - /** - * The `authorityPath` parameter is `/`, or really - * whatever comes after `://` and before `?`. - */ - std::function ( - std::string_view scheme, - std::string_view authorityPath, - const Store::Params & params)> create; - std::function ()> getConfig; -}; - -struct Implementations -{ - static std::vector * registered; - - template - static void add() - { - if (!registered) registered = new std::vector(); - StoreFactory factory{ - .uriSchemes = TConfig::uriSchemes(), - .create = - ([](auto scheme, auto uri, auto & params) - -> std::shared_ptr - { return std::make_shared(scheme, uri, params); }), - .getConfig = - ([]() - -> std::shared_ptr - { return std::make_shared(StringMap({})); }) - }; - registered->push_back(factory); - } -}; - -template -struct RegisterStoreImplementation -{ - RegisterStoreImplementation() - { - Implementations::add(); - } -}; - - /** * Display a set of paths in human-readable form (i.e., between quotes * and separated by commas). diff --git a/src/libstore/store-dir-config.cc b/src/libstore/store-dir-config.cc index 775f678be4a..960f2b88266 100644 --- a/src/libstore/store-dir-config.cc +++ b/src/libstore/store-dir-config.cc @@ -1,18 +1,15 @@ #include "store-dir-config.hh" -#include "config-parse.hh" +#include "config-parse-impl.hh" #include "util.hh" namespace nix { -const StoreDirConfigT storeDirConfigDefaults = { - .storeDir = - { - .value = settings.nixStore, - }, +const StoreDirConfigT storeDirConfigDefaults = { + ._storeDir = {.value = settings.nixStore}, }; -const StoreDirConfigT storeDirConfigDescriptions = { - .storeDir = +const StoreDirConfigT storeDirConfigDescriptions = { + ._storeDir = { .name = "store", .description = R"( @@ -23,18 +20,18 @@ const StoreDirConfigT storeDirConfigDescriptions = { }, }; -StoreDirConfigT parseStoreDirConfig(const StoreReference::Params & params) +StoreDirConfigT parseStoreDirConfig(const StoreReference::Params & params) { constexpr auto & defaults = storeDirConfigDefaults; constexpr auto & descriptions = storeDirConfigDescriptions; return { - CONFIG_ROW(storeDir), + CONFIG_ROW(_storeDir), }; } StoreDirConfig::StoreDirConfig(const StoreReference::Params & params) - : StoreDirConfigT{parseStoreDirConfig(params)} + : StoreDirConfigT{parseStoreDirConfig(params)} { } diff --git a/src/libstore/store-dir-config.hh b/src/libstore/store-dir-config.hh index e2f61a72995..a2717534501 100644 --- a/src/libstore/store-dir-config.hh +++ b/src/libstore/store-dir-config.hh @@ -4,7 +4,7 @@ #include "hash.hh" #include "content-address.hh" #include "store-reference.hh" -#include "config-abstract.hh" +#include "config-parse.hh" #include "globals.hh" #include @@ -22,21 +22,23 @@ MakeError(BadStorePathName, BadStorePath); template class F> struct StoreDirConfigT { - F storeDir; + F _storeDir; }; -extern const StoreDirConfigT storeDirConfigDefaults; +extern const StoreDirConfigT storeDirConfigDefaults; -extern const StoreDirConfigT storeDirConfigDescriptions; +extern const StoreDirConfigT storeDirConfigDescriptions; -StoreDirConfigT parseStoreDirConfig(const StoreReference::Params &); +StoreDirConfigT parseStoreDirConfig(const StoreReference::Params &); -struct StoreDirConfig : StoreDirConfigT +struct StoreDirConfig : StoreDirConfigT { StoreDirConfig(const StoreReference::Params & params); virtual ~StoreDirConfig() = default; + const Path & storeDir = _storeDir.value; + // pure methods StorePath parseStorePath(std::string_view path) const; diff --git a/src/libstore/store-open.hh b/src/libstore/store-open.hh new file mode 100644 index 00000000000..ab6f3cb7739 --- /dev/null +++ b/src/libstore/store-open.hh @@ -0,0 +1,38 @@ +#pragma once +/** + * @file + * + * For opening a store described by an `StoreReference`, which is an "untyped" + * notion which needs to be decoded against a collection of specific + * implementations. + * + * For consumers of the store registration machinery defined in + * `store-registration.hh`. Not needed by store implementation definitions, or + * usages of a given `Store` which will be passed in. + */ + +#include "store-api.hh" + +namespace nix { + +/** + * @return a Store object to access the Nix store denoted by + * ‘uri’ (slight misnomer...). + */ +ref openStore(StoreReference && storeURI); + +/** + * Opens the store at `uri`, where `uri` is in the format expected by `StoreReference::parse` + + */ +ref openStore( + const std::string & uri = settings.storeUri.get(), + const StoreReference::Params & extraParams = StoreReference::Params()); + +/** + * @return the default substituter stores, defined by the + * ‘substituters’ option and various legacy options. + */ +std::list> getDefaultSubstituters(); + +} diff --git a/src/libstore/store-reference.hh b/src/libstore/store-reference.hh index 459cea9c20e..c1066296afe 100644 --- a/src/libstore/store-reference.hh +++ b/src/libstore/store-reference.hh @@ -2,6 +2,7 @@ ///@file #include +#include #include "types.hh" @@ -41,7 +42,7 @@ namespace nix { */ struct StoreReference { - using Params = std::map; + using Params = nlohmann::json::object_t; /** * Special store reference `""` or `"auto"` diff --git a/src/libstore/store-registration.cc b/src/libstore/store-registration.cc new file mode 100644 index 00000000000..85ece9623cf --- /dev/null +++ b/src/libstore/store-registration.cc @@ -0,0 +1,95 @@ +#include "store-registration.hh" +#include "store-open.hh" +#include "local-store.hh" +#include "uds-remote-store.hh" + +namespace nix { + +ref openStore(const std::string & uri, const Store::Params & extraParams) +{ + return openStore(StoreReference::parse(uri, extraParams)); +} + +ref openStore(StoreReference && storeURI) +{ + auto & params = storeURI.params; + + auto store = std::visit( + overloaded{ + [&](const StoreReference::Auto &) -> std::shared_ptr { + auto stateDir = getOr(params, "state", settings.nixStateDir); + if (access(stateDir.c_str(), R_OK | W_OK) == 0) + return std::make_shared(params); + else if (pathExists(settings.nixDaemonSocketFile)) + return std::make_shared(params); +#if __linux__ + else if ( + !pathExists(stateDir) && params.empty() && !isRootUser() && !getEnv("NIX_STORE_DIR").has_value() + && !getEnv("NIX_STATE_DIR").has_value()) { + /* If /nix doesn't exist, there is no daemon socket, and + we're not root, then automatically set up a chroot + store in ~/.local/share/nix/root. */ + auto chrootStore = getDataDir() + "/nix/root"; + if (!pathExists(chrootStore)) { + try { + createDirs(chrootStore); + } catch (SystemError & e) { + return std::make_shared(params); + } + warn("'%s' does not exist, so Nix will use '%s' as a chroot store", stateDir, chrootStore); + } else + debug("'%s' does not exist, so Nix will use '%s' as a chroot store", stateDir, chrootStore); + return std::make_shared("local", chrootStore, params); + } +#endif + else + return std::make_shared(params); + }, + [&](const StoreReference::Specified & g) { + for (auto implem : *Implementations::registered) + if (implem.uriSchemes.count(g.scheme)) + return implem.create(g.scheme, g.authority, params); + + throw Error("don't know how to open Nix store with scheme '%s'", g.scheme); + }, + }, + storeURI.variant); + + experimentalFeatureSettings.require(store->experimentalFeature()); + store->warnUnknownSettings(); + store->init(); + + return ref{store}; +} + +std::vector * Implementations::registered = 0; + +std::list> getDefaultSubstituters() +{ + static auto stores([]() { + std::list> stores; + + StringSet done; + + auto addStore = [&](const std::string & uri) { + if (!done.insert(uri).second) + return; + try { + stores.push_back(openStore(uri)); + } catch (Error & e) { + logWarning(e.info()); + } + }; + + for (auto uri : settings.substituters.get()) + addStore(uri); + + stores.sort([](ref & a, ref & b) { return a->priority < b->priority; }); + + return stores; + }()); + + return stores; +} + +} diff --git a/src/libstore/store-registration.hh b/src/libstore/store-registration.hh new file mode 100644 index 00000000000..29cee23a3fb --- /dev/null +++ b/src/libstore/store-registration.hh @@ -0,0 +1,55 @@ +#pragma once +/** + * @file + * + * Infrastructure for "registering" store implementations. Used by the + * store implementation definitions themselves but not by consumers of + * those implementations. + */ + +#include "store-api.hh" + +namespace nix { + +struct StoreFactory +{ + std::set uriSchemes; + /** + * The `authorityPath` parameter is `/`, or really + * whatever comes after `://` and before `?`. + */ + std::function( + std::string_view scheme, std::string_view authorityPath, const Store::Params & params)> + create; + std::function()> getConfig; +}; + +struct Implementations +{ + static std::vector * registered; + + template + static void add() + { + if (!registered) + registered = new std::vector(); + StoreFactory factory{ + .uriSchemes = TConfig::uriSchemes(), + .create = ([](auto scheme, auto uri, auto & params) -> std::shared_ptr { + return std::make_shared(scheme, uri, params); + }), + .getConfig = ([]() -> std::shared_ptr { return std::make_shared(StringMap({})); })}; + registered->push_back(factory); + } +}; + +template +struct RegisterStoreImplementation +{ + RegisterStoreImplementation() + { + Implementations::add(); + } +}; + +} diff --git a/src/libutil/config-abstract.hh b/src/libutil/config-abstract.hh index 081478dc195..68f20605e06 100644 --- a/src/libutil/config-abstract.hh +++ b/src/libutil/config-abstract.hh @@ -1,7 +1,7 @@ #pragma once ///@type -namespace nix { +namespace nix::config { template struct JustValue @@ -22,11 +22,4 @@ struct JustValue } }; -template -struct SettingInfo -{ - std::string name; - std::string description; -}; - } From f0a1c130a1a5543d09baf731f59845f8cff45b9f Mon Sep 17 00:00:00 2001 From: RTUnreal <22859658+RTUnreal@users.noreply.github.com> Date: Wed, 17 Jul 2024 21:08:33 +0200 Subject: [PATCH 46/54] doc: add example usage for Gitea in tarball fetcher (#11116) Co-authored-by: Valentin Gagarin --- doc/manual/src/protocols/tarball-fetcher.md | 26 +++++++++++++++++++++ src/nix/flake.md | 2 ++ 2 files changed, 28 insertions(+) diff --git a/doc/manual/src/protocols/tarball-fetcher.md b/doc/manual/src/protocols/tarball-fetcher.md index 24ec7ae14aa..5cff05d66ce 100644 --- a/doc/manual/src/protocols/tarball-fetcher.md +++ b/doc/manual/src/protocols/tarball-fetcher.md @@ -41,4 +41,30 @@ Link: ///archive/.tar.gz +``` + +> **Example** +> +> +> ```nix +> # flake.nix +> { +> inputs = { +> foo.url = "https://gitea.example.org/some-person/some-flake/archive/main.tar.gz"; +> bar.url = "https://gitea.example.org/some-other-person/other-flake/archive/442793d9ec0584f6a6e82fa253850c8085bb150a.tar.gz"; +> qux = { +> url = "https://forgejo.example.org/another-person/some-non-flake-repo/archive/development.tar.gz"; +> flake = false; +> }; +> }; +> outputs = { foo, bar, qux }: { /* ... */ }; +> } +``` + [Nix Archive]: @docroot@/store/file-system-object/content-address.md#serial-nix-archive diff --git a/src/nix/flake.md b/src/nix/flake.md index 2f43d02640d..46d5a3867bf 100644 --- a/src/nix/flake.md +++ b/src/nix/flake.md @@ -259,6 +259,8 @@ Currently the `type` attribute can be one of the following: `.tgz`, `.tar.gz`, `.tar.xz`, `.tar.bz2` or `.tar.zst`), then the `tarball+` can be dropped. + This can also be used to set the location of gitea/forgejo branches. [See here](@docroot@/protocols/tarball-fetcher.md#gitea-and-forgejo-support) + * `file`: Plain files or directory tarballs, either over http(s) or from the local disk. From 7b135d5e68707e1ba22d1d79ca702e9538cdedd6 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Wed, 17 Jul 2024 12:18:13 -0700 Subject: [PATCH 47/54] Comment out tests --- tests/unit/libstore/local-store.cc | 4 ++++ tests/unit/libstore/ssh-store.cc | 4 ++++ tests/unit/libstore/uds-remote-store.cc | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/tests/unit/libstore/local-store.cc b/tests/unit/libstore/local-store.cc index e85cf0b83f0..2ab9f43cc19 100644 --- a/tests/unit/libstore/local-store.cc +++ b/tests/unit/libstore/local-store.cc @@ -1,3 +1,6 @@ +//FIXME: Odd failures for templates that are causing the PR to break +// for now with discussion with @Ericson2314 to comment out. +#if 0 #include #include "local-store.hh" @@ -34,3 +37,4 @@ TEST(LocalStore, constructConfig_rootPath) } } // namespace nix +#endif \ No newline at end of file diff --git a/tests/unit/libstore/ssh-store.cc b/tests/unit/libstore/ssh-store.cc index a1fcef6b863..f0559c1fea2 100644 --- a/tests/unit/libstore/ssh-store.cc +++ b/tests/unit/libstore/ssh-store.cc @@ -1,3 +1,6 @@ +//FIXME: Odd failures for templates that are causing the PR to break +// for now with discussion with @Ericson2314 to comment out. +#if 0 #include #include "ssh-store.hh" @@ -49,3 +52,4 @@ TEST(MountedSSHStore, constructConfig) } } +#endif \ No newline at end of file diff --git a/tests/unit/libstore/uds-remote-store.cc b/tests/unit/libstore/uds-remote-store.cc index 4bd3bc18f5f..d5f495428cd 100644 --- a/tests/unit/libstore/uds-remote-store.cc +++ b/tests/unit/libstore/uds-remote-store.cc @@ -1,3 +1,6 @@ +//FIXME: Odd failures for templates that are causing the PR to break +// for now with discussion with @Ericson2314 to comment out. +#if 0 #include #include "uds-remote-store.hh" @@ -17,3 +20,4 @@ TEST(UDSRemoteStore, constructConfigWrongScheme) } } // namespace nix +#endif \ No newline at end of file From 596a682a87b98b079c729b84b1efc3925c9570fd Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 17 Jul 2024 16:18:29 -0400 Subject: [PATCH 48/54] Dummy store compiles --- src/libstore/dummy-store.cc | 35 ++++++++++++++++++++++-------- src/libstore/store-api.cc | 10 ++++----- src/libstore/store-api.hh | 22 ++++++++++++++++--- src/libstore/store-registration.hh | 23 ++++++++++---------- 4 files changed, 62 insertions(+), 28 deletions(-) diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index c1e871e9384..4d0bbec2975 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -1,12 +1,22 @@ #include "store-api.hh" +#include "store-registration.hh" #include "callback.hh" namespace nix { +struct DummyStoreConfigDescription : virtual Store::Config::Description +{ + DummyStoreConfigDescription() : StoreConfigDescription{Store::Config::schema} {} +}; + struct DummyStoreConfig : virtual StoreConfig { using StoreConfig::StoreConfig; - DummyStoreConfig(std::string_view scheme, std::string_view authority, const Params & params) + using Description = DummyStoreConfigDescription; + + static const Description schema; + + DummyStoreConfig(std::string_view scheme, std::string_view authority, const StoreReference::Params & params) : StoreConfig(params) { if (!authority.empty()) @@ -25,18 +35,20 @@ struct DummyStoreConfig : virtual StoreConfig { static std::set uriSchemes() { return {"dummy"}; } + + std::shared_ptr open() override; }; +decltype(DummyStoreConfig::schema) DummyStoreConfig::schema{}; + struct DummyStore : public virtual DummyStoreConfig, public virtual Store { - DummyStore(std::string_view scheme, std::string_view authority, const Params & params) - : StoreConfig(params) - , DummyStoreConfig(scheme, authority, params) - , Store(params) - { } + using Config = DummyStoreConfig; - DummyStore(const Params & params) - : DummyStore("dummy", "", params) + DummyStore(const DummyStoreConfig & config) + : StoreConfig(config) + , DummyStoreConfig(config) + , Store(config) { } std::string getUri() override @@ -86,6 +98,11 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store { unsupported("getFSAccessor"); } }; -static RegisterStoreImplementation regDummyStore; +std::shared_ptr DummyStoreConfig::open() +{ + return std::make_shared(*this); +} + +static RegisterStoreImplementation regDummyStore; } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index bf47026ed47..381bb3ed6c4 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -197,7 +197,7 @@ const StoreConfigT storeConfigDefaults = { .systemFeatures = { .value = StoreConfig::getDefaultSystemFeatures() }, }; -const StoreConfigT storeConfigDescriptions = { +decltype(StoreConfig::schema) StoreConfig::schema = {{ .pathInfoCacheSize = { .name = "path-info-cache-size", .description = "Size of the in-memory store path metadata cache.", @@ -236,12 +236,12 @@ const StoreConfigT storeConfigDescriptions = { // unsuitable to be rendered in the documentation. .documentDefault = false, }, -}; +}}; StoreConfigT parseStoreConfig(const StoreReference::Params & params) { constexpr auto & defaults = storeConfigDefaults; - constexpr auto & descriptions = storeConfigDescriptions; + constexpr auto & descriptions = StoreConfig::schema; return { CONFIG_ROW(pathInfoCacheSize), @@ -497,8 +497,8 @@ StringSet StoreConfig::getDefaultSystemFeatures() return res; } -Store::Store(const StoreReference::Params & params) - : StoreConfig(params) +Store::Store(const Store::Config & config) + : Store::Config(config) , state({(size_t) pathInfoCacheSize}) { assertLibStoreInitialized(); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 11d5d1c61f6..924f037af24 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -112,16 +112,22 @@ struct StoreConfigT extern const StoreConfigT storeConfigDefaults; -extern const StoreConfigT storeConfigDescriptions; - StoreConfigT parseStoreConfig(const StoreReference::Params &); +struct StoreConfigDescription : StoreConfigT, StoreDirConfigT +{ +}; + struct StoreConfig : StoreConfigT, StoreDirConfig { using StoreDirConfig::StoreDirConfig; StoreConfig() = delete; + using Description = StoreConfigDescription; + + static const Description schema; + static StringSet getDefaultSystemFeatures(); virtual ~StoreConfig() { } @@ -147,10 +153,20 @@ struct StoreConfig : StoreConfigT, StoreDirConfig { return std::nullopt; } + + /** + * Open a store of the type corresponding to this configuration + * type. + */ + virtual std::shared_ptr open() = 0; }; class Store : public std::enable_shared_from_this, public virtual StoreConfig { +public: + + using Config = StoreConfig; + protected: struct PathInfoCacheValue { @@ -189,7 +205,7 @@ protected: std::shared_ptr diskCache; - Store(const StoreReference::Params & params); + Store(const Store::Config & config); public: /** diff --git a/src/libstore/store-registration.hh b/src/libstore/store-registration.hh index 29cee23a3fb..9bc1869eae1 100644 --- a/src/libstore/store-registration.hh +++ b/src/libstore/store-registration.hh @@ -18,37 +18,38 @@ struct StoreFactory * The `authorityPath` parameter is `/`, or really * whatever comes after `://` and before `?`. */ - std::function( - std::string_view scheme, std::string_view authorityPath, const Store::Params & params)> - create; - std::function()> getConfig; + std::function( + std::string_view scheme, std::string_view authorityPath, const StoreReference::Params & params)> + parseConfig; + const StoreConfigDescription & configSchema; }; struct Implementations { static std::vector * registered; - template + template static void add() { if (!registered) registered = new std::vector(); StoreFactory factory{ - .uriSchemes = TConfig::uriSchemes(), - .create = ([](auto scheme, auto uri, auto & params) -> std::shared_ptr { - return std::make_shared(scheme, uri, params); + .uriSchemes = T::Config::uriSchemes(), + .parseConfig = ([](auto scheme, auto uri, auto & params) -> std::shared_ptr { + return std::make_shared(scheme, uri, params); }), - .getConfig = ([]() -> std::shared_ptr { return std::make_shared(StringMap({})); })}; + .configSchema = T::Config::schema, + }; registered->push_back(factory); } }; -template +template struct RegisterStoreImplementation { RegisterStoreImplementation() { - Implementations::add(); + Implementations::add(); } }; From 34f876f350f6cb366580dd8755bbcaec866d8535 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 17 Jul 2024 16:19:57 -0400 Subject: [PATCH 49/54] Format, CI hopefully happy now --- tests/unit/libstore/local-store.cc | 14 +++++++------- tests/unit/libstore/ssh-store.cc | 8 ++++---- tests/unit/libstore/uds-remote-store.cc | 8 ++++---- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/unit/libstore/local-store.cc b/tests/unit/libstore/local-store.cc index 2ab9f43cc19..abc3ea7963f 100644 --- a/tests/unit/libstore/local-store.cc +++ b/tests/unit/libstore/local-store.cc @@ -1,15 +1,15 @@ -//FIXME: Odd failures for templates that are causing the PR to break +// FIXME: Odd failures for templates that are causing the PR to break // for now with discussion with @Ericson2314 to comment out. #if 0 -#include +# include -#include "local-store.hh" +# include "local-store.hh" // Needed for template specialisations. This is not good! When we // overhaul how store configs work, this should be fixed. -#include "args.hh" -#include "config-impl.hh" -#include "abstract-setting-to-json.hh" +# include "args.hh" +# include "config-impl.hh" +# include "abstract-setting-to-json.hh" namespace nix { @@ -37,4 +37,4 @@ TEST(LocalStore, constructConfig_rootPath) } } // namespace nix -#endif \ No newline at end of file +#endif diff --git a/tests/unit/libstore/ssh-store.cc b/tests/unit/libstore/ssh-store.cc index f0559c1fea2..b853a5f1fb9 100644 --- a/tests/unit/libstore/ssh-store.cc +++ b/tests/unit/libstore/ssh-store.cc @@ -1,9 +1,9 @@ -//FIXME: Odd failures for templates that are causing the PR to break +// FIXME: Odd failures for templates that are causing the PR to break // for now with discussion with @Ericson2314 to comment out. #if 0 -#include +# include -#include "ssh-store.hh" +# include "ssh-store.hh" namespace nix { @@ -52,4 +52,4 @@ TEST(MountedSSHStore, constructConfig) } } -#endif \ No newline at end of file +#endif diff --git a/tests/unit/libstore/uds-remote-store.cc b/tests/unit/libstore/uds-remote-store.cc index d5f495428cd..5ccb208714f 100644 --- a/tests/unit/libstore/uds-remote-store.cc +++ b/tests/unit/libstore/uds-remote-store.cc @@ -1,9 +1,9 @@ -//FIXME: Odd failures for templates that are causing the PR to break +// FIXME: Odd failures for templates that are causing the PR to break // for now with discussion with @Ericson2314 to comment out. #if 0 -#include +# include -#include "uds-remote-store.hh" +# include "uds-remote-store.hh" namespace nix { @@ -20,4 +20,4 @@ TEST(UDSRemoteStore, constructConfigWrongScheme) } } // namespace nix -#endif \ No newline at end of file +#endif From 57399bfc0e438d699700c6a4f2b6b63b157bcd2a Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Wed, 17 Jul 2024 23:32:27 -0400 Subject: [PATCH 50/54] Refactor unix domain socket store config (#11109) Following what is outlined in #10766 refactor the uds-remote-store such that the member variables (state) don't live in the store itself but in the config object. Additionally, the config object includes a new necessary constructor that takes a scheme & authority. Tests are commented out because of linking errors with the current config system. When there is a new config system we can reenable them. Co-authored-by: John Ericson --- src/libstore/dummy-store.cc | 20 ++-- src/libstore/http-binary-cache-store.cc | 50 ++++----- src/libstore/http-binary-cache-store.hh | 21 ++++ src/libstore/local-binary-cache-store.cc | 36 +++---- src/libstore/local-binary-cache-store.hh | 21 ++++ src/libstore/local-fs-store.cc | 14 +++ src/libstore/local-fs-store.hh | 9 ++ src/libstore/local-overlay-store.cc | 6 +- src/libstore/local-overlay-store.hh | 21 ++-- src/libstore/local-store.cc | 33 +++--- src/libstore/local-store.hh | 5 + src/libstore/meson.build | 2 + src/libstore/s3-binary-cache-store.cc | 100 +++++------------- src/libstore/s3-binary-cache-store.hh | 90 ++++++++++++++++ src/libstore/ssh-store.cc | 55 ++++------ src/libstore/ssh-store.hh | 23 ++++ src/libstore/uds-remote-store.cc | 54 ++++++---- src/libstore/uds-remote-store.hh | 41 +++++-- .../unit/libstore/http-binary-cache-store.cc | 21 ++++ .../unit/libstore/local-binary-cache-store.cc | 14 +++ tests/unit/libstore/local-overlay-store.cc | 34 ++++++ tests/unit/libstore/local-store.cc | 40 +++++++ tests/unit/libstore/meson.build | 6 ++ tests/unit/libstore/s3-binary-cache-store.cc | 18 ++++ tests/unit/libstore/ssh-store.cc | 35 +++++- tests/unit/libstore/uds-remote-store.cc | 23 ++++ 26 files changed, 574 insertions(+), 218 deletions(-) create mode 100644 src/libstore/http-binary-cache-store.hh create mode 100644 src/libstore/local-binary-cache-store.hh create mode 100644 tests/unit/libstore/http-binary-cache-store.cc create mode 100644 tests/unit/libstore/local-binary-cache-store.cc create mode 100644 tests/unit/libstore/local-overlay-store.cc create mode 100644 tests/unit/libstore/local-store.cc create mode 100644 tests/unit/libstore/s3-binary-cache-store.cc create mode 100644 tests/unit/libstore/uds-remote-store.cc diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 17ebaace6a8..af0dd909214 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -6,6 +6,13 @@ namespace nix { struct DummyStoreConfig : virtual StoreConfig { using StoreConfig::StoreConfig; + DummyStoreConfig(std::string_view scheme, std::string_view authority, const Params & params) + : StoreConfig(params) + { + if (!authority.empty()) + throw UsageError("`%s` store URIs must not contain an authority part %s", scheme, authority); + } + const std::string name() override { return "Dummy Store"; } std::string doc() override @@ -19,18 +26,15 @@ struct DummyStoreConfig : virtual StoreConfig { struct DummyStore : public virtual DummyStoreConfig, public virtual Store { DummyStore(std::string_view scheme, std::string_view authority, const Params & params) - : DummyStore(params) - { - if (!authority.empty()) - throw UsageError("`%s` store URIs must not contain an authority part %s", scheme, authority); - } - - DummyStore(const Params & params) : StoreConfig(params) - , DummyStoreConfig(params) + , DummyStoreConfig(scheme, authority, params) , Store(params) { } + DummyStore(const Params & params) + : DummyStore("dummy", "", params) + { } + std::string getUri() override { return *uriSchemes().begin(); diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index 3328caef9c4..49ec26b9f93 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -1,4 +1,4 @@ -#include "binary-cache-store.hh" +#include "http-binary-cache-store.hh" #include "filetransfer.hh" #include "globals.hh" #include "nar-info-disk-cache.hh" @@ -8,26 +8,37 @@ namespace nix { MakeError(UploadToHTTP, Error); -struct HttpBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig + +HttpBinaryCacheStoreConfig::HttpBinaryCacheStoreConfig( + std::string_view scheme, + std::string_view _cacheUri, + const Params & params) + : StoreConfig(params) + , BinaryCacheStoreConfig(params) + , cacheUri( + std::string { scheme } + + "://" + + (!_cacheUri.empty() + ? _cacheUri + : throw UsageError("`%s` Store requires a non-empty authority in Store URL", scheme))) { - using BinaryCacheStoreConfig::BinaryCacheStoreConfig; + while (!cacheUri.empty() && cacheUri.back() == '/') + cacheUri.pop_back(); +} - const std::string name() override { return "HTTP Binary Cache Store"; } - std::string doc() override - { - return - #include "http-binary-cache-store.md" - ; - } -}; +std::string HttpBinaryCacheStoreConfig::doc() +{ + return + #include "http-binary-cache-store.md" + ; +} + class HttpBinaryCacheStore : public virtual HttpBinaryCacheStoreConfig, public virtual BinaryCacheStore { private: - Path cacheUri; - struct State { bool enabled = true; @@ -40,23 +51,14 @@ class HttpBinaryCacheStore : public virtual HttpBinaryCacheStoreConfig, public v HttpBinaryCacheStore( std::string_view scheme, - PathView _cacheUri, + PathView cacheUri, const Params & params) : StoreConfig(params) , BinaryCacheStoreConfig(params) - , HttpBinaryCacheStoreConfig(params) + , HttpBinaryCacheStoreConfig(scheme, cacheUri, params) , Store(params) , BinaryCacheStore(params) - , cacheUri( - std::string { scheme } - + "://" - + (!_cacheUri.empty() - ? _cacheUri - : throw UsageError("`%s` Store requires a non-empty authority in Store URL", scheme))) { - while (!cacheUri.empty() && cacheUri.back() == '/') - cacheUri.pop_back(); - diskCache = getNarInfoDiskCache(); } diff --git a/src/libstore/http-binary-cache-store.hh b/src/libstore/http-binary-cache-store.hh new file mode 100644 index 00000000000..230e52b3373 --- /dev/null +++ b/src/libstore/http-binary-cache-store.hh @@ -0,0 +1,21 @@ +#include "binary-cache-store.hh" + +namespace nix { + +struct HttpBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig +{ + using BinaryCacheStoreConfig::BinaryCacheStoreConfig; + + HttpBinaryCacheStoreConfig(std::string_view scheme, std::string_view _cacheUri, const Params & params); + + Path cacheUri; + + const std::string name() override + { + return "HTTP Binary Cache Store"; + } + + std::string doc() override; +}; + +} diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index aa2efdb8fb6..106663132c1 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -1,4 +1,4 @@ -#include "binary-cache-store.hh" +#include "local-binary-cache-store.hh" #include "globals.hh" #include "nar-info-disk-cache.hh" #include "signals.hh" @@ -7,28 +7,27 @@ namespace nix { -struct LocalBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig +LocalBinaryCacheStoreConfig::LocalBinaryCacheStoreConfig( + std::string_view scheme, + PathView binaryCacheDir, + const Params & params) + : StoreConfig(params) + , BinaryCacheStoreConfig(params) + , binaryCacheDir(binaryCacheDir) { - using BinaryCacheStoreConfig::BinaryCacheStoreConfig; - - const std::string name() override { return "Local Binary Cache Store"; } +} - std::string doc() override - { - return - #include "local-binary-cache-store.md" - ; - } -}; -class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public virtual BinaryCacheStore +std::string LocalBinaryCacheStoreConfig::doc() { -private: - - Path binaryCacheDir; + return + #include "local-binary-cache-store.md" + ; +} -public: +struct LocalBinaryCacheStore : virtual LocalBinaryCacheStoreConfig, virtual BinaryCacheStore +{ /** * @param binaryCacheDir `file://` is a short-hand for `file:///` * for now. @@ -39,10 +38,9 @@ class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public const Params & params) : StoreConfig(params) , BinaryCacheStoreConfig(params) - , LocalBinaryCacheStoreConfig(params) + , LocalBinaryCacheStoreConfig(scheme, binaryCacheDir, params) , Store(params) , BinaryCacheStore(params) - , binaryCacheDir(binaryCacheDir) { } diff --git a/src/libstore/local-binary-cache-store.hh b/src/libstore/local-binary-cache-store.hh new file mode 100644 index 00000000000..7701eb38b99 --- /dev/null +++ b/src/libstore/local-binary-cache-store.hh @@ -0,0 +1,21 @@ +#include "binary-cache-store.hh" + +namespace nix { + +struct LocalBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig +{ + using BinaryCacheStoreConfig::BinaryCacheStoreConfig; + + LocalBinaryCacheStoreConfig(std::string_view scheme, PathView binaryCacheDir, const Params & params); + + Path binaryCacheDir; + + const std::string name() override + { + return "Local Binary Cache Store"; + } + + std::string doc() override; +}; + +} diff --git a/src/libstore/local-fs-store.cc b/src/libstore/local-fs-store.cc index 843c0d28881..5449b20eb3b 100644 --- a/src/libstore/local-fs-store.cc +++ b/src/libstore/local-fs-store.cc @@ -8,6 +8,20 @@ namespace nix { +LocalFSStoreConfig::LocalFSStoreConfig(PathView rootDir, const Params & params) + : StoreConfig(params) + // Default `?root` from `rootDir` if non set + // FIXME don't duplicate description once we don't have root setting + , rootDir{ + this, + !rootDir.empty() && params.count("root") == 0 + ? (std::optional{rootDir}) + : std::nullopt, + "root", + "Directory prefixed to all other paths."} +{ +} + LocalFSStore::LocalFSStore(const Params & params) : Store(params) { diff --git a/src/libstore/local-fs-store.hh b/src/libstore/local-fs-store.hh index 8fb08120012..9bb569f0b25 100644 --- a/src/libstore/local-fs-store.hh +++ b/src/libstore/local-fs-store.hh @@ -11,6 +11,15 @@ struct LocalFSStoreConfig : virtual StoreConfig { using StoreConfig::StoreConfig; + /** + * Used to override the `root` settings. Can't be done via modifying + * `params` reliably because this parameter is unused except for + * passing to base class constructors. + * + * @todo Make this less error-prone with new store settings system. + */ + LocalFSStoreConfig(PathView path, const Params & params); + const OptionalPathSetting rootDir{this, std::nullopt, "root", "Directory prefixed to all other paths."}; diff --git a/src/libstore/local-overlay-store.cc b/src/libstore/local-overlay-store.cc index 598415db837..ec2c5f4e936 100644 --- a/src/libstore/local-overlay-store.cc +++ b/src/libstore/local-overlay-store.cc @@ -18,11 +18,11 @@ Path LocalOverlayStoreConfig::toUpperPath(const StorePath & path) { return upperLayer + "/" + path.to_string(); } -LocalOverlayStore::LocalOverlayStore(const Params & params) +LocalOverlayStore::LocalOverlayStore(std::string_view scheme, PathView path, const Params & params) : StoreConfig(params) - , LocalFSStoreConfig(params) + , LocalFSStoreConfig(path, params) , LocalStoreConfig(params) - , LocalOverlayStoreConfig(params) + , LocalOverlayStoreConfig(scheme, path, params) , Store(params) , LocalFSStore(params) , LocalStore(params) diff --git a/src/libstore/local-overlay-store.hh b/src/libstore/local-overlay-store.hh index 35a30101391..a3f15e484f0 100644 --- a/src/libstore/local-overlay-store.hh +++ b/src/libstore/local-overlay-store.hh @@ -8,11 +8,16 @@ namespace nix { struct LocalOverlayStoreConfig : virtual LocalStoreConfig { LocalOverlayStoreConfig(const StringMap & params) - : StoreConfig(params) - , LocalFSStoreConfig(params) - , LocalStoreConfig(params) + : LocalOverlayStoreConfig("local-overlay", "", params) { } + LocalOverlayStoreConfig(std::string_view scheme, PathView path, const Params & params) + : StoreConfig(params) + , LocalFSStoreConfig(path, params) + , LocalStoreConfig(scheme, path, params) + { + } + const Setting lowerStoreUri{(StoreConfig*) this, "", "lower-store", R"( [Store URL](@docroot@/command-ref/new-cli/nix3-help-stores.md#store-url-format) @@ -90,15 +95,13 @@ class LocalOverlayStore : public virtual LocalOverlayStoreConfig, public virtual ref lowerStore; public: - LocalOverlayStore(const Params & params); - - LocalOverlayStore(std::string_view scheme, PathView path, const Params & params) - : LocalOverlayStore(params) + LocalOverlayStore(const Params & params) + : LocalOverlayStore("local-overlay", "", params) { - if (!path.empty()) - throw UsageError("local-overlay:// store url doesn't support path part, only scheme and query params"); } + LocalOverlayStore(std::string_view scheme, PathView path, const Params & params); + static std::set uriSchemes() { return { "local-overlay" }; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 82b70ff2135..819cee34532 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -56,6 +56,15 @@ namespace nix { +LocalStoreConfig::LocalStoreConfig( + std::string_view scheme, + std::string_view authority, + const Params & params) + : StoreConfig(params) + , LocalFSStoreConfig(authority, params) +{ +} + std::string LocalStoreConfig::doc() { return @@ -183,10 +192,13 @@ void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd) } } -LocalStore::LocalStore(const Params & params) +LocalStore::LocalStore( + std::string_view scheme, + PathView path, + const Params & params) : StoreConfig(params) - , LocalFSStoreConfig(params) - , LocalStoreConfig(params) + , LocalFSStoreConfig(path, params) + , LocalStoreConfig(scheme, path, params) , Store(params) , LocalFSStore(params) , dbDir(stateDir + "/db") @@ -465,19 +477,8 @@ LocalStore::LocalStore(const Params & params) } -LocalStore::LocalStore( - std::string_view scheme, - PathView path, - const Params & _params) - : LocalStore([&]{ - // Default `?root` from `path` if non set - if (!path.empty() && _params.count("root") == 0) { - auto params = _params; - params.insert_or_assign("root", std::string { path }); - return params; - } - return _params; - }()) +LocalStore::LocalStore(const Params & params) + : LocalStore("local", "", params) { } diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index b0a0def9aae..026729bf24c 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -38,6 +38,11 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig { using LocalFSStoreConfig::LocalFSStoreConfig; + LocalStoreConfig( + std::string_view scheme, + std::string_view authority, + const Params & params); + Setting requireSigs{this, settings.requireSigs, "require-sigs", diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 297bf71870c..29ee95b75bc 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -243,10 +243,12 @@ headers = [config_h] + files( 'filetransfer.hh', 'gc-store.hh', 'globals.hh', + 'http-binary-cache-store.hh', 'indirect-root-store.hh', 'keys.hh', 'legacy-ssh-store.hh', 'length-prefixed-protocol-helper.hh', + 'local-binary-cache-store.hh', 'local-fs-store.hh', 'local-overlay-store.hh', 'local-store.hh', diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 27013c0f17a..d865684d7aa 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -1,5 +1,7 @@ #if ENABLE_S3 +#include + #include "s3.hh" #include "s3-binary-cache-store.hh" #include "nar-info.hh" @@ -190,76 +192,31 @@ S3BinaryCacheStore::S3BinaryCacheStore(const Params & params) , BinaryCacheStore(params) { } -struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig + +S3BinaryCacheStoreConfig::S3BinaryCacheStoreConfig( + std::string_view uriScheme, + std::string_view bucketName, + const Params & params) + : StoreConfig(params) + , BinaryCacheStoreConfig(params) + , bucketName(bucketName) { - using BinaryCacheStoreConfig::BinaryCacheStoreConfig; - - const Setting profile{this, "", "profile", - R"( - The name of the AWS configuration profile to use. By default - Nix will use the `default` profile. - )"}; - - const Setting region{this, Aws::Region::US_EAST_1, "region", - R"( - The region of the S3 bucket. If your bucket is not in - `us–east-1`, you should always explicitly specify the region - parameter. - )"}; - - const Setting scheme{this, "", "scheme", - R"( - The scheme used for S3 requests, `https` (default) or `http`. This - option allows you to disable HTTPS for binary caches which don't - support it. - - > **Note** - > - > HTTPS should be used if the cache might contain sensitive - > information. - )"}; - - const Setting endpoint{this, "", "endpoint", - R"( - The URL of the endpoint of an S3-compatible service such as MinIO. - Do not specify this setting if you're using Amazon S3. - - > **Note** - > - > This endpoint must support HTTPS and will use path-based - > addressing instead of virtual host based addressing. - )"}; - - const Setting narinfoCompression{this, "", "narinfo-compression", - "Compression method for `.narinfo` files."}; - - const Setting lsCompression{this, "", "ls-compression", - "Compression method for `.ls` files."}; - - const Setting logCompression{this, "", "log-compression", - R"( - Compression method for `log/*` files. It is recommended to - use a compression method supported by most web browsers - (e.g. `brotli`). - )"}; - - const Setting multipartUpload{ - this, false, "multipart-upload", - "Whether to use multi-part uploads."}; - - const Setting bufferSize{ - this, 5 * 1024 * 1024, "buffer-size", - "Size (in bytes) of each part in multi-part uploads."}; - - const std::string name() override { return "S3 Binary Cache Store"; } - - std::string doc() override - { - return - #include "s3-binary-cache-store.md" - ; - } -}; + // Don't want to use use AWS SDK in header, so we check the default + // here. TODO do this better after we overhaul the store settings + // system. + assert(std::string{defaultRegion} == std::string{Aws::Region::US_EAST_1}); + + if (bucketName.empty()) + throw UsageError("`%s` store requires a bucket name in its Store URI", uriScheme); +} + +std::string S3BinaryCacheStoreConfig::doc() +{ + return + #include "s3-binary-cache-store.md" + ; +} + struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual S3BinaryCacheStore { @@ -275,15 +232,12 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual const Params & params) : StoreConfig(params) , BinaryCacheStoreConfig(params) - , S3BinaryCacheStoreConfig(params) + , S3BinaryCacheStoreConfig(uriScheme, bucketName, params) , Store(params) , BinaryCacheStore(params) , S3BinaryCacheStore(params) - , bucketName(bucketName) , s3Helper(profile, region, scheme, endpoint) { - if (bucketName.empty()) - throw UsageError("`%s` store requires a bucket name in its Store URI", uriScheme); diskCache = getNarInfoDiskCache(); } diff --git a/src/libstore/s3-binary-cache-store.hh b/src/libstore/s3-binary-cache-store.hh index c62ea5147f3..bca5196cde0 100644 --- a/src/libstore/s3-binary-cache-store.hh +++ b/src/libstore/s3-binary-cache-store.hh @@ -7,6 +7,96 @@ namespace nix { +struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig +{ + std::string bucketName; + + using BinaryCacheStoreConfig::BinaryCacheStoreConfig; + + S3BinaryCacheStoreConfig(std::string_view uriScheme, std::string_view bucketName, const Params & params); + + const Setting profile{ + this, + "", + "profile", + R"( + The name of the AWS configuration profile to use. By default + Nix will use the `default` profile. + )"}; + +protected: + + constexpr static const char * defaultRegion = "us-east-1"; + +public: + + const Setting region{ + this, + defaultRegion, + "region", + R"( + The region of the S3 bucket. If your bucket is not in + `us–east-1`, you should always explicitly specify the region + parameter. + )"}; + + const Setting scheme{ + this, + "", + "scheme", + R"( + The scheme used for S3 requests, `https` (default) or `http`. This + option allows you to disable HTTPS for binary caches which don't + support it. + + > **Note** + > + > HTTPS should be used if the cache might contain sensitive + > information. + )"}; + + const Setting endpoint{ + this, + "", + "endpoint", + R"( + The URL of the endpoint of an S3-compatible service such as MinIO. + Do not specify this setting if you're using Amazon S3. + + > **Note** + > + > This endpoint must support HTTPS and will use path-based + > addressing instead of virtual host based addressing. + )"}; + + const Setting narinfoCompression{ + this, "", "narinfo-compression", "Compression method for `.narinfo` files."}; + + const Setting lsCompression{this, "", "ls-compression", "Compression method for `.ls` files."}; + + const Setting logCompression{ + this, + "", + "log-compression", + R"( + Compression method for `log/*` files. It is recommended to + use a compression method supported by most web browsers + (e.g. `brotli`). + )"}; + + const Setting multipartUpload{this, false, "multipart-upload", "Whether to use multi-part uploads."}; + + const Setting bufferSize{ + this, 5 * 1024 * 1024, "buffer-size", "Size (in bytes) of each part in multi-part uploads."}; + + const std::string name() override + { + return "S3 Binary Cache Store"; + } + + std::string doc() override; +}; + class S3BinaryCacheStore : public virtual BinaryCacheStore { protected: diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index b21c22d7e08..d63995bf3bc 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -89,43 +89,32 @@ class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore }; }; -struct MountedSSHStoreConfig : virtual SSHStoreConfig, virtual LocalFSStoreConfig -{ - using SSHStoreConfig::SSHStoreConfig; - using LocalFSStoreConfig::LocalFSStoreConfig; - - MountedSSHStoreConfig(StringMap params) - : StoreConfig(params) - , RemoteStoreConfig(params) - , CommonSSHStoreConfig(params) - , SSHStoreConfig(params) - , LocalFSStoreConfig(params) - { - } - MountedSSHStoreConfig(std::string_view scheme, std::string_view host, StringMap params) - : StoreConfig(params) - , RemoteStoreConfig(params) - , CommonSSHStoreConfig(scheme, host, params) - , SSHStoreConfig(params) - , LocalFSStoreConfig(params) - { - } +MountedSSHStoreConfig::MountedSSHStoreConfig(StringMap params) + : StoreConfig(params) + , RemoteStoreConfig(params) + , CommonSSHStoreConfig(params) + , SSHStoreConfig(params) + , LocalFSStoreConfig(params) +{ +} - const std::string name() override { return "Experimental SSH Store with filesystem mounted"; } +MountedSSHStoreConfig::MountedSSHStoreConfig(std::string_view scheme, std::string_view host, StringMap params) + : StoreConfig(params) + , RemoteStoreConfig(params) + , CommonSSHStoreConfig(scheme, host, params) + , SSHStoreConfig(params) + , LocalFSStoreConfig(params) +{ +} - std::string doc() override - { - return - #include "mounted-ssh-store.md" - ; - } +std::string MountedSSHStoreConfig::doc() +{ + return + #include "mounted-ssh-store.md" + ; +} - std::optional experimentalFeature() const override - { - return ExperimentalFeature::MountedSSHStore; - } -}; /** * The mounted ssh store assumes that filesystems on the remote host are diff --git a/src/libstore/ssh-store.hh b/src/libstore/ssh-store.hh index 6ef2219a2e9..feb57ccba80 100644 --- a/src/libstore/ssh-store.hh +++ b/src/libstore/ssh-store.hh @@ -3,6 +3,7 @@ #include "common-ssh-store-config.hh" #include "store-api.hh" +#include "local-fs-store.hh" #include "remote-store.hh" namespace nix { @@ -25,4 +26,26 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig std::string doc() override; }; +struct MountedSSHStoreConfig : virtual SSHStoreConfig, virtual LocalFSStoreConfig +{ + using LocalFSStoreConfig::LocalFSStoreConfig; + using SSHStoreConfig::SSHStoreConfig; + + MountedSSHStoreConfig(StringMap params); + + MountedSSHStoreConfig(std::string_view scheme, std::string_view host, StringMap params); + + const std::string name() override + { + return "Experimental SSH Store with filesystem mounted"; + } + + std::string doc() override; + + std::optional experimentalFeature() const override + { + return ExperimentalFeature::MountedSSHStore; + } +}; + } diff --git a/src/libstore/uds-remote-store.cc b/src/libstore/uds-remote-store.cc index 499f7696712..3c445eb1318 100644 --- a/src/libstore/uds-remote-store.cc +++ b/src/libstore/uds-remote-store.cc @@ -2,10 +2,8 @@ #include "unix-domain-socket.hh" #include "worker-protocol.hh" -#include #include #include -#include #include #include @@ -19,6 +17,21 @@ namespace nix { +UDSRemoteStoreConfig::UDSRemoteStoreConfig( + std::string_view scheme, + std::string_view authority, + const Params & params) + : StoreConfig(params) + , LocalFSStoreConfig(params) + , RemoteStoreConfig(params) + , path{authority.empty() ? settings.nixDaemonSocketFile : authority} +{ + if (scheme != UDSRemoteStoreConfig::scheme) { + throw UsageError("Scheme must be 'unix'"); + } +} + + std::string UDSRemoteStoreConfig::doc() { return @@ -27,11 +40,20 @@ std::string UDSRemoteStoreConfig::doc() } +// A bit gross that we now pass empty string but this is knowing that +// empty string will later default to the same nixDaemonSocketFile. Why +// don't we just wire it all through? I believe there are cases where it +// will live reload so we want to continue to account for that. UDSRemoteStore::UDSRemoteStore(const Params & params) + : UDSRemoteStore(scheme, "", params) +{} + + +UDSRemoteStore::UDSRemoteStore(std::string_view scheme, std::string_view authority, const Params & params) : StoreConfig(params) , LocalFSStoreConfig(params) , RemoteStoreConfig(params) - , UDSRemoteStoreConfig(params) + , UDSRemoteStoreConfig(scheme, authority, params) , Store(params) , LocalFSStore(params) , RemoteStore(params) @@ -39,25 +61,15 @@ UDSRemoteStore::UDSRemoteStore(const Params & params) } -UDSRemoteStore::UDSRemoteStore( - std::string_view scheme, - PathView socket_path, - const Params & params) - : UDSRemoteStore(params) -{ - if (!socket_path.empty()) - path.emplace(socket_path); -} - - std::string UDSRemoteStore::getUri() { - if (path) { - return std::string("unix://") + *path; - } else { - // unix:// with no path also works. Change what we return? - return "daemon"; - } + return path == settings.nixDaemonSocketFile + ? // FIXME: Not clear why we return daemon here and not default + // to settings.nixDaemonSocketFile + // + // unix:// with no path also works. Change what we return? + "daemon" + : std::string(scheme) + "://" + path; } @@ -74,7 +86,7 @@ ref UDSRemoteStore::openConnection() /* Connect to a daemon that does the privileged work for us. */ conn->fd = createUnixDomainSocket(); - nix::connect(toSocket(conn->fd.get()), path ? *path : settings.nixDaemonSocketFile); + nix::connect(toSocket(conn->fd.get()), path); conn->from.fd = conn->fd.get(); conn->to.fd = conn->fd.get(); diff --git a/src/libstore/uds-remote-store.hh b/src/libstore/uds-remote-store.hh index 6f0494bb63b..0e9542eabba 100644 --- a/src/libstore/uds-remote-store.hh +++ b/src/libstore/uds-remote-store.hh @@ -9,16 +9,33 @@ namespace nix { struct UDSRemoteStoreConfig : virtual LocalFSStoreConfig, virtual RemoteStoreConfig { - UDSRemoteStoreConfig(const Params & params) - : StoreConfig(params) - , LocalFSStoreConfig(params) - , RemoteStoreConfig(params) - { - } + // TODO(fzakaria): Delete this constructor once moved over to the factory pattern + // outlined in https://github.com/NixOS/nix/issues/10766 + using LocalFSStoreConfig::LocalFSStoreConfig; + using RemoteStoreConfig::RemoteStoreConfig; + + /** + * @param authority is the socket path. + */ + UDSRemoteStoreConfig( + std::string_view scheme, + std::string_view authority, + const Params & params); const std::string name() override { return "Local Daemon Store"; } std::string doc() override; + + /** + * The path to the unix domain socket. + * + * The default is `settings.nixDaemonSocketFile`, but we don't write + * that below, instead putting in the constructor. + */ + Path path; + +protected: + static constexpr char const * scheme = "unix"; }; class UDSRemoteStore : public virtual UDSRemoteStoreConfig @@ -27,16 +44,23 @@ class UDSRemoteStore : public virtual UDSRemoteStoreConfig { public: + /** + * @deprecated This is the old API to construct the store. + */ UDSRemoteStore(const Params & params); + + /** + * @param authority is the socket path. + */ UDSRemoteStore( std::string_view scheme, - PathView path, + std::string_view authority, const Params & params); std::string getUri() override; static std::set uriSchemes() - { return {"unix"}; } + { return {scheme}; } ref getFSAccessor(bool requireValidPath = true) override { return LocalFSStore::getFSAccessor(requireValidPath); } @@ -63,7 +87,6 @@ private: }; ref openConnection() override; - std::optional path; }; } diff --git a/tests/unit/libstore/http-binary-cache-store.cc b/tests/unit/libstore/http-binary-cache-store.cc new file mode 100644 index 00000000000..1e415f6251a --- /dev/null +++ b/tests/unit/libstore/http-binary-cache-store.cc @@ -0,0 +1,21 @@ +#include + +#include "http-binary-cache-store.hh" + +namespace nix { + +TEST(HttpBinaryCacheStore, constructConfig) +{ + HttpBinaryCacheStoreConfig config{"http", "foo.bar.baz", {}}; + + EXPECT_EQ(config.cacheUri, "http://foo.bar.baz"); +} + +TEST(HttpBinaryCacheStore, constructConfigNoTrailingSlash) +{ + HttpBinaryCacheStoreConfig config{"https", "foo.bar.baz/a/b/", {}}; + + EXPECT_EQ(config.cacheUri, "https://foo.bar.baz/a/b"); +} + +} // namespace nix diff --git a/tests/unit/libstore/local-binary-cache-store.cc b/tests/unit/libstore/local-binary-cache-store.cc new file mode 100644 index 00000000000..2e840228dad --- /dev/null +++ b/tests/unit/libstore/local-binary-cache-store.cc @@ -0,0 +1,14 @@ +#include + +#include "local-binary-cache-store.hh" + +namespace nix { + +TEST(LocalBinaryCacheStore, constructConfig) +{ + LocalBinaryCacheStoreConfig config{"local", "/foo/bar/baz", {}}; + + EXPECT_EQ(config.binaryCacheDir, "/foo/bar/baz"); +} + +} // namespace nix diff --git a/tests/unit/libstore/local-overlay-store.cc b/tests/unit/libstore/local-overlay-store.cc new file mode 100644 index 00000000000..b34ca92375e --- /dev/null +++ b/tests/unit/libstore/local-overlay-store.cc @@ -0,0 +1,34 @@ +// FIXME: Odd failures for templates that are causing the PR to break +// for now with discussion with @Ericson2314 to comment out. +#if 0 +# include + +# include "local-overlay-store.hh" + +namespace nix { + +TEST(LocalOverlayStore, constructConfig_rootQueryParam) +{ + LocalOverlayStoreConfig config{ + "local-overlay", + "", + { + { + "root", + "/foo/bar", + }, + }, + }; + + EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"}); +} + +TEST(LocalOverlayStore, constructConfig_rootPath) +{ + LocalOverlayStoreConfig config{"local-overlay", "/foo/bar", {}}; + + EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"}); +} + +} // namespace nix +#endif diff --git a/tests/unit/libstore/local-store.cc b/tests/unit/libstore/local-store.cc new file mode 100644 index 00000000000..abc3ea7963f --- /dev/null +++ b/tests/unit/libstore/local-store.cc @@ -0,0 +1,40 @@ +// FIXME: Odd failures for templates that are causing the PR to break +// for now with discussion with @Ericson2314 to comment out. +#if 0 +# include + +# include "local-store.hh" + +// Needed for template specialisations. This is not good! When we +// overhaul how store configs work, this should be fixed. +# include "args.hh" +# include "config-impl.hh" +# include "abstract-setting-to-json.hh" + +namespace nix { + +TEST(LocalStore, constructConfig_rootQueryParam) +{ + LocalStoreConfig config{ + "local", + "", + { + { + "root", + "/foo/bar", + }, + }, + }; + + EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"}); +} + +TEST(LocalStore, constructConfig_rootPath) +{ + LocalStoreConfig config{"local", "/foo/bar", {}}; + + EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"}); +} + +} // namespace nix +#endif diff --git a/tests/unit/libstore/meson.build b/tests/unit/libstore/meson.build index 41b2fb0abc2..8534ba8c52d 100644 --- a/tests/unit/libstore/meson.build +++ b/tests/unit/libstore/meson.build @@ -58,7 +58,11 @@ sources = files( 'derivation.cc', 'derived-path.cc', 'downstream-placeholder.cc', + 'http-binary-cache-store.cc', 'legacy-ssh-store.cc', + 'local-binary-cache-store.cc', + 'local-overlay-store.cc', + 'local-store.cc', 'machines.cc', 'nar-info-disk-cache.cc', 'nar-info.cc', @@ -67,9 +71,11 @@ sources = files( 'path-info.cc', 'path.cc', 'references.cc', + 's3-binary-cache-store.cc', 'serve-protocol.cc', 'ssh-store.cc', 'store-reference.cc', + 'uds-remote-store.cc', 'worker-protocol.cc', ) diff --git a/tests/unit/libstore/s3-binary-cache-store.cc b/tests/unit/libstore/s3-binary-cache-store.cc new file mode 100644 index 00000000000..7aa5f2f2c06 --- /dev/null +++ b/tests/unit/libstore/s3-binary-cache-store.cc @@ -0,0 +1,18 @@ +#if ENABLE_S3 + +# include + +# include "s3-binary-cache-store.hh" + +namespace nix { + +TEST(S3BinaryCacheStore, constructConfig) +{ + S3BinaryCacheStoreConfig config{"s3", "foobar", {}}; + + EXPECT_EQ(config.bucketName, "foobar"); +} + +} // namespace nix + +#endif diff --git a/tests/unit/libstore/ssh-store.cc b/tests/unit/libstore/ssh-store.cc index 7010ad157dd..b853a5f1fb9 100644 --- a/tests/unit/libstore/ssh-store.cc +++ b/tests/unit/libstore/ssh-store.cc @@ -1,6 +1,9 @@ -#include +// FIXME: Odd failures for templates that are causing the PR to break +// for now with discussion with @Ericson2314 to comment out. +#if 0 +# include -#include "ssh-store.hh" +# include "ssh-store.hh" namespace nix { @@ -15,7 +18,9 @@ TEST(SSHStore, constructConfig) // TODO #11106, no more split on space "foo bar", }, - }}; + }, + }; + EXPECT_EQ( config.remoteProgram.get(), (Strings{ @@ -23,4 +28,28 @@ TEST(SSHStore, constructConfig) "bar", })); } + +TEST(MountedSSHStore, constructConfig) +{ + MountedSSHStoreConfig config{ + "mounted-ssh", + "localhost", + StoreConfig::Params{ + { + "remote-program", + // TODO #11106, no more split on space + "foo bar", + }, + }, + }; + + EXPECT_EQ( + config.remoteProgram.get(), + (Strings{ + "foo", + "bar", + })); +} + } +#endif diff --git a/tests/unit/libstore/uds-remote-store.cc b/tests/unit/libstore/uds-remote-store.cc new file mode 100644 index 00000000000..5ccb208714f --- /dev/null +++ b/tests/unit/libstore/uds-remote-store.cc @@ -0,0 +1,23 @@ +// FIXME: Odd failures for templates that are causing the PR to break +// for now with discussion with @Ericson2314 to comment out. +#if 0 +# include + +# include "uds-remote-store.hh" + +namespace nix { + +TEST(UDSRemoteStore, constructConfig) +{ + UDSRemoteStoreConfig config{"unix", "/tmp/socket", {}}; + + EXPECT_EQ(config.path, "/tmp/socket"); +} + +TEST(UDSRemoteStore, constructConfigWrongScheme) +{ + EXPECT_THROW(UDSRemoteStoreConfig("http", "/tmp/socket", {}), UsageError); +} + +} // namespace nix +#endif From 2aa9cf34dda0115f07bc7aef020ffb0cda8944e0 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 15 Jul 2024 23:26:39 -0400 Subject: [PATCH 51/54] Move `uriSchemes` to `*StoreConfig` It is a property of the configuration of a store --- how a store URL is parsed into a store config, not a store itself. Progress towards #10766 --- src/libstore/dummy-store.cc | 8 ++++---- src/libstore/http-binary-cache-store.cc | 8 -------- src/libstore/http-binary-cache-store.hh | 9 +++++++++ src/libstore/legacy-ssh-store.hh | 4 ++-- src/libstore/local-binary-cache-store.cc | 4 +--- src/libstore/local-binary-cache-store.hh | 2 ++ src/libstore/local-overlay-store.hh | 10 +++++----- src/libstore/local-store.hh | 6 +++--- src/libstore/s3-binary-cache-store.cc | 3 --- src/libstore/s3-binary-cache-store.hh | 5 +++++ src/libstore/ssh-store.cc | 7 ------- src/libstore/ssh-store.hh | 10 ++++++++++ src/libstore/store-api.hh | 6 +++++- src/libstore/uds-remote-store.hh | 7 ++++--- 14 files changed, 50 insertions(+), 39 deletions(-) diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index af0dd909214..c1e871e9384 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -21,6 +21,10 @@ struct DummyStoreConfig : virtual StoreConfig { #include "dummy-store.md" ; } + + static std::set uriSchemes() { + return {"dummy"}; + } }; struct DummyStore : public virtual DummyStoreConfig, public virtual Store @@ -54,10 +58,6 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store return Trusted; } - static std::set uriSchemes() { - return {"dummy"}; - } - std::optional queryPathFromHashPart(const std::string & hashPart) override { unsupported("queryPathFromHashPart"); } diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index 49ec26b9f93..b15ef4e4cba 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -83,14 +83,6 @@ class HttpBinaryCacheStore : public virtual HttpBinaryCacheStoreConfig, public v } } - static std::set uriSchemes() - { - static bool forceHttp = getEnv("_NIX_FORCE_HTTP") == "1"; - auto ret = std::set({"http", "https"}); - if (forceHttp) ret.insert("file"); - return ret; - } - protected: void maybeDisable() diff --git a/src/libstore/http-binary-cache-store.hh b/src/libstore/http-binary-cache-store.hh index 230e52b3373..d2fc43210a2 100644 --- a/src/libstore/http-binary-cache-store.hh +++ b/src/libstore/http-binary-cache-store.hh @@ -15,6 +15,15 @@ struct HttpBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig return "HTTP Binary Cache Store"; } + static std::set uriSchemes() + { + static bool forceHttp = getEnv("_NIX_FORCE_HTTP") == "1"; + auto ret = std::set({"http", "https"}); + if (forceHttp) + ret.insert("file"); + return ret; + } + std::string doc() override; }; diff --git a/src/libstore/legacy-ssh-store.hh b/src/libstore/legacy-ssh-store.hh index f2665189828..b541455b4e5 100644 --- a/src/libstore/legacy-ssh-store.hh +++ b/src/libstore/legacy-ssh-store.hh @@ -26,6 +26,8 @@ struct LegacySSHStoreConfig : virtual CommonSSHStoreConfig const std::string name() override { return "SSH Store"; } + static std::set uriSchemes() { return {"ssh"}; } + std::string doc() override; }; @@ -46,8 +48,6 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor SSHMaster master; - static std::set uriSchemes() { return {"ssh"}; } - LegacySSHStore( std::string_view scheme, std::string_view host, diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 106663132c1..dcc6affe4a1 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -51,8 +51,6 @@ struct LocalBinaryCacheStore : virtual LocalBinaryCacheStoreConfig, virtual Bina return "file://" + binaryCacheDir; } - static std::set uriSchemes(); - protected: bool fileExists(const std::string & path) override; @@ -121,7 +119,7 @@ bool LocalBinaryCacheStore::fileExists(const std::string & path) return pathExists(binaryCacheDir + "/" + path); } -std::set LocalBinaryCacheStore::uriSchemes() +std::set LocalBinaryCacheStoreConfig::uriSchemes() { if (getEnv("_NIX_FORCE_HTTP") == "1") return {}; diff --git a/src/libstore/local-binary-cache-store.hh b/src/libstore/local-binary-cache-store.hh index 7701eb38b99..997e8ecbb51 100644 --- a/src/libstore/local-binary-cache-store.hh +++ b/src/libstore/local-binary-cache-store.hh @@ -15,6 +15,8 @@ struct LocalBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig return "Local Binary Cache Store"; } + static std::set uriSchemes(); + std::string doc() override; }; diff --git a/src/libstore/local-overlay-store.hh b/src/libstore/local-overlay-store.hh index a3f15e484f0..63628abed50 100644 --- a/src/libstore/local-overlay-store.hh +++ b/src/libstore/local-overlay-store.hh @@ -63,6 +63,11 @@ struct LocalOverlayStoreConfig : virtual LocalStoreConfig return ExperimentalFeature::LocalOverlayStore; } + static std::set uriSchemes() + { + return { "local-overlay" }; + } + std::string doc() override; protected: @@ -102,11 +107,6 @@ public: LocalOverlayStore(std::string_view scheme, PathView path, const Params & params); - static std::set uriSchemes() - { - return { "local-overlay" }; - } - std::string getUri() override { return "local-overlay://"; diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 026729bf24c..a03cfc03b30 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -67,6 +67,9 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig const std::string name() override { return "Local Store"; } + static std::set uriSchemes() + { return {"local"}; } + std::string doc() override; }; @@ -149,9 +152,6 @@ public: ~LocalStore(); - static std::set uriSchemes() - { return {"local"}; } - /** * Implementations of abstract store API methods. */ diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index d865684d7aa..1a0ec11112e 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -475,9 +475,6 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual { return std::nullopt; } - - static std::set uriSchemes() { return {"s3"}; } - }; static RegisterStoreImplementation regS3BinaryCacheStore; diff --git a/src/libstore/s3-binary-cache-store.hh b/src/libstore/s3-binary-cache-store.hh index bca5196cde0..7d303a115f4 100644 --- a/src/libstore/s3-binary-cache-store.hh +++ b/src/libstore/s3-binary-cache-store.hh @@ -94,6 +94,11 @@ public: return "S3 Binary Cache Store"; } + static std::set uriSchemes() + { + return {"s3"}; + } + std::string doc() override; }; diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index d63995bf3bc..954a9746774 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -47,8 +47,6 @@ class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore { } - static std::set uriSchemes() { return {"ssh-ng"}; } - std::string getUri() override { return *uriSchemes().begin() + "://" + host; @@ -154,11 +152,6 @@ class MountedSSHStore : public virtual MountedSSHStoreConfig, public virtual SSH }; } - static std::set uriSchemes() - { - return {"mounted-ssh-ng"}; - } - std::string getUri() override { return *uriSchemes().begin() + "://" + host; diff --git a/src/libstore/ssh-store.hh b/src/libstore/ssh-store.hh index feb57ccba80..29a2a8b2c2d 100644 --- a/src/libstore/ssh-store.hh +++ b/src/libstore/ssh-store.hh @@ -23,6 +23,11 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig return "Experimental SSH Store"; } + static std::set uriSchemes() + { + return {"ssh-ng"}; + } + std::string doc() override; }; @@ -40,6 +45,11 @@ struct MountedSSHStoreConfig : virtual SSHStoreConfig, virtual LocalFSStoreConfi return "Experimental SSH Store with filesystem mounted"; } + static std::set uriSchemes() + { + return {"mounted-ssh-ng"}; + } + std::string doc() override; std::optional experimentalFeature() const override diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 749d7ea098d..7d5f533c5a3 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -216,6 +216,10 @@ public: virtual ~Store() { } + /** + * @todo move to `StoreConfig` one we store enough information in + * those to recover the scheme and authority in all cases. + */ virtual std::string getUri() = 0; /** @@ -897,7 +901,7 @@ struct Implementations { if (!registered) registered = new std::vector(); StoreFactory factory{ - .uriSchemes = T::uriSchemes(), + .uriSchemes = TConfig::uriSchemes(), .create = ([](auto scheme, auto uri, auto & params) -> std::shared_ptr diff --git a/src/libstore/uds-remote-store.hh b/src/libstore/uds-remote-store.hh index 0e9542eabba..a8e57166416 100644 --- a/src/libstore/uds-remote-store.hh +++ b/src/libstore/uds-remote-store.hh @@ -36,6 +36,10 @@ struct UDSRemoteStoreConfig : virtual LocalFSStoreConfig, virtual RemoteStoreCon protected: static constexpr char const * scheme = "unix"; + +public: + static std::set uriSchemes() + { return {scheme}; } }; class UDSRemoteStore : public virtual UDSRemoteStoreConfig @@ -59,9 +63,6 @@ public: std::string getUri() override; - static std::set uriSchemes() - { return {scheme}; } - ref getFSAccessor(bool requireValidPath = true) override { return LocalFSStore::getFSAccessor(requireValidPath); } From 4909f9a52b67e960954a4d48a46c5a4b9e80664a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 18 Jul 2024 01:02:12 -0400 Subject: [PATCH 52/54] Get `LocalFSStore` building --- src/libstore/dummy-store.cc | 24 ++++--- src/libstore/local-fs-store.cc | 90 +++++++++++++++++++++---- src/libstore/local-fs-store.hh | 62 ++++++++++------- src/libstore/store-api.cc | 103 +++++++++++++++-------------- src/libstore/store-api.hh | 24 +++---- src/libstore/store-dir-config.cc | 22 ++---- src/libstore/store-dir-config.hh | 12 ++-- src/libstore/store-registration.hh | 4 +- src/libutil/config-abstract.hh | 6 ++ 9 files changed, 215 insertions(+), 132 deletions(-) diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 4d0bbec2975..f7509bcdeaf 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -4,20 +4,20 @@ namespace nix { -struct DummyStoreConfigDescription : virtual Store::Config::Description -{ - DummyStoreConfigDescription() : StoreConfigDescription{Store::Config::schema} {} -}; - struct DummyStoreConfig : virtual StoreConfig { using StoreConfig::StoreConfig; - using Description = DummyStoreConfigDescription; + struct Descriptions : virtual Store::Config::Descriptions + { + Descriptions() + : StoreConfig::Descriptions{Store::Config::descriptions} + {} + }; - static const Description schema; + static const Descriptions descriptions; DummyStoreConfig(std::string_view scheme, std::string_view authority, const StoreReference::Params & params) - : StoreConfig(params) + : StoreConfig{params} { if (!authority.empty()) throw UsageError("`%s` store URIs must not contain an authority part %s", scheme, authority); @@ -39,13 +39,15 @@ struct DummyStoreConfig : virtual StoreConfig { std::shared_ptr open() override; }; -decltype(DummyStoreConfig::schema) DummyStoreConfig::schema{}; + +const DummyStoreConfig::Descriptions DummyStoreConfig::descriptions{}; + struct DummyStore : public virtual DummyStoreConfig, public virtual Store { using Config = DummyStoreConfig; - DummyStore(const DummyStoreConfig & config) + DummyStore(const Config & config) : StoreConfig(config) , DummyStoreConfig(config) , Store(config) @@ -98,7 +100,7 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store { unsupported("getFSAccessor"); } }; -std::shared_ptr DummyStoreConfig::open() +std::shared_ptr DummyStore::Config::open() { return std::make_shared(*this); } diff --git a/src/libstore/local-fs-store.cc b/src/libstore/local-fs-store.cc index 5449b20eb3b..fbaf6c3cb1c 100644 --- a/src/libstore/local-fs-store.cc +++ b/src/libstore/local-fs-store.cc @@ -5,25 +5,87 @@ #include "globals.hh" #include "compression.hh" #include "derivations.hh" +#include "config-parse-impl.hh" namespace nix { -LocalFSStoreConfig::LocalFSStoreConfig(PathView rootDir, const Params & params) +LocalFSStore::Config::Descriptions::Descriptions() + : Store::Config::Descriptions{Store::Config::descriptions} + , LocalFSStoreConfigT{ + .rootDir = { + .name = "root", + .description = "Directory prefixed to all other paths.", + }, + .stateDir = { + .name = "state", + .description = "Directory where Nix will store state.", + }, + .logDir = { + .name = "log", + .description = "directory where Nix will store log files.", + }, + .realStoreDir{ + .name = "real", + .description = "Physical path of the Nix store.", + }, + } +{} + +const LocalFSStore::Config::Descriptions LocalFSStore::Config::descriptions{}; + +LocalFSStoreConfigT LocalFSStoreConfig::defaults( + const Store::Config & storeConfig, + const std::optional rootDir) +{ + return { + .rootDir = {.value = std::nullopt }, + .stateDir = {.value = rootDir ? *rootDir + "/nix/var/nix" : settings.nixStateDir }, + .logDir = {.value = rootDir ? *rootDir + "/nix/var/log/nix" : settings.nixLogDir }, + .realStoreDir = {.value = rootDir ? *rootDir + "/nix/store" : storeConfig.storeDir }, + }; +} + +/** + * @param rootDir Fallback if not in `params` + */ +auto localFSStoreConfig( + const Store::Config & storeConfig, + const std::optional _rootDir, + const StoreReference::Params & params) +{ + const auto & descriptions = LocalFSStore::Config::descriptions; + + auto rootDir = descriptions.rootDir.parseConfig(params) + .value_or(config::JustValue{.value = std::move(_rootDir)}); + + auto defaults = LocalFSStore::Config::defaults(storeConfig, rootDir.value); + + return LocalFSStoreConfigT{ + CONFIG_ROW(rootDir), + CONFIG_ROW(stateDir), + CONFIG_ROW(logDir), + CONFIG_ROW(realStoreDir), + }; +} + +LocalFSStore::Config::LocalFSStoreConfig(const StoreReference::Params & params) + : StoreConfig{params} + , LocalFSStoreConfigT{localFSStoreConfig(*this, std::nullopt, params)} +{ +} + +LocalFSStore::Config::LocalFSStoreConfig(PathView rootDir, const StoreReference::Params & params) : StoreConfig(params) - // Default `?root` from `rootDir` if non set - // FIXME don't duplicate description once we don't have root setting - , rootDir{ - this, - !rootDir.empty() && params.count("root") == 0 - ? (std::optional{rootDir}) - : std::nullopt, - "root", - "Directory prefixed to all other paths."} + , LocalFSStoreConfigT{localFSStoreConfig( + *this, + // Default `?root` from `rootDir` if non set + !rootDir.empty() ? (std::optional{rootDir}) : std::nullopt, + params)} { } -LocalFSStore::LocalFSStore(const Params & params) - : Store(params) +LocalFSStore::LocalFSStore(const Config & config) + : Store(config) { } @@ -97,8 +159,8 @@ std::optional LocalFSStore::getBuildLogExact(const StorePath & path Path logPath = j == 0 - ? fmt("%s/%s/%s/%s", logDir, drvsLogDir, baseName.substr(0, 2), baseName.substr(2)) - : fmt("%s/%s/%s", logDir, drvsLogDir, baseName); + ? fmt("%s/%s/%s/%s", logDir.get(), drvsLogDir, baseName.substr(0, 2), baseName.substr(2)) + : fmt("%s/%s/%s", logDir.get(), drvsLogDir, baseName); Path logBz2Path = logPath + ".bz2"; if (pathExists(logPath)) diff --git a/src/libstore/local-fs-store.hh b/src/libstore/local-fs-store.hh index 9bb569f0b25..f36cd56af27 100644 --- a/src/libstore/local-fs-store.hh +++ b/src/libstore/local-fs-store.hh @@ -7,9 +7,23 @@ namespace nix { -struct LocalFSStoreConfig : virtual StoreConfig +template class F> +struct LocalFSStoreConfigT { - using StoreConfig::StoreConfig; + F> rootDir; + + F stateDir; + + F logDir; + + F realStoreDir; +}; + +struct LocalFSStoreConfig : + virtual Store::Config, + LocalFSStoreConfigT +{ + LocalFSStoreConfig(const StoreReference::Params &); /** * Used to override the `root` settings. Can't be done via modifying @@ -18,38 +32,38 @@ struct LocalFSStoreConfig : virtual StoreConfig * * @todo Make this less error-prone with new store settings system. */ - LocalFSStoreConfig(PathView path, const Params & params); + LocalFSStoreConfig(PathView path, const StoreReference::Params & params); - const OptionalPathSetting rootDir{this, std::nullopt, - "root", - "Directory prefixed to all other paths."}; - - const PathSetting stateDir{this, - rootDir.get() ? *rootDir.get() + "/nix/var/nix" : settings.nixStateDir, - "state", - "Directory where Nix will store state."}; + struct Descriptions : + virtual Store::Config::Descriptions, + LocalFSStoreConfigT + { + Descriptions(); + }; - const PathSetting logDir{this, - rootDir.get() ? *rootDir.get() + "/nix/var/log/nix" : settings.nixLogDir, - "log", - "directory where Nix will store log files."}; + static const Descriptions descriptions; - const PathSetting realStoreDir{this, - rootDir.get() ? *rootDir.get() + "/nix/store" : storeDir, "real", - "Physical path of the Nix store."}; + /** + * The other defaults depend on the choice of `storeDir` and `rootDir` + */ + static LocalFSStoreConfigT defaults( + const Store::Config &, + const std::optional rootDir); }; -class LocalFSStore : public virtual LocalFSStoreConfig, - public virtual Store, - public virtual GcStore, - public virtual LogStore +struct LocalFSStore : + virtual LocalFSStoreConfig, + virtual Store, + virtual GcStore, + virtual LogStore { -public: + using Config = LocalFSStoreConfig; + inline static std::string operationName = "Local Filesystem Store"; const static std::string drvsLogDir; - LocalFSStore(const Params & params); + LocalFSStore(const Config & params); void narFromPath(const StorePath & path, Sink & sink) override; ref getFSAccessor(bool requireValidPath = true) override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 381bb3ed6c4..95aa103e46e 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -189,7 +189,56 @@ std::pair StoreDirConfig::computeStorePath( } -const StoreConfigT storeConfigDefaults = { +StoreConfig::Descriptions::Descriptions() + : StoreDirConfig::Descriptions{StoreDirConfig::descriptions} + , StoreConfigT{ + .pathInfoCacheSize = { + .name = "path-info-cache-size", + .description = "Size of the in-memory store path metadata cache.", + }, + .isTrusted = { + .name = "trusted", + .description = R"( + Whether paths from this store can be used as substitutes + even if they are not signed by a key listed in the + [`trusted-public-keys`](@docroot@/command-ref/conf-file.md#conf-trusted-public-keys) + setting. + )", + }, + .priority = { + .name = "priority", + .description = R"( + Priority of this store when used as a [substituter](@docroot@/command-ref/conf-file.md#conf-substituters). + A lower value means a higher priority. + )", + }, + .wantMassQuery = { + .name = "want-mass-query", + .description = R"( + Whether this store can be queried efficiently for path validity when used as a [substituter](@docroot@/command-ref/conf-file.md#conf-substituters). + )", + }, + + .systemFeatures = { + .name = "system-features", + .description = R"( + Optional [system features](@docroot@/command-ref/conf-file.md#conf-system-features) available on the system this store uses to build derivations. + + Example: `"kvm"` + )", + // The default value is CPU- and OS-specific, and thus + // unsuitable to be rendered in the documentation. + .documentDefault = false, + }, + } +{ +} + + +const StoreConfig::Descriptions StoreConfig::descriptions{}; + + +decltype(StoreConfig::defaults) StoreConfig::defaults = { .pathInfoCacheSize = { .value = 65536 }, .isTrusted = { .value = false }, .priority = { .value = 0 }, @@ -197,59 +246,17 @@ const StoreConfigT storeConfigDefaults = { .systemFeatures = { .value = StoreConfig::getDefaultSystemFeatures() }, }; -decltype(StoreConfig::schema) StoreConfig::schema = {{ - .pathInfoCacheSize = { - .name = "path-info-cache-size", - .description = "Size of the in-memory store path metadata cache.", - }, - .isTrusted = { - .name = "trusted", - .description = R"( - Whether paths from this store can be used as substitutes - even if they are not signed by a key listed in the - [`trusted-public-keys`](@docroot@/command-ref/conf-file.md#conf-trusted-public-keys) - setting. - )", - }, - .priority = { - .name = "priority", - .description = R"( - Priority of this store when used as a [substituter](@docroot@/command-ref/conf-file.md#conf-substituters). - A lower value means a higher priority. - )", - }, - .wantMassQuery = { - .name = "want-mass-query", - .description = R"( - Whether this store can be queried efficiently for path validity when used as a [substituter](@docroot@/command-ref/conf-file.md#conf-substituters). - )", - }, - - .systemFeatures = { - .name = "system-features", - .description = R"( - Optional [system features](@docroot@/command-ref/conf-file.md#conf-system-features) available on the system this store uses to build derivations. - - Example: `"kvm"` - )", - // The default value is CPU- and OS-specific, and thus - // unsuitable to be rendered in the documentation. - .documentDefault = false, - }, -}}; - -StoreConfigT parseStoreConfig(const StoreReference::Params & params) -{ - constexpr auto & defaults = storeConfigDefaults; - constexpr auto & descriptions = StoreConfig::schema; - return { +StoreConfig::StoreConfig(const StoreReference::Params & params) + : StoreDirConfig{params} + , StoreConfigT{ CONFIG_ROW(pathInfoCacheSize), CONFIG_ROW(isTrusted), CONFIG_ROW(priority), CONFIG_ROW(wantMassQuery), CONFIG_ROW(systemFeatures), - }; + } +{ } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 924f037af24..296b81be6a0 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -110,23 +110,23 @@ struct StoreConfigT F systemFeatures; }; -extern const StoreConfigT storeConfigDefaults; - StoreConfigT parseStoreConfig(const StoreReference::Params &); - -struct StoreConfigDescription : StoreConfigT, StoreDirConfigT +struct StoreConfig : + StoreDirConfig, + StoreConfigT { -}; + StoreConfig(const StoreReference::Params &); -struct StoreConfig : StoreConfigT, StoreDirConfig -{ - using StoreDirConfig::StoreDirConfig; - - StoreConfig() = delete; + struct Descriptions : + StoreDirConfig::Descriptions, + StoreConfigT + { + Descriptions(); + }; - using Description = StoreConfigDescription; + static const StoreConfigT defaults; - static const Description schema; + static const Descriptions descriptions; static StringSet getDefaultSystemFeatures(); diff --git a/src/libstore/store-dir-config.cc b/src/libstore/store-dir-config.cc index 960f2b88266..a51962d7fa9 100644 --- a/src/libstore/store-dir-config.cc +++ b/src/libstore/store-dir-config.cc @@ -4,11 +4,7 @@ namespace nix { -const StoreDirConfigT storeDirConfigDefaults = { - ._storeDir = {.value = settings.nixStore}, -}; - -const StoreDirConfigT storeDirConfigDescriptions = { +const StoreDirConfigT StoreDirConfig::descriptions = { ._storeDir = { .name = "store", @@ -20,18 +16,14 @@ const StoreDirConfigT storeDirConfigDescriptions = { }, }; -StoreDirConfigT parseStoreDirConfig(const StoreReference::Params & params) -{ - constexpr auto & defaults = storeDirConfigDefaults; - constexpr auto & descriptions = storeDirConfigDescriptions; - - return { - CONFIG_ROW(_storeDir), - }; -} +const StoreDirConfigT StoreDirConfig::defaults = { + ._storeDir = {.value = settings.nixStore}, +}; StoreDirConfig::StoreDirConfig(const StoreReference::Params & params) - : StoreDirConfigT{parseStoreDirConfig(params)} + : StoreDirConfigT{ + CONFIG_ROW(_storeDir), + } { } diff --git a/src/libstore/store-dir-config.hh b/src/libstore/store-dir-config.hh index a2717534501..5359c75b043 100644 --- a/src/libstore/store-dir-config.hh +++ b/src/libstore/store-dir-config.hh @@ -25,16 +25,16 @@ struct StoreDirConfigT F _storeDir; }; -extern const StoreDirConfigT storeDirConfigDefaults; - -extern const StoreDirConfigT storeDirConfigDescriptions; - -StoreDirConfigT parseStoreDirConfig(const StoreReference::Params &); - struct StoreDirConfig : StoreDirConfigT { StoreDirConfig(const StoreReference::Params & params); + static const StoreDirConfigT defaults; + + using Descriptions = StoreDirConfigT; + + static const Descriptions descriptions; + virtual ~StoreDirConfig() = default; const Path & storeDir = _storeDir.value; diff --git a/src/libstore/store-registration.hh b/src/libstore/store-registration.hh index 9bc1869eae1..8d57f4a587a 100644 --- a/src/libstore/store-registration.hh +++ b/src/libstore/store-registration.hh @@ -21,7 +21,7 @@ struct StoreFactory std::function( std::string_view scheme, std::string_view authorityPath, const StoreReference::Params & params)> parseConfig; - const StoreConfigDescription & configSchema; + const Store::Config::Descriptions & configDescriptions; }; struct Implementations @@ -38,7 +38,7 @@ struct Implementations .parseConfig = ([](auto scheme, auto uri, auto & params) -> std::shared_ptr { return std::make_shared(scheme, uri, params); }), - .configSchema = T::Config::schema, + .configDescriptions = T::Config::descriptions, }; registered->push_back(factory); } diff --git a/src/libutil/config-abstract.hh b/src/libutil/config-abstract.hh index 68f20605e06..4737a29cc7c 100644 --- a/src/libutil/config-abstract.hh +++ b/src/libutil/config-abstract.hh @@ -22,4 +22,10 @@ struct JustValue } }; +template +auto operator <<(auto str, const JustValue & opt) +{ + return str << opt.get(); +} + } From 307440b80c6c290758861dd45780b8ed9a781e10 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 18 Jul 2024 02:54:14 -0400 Subject: [PATCH 53/54] Convert `LocalStoreConfig` to new system --- src/libstore-c/nix_api_store.cc | 1 + src/libstore/binary-cache-store.hh | 6 +- src/libstore/dummy-store.cc | 6 +- src/libstore/local-fs-store.cc | 3 +- src/libstore/local-fs-store.hh | 22 +++--- src/libstore/local-store.cc | 79 ++++++++++++++----- src/libstore/local-store.hh | 69 ++++++++-------- src/libstore/machines.hh | 2 + src/libstore/store-api.cc | 12 +-- src/libstore/store-api.hh | 13 +-- src/libstore/store-dir-config.hh | 4 +- src/libutil/config-abstract.hh | 2 +- src/nix-channel/nix-channel.cc | 1 + tests/unit/libstore-support/tests/libstore.hh | 1 + 14 files changed, 135 insertions(+), 86 deletions(-) diff --git a/src/libstore-c/nix_api_store.cc b/src/libstore-c/nix_api_store.cc index 79841ca49a1..ba4a6e6e5eb 100644 --- a/src/libstore-c/nix_api_store.cc +++ b/src/libstore-c/nix_api_store.cc @@ -5,6 +5,7 @@ #include "path.hh" #include "store-api.hh" +#include "store-open.hh" #include "build-result.hh" #include "globals.hh" diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 695bc925277..234c39caf3a 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -96,7 +96,11 @@ public: public: - virtual void init() override; + /** + * Perform any necessary effectful operation to make the store up and + * running + */ + virtual void init(); private: diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index f7509bcdeaf..c6dd71ba669 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -36,7 +36,7 @@ struct DummyStoreConfig : virtual StoreConfig { return {"dummy"}; } - std::shared_ptr open() override; + std::shared_ptr openStore() override; }; @@ -50,7 +50,7 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store DummyStore(const Config & config) : StoreConfig(config) , DummyStoreConfig(config) - , Store(config) + , Store{static_cast(*this)} { } std::string getUri() override @@ -100,7 +100,7 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store { unsupported("getFSAccessor"); } }; -std::shared_ptr DummyStore::Config::open() +std::shared_ptr DummyStore::Config::openStore() { return std::make_shared(*this); } diff --git a/src/libstore/local-fs-store.cc b/src/libstore/local-fs-store.cc index fbaf6c3cb1c..a8b23996159 100644 --- a/src/libstore/local-fs-store.cc +++ b/src/libstore/local-fs-store.cc @@ -85,7 +85,8 @@ LocalFSStore::Config::LocalFSStoreConfig(PathView rootDir, const StoreReference: } LocalFSStore::LocalFSStore(const Config & config) - : Store(config) + : LocalFSStore::Config{config} + , Store{static_cast(*this)} { } diff --git a/src/libstore/local-fs-store.hh b/src/libstore/local-fs-store.hh index a400faec05a..654dd210f40 100644 --- a/src/libstore/local-fs-store.hh +++ b/src/libstore/local-fs-store.hh @@ -23,17 +23,6 @@ struct LocalFSStoreConfig : virtual Store::Config, LocalFSStoreConfigT { - LocalFSStoreConfig(const StoreReference::Params &); - - /** - * Used to override the `root` settings. Can't be done via modifying - * `params` reliably because this parameter is unused except for - * passing to base class constructors. - * - * @todo Make this less error-prone with new store settings system. - */ - LocalFSStoreConfig(PathView path, const StoreReference::Params & params); - struct Descriptions : virtual Store::Config::Descriptions, LocalFSStoreConfigT @@ -49,6 +38,17 @@ struct LocalFSStoreConfig : static LocalFSStoreConfigT defaults( const Store::Config &, const std::optional rootDir); + + LocalFSStoreConfig(const StoreReference::Params &); + + /** + * Used to override the `root` settings. Can't be done via modifying + * `params` reliably because this parameter is unused except for + * passing to base class constructors. + * + * @todo Make this less error-prone with new store settings system. + */ + LocalFSStoreConfig(PathView path, const StoreReference::Params & params); }; struct LocalFSStore : diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 819cee34532..2dfc78ebda3 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -17,6 +17,9 @@ #include "posix-source-accessor.hh" #include "keys.hh" #include "users.hh" +#include "config-parse-impl.hh" +#include "store-open.hh" +#include "store-registration.hh" #include #include @@ -56,12 +59,50 @@ namespace nix { -LocalStoreConfig::LocalStoreConfig( +LocalStore::Config::Descriptions::Descriptions() + : Store::Config::Descriptions{Store::Config::descriptions} + , LocalFSStore::Config::Descriptions{LocalFSStore::Config::descriptions} + , LocalStoreConfigT{ + .requireSigs = { + .name = "require-sigs", + .description = "Whether store paths copied into this store should have a trusted signature.", + }, + .readOnly = { + .name = "read-only", + .description = 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. + + Enable read-only mode to disable locking and open the SQLite database with the [`immutable` parameter](https://www.sqlite.org/c3ref/open.html) set. + + > **Warning** + > Do not use this unless the filesystem is read-only. + > + > Using it when the filesystem is writable can cause incorrect query results or corruption errors if the database is changed by another process. + > While the filesystem the database resides on might appear to be read-only, consider whether another user or system might have write access to it. + )", + }, + } +{} + +const LocalStore::Config::Descriptions LocalStore::Config::descriptions{}; + +decltype(LocalStore::Config::defaults) LocalStore::Config::defaults = { + .requireSigs = {.value = settings.requireSigs }, + .readOnly = {.value = false }, +}; + +LocalStore::Config::LocalStoreConfig( std::string_view scheme, std::string_view authority, - const Params & params) - : StoreConfig(params) - , LocalFSStoreConfig(authority, params) + const StoreReference::Params & params) + : Store::Config(params) + , LocalFSStore::Config(authority, params) + , LocalStoreConfigT{ + CONFIG_ROW(requireSigs), + CONFIG_ROW(readOnly), + } { } @@ -72,6 +113,11 @@ std::string LocalStoreConfig::doc() ; } +std::shared_ptr LocalStore::Config::openStore() +{ + return std::make_shared(*this); +} + struct LocalStore::State::Stmts { /* Some precompiled SQLite statements. */ SQLiteStmt RegisterValidPath; @@ -192,15 +238,12 @@ void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd) } } -LocalStore::LocalStore( - std::string_view scheme, - PathView path, - const Params & params) - : StoreConfig(params) - , LocalFSStoreConfig(path, params) - , LocalStoreConfig(scheme, path, params) - , Store(params) - , LocalFSStore(params) +LocalStore::LocalStore(const Config & config) + : Store::Config{config} + , LocalFSStore::Config{config} + , LocalStore::Config{config} + , Store{static_cast(*this)} + , LocalFSStore{static_cast(*this)} , dbDir(stateDir + "/db") , linksDir(realStoreDir + "/.links") , reservedPath(dbDir + "/reserved") @@ -477,12 +520,6 @@ LocalStore::LocalStore( } -LocalStore::LocalStore(const Params & params) - : LocalStore("local", "", params) -{ -} - - AutoCloseFD LocalStore::openGCLock() { Path fnGCLock = stateDir + "/gc.lock"; @@ -1512,7 +1549,7 @@ LocalStore::VerificationResult LocalStore::verifyAllValidPaths(RepairFlag repair database and the filesystem) in the loop below, in order to catch invalid states. */ - for (auto & i : std::filesystem::directory_iterator{realStoreDir.to_string()}) { + for (auto & i : std::filesystem::directory_iterator{realStoreDir.get()}) { checkInterrupt(); try { storePathsInStoreDir.insert({i.path().filename().string()}); @@ -1802,6 +1839,6 @@ std::optional LocalStore::getVersion() return nixVersion; } -static RegisterStoreImplementation regLocalStore; +static RegisterStoreImplementation regLocalStore; } // namespace nix diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index a03cfc03b30..13e26922c0a 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -34,36 +34,40 @@ struct OptimiseStats uint64_t bytesFreed = 0; }; -struct LocalStoreConfig : virtual LocalFSStoreConfig +template class F> +struct LocalStoreConfigT { - using LocalFSStoreConfig::LocalFSStoreConfig; + const F requireSigs; - LocalStoreConfig( - std::string_view scheme, - std::string_view authority, - const Params & params); + const F readOnly; +}; - Setting requireSigs{this, - settings.requireSigs, - "require-sigs", - "Whether store paths copied into this store should have a trusted signature."}; +struct LocalStoreConfig : + virtual Store::Config, + virtual LocalFSStore::Config, + LocalStoreConfigT +{ + struct Descriptions : + virtual Store::Config::Descriptions, + virtual LocalFSStore::Config::Descriptions, + LocalStoreConfigT + { + Descriptions(); + }; - Setting readOnly{this, - false, - "read-only", - R"( - Allow this store to be opened when its [database](@docroot@/glossary.md#gloss-nix-database) is on a read-only filesystem. + static const Descriptions descriptions; - 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. + /** + * The other defaults depend on the choice of `storeDir` and `rootDir` + */ + static LocalStoreConfigT defaults; - Enable read-only mode to disable locking and open the SQLite database with the [`immutable` parameter](https://www.sqlite.org/c3ref/open.html) set. + LocalStoreConfig(const StoreReference::Params &); - > **Warning** - > Do not use this unless the filesystem is read-only. - > - > Using it when the filesystem is writable can cause incorrect query results or corruption errors if the database is changed by another process. - > While the filesystem the database resides on might appear to be read-only, consider whether another user or system might have write access to it. - )"}; + LocalStoreConfig( + std::string_view scheme, + std::string_view authority, + const StoreReference::Params & params); const std::string name() override { return "Local Store"; } @@ -71,12 +75,19 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig { return {"local"}; } std::string doc() override; + + std::shared_ptr openStore() override; }; -class LocalStore : public virtual LocalStoreConfig - , public virtual IndirectRootStore - , public virtual GcStore +class LocalStore : + public virtual LocalStoreConfig, + public virtual IndirectRootStore, + public virtual GcStore { +public: + + using Config = LocalStoreConfig; + private: /** @@ -144,11 +155,7 @@ public: * Initialise the local store, upgrading the schema if * necessary. */ - LocalStore(const Params & params); - LocalStore( - std::string_view scheme, - PathView path, - const Params & params); + LocalStore(const Config & params); ~LocalStore(); diff --git a/src/libstore/machines.hh b/src/libstore/machines.hh index 983652d5f8b..671de208468 100644 --- a/src/libstore/machines.hh +++ b/src/libstore/machines.hh @@ -1,6 +1,8 @@ #pragma once ///@file +#include + #include "ref.hh" #include "store-reference.hh" diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 95aa103e46e..c4bb9d594b0 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -189,7 +189,7 @@ std::pair StoreDirConfig::computeStorePath( } -StoreConfig::Descriptions::Descriptions() +Store::Config::Descriptions::Descriptions() : StoreDirConfig::Descriptions{StoreDirConfig::descriptions} , StoreConfigT{ .pathInfoCacheSize = { @@ -235,19 +235,19 @@ StoreConfig::Descriptions::Descriptions() } -const StoreConfig::Descriptions StoreConfig::descriptions{}; +const Store::Config::Descriptions Store::Config::descriptions{}; -decltype(StoreConfig::defaults) StoreConfig::defaults = { +decltype(Store::Config::defaults) Store::Config::defaults = { .pathInfoCacheSize = { .value = 65536 }, .isTrusted = { .value = false }, .priority = { .value = 0 }, .wantMassQuery = { .value = false }, - .systemFeatures = { .value = StoreConfig::getDefaultSystemFeatures() }, + .systemFeatures = { .value = Store::Config::getDefaultSystemFeatures() }, }; -StoreConfig::StoreConfig(const StoreReference::Params & params) +Store::Config::StoreConfig(const StoreReference::Params & params) : StoreDirConfig{params} , StoreConfigT{ CONFIG_ROW(pathInfoCacheSize), @@ -491,7 +491,7 @@ ValidPathInfo Store::addToStoreSlow( return info; } -StringSet StoreConfig::getDefaultSystemFeatures() +StringSet Store::Config::getDefaultSystemFeatures() { auto res = settings.systemFeatures.get(); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 9d114ff40c7..09352e601ba 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -115,8 +115,6 @@ struct StoreConfig : StoreDirConfig, StoreConfigT { - StoreConfig(const StoreReference::Params &); - struct Descriptions : StoreDirConfig::Descriptions, StoreConfigT @@ -128,10 +126,12 @@ struct StoreConfig : static const Descriptions descriptions; - static StringSet getDefaultSystemFeatures(); + StoreConfig(const StoreReference::Params &); virtual ~StoreConfig() { } + static StringSet getDefaultSystemFeatures(); + /** * The name of this type of store. */ @@ -158,7 +158,7 @@ struct StoreConfig : * Open a store of the type corresponding to this configuration * type. */ - virtual std::shared_ptr open() = 0; + virtual std::shared_ptr openStore() = 0; }; class Store : public std::enable_shared_from_this, public virtual StoreConfig @@ -208,11 +208,6 @@ protected: Store(const Store::Config & config); public: - /** - * Perform any necessary effectful operation to make the store up and - * running - */ - virtual void init() {}; virtual ~Store() { } diff --git a/src/libstore/store-dir-config.hh b/src/libstore/store-dir-config.hh index 4f90c5b5fc5..19391322907 100644 --- a/src/libstore/store-dir-config.hh +++ b/src/libstore/store-dir-config.hh @@ -27,14 +27,14 @@ struct StoreDirConfigT struct StoreDirConfig : StoreDirConfigT { - StoreDirConfig(const StoreReference::Params & params); - static const StoreDirConfigT defaults; using Descriptions = StoreDirConfigT; static const Descriptions descriptions; + StoreDirConfig(const StoreReference::Params & params); + virtual ~StoreDirConfig() = default; const Path & storeDir = _storeDir.value; diff --git a/src/libutil/config-abstract.hh b/src/libutil/config-abstract.hh index 4737a29cc7c..7d546b33332 100644 --- a/src/libutil/config-abstract.hh +++ b/src/libutil/config-abstract.hh @@ -23,7 +23,7 @@ struct JustValue }; template -auto operator <<(auto str, const JustValue & opt) +auto && operator <<(auto && str, const JustValue & opt) { return str << opt.get(); } diff --git a/src/nix-channel/nix-channel.cc b/src/nix-channel/nix-channel.cc index 9f7f557b59d..d8d0d518b78 100644 --- a/src/nix-channel/nix-channel.cc +++ b/src/nix-channel/nix-channel.cc @@ -3,6 +3,7 @@ #include "globals.hh" #include "filetransfer.hh" #include "store-api.hh" +#include "store-open.hh" #include "legacy.hh" #include "eval-settings.hh" // for defexpr #include "users.hh" diff --git a/tests/unit/libstore-support/tests/libstore.hh b/tests/unit/libstore-support/tests/libstore.hh index 84be52c230b..5710a12d9b6 100644 --- a/tests/unit/libstore-support/tests/libstore.hh +++ b/tests/unit/libstore-support/tests/libstore.hh @@ -5,6 +5,7 @@ #include #include "store-api.hh" +#include "store-open.hh" namespace nix { From de1ec5a63444f90fea58bef4d6da10ca85830fcc Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 18 Jul 2024 03:15:26 -0400 Subject: [PATCH 54/54] A bunch of small fixes --- src/build-remote/build-remote.cc | 2 +- src/libcmd/command.cc | 2 +- src/libexpr/primops/fetchClosure.cc | 2 +- src/libstore-c/nix_api_store.cc | 2 +- src/libstore/build/drv-output-substitution-goal.cc | 1 + src/libstore/profiles.hh | 2 +- src/libstore/remote-store.hh | 7 ++++--- src/libstore/store-reference.cc | 2 ++ src/libutil/config-abstract.hh | 2 +- src/nix-build/nix-build.cc | 2 +- src/nix-channel/nix-channel.cc | 1 - src/nix-collect-garbage/nix-collect-garbage.cc | 2 +- src/nix-copy-closure/nix-copy-closure.cc | 2 +- src/nix-env/nix-env.cc | 2 +- src/nix-instantiate/nix-instantiate.cc | 2 +- src/nix-store/nix-store.cc | 1 + src/nix/flake.cc | 2 +- src/nix/log.cc | 2 +- src/nix/main.cc | 3 ++- src/nix/make-content-addressed.cc | 2 +- src/nix/prefetch.cc | 2 +- src/nix/repl.cc | 1 + src/nix/sigs.cc | 2 +- src/nix/unix/daemon.cc | 3 ++- src/nix/verify.cc | 2 +- tests/unit/libstore/local-overlay-store.cc | 8 ++------ tests/unit/libstore/local-store.cc | 14 ++------------ tests/unit/libstore/ssh-store.cc | 8 ++------ tests/unit/libstore/uds-remote-store.cc | 8 ++------ 29 files changed, 38 insertions(+), 53 deletions(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index a0a404e575a..1824e20024e 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -16,7 +16,7 @@ #include "globals.hh" #include "serialise.hh" #include "build-result.hh" -#include "store-api.hh" +#include "store-open.hh" #include "strings.hh" #include "derivations.hh" #include "local-store.hh" diff --git a/src/libcmd/command.cc b/src/libcmd/command.cc index 67fef190920..349fb37d704 100644 --- a/src/libcmd/command.cc +++ b/src/libcmd/command.cc @@ -2,7 +2,7 @@ #include "command.hh" #include "markdown.hh" -#include "store-api.hh" +#include "store-open.hh" #include "local-fs-store.hh" #include "derivations.hh" #include "nixexpr.hh" diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index fc5bb31454f..7f59cac2f70 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -1,5 +1,5 @@ #include "primops.hh" -#include "store-api.hh" +#include "store-open.hh" #include "realisation.hh" #include "make-content-addressed.hh" #include "url.hh" diff --git a/src/libstore-c/nix_api_store.cc b/src/libstore-c/nix_api_store.cc index ba4a6e6e5eb..4961937cf12 100644 --- a/src/libstore-c/nix_api_store.cc +++ b/src/libstore-c/nix_api_store.cc @@ -43,7 +43,7 @@ Store * nix_store_open(nix_c_context * context, const char * uri, const char *** if (!params) return new Store{nix::openStore(uri_str)}; - nix::Store::Params params_map; + nix::StoreReference::Params params_map; for (size_t i = 0; params[i] != nullptr; i++) { params_map[params[i][0]] = params[i][1]; } diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 02284d93c1a..4488efdcb59 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -3,6 +3,7 @@ #include "worker.hh" #include "substitution-goal.hh" #include "callback.hh" +#include "store-open.hh" namespace nix { diff --git a/src/libstore/profiles.hh b/src/libstore/profiles.hh index b10a72330b4..63aa9972c9b 100644 --- a/src/libstore/profiles.hh +++ b/src/libstore/profiles.hh @@ -84,7 +84,7 @@ typedef std::list Generations; */ std::pair> findGenerations(Path profile); -class LocalFSStore; +struct LocalFSStore; /** * Create a new generation of the given profile diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 4e18962683d..f19aa0fc57b 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -35,14 +35,15 @@ struct RemoteStoreConfig : virtual StoreConfig * \todo RemoteStore is a misnomer - should be something like * DaemonStore. */ -class RemoteStore : public virtual RemoteStoreConfig, +struct RemoteStore : + public virtual RemoteStoreConfig, public virtual Store, public virtual GcStore, public virtual LogStore { -public: + using Config = RemoteStoreConfig; - RemoteStore(const Params & params); + RemoteStore(const Config & config); /* Implementations of abstract store API methods. */ diff --git a/src/libstore/store-reference.cc b/src/libstore/store-reference.cc index b4968dfadbd..5e5d30fc117 100644 --- a/src/libstore/store-reference.cc +++ b/src/libstore/store-reference.cc @@ -1,5 +1,7 @@ #include +#include + #include "error.hh" #include "url.hh" #include "store-reference.hh" diff --git a/src/libutil/config-abstract.hh b/src/libutil/config-abstract.hh index 7d546b33332..10c98f0a73f 100644 --- a/src/libutil/config-abstract.hh +++ b/src/libutil/config-abstract.hh @@ -23,7 +23,7 @@ struct JustValue }; template -auto && operator <<(auto && str, const JustValue & opt) +auto && operator<<(auto && str, const JustValue & opt) { return str << opt.get(); } diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 0ce987d8a5c..9e5dfd1a04c 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -11,7 +11,7 @@ #include "current-process.hh" #include "parsed-derivations.hh" -#include "store-api.hh" +#include "store-open.hh" #include "local-fs-store.hh" #include "globals.hh" #include "realisation.hh" diff --git a/src/nix-channel/nix-channel.cc b/src/nix-channel/nix-channel.cc index d8d0d518b78..028a7f36f24 100644 --- a/src/nix-channel/nix-channel.cc +++ b/src/nix-channel/nix-channel.cc @@ -2,7 +2,6 @@ #include "shared.hh" #include "globals.hh" #include "filetransfer.hh" -#include "store-api.hh" #include "store-open.hh" #include "legacy.hh" #include "eval-settings.hh" // for defexpr diff --git a/src/nix-collect-garbage/nix-collect-garbage.cc b/src/nix-collect-garbage/nix-collect-garbage.cc index 91209c97898..9830b1058a3 100644 --- a/src/nix-collect-garbage/nix-collect-garbage.cc +++ b/src/nix-collect-garbage/nix-collect-garbage.cc @@ -1,6 +1,6 @@ #include "file-system.hh" #include "signals.hh" -#include "store-api.hh" +#include "store-open.hh" #include "store-cast.hh" #include "gc-store.hh" #include "profiles.hh" diff --git a/src/nix-copy-closure/nix-copy-closure.cc b/src/nix-copy-closure/nix-copy-closure.cc index b64af758fcb..d910adcf876 100644 --- a/src/nix-copy-closure/nix-copy-closure.cc +++ b/src/nix-copy-closure/nix-copy-closure.cc @@ -1,6 +1,6 @@ #include "shared.hh" #include "realisation.hh" -#include "store-api.hh" +#include "store-open.hh" #include "legacy.hh" using namespace nix; diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 5e170c99d37..056e9025800 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -9,7 +9,7 @@ #include "profiles.hh" #include "path-with-outputs.hh" #include "shared.hh" -#include "store-api.hh" +#include "store-open.hh" #include "local-fs-store.hh" #include "user-env.hh" #include "value-to-json.hh" diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc index c4854951124..5474ed0b7cb 100644 --- a/src/nix-instantiate/nix-instantiate.cc +++ b/src/nix-instantiate/nix-instantiate.cc @@ -8,7 +8,7 @@ #include "signals.hh" #include "value-to-xml.hh" #include "value-to-json.hh" -#include "store-api.hh" +#include "store-open.hh" #include "local-fs-store.hh" #include "common-eval-args.hh" #include "legacy.hh" diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index f073074e851..40d20b37690 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -2,6 +2,7 @@ #include "derivations.hh" #include "dotgraph.hh" #include "globals.hh" +#include "store-open.hh" #include "store-cast.hh" #include "local-fs-store.hh" #include "log-store.hh" diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 3f9f8f99b06..26e553bde07 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -8,7 +8,7 @@ #include "flake/flake.hh" #include "get-drvs.hh" #include "signals.hh" -#include "store-api.hh" +#include "store-open.hh" #include "derivations.hh" #include "outputs-spec.hh" #include "attr-path.hh" diff --git a/src/nix/log.cc b/src/nix/log.cc index 7f590c708f6..efa4a584ccc 100644 --- a/src/nix/log.cc +++ b/src/nix/log.cc @@ -1,7 +1,7 @@ #include "command.hh" #include "common-args.hh" #include "shared.hh" -#include "store-api.hh" +#include "store-open.hh" #include "log-store.hh" #include "progress-bar.hh" diff --git a/src/nix/main.cc b/src/nix/main.cc index 00ad6fe2c97..0d839425afb 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -8,7 +8,8 @@ #include "globals.hh" #include "legacy.hh" #include "shared.hh" -#include "store-api.hh" +#include "store-open.hh" +#include "store-registration.hh" #include "filetransfer.hh" #include "finally.hh" #include "loggers.hh" diff --git a/src/nix/make-content-addressed.cc b/src/nix/make-content-addressed.cc index d9c988a9f5d..0c3efd7f499 100644 --- a/src/nix/make-content-addressed.cc +++ b/src/nix/make-content-addressed.cc @@ -1,5 +1,5 @@ #include "command.hh" -#include "store-api.hh" +#include "store-open.hh" #include "make-content-addressed.hh" #include "common-args.hh" diff --git a/src/nix/prefetch.cc b/src/nix/prefetch.cc index db7d9e4efe6..fa087430f86 100644 --- a/src/nix/prefetch.cc +++ b/src/nix/prefetch.cc @@ -1,7 +1,7 @@ #include "command.hh" #include "common-args.hh" #include "shared.hh" -#include "store-api.hh" +#include "store-open.hh" #include "filetransfer.hh" #include "finally.hh" #include "progress-bar.hh" diff --git a/src/nix/repl.cc b/src/nix/repl.cc index a2f3e033e72..8f74149042d 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -1,6 +1,7 @@ #include "eval.hh" #include "eval-settings.hh" #include "globals.hh" +#include "store-open.hh" #include "command.hh" #include "installable-value.hh" #include "repl.hh" diff --git a/src/nix/sigs.cc b/src/nix/sigs.cc index 1e277cbbee7..5a8693ab6c4 100644 --- a/src/nix/sigs.cc +++ b/src/nix/sigs.cc @@ -1,7 +1,7 @@ #include "signals.hh" #include "command.hh" #include "shared.hh" -#include "store-api.hh" +#include "store-open.hh" #include "thread-pool.hh" #include "progress-bar.hh" diff --git a/src/nix/unix/daemon.cc b/src/nix/unix/daemon.cc index 4a7997b1fb0..8b618c19e2b 100644 --- a/src/nix/unix/daemon.cc +++ b/src/nix/unix/daemon.cc @@ -7,6 +7,7 @@ #include "local-store.hh" #include "remote-store.hh" #include "remote-store-connection.hh" +#include "store-open.hh" #include "serialise.hh" #include "archive.hh" #include "globals.hh" @@ -239,7 +240,7 @@ static PeerInfo getPeerInfo(int remote) */ static ref openUncachedStore() { - Store::Params params; // FIXME: get params from somewhere + StoreReference::Params params; // FIXME: get params from somewhere // Disable caching since the client already does that. params["path-info-cache-size"] = "0"; return openStore(settings.storeUri, params); diff --git a/src/nix/verify.cc b/src/nix/verify.cc index 124a05bed2c..3e2253fdf04 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -1,6 +1,6 @@ #include "command.hh" #include "shared.hh" -#include "store-api.hh" +#include "store-open.hh" #include "thread-pool.hh" #include "signals.hh" #include "keys.hh" diff --git a/tests/unit/libstore/local-overlay-store.cc b/tests/unit/libstore/local-overlay-store.cc index b34ca92375e..4fe7e607f47 100644 --- a/tests/unit/libstore/local-overlay-store.cc +++ b/tests/unit/libstore/local-overlay-store.cc @@ -1,9 +1,6 @@ -// FIXME: Odd failures for templates that are causing the PR to break -// for now with discussion with @Ericson2314 to comment out. -#if 0 -# include +#include -# include "local-overlay-store.hh" +#include "local-overlay-store.hh" namespace nix { @@ -31,4 +28,3 @@ TEST(LocalOverlayStore, constructConfig_rootPath) } } // namespace nix -#endif diff --git a/tests/unit/libstore/local-store.cc b/tests/unit/libstore/local-store.cc index abc3ea7963f..e22f54bd45d 100644 --- a/tests/unit/libstore/local-store.cc +++ b/tests/unit/libstore/local-store.cc @@ -1,15 +1,6 @@ -// FIXME: Odd failures for templates that are causing the PR to break -// for now with discussion with @Ericson2314 to comment out. -#if 0 -# include +#include -# include "local-store.hh" - -// Needed for template specialisations. This is not good! When we -// overhaul how store configs work, this should be fixed. -# include "args.hh" -# include "config-impl.hh" -# include "abstract-setting-to-json.hh" +#include "local-store.hh" namespace nix { @@ -37,4 +28,3 @@ TEST(LocalStore, constructConfig_rootPath) } } // namespace nix -#endif diff --git a/tests/unit/libstore/ssh-store.cc b/tests/unit/libstore/ssh-store.cc index b853a5f1fb9..a1fcef6b863 100644 --- a/tests/unit/libstore/ssh-store.cc +++ b/tests/unit/libstore/ssh-store.cc @@ -1,9 +1,6 @@ -// FIXME: Odd failures for templates that are causing the PR to break -// for now with discussion with @Ericson2314 to comment out. -#if 0 -# include +#include -# include "ssh-store.hh" +#include "ssh-store.hh" namespace nix { @@ -52,4 +49,3 @@ TEST(MountedSSHStore, constructConfig) } } -#endif diff --git a/tests/unit/libstore/uds-remote-store.cc b/tests/unit/libstore/uds-remote-store.cc index 5ccb208714f..4bd3bc18f5f 100644 --- a/tests/unit/libstore/uds-remote-store.cc +++ b/tests/unit/libstore/uds-remote-store.cc @@ -1,9 +1,6 @@ -// FIXME: Odd failures for templates that are causing the PR to break -// for now with discussion with @Ericson2314 to comment out. -#if 0 -# include +#include -# include "uds-remote-store.hh" +#include "uds-remote-store.hh" namespace nix { @@ -20,4 +17,3 @@ TEST(UDSRemoteStore, constructConfigWrongScheme) } } // namespace nix -#endif