From 25b0242ca6aee3484111739e95b6788ea56d1a64 Mon Sep 17 00:00:00 2001 From: siddhantCodes Date: Sun, 9 Jun 2024 19:49:39 +0530 Subject: [PATCH 01/18] `std::filesystem::create_directories` for createDirs The implementation of `nix::createDirs` allows it to be a simple wrapper around `std::filesystem::create_directories` as its return value is not used anywhere. --- src/libcmd/repl-interacter.cc | 4 ++-- src/libutil/file-system.cc | 25 ++----------------------- src/libutil/file-system.hh | 7 +++---- 3 files changed, 7 insertions(+), 29 deletions(-) diff --git a/src/libcmd/repl-interacter.cc b/src/libcmd/repl-interacter.cc index eb4361e25..254a86d7b 100644 --- a/src/libcmd/repl-interacter.cc +++ b/src/libcmd/repl-interacter.cc @@ -107,8 +107,8 @@ ReadlineLikeInteracter::Guard ReadlineLikeInteracter::init(detail::ReplCompleter rl_readline_name = "nix-repl"; try { createDirs(dirOf(historyFile)); - } catch (SystemError & e) { - logWarning(e.info()); + } catch (std::filesystem::filesystem_error & e) { + warn(e.what()); } #ifndef USE_READLINE el_hist_size = 1000; diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 919bf5d50..cd5db31bb 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -413,30 +413,9 @@ void deletePath(const fs::path & path) } -Paths createDirs(const Path & path) +void createDirs(const Path & path) { - Paths created; - if (path == "/") return created; - - struct stat st; - if (STAT(path.c_str(), &st) == -1) { - created = createDirs(dirOf(path)); - if (mkdir(path.c_str() -#ifndef _WIN32 // TODO abstract mkdir perms for Windows - , 0777 -#endif - ) == -1 && errno != EEXIST) - throw SysError("creating directory '%1%'", path); - st = STAT(path); - created.push_back(path); - } - - if (S_ISLNK(st.st_mode) && stat(path.c_str(), &st) == -1) - throw SysError("statting symlink '%1%'", path); - - if (!S_ISDIR(st.st_mode)) throw Error("'%1%' is not a directory", path); - - return created; + fs::create_directories(path); } diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh index c6b6ecedb..9405cda0c 100644 --- a/src/libutil/file-system.hh +++ b/src/libutil/file-system.hh @@ -148,11 +148,10 @@ void deletePath(const std::filesystem::path & path); void deletePath(const std::filesystem::path & path, uint64_t & bytesFreed); /** - * Create a directory and all its parents, if necessary. Returns the - * list of created directories, in order of creation. + * Create a directory and all its parents, if necessary. */ -Paths createDirs(const Path & path); -inline Paths createDirs(PathView path) +void createDirs(const Path & path); +inline void createDirs(PathView path) { return createDirs(Path(path)); } From 7a21432e7774617731556d844d05686a8b3ff46d Mon Sep 17 00:00:00 2001 From: siddhantCodes Date: Mon, 10 Jun 2024 11:30:39 +0530 Subject: [PATCH 02/18] fix: catch `filesystem_error` thrown by `createDirs` --- src/libstore/store-api.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index c67ccd7d4..ed5275377 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -19,6 +19,7 @@ #include "signals.hh" #include "users.hh" +#include #include using json = nlohmann::json; @@ -1303,7 +1304,7 @@ ref openStore(StoreReference && storeURI) if (!pathExists(chrootStore)) { try { createDirs(chrootStore); - } catch (Error & e) { + } catch (std::filesystem::filesystem_error & e) { return std::make_shared(params); } warn("'%s' does not exist, so Nix will use '%s' as a chroot store", stateDir, chrootStore); From 4809e59b7ee0681d420f83a285d52e9424f2fad8 Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Mon, 10 Jun 2024 09:31:21 -0400 Subject: [PATCH 03/18] fix: warn and document when advanced attributes will have no impact due to __structuredAttrs --- doc/manual/src/language/advanced-attributes.md | 6 ++++++ src/libexpr/eval.cc | 6 ++++++ src/libexpr/eval.hh | 5 ++++- src/libexpr/primops.cc | 14 ++++++++++++++ .../unix/build/local-derivation-goal.cc | 18 ++++++++++++++++++ 5 files changed, 48 insertions(+), 1 deletion(-) diff --git a/doc/manual/src/language/advanced-attributes.md b/doc/manual/src/language/advanced-attributes.md index 113062db1..64a28a366 100644 --- a/doc/manual/src/language/advanced-attributes.md +++ b/doc/manual/src/language/advanced-attributes.md @@ -301,6 +301,12 @@ Derivations can declare some infrequently used optional attributes. (associative) arrays. For example, the attribute `hardening.format = true` ends up as the Bash associative array element `${hardening[format]}`. + > **Warning** + > + > If set to `true`, other advanced attributes such as [`allowedReferences`](#adv-attr-allowedReferences), [`allowedReferences`](#adv-attr-allowedReferences), [`allowedRequisites`](#adv-attr-allowedRequisites), + [`disallowedReferences`](#adv-attr-disallowedReferences) and [`disallowedRequisites`](#adv-attr-disallowedRequisites), maxSize, and maxClosureSize. + will have no effect. + - [`outputChecks`]{#adv-attr-outputChecks}\ When using [structured attributes](#adv-attr-structuredAttrs), the `outputChecks` attribute allows defining checks per-output. diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index f441977a5..5a94c67ec 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -423,6 +423,12 @@ EvalState::EvalState( , sRight(symbols.create("right")) , sWrong(symbols.create("wrong")) , sStructuredAttrs(symbols.create("__structuredAttrs")) + , sAllowedReferences(symbols.create("allowedReferences")) + , sAllowedRequisites(symbols.create("allowedRequisites")) + , sDisallowedReferences(symbols.create("disallowedReferences")) + , sDisallowedRequisites(symbols.create("disallowedRequisites")) + , sMaxSize(symbols.create("maxSize")) + , sMaxClosureSize(symbols.create("maxClosureSize")) , sBuilder(symbols.create("builder")) , sArgs(symbols.create("args")) , sContentAddressed(symbols.create("__contentAddressed")) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index dac763268..f916873f1 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -173,7 +173,10 @@ public: const Symbol sWith, sOutPath, sDrvPath, sType, sMeta, sName, sValue, sSystem, sOverrides, sOutputs, sOutputName, sIgnoreNulls, sFile, sLine, sColumn, sFunctor, sToString, - sRight, sWrong, sStructuredAttrs, sBuilder, sArgs, + sRight, sWrong, sStructuredAttrs, + sAllowedReferences, sAllowedRequisites, sDisallowedReferences, sDisallowedRequisites, + sMaxSize, sMaxClosureSize, + sBuilder, sArgs, sContentAddressed, sImpure, sOutputHash, sOutputHashAlgo, sOutputHashMode, sRecurseForDerivations, diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 9177b0a2f..98649f081 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1308,6 +1308,20 @@ static void derivationStrictInternal( handleOutputs(ss); } + if (i->name == state.sAllowedReferences) + warn("In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'allowedReferences'; use 'outputChecks..allowedReferences' instead", drvName); + if (i->name == state.sAllowedRequisites) + warn("In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'allowedRequisites'; use 'outputChecks..allowedRequisites' instead", drvName); + if (i->name == state.sDisallowedReferences) + warn("In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedReferences'; use 'outputChecks..disallowedReferences' instead", drvName); + if (i->name == state.sDisallowedRequisites) + warn("In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedRequisites'; use 'outputChecks..disallowedRequisites' instead", drvName); + if (i->name == state.sMaxSize) + warn("In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'maxSize'; use 'outputChecks..maxSize' instead", drvName); + if (i->name == state.sMaxClosureSize) + warn("In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'maxClosureSize'; use 'outputChecks..maxClosureSize' instead", drvName); + + } else { auto s = state.coerceToString(pos, *i->value, context, context_below, true).toOwned(); drv.env.emplace(key, s); diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 16095cf5d..54644dc3a 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -2904,6 +2904,24 @@ void LocalDerivationGoal::checkOutputs(const std::mapgetStructuredAttrs()) { + if (get(*structuredAttrs, "allowedReferences")){ + warn("'structuredAttrs' disables the effect of the top-level attribute 'allowedReferences'; use 'outputChecks' instead"); + } + if (get(*structuredAttrs, "allowedRequisites")){ + warn("'structuredAttrs' disables the effect of the top-level attribute 'allowedRequisites'; use 'outputChecks' instead"); + } + if (get(*structuredAttrs, "disallowedRequisites")){ + warn("'structuredAttrs' disables the effect of the top-level attribute 'disallowedRequisites'; use 'outputChecks' instead"); + } + if (get(*structuredAttrs, "disallowedReferences")){ + warn("'structuredAttrs' disables the effect of the top-level attribute 'disallowedReferences'; use 'outputChecks' instead"); + } + if (get(*structuredAttrs, "maxSize")){ + warn("'structuredAttrs' disables the effect of the top-level attribute 'maxSize'; use 'outputChecks' instead"); + } + if (get(*structuredAttrs, "maxClosureSize")){ + warn("'structuredAttrs' disables the effect of the top-level attribute 'maxClosureSize'; use 'outputChecks' instead"); + } if (auto outputChecks = get(*structuredAttrs, "outputChecks")) { if (auto output = get(*outputChecks, outputName)) { Checks checks; From de3fd52a95cedb57bc6d9dbda0c597cd2bfb3db5 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 10 Jun 2024 15:55:53 +0200 Subject: [PATCH 04/18] Add tests/f/lang/eval-okay-derivation-legacy --- .../lang/eval-okay-derivation-legacy.err.exp | 6 ++++++ .../functional/lang/eval-okay-derivation-legacy.exp | 1 + .../functional/lang/eval-okay-derivation-legacy.nix | 12 ++++++++++++ 3 files changed, 19 insertions(+) create mode 100644 tests/functional/lang/eval-okay-derivation-legacy.err.exp create mode 100644 tests/functional/lang/eval-okay-derivation-legacy.exp create mode 100644 tests/functional/lang/eval-okay-derivation-legacy.nix diff --git a/tests/functional/lang/eval-okay-derivation-legacy.err.exp b/tests/functional/lang/eval-okay-derivation-legacy.err.exp new file mode 100644 index 000000000..94f0854dd --- /dev/null +++ b/tests/functional/lang/eval-okay-derivation-legacy.err.exp @@ -0,0 +1,6 @@ +warning: In a derivation named 'eval-okay-derivation-legacy', 'structuredAttrs' disables the effect of the derivation attribute 'allowedReferences'; use 'outputChecks..allowedReferences' instead +warning: In a derivation named 'eval-okay-derivation-legacy', 'structuredAttrs' disables the effect of the derivation attribute 'allowedRequisites'; use 'outputChecks..allowedRequisites' instead +warning: In a derivation named 'eval-okay-derivation-legacy', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedReferences'; use 'outputChecks..disallowedReferences' instead +warning: In a derivation named 'eval-okay-derivation-legacy', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedRequisites'; use 'outputChecks..disallowedRequisites' instead +warning: In a derivation named 'eval-okay-derivation-legacy', 'structuredAttrs' disables the effect of the derivation attribute 'maxClosureSize'; use 'outputChecks..maxClosureSize' instead +warning: In a derivation named 'eval-okay-derivation-legacy', 'structuredAttrs' disables the effect of the derivation attribute 'maxSize'; use 'outputChecks..maxSize' instead diff --git a/tests/functional/lang/eval-okay-derivation-legacy.exp b/tests/functional/lang/eval-okay-derivation-legacy.exp new file mode 100644 index 000000000..4f374a1aa --- /dev/null +++ b/tests/functional/lang/eval-okay-derivation-legacy.exp @@ -0,0 +1 @@ +"/nix/store/mzgwvrjjir216ra58mwwizi8wj6y9ddr-eval-okay-derivation-legacy" diff --git a/tests/functional/lang/eval-okay-derivation-legacy.nix b/tests/functional/lang/eval-okay-derivation-legacy.nix new file mode 100644 index 000000000..b529cdf90 --- /dev/null +++ b/tests/functional/lang/eval-okay-derivation-legacy.nix @@ -0,0 +1,12 @@ +(builtins.derivationStrict { + name = "eval-okay-derivation-legacy"; + system = "x86_64-linux"; + builder = "/dontcare"; + __structuredAttrs = true; + allowedReferences = [ ]; + disallowedReferences = [ ]; + allowedRequisites = [ ]; + disallowedRequisites = [ ]; + maxSize = 1234; + maxClosureSize = 12345; +}).out From a58ca342cae1d6bee488a42b6fc054c1072cfb8f Mon Sep 17 00:00:00 2001 From: PoweredByPie Date: Mon, 17 Jun 2024 11:01:29 -0700 Subject: [PATCH 05/18] Initial runProgram implementation for Windows This is incomplete; proper shell escaping needs to be done --- src/libutil/processes.hh | 17 +- src/libutil/windows/processes.cc | 322 ++++++++++++++++++++++++++++++- 2 files changed, 331 insertions(+), 8 deletions(-) diff --git a/src/libutil/processes.hh b/src/libutil/processes.hh index 168fcaa55..bbbe7dcab 100644 --- a/src/libutil/processes.hh +++ b/src/libutil/processes.hh @@ -3,6 +3,7 @@ #include "types.hh" #include "error.hh" +#include "file-descriptor.hh" #include "logging.hh" #include "ansicolor.hh" @@ -23,26 +24,36 @@ namespace nix { struct Sink; struct Source; -#ifndef _WIN32 class Pid { +#ifndef _WIN32 pid_t pid = -1; bool separatePG = false; int killSignal = SIGKILL; +#else + AutoCloseFD pid = INVALID_DESCRIPTOR; +#endif public: Pid(); +#ifndef _WIN32 Pid(pid_t pid); - ~Pid(); void operator =(pid_t pid); operator pid_t(); +#else + Pid(AutoCloseFD pid); + void operator =(AutoCloseFD pid); +#endif + ~Pid(); int kill(); int wait(); + // TODO: Implement for Windows +#ifndef _WIN32 void setSeparatePG(bool separatePG); void setKillSignal(int signal); pid_t release(); -}; #endif +}; #ifndef _WIN32 diff --git a/src/libutil/windows/processes.cc b/src/libutil/windows/processes.cc index 44a32f6a1..bb796ce2e 100644 --- a/src/libutil/windows/processes.cc +++ b/src/libutil/windows/processes.cc @@ -1,9 +1,15 @@ #include "current-process.hh" #include "environment-variables.hh" +#include "error.hh" +#include "file-descriptor.hh" +#include "file-path.hh" #include "signals.hh" #include "processes.hh" #include "finally.hh" #include "serialise.hh" +#include "file-system.hh" +#include "util.hh" +#include "windows-error.hh" #include #include @@ -16,25 +22,332 @@ #include #include +#define WIN32_LEAN_AND_MEAN +#include + namespace nix { +using namespace nix::windows; + +Pid::Pid() {} + +Pid::Pid(AutoCloseFD pid) + : pid(std::move(pid)) +{ +} + +Pid::~Pid() +{ + if (pid.get() != INVALID_DESCRIPTOR) + kill(); +} + +void Pid::operator=(AutoCloseFD pid) +{ + if (this->pid.get() != INVALID_DESCRIPTOR && this->pid.get() != pid.get()) + kill(); + this->pid = std::move(pid); +} + +// TODO: Implement (not needed for process spawning yet) +int Pid::kill() +{ + assert(pid.get() != INVALID_DESCRIPTOR); + + debug("killing process %1%", pid.get()); + + throw UnimplementedError("Pid::kill unimplemented"); +} + +int Pid::wait() +{ + // https://github.com/nix-windows/nix/blob/windows-meson/src/libutil/util.cc#L1938 + assert(pid.get() != INVALID_DESCRIPTOR); + DWORD status = WaitForSingleObject(pid.get(), INFINITE); + if (status != WAIT_OBJECT_0) { + debug("WaitForSingleObject returned %1%", status); + } + + DWORD exitCode = 0; + if (GetExitCodeProcess(pid.get(), &exitCode) == FALSE) { + debug("GetExitCodeProcess failed on pid %1%", pid.get()); + } + + pid.close(); + return exitCode; +} + +// TODO: Merge this with Unix's runProgram since it's identical logic. std::string runProgram(Path program, bool lookupPath, const Strings & args, const std::optional & input, bool isInteractive) { - throw UnimplementedError("Cannot shell out to git on Windows yet"); + auto res = runProgram(RunOptions{ + .program = program, .lookupPath = lookupPath, .args = args, .input = input, .isInteractive = isInteractive}); + + if (!statusOk(res.first)) + throw ExecError(res.first, "program '%1%' %2%", program, statusToString(res.first)); + + return res.second; +} +// Looks at the $PATH environment variable to find the program. +// Adapted from https://github.com/nix-windows/nix/blob/windows/src/libutil/util.cc#L2276 +Path lookupPathForProgram(const Path & program) +{ + if (program.find('/') != program.npos || program.find('\\') != program.npos) { + throw Error("program '%1%' partially specifies its path", program); + } + + // Possible extensions. + // TODO: This should actually be sourced from $PATHEXT, not hardcoded. + static constexpr const char * exts[] = {"", ".exe", ".cmd", ".bat"}; + + auto path = getEnv("PATH"); + if (!path.has_value()) { + throw WinError("couldn't find PATH environment variable"); + } + + // Look through each directory listed in $PATH. + for (const std::string & dir : tokenizeString(*getEnv("PATH"), ";")) { + Path candidate = canonPath(dir) + '/' + program; + for (const auto ext : exts) { + if (pathExists(candidate + ext)) { + return candidate; + } + } + } + + throw WinError("program '%1%' not found on PATH", program); } +std::optional getProgramInterpreter(const Path & program) +{ + // These extensions are automatically handled by Windows and don't require an interpreter. + static constexpr const char * exts[] = {".exe", ".cmd", ".bat"}; + for (const auto ext : exts) { + if (hasSuffix(program, ext)) { + return {}; + } + } + // TODO: Open file and read the shebang + throw UnimplementedError("getProgramInterpreter unimplemented"); +} +// TODO: Not sure if this is needed in the unix version but it might be useful as a member func +void setFDInheritable(AutoCloseFD & fd, bool inherit) +{ + if (fd.get() != INVALID_DESCRIPTOR) { + if (!SetHandleInformation(fd.get(), HANDLE_FLAG_INHERIT, inherit ? HANDLE_FLAG_INHERIT : 0)) { + throw WinError("Couldn't disable inheriting of handle"); + } + } +} + +AutoCloseFD nullFD() +{ + // Create null handle to discard reads / writes + // https://stackoverflow.com/a/25609668 + // https://github.com/nix-windows/nix/blob/windows-meson/src/libutil/util.cc#L2228 + AutoCloseFD nul = CreateFileW( + L"NUL", + GENERIC_READ | GENERIC_WRITE, + // We don't care who reads / writes / deletes this file since it's NUL anyways + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + NULL, + OPEN_EXISTING, + 0, + NULL); + if (!nul.get()) { + throw WinError("Couldn't open NUL device"); + } + // Let this handle be inheritable by child processes + setFDInheritable(nul, true); + return nul; +} + +Pid spawnProcess(const Path & realProgram, const RunOptions & options, Pipe & out, Pipe & in) +{ + // Setup pipes. + if (options.standardOut) { + // Don't inherit the read end of the output pipe + setFDInheritable(out.readSide, false); + } else { + out.writeSide = nullFD(); + } + if (options.standardIn) { + // Don't inherit the write end of the input pipe + setFDInheritable(in.writeSide, false); + } else { + in.readSide = nullFD(); + } + + STARTUPINFOW startInfo = {0}; + startInfo.cb = sizeof(startInfo); + startInfo.dwFlags = STARTF_USESTDHANDLES; + startInfo.hStdInput = in.readSide.get(); + startInfo.hStdOutput = out.writeSide.get(); + startInfo.hStdError = out.writeSide.get(); + + std::string envline; + // Retain the current processes' environment variables. + for (const auto & envVar : getEnv()) { + envline += (envVar.first + '=' + envVar.second + '\0'); + } + // Also add new ones specified in options. + if (options.environment) { + for (const auto & envVar : *options.environment) { + envline += (envVar.first + '=' + envVar.second + '\0'); + } + } + + std::string cmdline = realProgram; + for (const auto & arg : options.args) { + // TODO: This isn't the right way to escape windows command + // See https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-commandlinetoargvw + cmdline += ' ' + shellEscape(arg); + } + + PROCESS_INFORMATION procInfo = {0}; + if (CreateProcessW( + string_to_os_string(realProgram).c_str(), + string_to_os_string(cmdline).data(), + NULL, + NULL, + TRUE, + CREATE_UNICODE_ENVIRONMENT | CREATE_SUSPENDED, + string_to_os_string(envline).data(), + options.chdir.has_value() ? string_to_os_string(*options.chdir).data() : NULL, + &startInfo, + &procInfo) + == 0) { + throw WinError("CreateProcessW failed (%1%)", cmdline); + } + + // Convert these to use RAII + AutoCloseFD process = procInfo.hProcess; + AutoCloseFD thread = procInfo.hThread; + + // Add current process and child to job object so child terminates when parent terminates + // TODO: This spawns one job per child process. We can probably keep this as a global, and + // add children a single job so we don't use so many jobs at once. + Descriptor job = CreateJobObjectW(NULL, NULL); + if (job == NULL) { + TerminateProcess(procInfo.hProcess, 0); + throw WinError("Couldn't create job object for child process"); + } + if (AssignProcessToJobObject(job, procInfo.hProcess) == FALSE) { + TerminateProcess(procInfo.hProcess, 0); + throw WinError("Couldn't assign child process to job object"); + } + if (ResumeThread(procInfo.hThread) == -1) { + TerminateProcess(procInfo.hProcess, 0); + throw WinError("Couldn't resume child process thread"); + } + + return process; +} + +// TODO: Merge this with Unix's runProgram since it's identical logic. // Output = error code + "standard out" output stream std::pair runProgram(RunOptions && options) { - throw UnimplementedError("Cannot shell out to git on Windows yet"); -} + StringSink sink; + options.standardOut = &sink; + int status = 0; + + try { + runProgram2(options); + } catch (ExecError & e) { + status = e.status; + } + + return {status, std::move(sink.s)}; +} void runProgram2(const RunOptions & options) { - throw UnimplementedError("Cannot shell out to git on Windows yet"); + checkInterrupt(); + + assert(!(options.standardIn && options.input)); + + std::unique_ptr source_; + Source * source = options.standardIn; + + if (options.input) { + source_ = std::make_unique(*options.input); + source = source_.get(); + } + + /* Create a pipe. */ + Pipe out, in; + // TODO: I copied this from unix but this is handled again in spawnProcess, so might be weird to split it up like + // this + if (options.standardOut) + out.create(); + if (source) + in.create(); + + Path realProgram = options.program; + if (options.lookupPath) { + realProgram = lookupPathForProgram(realProgram); + } + // TODO: Implement shebang / program interpreter lookup on Windows + auto interpreter = getProgramInterpreter(realProgram); + + std::optional>> resumeLoggerDefer; + if (options.isInteractive) { + logger->pause(); + resumeLoggerDefer.emplace([]() { logger->resume(); }); + } + + Pid pid = spawnProcess(interpreter.has_value() ? *interpreter : realProgram, options, out, in); + + // TODO: This is identical to unix, deduplicate? + out.writeSide.close(); + + std::thread writerThread; + + std::promise promise; + + Finally doJoin([&] { + if (writerThread.joinable()) + writerThread.join(); + }); + + if (source) { + in.readSide.close(); + writerThread = std::thread([&] { + try { + std::vector buf(8 * 1024); + while (true) { + size_t n; + try { + n = source->read(buf.data(), buf.size()); + } catch (EndOfFile &) { + break; + } + writeFull(in.writeSide.get(), {buf.data(), n}); + } + promise.set_value(); + } catch (...) { + promise.set_exception(std::current_exception()); + } + in.writeSide.close(); + }); + } + + if (options.standardOut) + drainFD(out.readSide.get(), *options.standardOut); + + /* Wait for the child to finish. */ + int status = pid.wait(); + + /* Wait for the writer thread to finish. */ + if (source) + promise.get_future().get(); + + if (status) + throw ExecError(status, "program '%1%' %2%", options.program, statusToString(status)); } std::string statusToString(int status) @@ -45,7 +358,6 @@ std::string statusToString(int status) return "succeeded"; } - bool statusOk(int status) { return status == 0; From b11cf8166f63fa061d98cd5812a94234fe974845 Mon Sep 17 00:00:00 2001 From: PoweredByPie Date: Mon, 17 Jun 2024 13:12:28 -0700 Subject: [PATCH 06/18] Format runProgram declaration --- src/libutil/windows/processes.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libutil/windows/processes.cc b/src/libutil/windows/processes.cc index bb796ce2e..1d8035c62 100644 --- a/src/libutil/windows/processes.cc +++ b/src/libutil/windows/processes.cc @@ -78,8 +78,8 @@ int Pid::wait() } // TODO: Merge this with Unix's runProgram since it's identical logic. -std::string runProgram(Path program, bool lookupPath, const Strings & args, - const std::optional & input, bool isInteractive) +std::string runProgram( + Path program, bool lookupPath, const Strings & args, const std::optional & input, bool isInteractive) { auto res = runProgram(RunOptions{ .program = program, .lookupPath = lookupPath, .args = args, .input = input, .isInteractive = isInteractive}); From 4662e7d8568f55f7f56c55e7976b68a25824d1a3 Mon Sep 17 00:00:00 2001 From: PoweredByPie Date: Mon, 17 Jun 2024 14:57:57 -0700 Subject: [PATCH 07/18] Implement windowsEscape --- src/libutil/windows/processes.cc | 48 +++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/src/libutil/windows/processes.cc b/src/libutil/windows/processes.cc index 1d8035c62..cf1949aa2 100644 --- a/src/libutil/windows/processes.cc +++ b/src/libutil/windows/processes.cc @@ -164,6 +164,52 @@ AutoCloseFD nullFD() return nul; } +// Adapted from +// https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/ +std::string windowsEscape(const std::string & str, bool cmd) +{ + // TODO: This doesn't handle cmd.exe escaping. + if (cmd) { + throw UnimplementedError("cmd.exe escaping is not implemented"); + } + + if (str.find_first_of(" \t\n\v\"") == str.npos && !str.empty()) { + // No need to escape this one, the nonempty contents don't have a special character + return str; + } + std::string buffer; + // Add the opening quote + buffer += '"'; + for (auto iter = str.begin();; ++iter) { + size_t backslashes = 0; + while (iter != str.end() && *iter == '\\') { + ++iter; + ++backslashes; + } + + // We only escape backslashes if: + // - They come immediately before the closing quote + // - They come immediately before a quote in the middle of the string + // Both of these cases break the escaping if not handled. Otherwise backslashes are fine as-is + if (iter == str.end()) { + // Need to escape each backslash + buffer.append(backslashes * 2, '\\'); + // Exit since we've reached the end of the string + break; + } else if (*iter == '"') { + // Need to escape each backslash and the intermediate quote character + buffer.append(backslashes * 2, '\\'); + buffer += "\\\""; + } else { + // Don't escape the backslashes since they won't break the delimiter + buffer.append(backslashes, '\\'); + buffer += *iter; + } + } + // Add the closing quote + return buffer + '"'; +} + Pid spawnProcess(const Path & realProgram, const RunOptions & options, Pipe & out, Pipe & in) { // Setup pipes. @@ -203,7 +249,7 @@ Pid spawnProcess(const Path & realProgram, const RunOptions & options, Pipe & ou for (const auto & arg : options.args) { // TODO: This isn't the right way to escape windows command // See https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-commandlinetoargvw - cmdline += ' ' + shellEscape(arg); + cmdline += ' ' + windowsEscape(arg, false); } PROCESS_INFORMATION procInfo = {0}; From d7537f695515339f811da73c98b64bb4e2e7dc9c Mon Sep 17 00:00:00 2001 From: PoweredByPie Date: Mon, 17 Jun 2024 14:58:17 -0700 Subject: [PATCH 08/18] Implement initial spawn tests (just testing windowsEscape for now) --- tests/unit/libutil/spawn.cc | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 tests/unit/libutil/spawn.cc diff --git a/tests/unit/libutil/spawn.cc b/tests/unit/libutil/spawn.cc new file mode 100644 index 000000000..6d9821f4e --- /dev/null +++ b/tests/unit/libutil/spawn.cc @@ -0,0 +1,35 @@ +#include + +#include "processes.hh" + +namespace nix { +/* +TEST(SpawnTest, spawnEcho) +{ +auto output = runProgram(RunOptions{.program = "echo", .args = {}}); +} +*/ + +#ifdef _WIN32 +std::string windowsEscape(const std::string & str, bool cmd); + +TEST(SpawnTest, windowsEscape) +{ + auto empty = windowsEscape("", false); + ASSERT_EQ(empty, R"("")"); + // There's no quotes in this argument so the input should equal the output + auto backslashStr = R"(\\\\)"; + auto backslashes = windowsEscape(backslashStr, false); + ASSERT_EQ(backslashes, backslashStr); + + auto nestedQuotes = windowsEscape(R"(he said: "hello there")", false); + ASSERT_EQ(nestedQuotes, R"("he said: \"hello there\"")"); + + auto middleQuote = windowsEscape(R"( \\\" )", false); + ASSERT_EQ(middleQuote, R"(" \\\\\\\" ")"); + + auto space = windowsEscape("hello world", false); + ASSERT_EQ(space, R"("hello world")"); +} +#endif +} From 4f6e3b940255053cd51e566416327c6120851075 Mon Sep 17 00:00:00 2001 From: PoweredByPie Date: Mon, 17 Jun 2024 18:44:23 -0700 Subject: [PATCH 09/18] Implement tests for lookupPathForProgram and fix bugs caught by tests --- src/libutil/windows/processes.cc | 8 ++++---- tests/unit/libutil/spawn.cc | 10 +++++++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/libutil/windows/processes.cc b/src/libutil/windows/processes.cc index cf1949aa2..a1104fa1e 100644 --- a/src/libutil/windows/processes.cc +++ b/src/libutil/windows/processes.cc @@ -94,7 +94,7 @@ std::string runProgram( Path lookupPathForProgram(const Path & program) { if (program.find('/') != program.npos || program.find('\\') != program.npos) { - throw Error("program '%1%' partially specifies its path", program); + throw UsageError("program '%1%' partially specifies its path", program); } // Possible extensions. @@ -107,8 +107,9 @@ Path lookupPathForProgram(const Path & program) } // Look through each directory listed in $PATH. - for (const std::string & dir : tokenizeString(*getEnv("PATH"), ";")) { - Path candidate = canonPath(dir) + '/' + program; + for (const std::string & dir : tokenizeString(*path, ";")) { + // TODO: This should actually be canonPath(dir), but that ends up appending two drive paths + Path candidate = dir + "/" + program; for (const auto ext : exts) { if (pathExists(candidate + ext)) { return candidate; @@ -408,5 +409,4 @@ bool statusOk(int status) { return status == 0; } - } diff --git a/tests/unit/libutil/spawn.cc b/tests/unit/libutil/spawn.cc index 6d9821f4e..e84de18f1 100644 --- a/tests/unit/libutil/spawn.cc +++ b/tests/unit/libutil/spawn.cc @@ -6,11 +6,19 @@ namespace nix { /* TEST(SpawnTest, spawnEcho) { -auto output = runProgram(RunOptions{.program = "echo", .args = {}}); +auto output = runProgram(RunOptions{.program = "cmd", .lookupPath = true, .args = {"/C", "echo \"hello world\""}}); +std::cout << output.second << std::endl; } */ #ifdef _WIN32 +Path lookupPathForProgram(const Path & program); +TEST(SpawnTest, pathSearch) +{ + ASSERT_NO_THROW(lookupPathForProgram("cmd")); + ASSERT_NO_THROW(lookupPathForProgram("cmd.exe")); + ASSERT_THROW(lookupPathForProgram("C:/System32/cmd.exe"), UsageError); +} std::string windowsEscape(const std::string & str, bool cmd); TEST(SpawnTest, windowsEscape) From fcb92b4fa4f260086aafdb7d9e8e51a26360b46e Mon Sep 17 00:00:00 2001 From: PoweredByPie Date: Mon, 17 Jun 2024 22:14:38 -0700 Subject: [PATCH 10/18] Fix DWORD vs. int comparison warning --- src/libutil/windows/processes.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/windows/processes.cc b/src/libutil/windows/processes.cc index a1104fa1e..973a2cab9 100644 --- a/src/libutil/windows/processes.cc +++ b/src/libutil/windows/processes.cc @@ -285,7 +285,7 @@ Pid spawnProcess(const Path & realProgram, const RunOptions & options, Pipe & ou TerminateProcess(procInfo.hProcess, 0); throw WinError("Couldn't assign child process to job object"); } - if (ResumeThread(procInfo.hThread) == -1) { + if (ResumeThread(procInfo.hThread) == (DWORD) -1) { TerminateProcess(procInfo.hProcess, 0); throw WinError("Couldn't resume child process thread"); } From 8b81d083a72b714a5599c8243f8e05ed15d2d7c0 Mon Sep 17 00:00:00 2001 From: PoweredByPie Date: Tue, 18 Jun 2024 00:55:47 -0700 Subject: [PATCH 11/18] Remove lookupPathForProgram and implement initial runProgram test Apparently, CreateProcessW already searches path, so manual path search isn't really necessary. --- src/libutil/windows/processes.cc | 38 +++----------------------------- tests/unit/libutil/spawn.cc | 17 +++++--------- 2 files changed, 8 insertions(+), 47 deletions(-) diff --git a/src/libutil/windows/processes.cc b/src/libutil/windows/processes.cc index 973a2cab9..9cd714f84 100644 --- a/src/libutil/windows/processes.cc +++ b/src/libutil/windows/processes.cc @@ -89,36 +89,6 @@ std::string runProgram( return res.second; } -// Looks at the $PATH environment variable to find the program. -// Adapted from https://github.com/nix-windows/nix/blob/windows/src/libutil/util.cc#L2276 -Path lookupPathForProgram(const Path & program) -{ - if (program.find('/') != program.npos || program.find('\\') != program.npos) { - throw UsageError("program '%1%' partially specifies its path", program); - } - - // Possible extensions. - // TODO: This should actually be sourced from $PATHEXT, not hardcoded. - static constexpr const char * exts[] = {"", ".exe", ".cmd", ".bat"}; - - auto path = getEnv("PATH"); - if (!path.has_value()) { - throw WinError("couldn't find PATH environment variable"); - } - - // Look through each directory listed in $PATH. - for (const std::string & dir : tokenizeString(*path, ";")) { - // TODO: This should actually be canonPath(dir), but that ends up appending two drive paths - Path candidate = dir + "/" + program; - for (const auto ext : exts) { - if (pathExists(candidate + ext)) { - return candidate; - } - } - } - - throw WinError("program '%1%' not found on PATH", program); -} std::optional getProgramInterpreter(const Path & program) { @@ -246,7 +216,7 @@ Pid spawnProcess(const Path & realProgram, const RunOptions & options, Pipe & ou } } - std::string cmdline = realProgram; + std::string cmdline = windowsEscape(realProgram, false); for (const auto & arg : options.args) { // TODO: This isn't the right way to escape windows command // See https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-commandlinetoargvw @@ -255,7 +225,8 @@ Pid spawnProcess(const Path & realProgram, const RunOptions & options, Pipe & ou PROCESS_INFORMATION procInfo = {0}; if (CreateProcessW( - string_to_os_string(realProgram).c_str(), + // EXE path is provided in the cmdline + NULL, string_to_os_string(cmdline).data(), NULL, NULL, @@ -335,9 +306,6 @@ void runProgram2(const RunOptions & options) in.create(); Path realProgram = options.program; - if (options.lookupPath) { - realProgram = lookupPathForProgram(realProgram); - } // TODO: Implement shebang / program interpreter lookup on Windows auto interpreter = getProgramInterpreter(realProgram); diff --git a/tests/unit/libutil/spawn.cc b/tests/unit/libutil/spawn.cc index e84de18f1..c617acae0 100644 --- a/tests/unit/libutil/spawn.cc +++ b/tests/unit/libutil/spawn.cc @@ -3,22 +3,15 @@ #include "processes.hh" namespace nix { -/* -TEST(SpawnTest, spawnEcho) -{ -auto output = runProgram(RunOptions{.program = "cmd", .lookupPath = true, .args = {"/C", "echo \"hello world\""}}); -std::cout << output.second << std::endl; -} -*/ #ifdef _WIN32 -Path lookupPathForProgram(const Path & program); -TEST(SpawnTest, pathSearch) +TEST(SpawnTest, spawnEcho) { - ASSERT_NO_THROW(lookupPathForProgram("cmd")); - ASSERT_NO_THROW(lookupPathForProgram("cmd.exe")); - ASSERT_THROW(lookupPathForProgram("C:/System32/cmd.exe"), UsageError); + auto output = runProgram(RunOptions{.program = "cmd.exe", .args = {"/C", "echo", "hello world"}}); + ASSERT_EQ(output.first, 0); + ASSERT_EQ(output.second, "\"hello world\"\r\n"); } + std::string windowsEscape(const std::string & str, bool cmd); TEST(SpawnTest, windowsEscape) From 85b7989764a00f5fa5a94d8ce29e4ac25434e7ac Mon Sep 17 00:00:00 2001 From: siddhantCodes Date: Thu, 20 Jun 2024 19:53:25 +0530 Subject: [PATCH 12/18] fix: handle errors in `nix::createDirs` the `std::filesystem::create_directories` can fail due to insufficient permissions. We convert this error into a `SysError` and catch it wherever required. --- src/libcmd/repl-interacter.cc | 4 ++-- src/libstore/store-api.cc | 2 +- src/libutil/file-system.cc | 6 +++++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libcmd/repl-interacter.cc b/src/libcmd/repl-interacter.cc index 254a86d7b..eb4361e25 100644 --- a/src/libcmd/repl-interacter.cc +++ b/src/libcmd/repl-interacter.cc @@ -107,8 +107,8 @@ ReadlineLikeInteracter::Guard ReadlineLikeInteracter::init(detail::ReplCompleter rl_readline_name = "nix-repl"; try { createDirs(dirOf(historyFile)); - } catch (std::filesystem::filesystem_error & e) { - warn(e.what()); + } catch (SystemError & e) { + logWarning(e.info()); } #ifndef USE_READLINE el_hist_size = 1000; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index ed5275377..2edb56510 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1304,7 +1304,7 @@ ref openStore(StoreReference && storeURI) if (!pathExists(chrootStore)) { try { createDirs(chrootStore); - } catch (std::filesystem::filesystem_error & e) { + } catch (SystemError & e) { return std::make_shared(params); } warn("'%s' does not exist, so Nix will use '%s' as a chroot store", stateDir, chrootStore); diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 5f269b7c0..443ccf829 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -415,7 +415,11 @@ void deletePath(const fs::path & path) void createDirs(const Path & path) { - fs::create_directories(path); + try { + fs::create_directories(path); + } catch (fs::filesystem_error & e) { + throw SysError("creating directory '%1%'", path); + } } From 0468061dd2f9e70042b0bd0b4bf503c54637fd1a Mon Sep 17 00:00:00 2001 From: Shogo Takata Date: Sun, 23 Jun 2024 00:52:19 +0900 Subject: [PATCH 13/18] accept response from gitlab with more than one entry --- src/libfetchers/github.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 267e8607f..ddb41e63f 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -433,7 +433,7 @@ struct GitLabInputScheme : GitArchiveInputScheme store->toRealPath( downloadFile(store, url, "source", headers).storePath))); - if (json.is_array() && json.size() == 1 && json[0]["id"] != nullptr) { + if (json.is_array() && json.size() >= 1 && json[0]["id"] != nullptr) { return RefInfo { .rev = Hash::parseAny(std::string(json[0]["id"]), HashAlgorithm::SHA1) }; From 490ca93cf886f046939ba4511dd88e5eb5dcddbc Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 23 Jun 2024 15:33:45 -0400 Subject: [PATCH 14/18] Factor out a bit more language testings infra Will be used in a second test after `lang.sh`. --- maintainers/flake-module.nix | 3 +- ...nfra.sh => characterisation-test-infra.sh} | 2 +- .../empty.exp => characterisation/empty} | 0 .../functional/characterisation/framework.sh | 77 +++++++++++++++++++ tests/functional/lang.sh | 32 +------- tests/functional/lang/framework.sh | 33 -------- tests/functional/local.mk | 2 +- 7 files changed, 82 insertions(+), 67 deletions(-) rename tests/functional/{lang-test-infra.sh => characterisation-test-infra.sh} (98%) rename tests/functional/{lang/empty.exp => characterisation/empty} (100%) create mode 100644 tests/functional/characterisation/framework.sh delete mode 100644 tests/functional/lang/framework.sh diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index b1d21e8fe..0bd353f1a 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -522,6 +522,7 @@ ''^tests/functional/ca/repl\.sh$'' ''^tests/functional/ca/selfref-gc\.sh$'' ''^tests/functional/ca/why-depends\.sh$'' + ''^tests/functional/characterisation-test-infra\.sh$'' ''^tests/functional/check\.sh$'' ''^tests/functional/common/vars-and-functions\.sh$'' ''^tests/functional/completions\.sh$'' @@ -579,9 +580,7 @@ ''^tests/functional/impure-env\.sh$'' ''^tests/functional/impure-eval\.sh$'' ''^tests/functional/install-darwin\.sh$'' - ''^tests/functional/lang-test-infra\.sh$'' ''^tests/functional/lang\.sh$'' - ''^tests/functional/lang/framework\.sh$'' ''^tests/functional/legacy-ssh-store\.sh$'' ''^tests/functional/linux-sandbox\.sh$'' ''^tests/functional/local-overlay-store/add-lower-inner\.sh$'' diff --git a/tests/functional/lang-test-infra.sh b/tests/functional/characterisation-test-infra.sh similarity index 98% rename from tests/functional/lang-test-infra.sh rename to tests/functional/characterisation-test-infra.sh index f32ccef05..279454550 100755 --- a/tests/functional/lang-test-infra.sh +++ b/tests/functional/characterisation-test-infra.sh @@ -3,7 +3,7 @@ # Test the function for lang.sh source common.sh -source lang/framework.sh +source characterisation/framework.sh # We are testing this, so don't want outside world to affect us. unset _NIX_TEST_ACCEPT diff --git a/tests/functional/lang/empty.exp b/tests/functional/characterisation/empty similarity index 100% rename from tests/functional/lang/empty.exp rename to tests/functional/characterisation/empty diff --git a/tests/functional/characterisation/framework.sh b/tests/functional/characterisation/framework.sh new file mode 100644 index 000000000..913fdd967 --- /dev/null +++ b/tests/functional/characterisation/framework.sh @@ -0,0 +1,77 @@ +# shellcheck shell=bash + +# Golden test support +# +# Test that the output of the given test matches what is expected. If +# `_NIX_TEST_ACCEPT` is non-empty also update the expected output so +# that next time the test succeeds. +function diffAndAcceptInner() { + local -r testName=$1 + local -r got="$2" + local -r expected="$3" + + # Absence of expected file indicates empty output expected. + if test -e "$expected"; then + local -r expectedOrEmpty="$expected" + else + local -r expectedOrEmpty=characterisation/empty + fi + + # Diff so we get a nice message + if ! diff --color=always --unified "$expectedOrEmpty" "$got"; then + echo "FAIL: evaluation result of $testName not as expected" + # shellcheck disable=SC2034 + badDiff=1 + fi + + # Update expected if `_NIX_TEST_ACCEPT` is non-empty. + if test -n "${_NIX_TEST_ACCEPT-}"; then + cp "$got" "$expected" + # Delete empty expected files to avoid bloating the repo with + # empty files. + if ! test -s "$expected"; then + rm "$expected" + fi + fi +} + +function characterisationTestExit() { + # Make sure shellcheck knows all these will be defined by the caller + : "${badDiff?} ${badExitCode?}" + + if test -n "${_NIX_TEST_ACCEPT-}"; then + if (( "$badDiff" )); then + set +x + echo 'Output did mot match, but accepted output as the persisted expected output.' + echo 'That means the next time the tests are run, they should pass.' + set -x + else + set +x + echo 'NOTE: Environment variable _NIX_TEST_ACCEPT is defined,' + echo 'indicating the unexpected output should be accepted as the expected output going forward,' + echo 'but no tests had unexpected output so there was no expected output to update.' + set -x + fi + if (( "$badExitCode" )); then + exit "$badExitCode" + else + skipTest "regenerating golden masters" + fi + else + if (( "$badDiff" )); then + set +x + echo '' + echo 'You can rerun this test with:' + echo '' + echo " _NIX_TEST_ACCEPT=1 make tests/functional/${TEST_NAME}.test" + echo '' + echo 'to regenerate the files containing the expected output,' + echo 'and then view the git diff to decide whether a change is' + echo 'good/intentional or bad/unintentional.' + echo 'If the diff contains arbitrary or impure information,' + echo 'please improve the normalization that the test applies to the output.' + set -x + fi + exit $(( "$badExitCode" + "$badDiff" )) + fi +} diff --git a/tests/functional/lang.sh b/tests/functional/lang.sh index 569e7082e..8cb8e98fb 100755 --- a/tests/functional/lang.sh +++ b/tests/functional/lang.sh @@ -4,7 +4,7 @@ source common.sh set -o pipefail -source lang/framework.sh +source characterisation/framework.sh # specialize function a bit function diffAndAccept() { @@ -138,32 +138,4 @@ for i in lang/eval-okay-*.nix; do fi done -if test -n "${_NIX_TEST_ACCEPT-}"; then - if (( "$badDiff" )); then - echo 'Output did mot match, but accepted output as the persisted expected output.' - echo 'That means the next time the tests are run, they should pass.' - else - echo 'NOTE: Environment variable _NIX_TEST_ACCEPT is defined,' - echo 'indicating the unexpected output should be accepted as the expected output going forward,' - echo 'but no tests had unexpected output so there was no expected output to update.' - fi - if (( "$badExitCode" )); then - exit "$badExitCode" - else - skipTest "regenerating golden masters" - fi -else - if (( "$badDiff" )); then - echo '' - echo 'You can rerun this test with:' - echo '' - echo ' _NIX_TEST_ACCEPT=1 make tests/functional/lang.sh.test' - echo '' - echo 'to regenerate the files containing the expected output,' - echo 'and then view the git diff to decide whether a change is' - echo 'good/intentional or bad/unintentional.' - echo 'If the diff contains arbitrary or impure information,' - echo 'please improve the normalization that the test applies to the output.' - fi - exit $(( "$badExitCode" + "$badDiff" )) -fi +characterisationTestExit diff --git a/tests/functional/lang/framework.sh b/tests/functional/lang/framework.sh deleted file mode 100644 index 9b886e983..000000000 --- a/tests/functional/lang/framework.sh +++ /dev/null @@ -1,33 +0,0 @@ -# Golden test support -# -# Test that the output of the given test matches what is expected. If -# `_NIX_TEST_ACCEPT` is non-empty also update the expected output so -# that next time the test succeeds. -function diffAndAcceptInner() { - local -r testName=$1 - local -r got="$2" - local -r expected="$3" - - # Absence of expected file indicates empty output expected. - if test -e "$expected"; then - local -r expectedOrEmpty="$expected" - else - local -r expectedOrEmpty=lang/empty.exp - fi - - # Diff so we get a nice message - if ! diff --color=always --unified "$expectedOrEmpty" "$got"; then - echo "FAIL: evaluation result of $testName not as expected" - badDiff=1 - fi - - # Update expected if `_NIX_TEST_ACCEPT` is non-empty. - if test -n "${_NIX_TEST_ACCEPT-}"; then - cp "$got" "$expected" - # Delete empty expected files to avoid bloating the repo with - # empty files. - if ! test -s "$expected"; then - rm "$expected" - fi - fi -} diff --git a/tests/functional/local.mk b/tests/functional/local.mk index b379eeefe..7e4536323 100644 --- a/tests/functional/local.mk +++ b/tests/functional/local.mk @@ -23,7 +23,7 @@ nix_tests = \ remote-store.sh \ legacy-ssh-store.sh \ lang.sh \ - lang-test-infra.sh \ + characterisation-test-infra.sh \ experimental-features.sh \ fetchMercurial.sh \ gc-auto.sh \ From 9f9984e4d07c3176a1781b2149ba7488383ddcde Mon Sep 17 00:00:00 2001 From: HaeNoe Date: Sat, 8 Jun 2024 13:10:51 +0200 Subject: [PATCH 15/18] Functional test for derivation "advanced attrs" This tests the Nix language side of things. We are purposely skipping most of `common.sh` because it is overkill for this test: we don't want to have an "overfit" test environment. Co-Authored-By: John Ericson --- tests/functional/common/paths.sh | 20 +++++++++ tests/functional/common/subst-vars.sh.in | 8 +++- tests/functional/common/test-root.sh | 4 ++ tests/functional/common/vars-and-functions.sh | 14 ++---- .../derivation-advanced-attributes.sh | 23 ++++++++++ .../advanced-attributes-defaults.drv | 1 + .../advanced-attributes-defaults.nix | 6 +++ ...d-attributes-structured-attrs-defaults.drv | 1 + ...d-attributes-structured-attrs-defaults.nix | 8 ++++ .../advanced-attributes-structured-attrs.drv | 1 + .../advanced-attributes-structured-attrs.nix | 45 +++++++++++++++++++ .../derivation/advanced-attributes.drv | 1 + .../derivation/advanced-attributes.nix | 33 ++++++++++++++ tests/functional/local.mk | 1 + 14 files changed, 155 insertions(+), 11 deletions(-) create mode 100644 tests/functional/common/paths.sh create mode 100644 tests/functional/common/test-root.sh create mode 100755 tests/functional/derivation-advanced-attributes.sh create mode 100644 tests/functional/derivation/advanced-attributes-defaults.drv create mode 100644 tests/functional/derivation/advanced-attributes-defaults.nix create mode 100644 tests/functional/derivation/advanced-attributes-structured-attrs-defaults.drv create mode 100644 tests/functional/derivation/advanced-attributes-structured-attrs-defaults.nix create mode 100644 tests/functional/derivation/advanced-attributes-structured-attrs.drv create mode 100644 tests/functional/derivation/advanced-attributes-structured-attrs.nix create mode 100644 tests/functional/derivation/advanced-attributes.drv create mode 100644 tests/functional/derivation/advanced-attributes.nix diff --git a/tests/functional/common/paths.sh b/tests/functional/common/paths.sh new file mode 100644 index 000000000..938b90899 --- /dev/null +++ b/tests/functional/common/paths.sh @@ -0,0 +1,20 @@ +# shellcheck shell=bash + +commonDir="$(readlink -f "$(dirname "${BASH_SOURCE[0]-$0}")")" + +# Since this is a generated file +# shellcheck disable=SC1091 +source "$commonDir/subst-vars.sh" +# Make sure shellcheck knows this will be defined by the above generated snippet +: "${bindir?}" + +export PATH="$bindir:$PATH" + +if [[ -n "${NIX_CLIENT_PACKAGE:-}" ]]; then + export PATH="$NIX_CLIENT_PACKAGE/bin":$PATH +fi + +DAEMON_PATH="$PATH" +if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then + DAEMON_PATH="${NIX_DAEMON_PACKAGE}/bin:$DAEMON_PATH" +fi diff --git a/tests/functional/common/subst-vars.sh.in b/tests/functional/common/subst-vars.sh.in index 4105e9f35..cd89a0033 100644 --- a/tests/functional/common/subst-vars.sh.in +++ b/tests/functional/common/subst-vars.sh.in @@ -1,6 +1,10 @@ # NOTE: instances of @variable@ are substituted as defined in /mk/templates.mk -export PATH=@bindir@:$PATH +if [[ -z "${COMMON_SUBST_VARS_SH_SOURCED-}" ]]; then + +COMMON_SUBST_VARS_SH_SOURCED=1 + +bindir=@bindir@ export coreutils=@coreutils@ #lsof=@lsof@ @@ -13,3 +17,5 @@ export version=@PACKAGE_VERSION@ export system=@system@ export BUILD_SHARED_LIBS=@BUILD_SHARED_LIBS@ + +fi diff --git a/tests/functional/common/test-root.sh b/tests/functional/common/test-root.sh new file mode 100644 index 000000000..b50a06267 --- /dev/null +++ b/tests/functional/common/test-root.sh @@ -0,0 +1,4 @@ +# shellcheck shell=bash + +TEST_ROOT=$(realpath "${TMPDIR:-/tmp}/nix-test")/${TEST_NAME:-default/tests\/functional//} +export TEST_ROOT diff --git a/tests/functional/common/vars-and-functions.sh b/tests/functional/common/vars-and-functions.sh index ad5b29a94..f6ccf9941 100644 --- a/tests/functional/common/vars-and-functions.sh +++ b/tests/functional/common/vars-and-functions.sh @@ -12,9 +12,11 @@ commonDir="$(readlink -f "$(dirname "${BASH_SOURCE[0]-$0}")")" source "$commonDir/subst-vars.sh" # Make sure shellcheck knows all these will be defined by the above generated snippet -: "${PATH?} ${coreutils?} ${dot?} ${SHELL?} ${PAGER?} ${busybox?} ${version?} ${system?} ${BUILD_SHARED_LIBS?}" +: "${bindir?} ${coreutils?} ${dot?} ${SHELL?} ${PAGER?} ${busybox?} ${version?} ${system?} ${BUILD_SHARED_LIBS?}" + +source "$commonDir/paths.sh" +source "$commonDir/test-root.sh" -export TEST_ROOT=$(realpath ${TMPDIR:-/tmp}/nix-test)/${TEST_NAME:-default/tests\/functional//} export NIX_STORE_DIR if ! NIX_STORE_DIR=$(readlink -f $TEST_ROOT/store 2> /dev/null); then # Maybe the build directory is symlinked. @@ -43,14 +45,6 @@ unset XDG_CONFIG_HOME unset XDG_CONFIG_DIRS unset XDG_CACHE_HOME -if [[ -n "${NIX_CLIENT_PACKAGE:-}" ]]; then - export PATH="$NIX_CLIENT_PACKAGE/bin":$PATH -fi -DAEMON_PATH="$PATH" -if [[ -n "${NIX_DAEMON_PACKAGE:-}" ]]; then - DAEMON_PATH="${NIX_DAEMON_PACKAGE}/bin:$DAEMON_PATH" -fi - export IMPURE_VAR1=foo export IMPURE_VAR2=bar diff --git a/tests/functional/derivation-advanced-attributes.sh b/tests/functional/derivation-advanced-attributes.sh new file mode 100755 index 000000000..6c0c76b4c --- /dev/null +++ b/tests/functional/derivation-advanced-attributes.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash + +source common/test-root.sh +source common/paths.sh + +set -o pipefail + +source characterisation/framework.sh + +badDiff=0 +badExitCode=0 + +store="$TEST_ROOT/store" + +for nixFile in derivation/*.nix; do + drvPath=$(nix-instantiate --store "$store" --pure-eval --expr "$(< "$nixFile")") + testName=$(basename "$nixFile" .nix) + got="${store}${drvPath}" + expected="derivation/$testName.drv" + diffAndAcceptInner "$testName" "$got" "$expected" +done + +characterisationTestExit diff --git a/tests/functional/derivation/advanced-attributes-defaults.drv b/tests/functional/derivation/advanced-attributes-defaults.drv new file mode 100644 index 000000000..391c6ab80 --- /dev/null +++ b/tests/functional/derivation/advanced-attributes-defaults.drv @@ -0,0 +1 @@ +Derive([("out","/nix/store/1qsc7svv43m4dw2prh6mvyf7cai5czji-advanced-attributes-defaults","","")],[],[],"my-system","/bin/bash",["-c","echo hello > $out"],[("builder","/bin/bash"),("name","advanced-attributes-defaults"),("out","/nix/store/1qsc7svv43m4dw2prh6mvyf7cai5czji-advanced-attributes-defaults"),("system","my-system")]) \ No newline at end of file diff --git a/tests/functional/derivation/advanced-attributes-defaults.nix b/tests/functional/derivation/advanced-attributes-defaults.nix new file mode 100644 index 000000000..51a8d0e7e --- /dev/null +++ b/tests/functional/derivation/advanced-attributes-defaults.nix @@ -0,0 +1,6 @@ +derivation { + name = "advanced-attributes-defaults"; + system = "my-system"; + builder = "/bin/bash"; + args = [ "-c" "echo hello > $out" ]; +} diff --git a/tests/functional/derivation/advanced-attributes-structured-attrs-defaults.drv b/tests/functional/derivation/advanced-attributes-structured-attrs-defaults.drv new file mode 100644 index 000000000..9dd402057 --- /dev/null +++ b/tests/functional/derivation/advanced-attributes-structured-attrs-defaults.drv @@ -0,0 +1 @@ +Derive([("dev","/nix/store/8bazivnbipbyi569623skw5zm91z6kc2-advanced-attributes-structured-attrs-defaults-dev","",""),("out","/nix/store/f8f8nvnx32bxvyxyx2ff7akbvwhwd9dw-advanced-attributes-structured-attrs-defaults","","")],[],[],"my-system","/bin/bash",["-c","echo hello > $out"],[("__json","{\"builder\":\"/bin/bash\",\"name\":\"advanced-attributes-structured-attrs-defaults\",\"outputs\":[\"out\",\"dev\"],\"system\":\"my-system\"}"),("dev","/nix/store/8bazivnbipbyi569623skw5zm91z6kc2-advanced-attributes-structured-attrs-defaults-dev"),("out","/nix/store/f8f8nvnx32bxvyxyx2ff7akbvwhwd9dw-advanced-attributes-structured-attrs-defaults")]) \ No newline at end of file diff --git a/tests/functional/derivation/advanced-attributes-structured-attrs-defaults.nix b/tests/functional/derivation/advanced-attributes-structured-attrs-defaults.nix new file mode 100644 index 000000000..0c13a7691 --- /dev/null +++ b/tests/functional/derivation/advanced-attributes-structured-attrs-defaults.nix @@ -0,0 +1,8 @@ +derivation { + name = "advanced-attributes-structured-attrs-defaults"; + system = "my-system"; + builder = "/bin/bash"; + args = [ "-c" "echo hello > $out" ]; + outputs = [ "out" "dev" ]; + __structuredAttrs = true; +} diff --git a/tests/functional/derivation/advanced-attributes-structured-attrs.drv b/tests/functional/derivation/advanced-attributes-structured-attrs.drv new file mode 100644 index 000000000..e47a41ad5 --- /dev/null +++ b/tests/functional/derivation/advanced-attributes-structured-attrs.drv @@ -0,0 +1 @@ +Derive([("bin","/nix/store/pbzb48v0ycf80jgligcp4n8z0rblna4n-advanced-attributes-structured-attrs-bin","",""),("dev","/nix/store/7xapi8jv7flcz1qq8jhw55ar8ag8hldh-advanced-attributes-structured-attrs-dev","",""),("out","/nix/store/mpq3l1l1qc2yr50q520g08kprprwv79f-advanced-attributes-structured-attrs","","")],[("/nix/store/4xm4wccqsvagz9gjksn24s7rip2fdy7v-foo.drv",["out"]),("/nix/store/plsq5jbr5nhgqwcgb2qxw7jchc09dnl8-bar.drv",["out"])],[],"my-system","/bin/bash",["-c","echo hello > $out"],[("__json","{\"__darwinAllowLocalNetworking\":true,\"__impureHostDeps\":[\"/usr/bin/ditto\"],\"__noChroot\":true,\"__sandboxProfile\":\"sandcastle\",\"allowSubstitutes\":false,\"builder\":\"/bin/bash\",\"impureEnvVars\":[\"UNICORN\"],\"name\":\"advanced-attributes-structured-attrs\",\"outputChecks\":{\"bin\":{\"disallowedReferences\":[\"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar\"],\"disallowedRequisites\":[\"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar\"]},\"dev\":{\"maxClosureSize\":5909,\"maxSize\":789},\"out\":{\"allowedReferences\":[\"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo\"],\"allowedRequisites\":[\"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo\"]}},\"outputs\":[\"out\",\"bin\",\"dev\"],\"preferLocalBuild\":true,\"requiredSystemFeatures\":[\"rainbow\",\"uid-range\"],\"system\":\"my-system\"}"),("bin","/nix/store/pbzb48v0ycf80jgligcp4n8z0rblna4n-advanced-attributes-structured-attrs-bin"),("dev","/nix/store/7xapi8jv7flcz1qq8jhw55ar8ag8hldh-advanced-attributes-structured-attrs-dev"),("out","/nix/store/mpq3l1l1qc2yr50q520g08kprprwv79f-advanced-attributes-structured-attrs")]) \ No newline at end of file diff --git a/tests/functional/derivation/advanced-attributes-structured-attrs.nix b/tests/functional/derivation/advanced-attributes-structured-attrs.nix new file mode 100644 index 000000000..0044b65fd --- /dev/null +++ b/tests/functional/derivation/advanced-attributes-structured-attrs.nix @@ -0,0 +1,45 @@ +let + system = "my-system"; + foo = derivation { + inherit system; + name = "foo"; + builder = "/bin/bash"; + args = ["-c" "echo foo > $out"]; + }; + bar = derivation { + inherit system; + name = "bar"; + builder = "/bin/bash"; + args = ["-c" "echo bar > $out"]; + }; +in +derivation { + inherit system; + name = "advanced-attributes-structured-attrs"; + builder = "/bin/bash"; + args = [ "-c" "echo hello > $out" ]; + __sandboxProfile = "sandcastle"; + __noChroot = true; + __impureHostDeps = ["/usr/bin/ditto"]; + impureEnvVars = ["UNICORN"]; + __darwinAllowLocalNetworking = true; + outputs = [ "out" "bin" "dev" ]; + __structuredAttrs = true; + outputChecks = { + out = { + allowedReferences = [foo]; + allowedRequisites = [foo]; + }; + bin = { + disallowedReferences = [bar]; + disallowedRequisites = [bar]; + }; + dev = { + maxSize = 789; + maxClosureSize = 5909; + }; + }; + requiredSystemFeatures = ["rainbow" "uid-range"]; + preferLocalBuild = true; + allowSubstitutes = false; +} diff --git a/tests/functional/derivation/advanced-attributes.drv b/tests/functional/derivation/advanced-attributes.drv new file mode 100644 index 000000000..ec3112ab2 --- /dev/null +++ b/tests/functional/derivation/advanced-attributes.drv @@ -0,0 +1 @@ +Derive([("out","/nix/store/33a6fdmn8q9ih9d7npbnrxn2q56a4l8q-advanced-attributes","","")],[("/nix/store/4xm4wccqsvagz9gjksn24s7rip2fdy7v-foo.drv",["out"]),("/nix/store/plsq5jbr5nhgqwcgb2qxw7jchc09dnl8-bar.drv",["out"])],[],"my-system","/bin/bash",["-c","echo hello > $out"],[("__darwinAllowLocalNetworking","1"),("__impureHostDeps","/usr/bin/ditto"),("__noChroot","1"),("__sandboxProfile","sandcastle"),("allowSubstitutes",""),("allowedReferences","/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"),("allowedRequisites","/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"),("builder","/bin/bash"),("disallowedReferences","/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"),("disallowedRequisites","/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"),("impureEnvVars","UNICORN"),("name","advanced-attributes"),("out","/nix/store/33a6fdmn8q9ih9d7npbnrxn2q56a4l8q-advanced-attributes"),("preferLocalBuild","1"),("requiredSystemFeatures","rainbow uid-range"),("system","my-system")]) \ No newline at end of file diff --git a/tests/functional/derivation/advanced-attributes.nix b/tests/functional/derivation/advanced-attributes.nix new file mode 100644 index 000000000..ff680c567 --- /dev/null +++ b/tests/functional/derivation/advanced-attributes.nix @@ -0,0 +1,33 @@ +let + system = "my-system"; + foo = derivation { + inherit system; + name = "foo"; + builder = "/bin/bash"; + args = ["-c" "echo foo > $out"]; + }; + bar = derivation { + inherit system; + name = "bar"; + builder = "/bin/bash"; + args = ["-c" "echo bar > $out"]; + }; +in +derivation { + inherit system; + name = "advanced-attributes"; + builder = "/bin/bash"; + args = [ "-c" "echo hello > $out" ]; + __sandboxProfile = "sandcastle"; + __noChroot = true; + __impureHostDeps = ["/usr/bin/ditto"]; + impureEnvVars = ["UNICORN"]; + __darwinAllowLocalNetworking = true; + allowedReferences = [foo]; + allowedRequisites = [foo]; + disallowedReferences = [bar]; + disallowedRequisites = [bar]; + requiredSystemFeatures = ["rainbow" "uid-range"]; + preferLocalBuild = true; + allowSubstitutes = false; +} diff --git a/tests/functional/local.mk b/tests/functional/local.mk index 7e4536323..49ee31284 100644 --- a/tests/functional/local.mk +++ b/tests/functional/local.mk @@ -106,6 +106,7 @@ nix_tests = \ eval-store.sh \ why-depends.sh \ derivation-json.sh \ + derivation-advanced-attributes.sh \ import-derivation.sh \ nix_path.sh \ case-hack.sh \ From 7fb14201afdeebc1327ff3ca75dcb4657c51cdc8 Mon Sep 17 00:00:00 2001 From: HaeNoe Date: Sat, 8 Jun 2024 13:10:51 +0200 Subject: [PATCH 16/18] Unit test for derivation "advanced attrs" This tests the parser and JSON format using the DRV files from the tests added in the previous commit. Co-Authored-By: John Ericson --- maintainers/flake-module.nix | 1 - tests/unit/libstore-support/tests/libstore.hh | 30 ++- .../advanced-attributes-defaults.drv | 1 + .../advanced-attributes-defaults.json | 22 ++ ...d-attributes-structured-attrs-defaults.drv | 1 + ...-attributes-structured-attrs-defaults.json | 24 ++ .../advanced-attributes-structured-attrs.drv | 1 + .../advanced-attributes-structured-attrs.json | 41 +++ .../data/derivation/advanced-attributes.drv | 1 + .../libstore/derivation-advanced-attrs.cc | 234 ++++++++++++++++++ .../libutil-support/tests/characterization.hh | 1 + 11 files changed, 345 insertions(+), 12 deletions(-) create mode 120000 tests/unit/libstore/data/derivation/advanced-attributes-defaults.drv create mode 100644 tests/unit/libstore/data/derivation/advanced-attributes-defaults.json create mode 120000 tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs-defaults.drv create mode 100644 tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs-defaults.json create mode 120000 tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs.drv create mode 100644 tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs.json create mode 120000 tests/unit/libstore/data/derivation/advanced-attributes.drv create mode 100644 tests/unit/libstore/derivation-advanced-attrs.cc diff --git a/maintainers/flake-module.nix b/maintainers/flake-module.nix index 0bd353f1a..5e4291fcb 100644 --- a/maintainers/flake-module.nix +++ b/maintainers/flake-module.nix @@ -447,7 +447,6 @@ ''^tests/unit/libfetchers/public-key\.cc'' ''^tests/unit/libstore-support/tests/derived-path\.cc'' ''^tests/unit/libstore-support/tests/derived-path\.hh'' - ''^tests/unit/libstore-support/tests/libstore\.hh'' ''^tests/unit/libstore-support/tests/nix_api_store\.hh'' ''^tests/unit/libstore-support/tests/outputs-spec\.cc'' ''^tests/unit/libstore-support/tests/outputs-spec\.hh'' diff --git a/tests/unit/libstore-support/tests/libstore.hh b/tests/unit/libstore-support/tests/libstore.hh index 267188224..84be52c23 100644 --- a/tests/unit/libstore-support/tests/libstore.hh +++ b/tests/unit/libstore-support/tests/libstore.hh @@ -8,19 +8,27 @@ namespace nix { -class LibStoreTest : public virtual ::testing::Test { - public: - static void SetUpTestSuite() { - initLibStore(false); - } +class LibStoreTest : public virtual ::testing::Test +{ +public: + static void SetUpTestSuite() + { + initLibStore(false); + } - protected: - LibStoreTest() - : store(openStore("dummy://")) - { } +protected: + LibStoreTest() + : store(openStore({ + .variant = + StoreReference::Specified{ + .scheme = "dummy", + }, + .params = {}, + })) + { + } - ref store; + ref store; }; - } /* namespace nix */ diff --git a/tests/unit/libstore/data/derivation/advanced-attributes-defaults.drv b/tests/unit/libstore/data/derivation/advanced-attributes-defaults.drv new file mode 120000 index 000000000..353090ad8 --- /dev/null +++ b/tests/unit/libstore/data/derivation/advanced-attributes-defaults.drv @@ -0,0 +1 @@ +../../../../functional/derivation/advanced-attributes-defaults.drv \ No newline at end of file diff --git a/tests/unit/libstore/data/derivation/advanced-attributes-defaults.json b/tests/unit/libstore/data/derivation/advanced-attributes-defaults.json new file mode 100644 index 000000000..d58e7d5b5 --- /dev/null +++ b/tests/unit/libstore/data/derivation/advanced-attributes-defaults.json @@ -0,0 +1,22 @@ +{ + "args": [ + "-c", + "echo hello > $out" + ], + "builder": "/bin/bash", + "env": { + "builder": "/bin/bash", + "name": "advanced-attributes-defaults", + "out": "/nix/store/1qsc7svv43m4dw2prh6mvyf7cai5czji-advanced-attributes-defaults", + "system": "my-system" + }, + "inputDrvs": {}, + "inputSrcs": [], + "name": "advanced-attributes-defaults", + "outputs": { + "out": { + "path": "/nix/store/1qsc7svv43m4dw2prh6mvyf7cai5czji-advanced-attributes-defaults" + } + }, + "system": "my-system" +} diff --git a/tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs-defaults.drv b/tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs-defaults.drv new file mode 120000 index 000000000..11713da12 --- /dev/null +++ b/tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs-defaults.drv @@ -0,0 +1 @@ +../../../../functional/derivation/advanced-attributes-structured-attrs-defaults.drv \ No newline at end of file diff --git a/tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs-defaults.json b/tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs-defaults.json new file mode 100644 index 000000000..473d006ac --- /dev/null +++ b/tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs-defaults.json @@ -0,0 +1,24 @@ +{ + "args": [ + "-c", + "echo hello > $out" + ], + "builder": "/bin/bash", + "env": { + "__json": "{\"builder\":\"/bin/bash\",\"name\":\"advanced-attributes-structured-attrs-defaults\",\"outputs\":[\"out\",\"dev\"],\"system\":\"my-system\"}", + "dev": "/nix/store/8bazivnbipbyi569623skw5zm91z6kc2-advanced-attributes-structured-attrs-defaults-dev", + "out": "/nix/store/f8f8nvnx32bxvyxyx2ff7akbvwhwd9dw-advanced-attributes-structured-attrs-defaults" + }, + "inputDrvs": {}, + "inputSrcs": [], + "name": "advanced-attributes-structured-attrs-defaults", + "outputs": { + "dev": { + "path": "/nix/store/8bazivnbipbyi569623skw5zm91z6kc2-advanced-attributes-structured-attrs-defaults-dev" + }, + "out": { + "path": "/nix/store/f8f8nvnx32bxvyxyx2ff7akbvwhwd9dw-advanced-attributes-structured-attrs-defaults" + } + }, + "system": "my-system" +} diff --git a/tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs.drv b/tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs.drv new file mode 120000 index 000000000..962f8ea3f --- /dev/null +++ b/tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs.drv @@ -0,0 +1 @@ +../../../../functional/derivation/advanced-attributes-structured-attrs.drv \ No newline at end of file diff --git a/tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs.json b/tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs.json new file mode 100644 index 000000000..324428124 --- /dev/null +++ b/tests/unit/libstore/data/derivation/advanced-attributes-structured-attrs.json @@ -0,0 +1,41 @@ +{ + "args": [ + "-c", + "echo hello > $out" + ], + "builder": "/bin/bash", + "env": { + "__json": "{\"__darwinAllowLocalNetworking\":true,\"__impureHostDeps\":[\"/usr/bin/ditto\"],\"__noChroot\":true,\"__sandboxProfile\":\"sandcastle\",\"allowSubstitutes\":false,\"builder\":\"/bin/bash\",\"impureEnvVars\":[\"UNICORN\"],\"name\":\"advanced-attributes-structured-attrs\",\"outputChecks\":{\"bin\":{\"disallowedReferences\":[\"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar\"],\"disallowedRequisites\":[\"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar\"]},\"dev\":{\"maxClosureSize\":5909,\"maxSize\":789},\"out\":{\"allowedReferences\":[\"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo\"],\"allowedRequisites\":[\"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo\"]}},\"outputs\":[\"out\",\"bin\",\"dev\"],\"preferLocalBuild\":true,\"requiredSystemFeatures\":[\"rainbow\",\"uid-range\"],\"system\":\"my-system\"}", + "bin": "/nix/store/pbzb48v0ycf80jgligcp4n8z0rblna4n-advanced-attributes-structured-attrs-bin", + "dev": "/nix/store/7xapi8jv7flcz1qq8jhw55ar8ag8hldh-advanced-attributes-structured-attrs-dev", + "out": "/nix/store/mpq3l1l1qc2yr50q520g08kprprwv79f-advanced-attributes-structured-attrs" + }, + "inputDrvs": { + "/nix/store/4xm4wccqsvagz9gjksn24s7rip2fdy7v-foo.drv": { + "dynamicOutputs": {}, + "outputs": [ + "out" + ] + }, + "/nix/store/plsq5jbr5nhgqwcgb2qxw7jchc09dnl8-bar.drv": { + "dynamicOutputs": {}, + "outputs": [ + "out" + ] + } + }, + "inputSrcs": [], + "name": "advanced-attributes-structured-attrs", + "outputs": { + "bin": { + "path": "/nix/store/pbzb48v0ycf80jgligcp4n8z0rblna4n-advanced-attributes-structured-attrs-bin" + }, + "dev": { + "path": "/nix/store/7xapi8jv7flcz1qq8jhw55ar8ag8hldh-advanced-attributes-structured-attrs-dev" + }, + "out": { + "path": "/nix/store/mpq3l1l1qc2yr50q520g08kprprwv79f-advanced-attributes-structured-attrs" + } + }, + "system": "my-system" +} diff --git a/tests/unit/libstore/data/derivation/advanced-attributes.drv b/tests/unit/libstore/data/derivation/advanced-attributes.drv new file mode 120000 index 000000000..2a53a05ca --- /dev/null +++ b/tests/unit/libstore/data/derivation/advanced-attributes.drv @@ -0,0 +1 @@ +../../../../functional/derivation/advanced-attributes.drv \ No newline at end of file diff --git a/tests/unit/libstore/derivation-advanced-attrs.cc b/tests/unit/libstore/derivation-advanced-attrs.cc new file mode 100644 index 000000000..26cf947a8 --- /dev/null +++ b/tests/unit/libstore/derivation-advanced-attrs.cc @@ -0,0 +1,234 @@ +#include +#include +#include + +#include "experimental-features.hh" +#include "derivations.hh" + +#include "tests/libstore.hh" +#include "tests/characterization.hh" +#include "parsed-derivations.hh" +#include "types.hh" + +namespace nix { + +using nlohmann::json; + +class DerivationAdvancedAttrsTest : public CharacterizationTest, public LibStoreTest +{ + Path unitTestData = getUnitTestData() + "/derivation"; + +public: + Path goldenMaster(std::string_view testStem) const override + { + return unitTestData + "/" + testStem; + } +}; + +#define TEST_ATERM_JSON(STEM, NAME) \ + TEST_F(DerivationAdvancedAttrsTest, Derivation_##STEM##_from_json) \ + { \ + readTest(NAME ".json", [&](const auto & encoded_) { \ + auto encoded = json::parse(encoded_); \ + /* Use DRV file instead of C++ literal as source of truth. */ \ + auto aterm = readFile(goldenMaster(NAME ".drv")); \ + auto expected = parseDerivation(*store, std::move(aterm), NAME); \ + Derivation got = Derivation::fromJSON(*store, encoded); \ + EXPECT_EQ(got, expected); \ + }); \ + } \ + \ + TEST_F(DerivationAdvancedAttrsTest, Derivation_##STEM##_to_json) \ + { \ + writeTest( \ + NAME ".json", \ + [&]() -> json { \ + /* Use DRV file instead of C++ literal as source of truth. */ \ + auto aterm = readFile(goldenMaster(NAME ".drv")); \ + return parseDerivation(*store, std::move(aterm), NAME).toJSON(*store); \ + }, \ + [](const auto & file) { return json::parse(readFile(file)); }, \ + [](const auto & file, const auto & got) { return writeFile(file, got.dump(2) + "\n"); }); \ + } \ + \ + TEST_F(DerivationAdvancedAttrsTest, Derivation_##STEM##_from_aterm) \ + { \ + readTest(NAME ".drv", [&](auto encoded) { \ + /* Use JSON file instead of C++ literal as source of truth. */ \ + auto json = json::parse(readFile(goldenMaster(NAME ".json"))); \ + auto expected = Derivation::fromJSON(*store, json); \ + auto got = parseDerivation(*store, std::move(encoded), NAME); \ + EXPECT_EQ(got.toJSON(*store), expected.toJSON(*store)); \ + EXPECT_EQ(got, expected); \ + }); \ + } \ + \ + /* No corresponding write test, because we need to read the drv to write the json file */ + +TEST_ATERM_JSON(advancedAttributes_defaults, "advanced-attributes-defaults"); +TEST_ATERM_JSON(advancedAttributes, "advanced-attributes-defaults"); +TEST_ATERM_JSON(advancedAttributes_structuredAttrs_defaults, "advanced-attributes-structured-attrs"); +TEST_ATERM_JSON(advancedAttributes_structuredAttrs, "advanced-attributes-structured-attrs-defaults"); + +#undef TEST_ATERM_JSON + +TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_defaults) +{ + readTest("advanced-attributes-defaults.drv", [&](auto encoded) { + auto got = parseDerivation(*store, std::move(encoded), "foo"); + + auto drvPath = writeDerivation(*store, got, NoRepair, true); + + ParsedDerivation parsedDrv(drvPath, got); + + EXPECT_EQ(parsedDrv.getStringAttr("__sandboxProfile").value_or(""), ""); + EXPECT_EQ(parsedDrv.getBoolAttr("__noChroot"), false); + EXPECT_EQ(parsedDrv.getStringsAttr("__impureHostDeps").value_or(Strings()), Strings()); + EXPECT_EQ(parsedDrv.getStringsAttr("impureEnvVars").value_or(Strings()), Strings()); + EXPECT_EQ(parsedDrv.getBoolAttr("__darwinAllowLocalNetworking"), false); + EXPECT_EQ(parsedDrv.getStringsAttr("allowedReferences"), std::nullopt); + EXPECT_EQ(parsedDrv.getStringsAttr("allowedRequisites"), std::nullopt); + EXPECT_EQ(parsedDrv.getStringsAttr("disallowedReferences"), std::nullopt); + EXPECT_EQ(parsedDrv.getStringsAttr("disallowedRequisites"), std::nullopt); + EXPECT_EQ(parsedDrv.getRequiredSystemFeatures(), StringSet()); + EXPECT_EQ(parsedDrv.canBuildLocally(*store), false); + EXPECT_EQ(parsedDrv.willBuildLocally(*store), false); + EXPECT_EQ(parsedDrv.substitutesAllowed(), true); + EXPECT_EQ(parsedDrv.useUidRange(), false); + }); +}; + +TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes) +{ + readTest("advanced-attributes.drv", [&](auto encoded) { + auto got = parseDerivation(*store, std::move(encoded), "foo"); + + auto drvPath = writeDerivation(*store, got, NoRepair, true); + + ParsedDerivation parsedDrv(drvPath, got); + + StringSet systemFeatures{"rainbow", "uid-range"}; + + EXPECT_EQ(parsedDrv.getStringAttr("__sandboxProfile").value_or(""), "sandcastle"); + EXPECT_EQ(parsedDrv.getBoolAttr("__noChroot"), true); + EXPECT_EQ(parsedDrv.getStringsAttr("__impureHostDeps").value_or(Strings()), Strings{"/usr/bin/ditto"}); + EXPECT_EQ(parsedDrv.getStringsAttr("impureEnvVars").value_or(Strings()), Strings{"UNICORN"}); + EXPECT_EQ(parsedDrv.getBoolAttr("__darwinAllowLocalNetworking"), true); + EXPECT_EQ( + parsedDrv.getStringsAttr("allowedReferences"), Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"}); + EXPECT_EQ( + parsedDrv.getStringsAttr("allowedRequisites"), Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"}); + EXPECT_EQ( + parsedDrv.getStringsAttr("disallowedReferences"), + Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"}); + EXPECT_EQ( + parsedDrv.getStringsAttr("disallowedRequisites"), + Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"}); + EXPECT_EQ(parsedDrv.getRequiredSystemFeatures(), systemFeatures); + EXPECT_EQ(parsedDrv.canBuildLocally(*store), false); + EXPECT_EQ(parsedDrv.willBuildLocally(*store), false); + EXPECT_EQ(parsedDrv.substitutesAllowed(), false); + EXPECT_EQ(parsedDrv.useUidRange(), true); + }); +}; + +TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_structuredAttrs_defaults) +{ + readTest("advanced-attributes-structured-attrs-defaults.drv", [&](auto encoded) { + auto got = parseDerivation(*store, std::move(encoded), "foo"); + + auto drvPath = writeDerivation(*store, got, NoRepair, true); + + ParsedDerivation parsedDrv(drvPath, got); + + EXPECT_EQ(parsedDrv.getStringAttr("__sandboxProfile").value_or(""), ""); + EXPECT_EQ(parsedDrv.getBoolAttr("__noChroot"), false); + EXPECT_EQ(parsedDrv.getStringsAttr("__impureHostDeps").value_or(Strings()), Strings()); + EXPECT_EQ(parsedDrv.getStringsAttr("impureEnvVars").value_or(Strings()), Strings()); + EXPECT_EQ(parsedDrv.getBoolAttr("__darwinAllowLocalNetworking"), false); + + { + auto structuredAttrs_ = parsedDrv.getStructuredAttrs(); + ASSERT_TRUE(structuredAttrs_); + auto & structuredAttrs = *structuredAttrs_; + + auto outputChecks_ = get(structuredAttrs, "outputChecks"); + ASSERT_FALSE(outputChecks_); + } + + EXPECT_EQ(parsedDrv.getRequiredSystemFeatures(), StringSet()); + EXPECT_EQ(parsedDrv.canBuildLocally(*store), false); + EXPECT_EQ(parsedDrv.willBuildLocally(*store), false); + EXPECT_EQ(parsedDrv.substitutesAllowed(), true); + EXPECT_EQ(parsedDrv.useUidRange(), false); + }); +}; + +TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_structuredAttrs) +{ + readTest("advanced-attributes-structured-attrs.drv", [&](auto encoded) { + auto got = parseDerivation(*store, std::move(encoded), "foo"); + + auto drvPath = writeDerivation(*store, got, NoRepair, true); + + ParsedDerivation parsedDrv(drvPath, got); + + StringSet systemFeatures{"rainbow", "uid-range"}; + + EXPECT_EQ(parsedDrv.getStringAttr("__sandboxProfile").value_or(""), "sandcastle"); + EXPECT_EQ(parsedDrv.getBoolAttr("__noChroot"), true); + EXPECT_EQ(parsedDrv.getStringsAttr("__impureHostDeps").value_or(Strings()), Strings{"/usr/bin/ditto"}); + EXPECT_EQ(parsedDrv.getStringsAttr("impureEnvVars").value_or(Strings()), Strings{"UNICORN"}); + EXPECT_EQ(parsedDrv.getBoolAttr("__darwinAllowLocalNetworking"), true); + + { + auto structuredAttrs_ = parsedDrv.getStructuredAttrs(); + ASSERT_TRUE(structuredAttrs_); + auto & structuredAttrs = *structuredAttrs_; + + auto outputChecks_ = get(structuredAttrs, "outputChecks"); + ASSERT_TRUE(outputChecks_); + auto & outputChecks = *outputChecks_; + + { + auto output_ = get(outputChecks, "out"); + ASSERT_TRUE(output_); + auto & output = *output_; + EXPECT_EQ( + get(output, "allowedReferences")->get(), + Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"}); + EXPECT_EQ( + get(output, "allowedRequisites")->get(), + Strings{"/nix/store/3c08bzb71z4wiag719ipjxr277653ynp-foo"}); + } + + { + auto output_ = get(outputChecks, "bin"); + ASSERT_TRUE(output_); + auto & output = *output_; + EXPECT_EQ( + get(output, "disallowedReferences")->get(), + Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"}); + EXPECT_EQ( + get(output, "disallowedRequisites")->get(), + Strings{"/nix/store/7rhsm8i393hm1wcsmph782awg1hi2f7x-bar"}); + } + + { + auto output_ = get(outputChecks, "dev"); + ASSERT_TRUE(output_); + auto & output = *output_; + EXPECT_EQ(get(output, "maxSize")->get(), 789); + EXPECT_EQ(get(output, "maxClosureSize")->get(), 5909); + } + } + + EXPECT_EQ(parsedDrv.getRequiredSystemFeatures(), systemFeatures); + EXPECT_EQ(parsedDrv.canBuildLocally(*store), false); + EXPECT_EQ(parsedDrv.willBuildLocally(*store), false); + EXPECT_EQ(parsedDrv.substitutesAllowed(), false); + EXPECT_EQ(parsedDrv.useUidRange(), true); + }); +}; + +} diff --git a/tests/unit/libutil-support/tests/characterization.hh b/tests/unit/libutil-support/tests/characterization.hh index c2f686dbf..19ba824ac 100644 --- a/tests/unit/libutil-support/tests/characterization.hh +++ b/tests/unit/libutil-support/tests/characterization.hh @@ -5,6 +5,7 @@ #include "types.hh" #include "environment-variables.hh" +#include "file-system.hh" namespace nix { From 64e599ebe162758a7e077a6c4e319649374307e2 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 16 May 2024 17:46:43 -0400 Subject: [PATCH 17/18] Rename `Recursive` -> `NixArchive` For enums: - `FileIngestionMethod` - `FileSerialisationMethod` --- src/libexpr/eval.cc | 2 +- src/libexpr/primops.cc | 10 +++++----- src/libexpr/primops/fetchTree.cc | 2 +- src/libfetchers/fetch-to-store.hh | 2 +- src/libfetchers/fetchers.cc | 2 +- src/libfetchers/mercurial.cc | 2 +- src/libstore/binary-cache-store.cc | 4 ++-- src/libstore/build/worker.cc | 2 +- src/libstore/content-address.cc | 6 +++--- src/libstore/daemon.cc | 12 ++++++------ src/libstore/dummy-store.cc | 4 ++-- src/libstore/legacy-ssh-store.hh | 4 ++-- src/libstore/local-store.cc | 8 ++++---- src/libstore/make-content-addressed.cc | 2 +- src/libstore/optimise-store.cc | 4 ++-- src/libstore/remote-store.cc | 12 ++++++------ src/libstore/remote-store.hh | 4 ++-- src/libstore/store-api.cc | 12 ++++++------ src/libstore/store-api.hh | 10 +++++----- src/libstore/store-dir-config.hh | 2 +- src/libstore/unix/build/local-derivation-goal.cc | 6 +++--- src/libutil/file-content-address.cc | 12 ++++++------ src/libutil/file-content-address.hh | 10 +++++----- src/nix-store/nix-store.cc | 6 +++--- src/nix/add-to-store.cc | 2 +- src/nix/hash.cc | 8 ++++---- src/nix/prefetch.cc | 2 +- src/nix/profile.cc | 2 +- src/perl/lib/Nix/Store.xs | 6 +++--- tests/unit/libstore/common-protocol.cc | 2 +- tests/unit/libstore/content-address.cc | 2 +- tests/unit/libstore/derivation.cc | 6 +++--- tests/unit/libstore/nar-info.cc | 2 +- tests/unit/libstore/path-info.cc | 2 +- tests/unit/libstore/serve-protocol.cc | 4 ++-- tests/unit/libstore/worker-protocol.cc | 4 ++-- tests/unit/libutil/file-content-address.cc | 4 ++-- 37 files changed, 93 insertions(+), 93 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 7aaec2e73..fc211e694 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2303,7 +2303,7 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat path.resolveSymlinks(), settings.readOnlyMode ? FetchMode::DryRun : FetchMode::Copy, path.baseName(), - FileIngestionMethod::Recursive, + FileIngestionMethod::NixArchive, nullptr, repair); allowPath(dstPath); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 98649f081..02b970112 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1209,7 +1209,7 @@ static void derivationStrictInternal( auto handleHashMode = [&](const std::string_view s) { if (s == "recursive") { // back compat, new name is "nar" - ingestionMethod = FileIngestionMethod::Recursive; + ingestionMethod = FileIngestionMethod::NixArchive; } else try { ingestionMethod = ContentAddressMethod::parse(s); } catch (UsageError &) { @@ -1432,7 +1432,7 @@ static void derivationStrictInternal( .atPos(v).debugThrow(); auto ha = outputHashAlgo.value_or(HashAlgorithm::SHA256); - auto method = ingestionMethod.value_or(FileIngestionMethod::Recursive); + auto method = ingestionMethod.value_or(FileIngestionMethod::NixArchive); for (auto & i : outputs) { drv.env[i] = hashPlaceholder(i); @@ -2391,7 +2391,7 @@ static void prim_filterSource(EvalState & state, const PosIdx pos, Value * * arg "while evaluating the second argument (the path to filter) passed to 'builtins.filterSource'"); state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.filterSource"); - addPath(state, pos, path.baseName(), path, args[0], FileIngestionMethod::Recursive, std::nullopt, v, context); + addPath(state, pos, path.baseName(), path, args[0], FileIngestionMethod::NixArchive, std::nullopt, v, context); } static RegisterPrimOp primop_filterSource({ @@ -2454,7 +2454,7 @@ static void prim_path(EvalState & state, const PosIdx pos, Value * * args, Value std::optional path; std::string name; Value * filterFun = nullptr; - ContentAddressMethod method = FileIngestionMethod::Recursive; + ContentAddressMethod method = FileIngestionMethod::NixArchive; std::optional expectedHash; NixStringContext context; @@ -2470,7 +2470,7 @@ static void prim_path(EvalState & state, const PosIdx pos, Value * * args, Value state.forceFunction(*(filterFun = attr.value), attr.pos, "while evaluating the `filter` parameter passed to builtins.path"); else if (n == "recursive") method = state.forceBool(*attr.value, attr.pos, "while evaluating the `recursive` attribute passed to builtins.path") - ? FileIngestionMethod::Recursive + ? FileIngestionMethod::NixArchive : FileIngestionMethod::Flat; else if (n == "sha256") expectedHash = newHashAllowEmpty(state.forceStringNoCtx(*attr.value, attr.pos, "while evaluating the `sha256` attribute passed to builtins.path"), HashAlgorithm::SHA256); diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index a99a71577..af152f4af 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -468,7 +468,7 @@ static void fetch(EvalState & state, const PosIdx pos, Value * * args, Value & v auto expectedPath = state.store->makeFixedOutputPath( name, FixedOutputInfo { - .method = unpack ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat, + .method = unpack ? FileIngestionMethod::NixArchive : FileIngestionMethod::Flat, .hash = *expectedHash, .references = {} }); diff --git a/src/libfetchers/fetch-to-store.hh b/src/libfetchers/fetch-to-store.hh index 81af1e240..95d1e6b01 100644 --- a/src/libfetchers/fetch-to-store.hh +++ b/src/libfetchers/fetch-to-store.hh @@ -18,7 +18,7 @@ StorePath fetchToStore( const SourcePath & path, FetchMode mode, std::string_view name = "source", - ContentAddressMethod method = FileIngestionMethod::Recursive, + ContentAddressMethod method = FileIngestionMethod::NixArchive, PathFilter * filter = nullptr, RepairFlag repair = NoRepair); diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 73923907c..170a8910c 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -305,7 +305,7 @@ StorePath Input::computeStorePath(Store & store) const if (!narHash) throw Error("cannot compute store path for unlocked input '%s'", to_string()); return store.makeFixedOutputPath(getName(), FixedOutputInfo { - .method = FileIngestionMethod::Recursive, + .method = FileIngestionMethod::NixArchive, .hash = *narHash, .references = {}, }); diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 7bdf1e937..4b80cbe9b 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -213,7 +213,7 @@ struct MercurialInputScheme : InputScheme auto storePath = store->addToStore( input.getName(), {getFSSourceAccessor(), CanonPath(actualPath)}, - FileIngestionMethod::Recursive, HashAlgorithm::SHA256, {}, + FileIngestionMethod::NixArchive, HashAlgorithm::SHA256, {}, filter); return storePath; diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 95a8d5a7a..e8c8892b3 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -322,7 +322,7 @@ StorePath BinaryCacheStore::addToStoreFromDump( if (static_cast(dumpMethod) == hashMethod.getFileIngestionMethod()) caHash = hashString(HashAlgorithm::SHA256, dump2.s); switch (dumpMethod) { - case FileSerialisationMethod::Recursive: + case FileSerialisationMethod::NixArchive: // The dump is already NAR in this case, just use it. nar = dump2.s; break; @@ -339,7 +339,7 @@ StorePath BinaryCacheStore::addToStoreFromDump( } else { // Otherwise, we have to do th same hashing as NAR so our single // hash will suffice for both purposes. - if (dumpMethod != FileSerialisationMethod::Recursive || hashAlgo != HashAlgorithm::SHA256) + if (dumpMethod != FileSerialisationMethod::NixArchive || hashAlgo != HashAlgorithm::SHA256) unsupported("addToStoreFromDump"); } StringSource narDump { nar }; diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index b53dc771a..31cfa8adc 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -530,7 +530,7 @@ bool Worker::pathContentsGood(const StorePath & path) else { auto current = hashPath( {store.getFSAccessor(), CanonPath(store.printStorePath(path))}, - FileIngestionMethod::Recursive, info->narHash.algo).first; + FileIngestionMethod::NixArchive, info->narHash.algo).first; Hash nullHash(HashAlgorithm::SHA256); res = info->narHash == nullHash || info->narHash == current; } diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 4ed4f2de5..fa06c4aa3 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -9,7 +9,7 @@ std::string_view makeFileIngestionPrefix(FileIngestionMethod m) switch (m) { case FileIngestionMethod::Flat: return ""; - case FileIngestionMethod::Recursive: + case FileIngestionMethod::NixArchive: return "r:"; case FileIngestionMethod::Git: experimentalFeatureSettings.require(Xp::GitHashing); @@ -52,7 +52,7 @@ std::string_view ContentAddressMethod::renderPrefix() const ContentAddressMethod ContentAddressMethod::parsePrefix(std::string_view & m) { if (splitPrefix(m, "r:")) { - return FileIngestionMethod::Recursive; + return FileIngestionMethod::NixArchive; } else if (splitPrefix(m, "git:")) { experimentalFeatureSettings.require(Xp::GitHashing); @@ -137,7 +137,7 @@ static std::pair parseContentAddressMethodP // Parse method auto method = FileIngestionMethod::Flat; if (splitPrefix(rest, "r:")) - method = FileIngestionMethod::Recursive; + method = FileIngestionMethod::NixArchive; else if (splitPrefix(rest, "git:")) { experimentalFeatureSettings.require(Xp::GitHashing); method = FileIngestionMethod::Git; diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index fe60cb918..788c5e2ea 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -415,12 +415,12 @@ static void performOp(TunnelLogger * logger, ref store, case FileIngestionMethod::Flat: dumpMethod = FileSerialisationMethod::Flat; break; - case FileIngestionMethod::Recursive: - dumpMethod = FileSerialisationMethod::Recursive; + case FileIngestionMethod::NixArchive: + dumpMethod = FileSerialisationMethod::NixArchive; break; case FileIngestionMethod::Git: // Use NAR; Git is not a serialization method - dumpMethod = FileSerialisationMethod::Recursive; + dumpMethod = FileSerialisationMethod::NixArchive; break; default: assert(false); @@ -441,13 +441,13 @@ static void performOp(TunnelLogger * logger, ref store, uint8_t recursive; std::string hashAlgoRaw; from >> baseName >> fixed /* obsolete */ >> recursive >> hashAlgoRaw; - if (recursive > (uint8_t) FileIngestionMethod::Recursive) + if (recursive > (uint8_t) FileIngestionMethod::NixArchive) throw Error("unsupported FileIngestionMethod with value of %i; you may need to upgrade nix-daemon", recursive); method = FileIngestionMethod { recursive }; /* Compatibility hack. */ if (!fixed) { hashAlgoRaw = "sha256"; - method = FileIngestionMethod::Recursive; + method = FileIngestionMethod::NixArchive; } hashAlgo = parseHashAlgo(hashAlgoRaw); } @@ -468,7 +468,7 @@ static void performOp(TunnelLogger * logger, ref store, }); logger->startWork(); auto path = store->addToStoreFromDump( - *dumpSource, baseName, FileSerialisationMethod::Recursive, method, hashAlgo); + *dumpSource, baseName, FileSerialisationMethod::NixArchive, method, hashAlgo); logger->stopWork(); to << store->printStorePath(path); diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 0d5d03091..17ebaace6 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -64,8 +64,8 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store virtual StorePath addToStoreFromDump( Source & dump, std::string_view name, - FileSerialisationMethod dumpMethod = FileSerialisationMethod::Recursive, - ContentAddressMethod hashMethod = FileIngestionMethod::Recursive, + FileSerialisationMethod dumpMethod = FileSerialisationMethod::NixArchive, + ContentAddressMethod hashMethod = FileIngestionMethod::NixArchive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, const StorePathSet & references = StorePathSet(), RepairFlag repair = NoRepair) override diff --git a/src/libstore/legacy-ssh-store.hh b/src/libstore/legacy-ssh-store.hh index b683ed580..db49188ec 100644 --- a/src/libstore/legacy-ssh-store.hh +++ b/src/libstore/legacy-ssh-store.hh @@ -76,8 +76,8 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor virtual StorePath addToStoreFromDump( Source & dump, std::string_view name, - FileSerialisationMethod dumpMethod = FileSerialisationMethod::Recursive, - ContentAddressMethod hashMethod = FileIngestionMethod::Recursive, + FileSerialisationMethod dumpMethod = FileSerialisationMethod::NixArchive, + ContentAddressMethod hashMethod = FileIngestionMethod::NixArchive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, const StorePathSet & references = StorePathSet(), RepairFlag repair = NoRepair) override diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 676a035fa..07ace70d0 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1155,7 +1155,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, auto fim = specified.method.getFileIngestionMethod(); switch (fim) { case FileIngestionMethod::Flat: - case FileIngestionMethod::Recursive: + case FileIngestionMethod::NixArchive: { HashModuloSink caSink { specified.hash.algo, @@ -1314,7 +1314,7 @@ StorePath LocalStore::addToStoreFromDump( auto fim = hashMethod.getFileIngestionMethod(); switch (fim) { case FileIngestionMethod::Flat: - case FileIngestionMethod::Recursive: + case FileIngestionMethod::NixArchive: restorePath(realPath, dumpSource, (FileSerialisationMethod) fim); break; case FileIngestionMethod::Git: @@ -1330,7 +1330,7 @@ StorePath LocalStore::addToStoreFromDump( /* For computing the nar hash. In recursive SHA-256 mode, this is the same as the store hash, so no need to do it again. */ auto narHash = std::pair { dumpHash, size }; - if (dumpMethod != FileSerialisationMethod::Recursive || hashAlgo != HashAlgorithm::SHA256) { + if (dumpMethod != FileSerialisationMethod::NixArchive || hashAlgo != HashAlgorithm::SHA256) { HashSink narSink { HashAlgorithm::SHA256 }; dumpPath(realPath, narSink); narHash = narSink.finish(); @@ -1423,7 +1423,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair) PosixSourceAccessor accessor; std::string hash = hashPath( PosixSourceAccessor::createAtRoot(link.path()), - FileIngestionMethod::Recursive, HashAlgorithm::SHA256).first.to_string(HashFormat::Nix32, false); + FileIngestionMethod::NixArchive, HashAlgorithm::SHA256).first.to_string(HashFormat::Nix32, false); if (hash != name.string()) { printError("link '%s' was modified! expected hash '%s', got '%s'", link.path(), name, hash); diff --git a/src/libstore/make-content-addressed.cc b/src/libstore/make-content-addressed.cc index 170fe67b9..a3130d7cc 100644 --- a/src/libstore/make-content-addressed.cc +++ b/src/libstore/make-content-addressed.cc @@ -52,7 +52,7 @@ std::map makeContentAddressed( dstStore, path.name(), FixedOutputInfo { - .method = FileIngestionMethod::Recursive, + .method = FileIngestionMethod::NixArchive, .hash = narModuloHash, .references = std::move(refs), }, diff --git a/src/libstore/optimise-store.cc b/src/libstore/optimise-store.cc index e9b6d2d50..9d903f218 100644 --- a/src/libstore/optimise-store.cc +++ b/src/libstore/optimise-store.cc @@ -151,7 +151,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, Hash hash = ({ hashPath( {make_ref(), CanonPath(path)}, - FileSerialisationMethod::Recursive, HashAlgorithm::SHA256).first; + FileSerialisationMethod::NixArchive, HashAlgorithm::SHA256).first; }); debug("'%1%' has hash '%2%'", path, hash.to_string(HashFormat::Nix32, true)); @@ -165,7 +165,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, || (repair && hash != ({ hashPath( PosixSourceAccessor::createAtRoot(linkPath), - FileSerialisationMethod::Recursive, HashAlgorithm::SHA256).first; + FileSerialisationMethod::NixArchive, HashAlgorithm::SHA256).first; }))) { // XXX: Consider overwriting linkPath with our valid version. diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index d6efc14f9..9adad9c2a 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -406,8 +406,8 @@ ref RemoteStore::addCAToStore( conn->to << WorkerProto::Op::AddToStore << name - << ((hashAlgo == HashAlgorithm::SHA256 && fim == FileIngestionMethod::Recursive) ? 0 : 1) /* backwards compatibility hack */ - << (fim == FileIngestionMethod::Recursive ? 1 : 0) + << ((hashAlgo == HashAlgorithm::SHA256 && fim == FileIngestionMethod::NixArchive) ? 0 : 1) /* backwards compatibility hack */ + << (fim == FileIngestionMethod::NixArchive ? 1 : 0) << printHashAlgo(hashAlgo); try { @@ -415,7 +415,7 @@ ref RemoteStore::addCAToStore( connections->incCapacity(); { Finally cleanup([&]() { connections->decCapacity(); }); - if (fim == FileIngestionMethod::Recursive) { + if (fim == FileIngestionMethod::NixArchive) { dump.drainInto(conn->to); } else { std::string contents = dump.drain(); @@ -457,12 +457,12 @@ StorePath RemoteStore::addToStoreFromDump( case FileIngestionMethod::Flat: fsm = FileSerialisationMethod::Flat; break; - case FileIngestionMethod::Recursive: - fsm = FileSerialisationMethod::Recursive; + case FileIngestionMethod::NixArchive: + fsm = FileSerialisationMethod::NixArchive; break; case FileIngestionMethod::Git: // Use NAR; Git is not a serialization method - fsm = FileSerialisationMethod::Recursive; + fsm = FileSerialisationMethod::NixArchive; break; default: assert(false); diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index d630adc08..4e1896268 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -87,8 +87,8 @@ public: StorePath addToStoreFromDump( Source & dump, std::string_view name, - FileSerialisationMethod dumpMethod = FileSerialisationMethod::Recursive, - ContentAddressMethod hashMethod = FileIngestionMethod::Recursive, + FileSerialisationMethod dumpMethod = FileSerialisationMethod::NixArchive, + ContentAddressMethod hashMethod = FileIngestionMethod::NixArchive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, const StorePathSet & references = StorePathSet(), RepairFlag repair = NoRepair) override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 2edb56510..8c957ff1a 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -122,7 +122,7 @@ StorePath StoreDirConfig::makeFixedOutputPath(std::string_view name, const Fixed if (info.method == FileIngestionMethod::Git && info.hash.algo != HashAlgorithm::SHA1) throw Error("Git file ingestion must use SHA-1 hash"); - if (info.hash.algo == HashAlgorithm::SHA256 && info.method == FileIngestionMethod::Recursive) { + if (info.hash.algo == HashAlgorithm::SHA256 && info.method == FileIngestionMethod::NixArchive) { return makeStorePath(makeType(*this, "source", info.references), info.hash, name); } else { if (!info.references.empty()) { @@ -200,12 +200,12 @@ StorePath Store::addToStore( case FileIngestionMethod::Flat: fsm = FileSerialisationMethod::Flat; break; - case FileIngestionMethod::Recursive: - fsm = FileSerialisationMethod::Recursive; + case FileIngestionMethod::NixArchive: + fsm = FileSerialisationMethod::NixArchive; break; case FileIngestionMethod::Git: // Use NAR; Git is not a serialization method - fsm = FileSerialisationMethod::Recursive; + fsm = FileSerialisationMethod::NixArchive; break; } auto source = sinkToSource([&](Sink & sink) { @@ -356,7 +356,7 @@ ValidPathInfo Store::addToStoreSlow( RegularFileSink fileSink { caHashSink }; TeeSink unusualHashTee { narHashSink, caHashSink }; - auto & narSink = method == FileIngestionMethod::Recursive && hashAlgo != HashAlgorithm::SHA256 + auto & narSink = method == FileIngestionMethod::NixArchive && hashAlgo != HashAlgorithm::SHA256 ? static_cast(unusualHashTee) : narHashSink; @@ -384,7 +384,7 @@ ValidPathInfo Store::addToStoreSlow( finish. */ auto [narHash, narSize] = narHashSink.finish(); - auto hash = method == FileIngestionMethod::Recursive && hashAlgo == HashAlgorithm::SHA256 + auto hash = method == FileIngestionMethod::NixArchive && hashAlgo == HashAlgorithm::SHA256 ? narHash : method == FileIngestionMethod::Git ? git::dumpHash(hashAlgo, srcPath).hash diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 15712458c..e719f9bf9 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -441,7 +441,7 @@ public: virtual StorePath addToStore( std::string_view name, const SourcePath & path, - ContentAddressMethod method = FileIngestionMethod::Recursive, + ContentAddressMethod method = FileIngestionMethod::NixArchive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, const StorePathSet & references = StorePathSet(), PathFilter & filter = defaultPathFilter, @@ -455,7 +455,7 @@ public: ValidPathInfo addToStoreSlow( std::string_view name, const SourcePath & path, - ContentAddressMethod method = FileIngestionMethod::Recursive, + ContentAddressMethod method = FileIngestionMethod::NixArchive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, const StorePathSet & references = StorePathSet(), std::optional expectedCAHash = {}); @@ -470,7 +470,7 @@ public: * * @param dumpMethod What serialisation format is `dump`, i.e. how * to deserialize it. Must either match hashMethod or be - * `FileSerialisationMethod::Recursive`. + * `FileSerialisationMethod::NixArchive`. * * @param hashMethod How content addressing? Need not match be the * same as `dumpMethod`. @@ -480,8 +480,8 @@ public: virtual StorePath addToStoreFromDump( Source & dump, std::string_view name, - FileSerialisationMethod dumpMethod = FileSerialisationMethod::Recursive, - ContentAddressMethod hashMethod = FileIngestionMethod::Recursive, + FileSerialisationMethod dumpMethod = FileSerialisationMethod::NixArchive, + ContentAddressMethod hashMethod = FileIngestionMethod::NixArchive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, const StorePathSet & references = StorePathSet(), RepairFlag repair = NoRepair) = 0; diff --git a/src/libstore/store-dir-config.hh b/src/libstore/store-dir-config.hh index 643f8854d..fe86ce40d 100644 --- a/src/libstore/store-dir-config.hh +++ b/src/libstore/store-dir-config.hh @@ -97,7 +97,7 @@ struct StoreDirConfig : public Config std::pair computeStorePath( std::string_view name, const SourcePath & path, - ContentAddressMethod method = FileIngestionMethod::Recursive, + ContentAddressMethod method = FileIngestionMethod::NixArchive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, const StorePathSet & references = {}, PathFilter & filter = defaultPathFilter) const; diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 95df0fdee..ab461d428 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -2489,7 +2489,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() auto fim = outputHash.method.getFileIngestionMethod(); switch (fim) { case FileIngestionMethod::Flat: - case FileIngestionMethod::Recursive: + case FileIngestionMethod::NixArchive: { HashModuloSink caSink { outputHash.hashAlgo, oldHashPart }; auto fim = outputHash.method.getFileIngestionMethod(); @@ -2531,7 +2531,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() { HashResult narHashAndSize = hashPath( {getFSSourceAccessor(), CanonPath(actualPath)}, - FileSerialisationMethod::Recursive, HashAlgorithm::SHA256); + FileSerialisationMethod::NixArchive, HashAlgorithm::SHA256); newInfo0.narHash = narHashAndSize.first; newInfo0.narSize = narHashAndSize.second; } @@ -2554,7 +2554,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() rewriteOutput(outputRewrites); HashResult narHashAndSize = hashPath( {getFSSourceAccessor(), CanonPath(actualPath)}, - FileSerialisationMethod::Recursive, HashAlgorithm::SHA256); + FileSerialisationMethod::NixArchive, HashAlgorithm::SHA256); ValidPathInfo newInfo0 { requiredFinalPath, narHashAndSize.first }; newInfo0.narSize = narHashAndSize.second; auto refs = rewriteRefs(); diff --git a/src/libutil/file-content-address.cc b/src/libutil/file-content-address.cc index 8b1e3117a..438dac7da 100644 --- a/src/libutil/file-content-address.cc +++ b/src/libutil/file-content-address.cc @@ -10,7 +10,7 @@ static std::optional parseFileSerialisationMethodOpt(st if (input == "flat") { return FileSerialisationMethod::Flat; } else if (input == "nar") { - return FileSerialisationMethod::Recursive; + return FileSerialisationMethod::NixArchive; } else { return std::nullopt; } @@ -45,7 +45,7 @@ std::string_view renderFileSerialisationMethod(FileSerialisationMethod method) switch (method) { case FileSerialisationMethod::Flat: return "flat"; - case FileSerialisationMethod::Recursive: + case FileSerialisationMethod::NixArchive: return "nar"; default: assert(false); @@ -57,7 +57,7 @@ std::string_view renderFileIngestionMethod(FileIngestionMethod method) { switch (method) { case FileIngestionMethod::Flat: - case FileIngestionMethod::Recursive: + case FileIngestionMethod::NixArchive: return renderFileSerialisationMethod( static_cast(method)); case FileIngestionMethod::Git: @@ -78,7 +78,7 @@ void dumpPath( case FileSerialisationMethod::Flat: path.readFile(sink); break; - case FileSerialisationMethod::Recursive: + case FileSerialisationMethod::NixArchive: path.dumpPath(sink, filter); break; } @@ -94,7 +94,7 @@ void restorePath( case FileSerialisationMethod::Flat: writeFile(path, source); break; - case FileSerialisationMethod::Recursive: + case FileSerialisationMethod::NixArchive: restorePath(path, source); break; } @@ -119,7 +119,7 @@ std::pair> hashPath( { switch (method) { case FileIngestionMethod::Flat: - case FileIngestionMethod::Recursive: { + case FileIngestionMethod::NixArchive: { auto res = hashPath(path, (FileSerialisationMethod) method, ht, filter); return {res.first, {res.second}}; } diff --git a/src/libutil/file-content-address.hh b/src/libutil/file-content-address.hh index e216ee4a7..f3f5589de 100644 --- a/src/libutil/file-content-address.hh +++ b/src/libutil/file-content-address.hh @@ -35,14 +35,14 @@ enum struct FileSerialisationMethod : uint8_t { * See `file-system-object/content-address.md#serial-nix-archive` in * the manual. */ - Recursive, + NixArchive, }; /** * Parse a `FileSerialisationMethod` by name. Choice of: * * - `flat`: `FileSerialisationMethod::Flat` - * - `nar`: `FileSerialisationMethod::Recursive` + * - `nar`: `FileSerialisationMethod::NixArchive` * * Opposite of `renderFileSerialisationMethod`. */ @@ -107,12 +107,12 @@ enum struct FileIngestionMethod : uint8_t { Flat, /** - * Hash `FileSerialisationMethod::Recursive` serialisation. + * Hash `FileSerialisationMethod::NixArchive` serialisation. * * See `file-system-object/content-address.md#serial-flat` in the * manual. */ - Recursive, + NixArchive, /** * Git hashing. @@ -127,7 +127,7 @@ enum struct FileIngestionMethod : uint8_t { * Parse a `FileIngestionMethod` by name. Choice of: * * - `flat`: `FileIngestionMethod::Flat` - * - `nar`: `FileIngestionMethod::Recursive` + * - `nar`: `FileIngestionMethod::NixArchive` * - `git`: `FileIngestionMethod::Git` * * Opposite of `renderFileIngestionMethod`. diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 6d028e0a7..178702f91 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -197,7 +197,7 @@ static void opAddFixed(Strings opFlags, Strings opArgs) auto method = FileIngestionMethod::Flat; for (auto & i : opFlags) - if (i == "--recursive") method = FileIngestionMethod::Recursive; + if (i == "--recursive") method = FileIngestionMethod::NixArchive; else throw UsageError("unknown flag '%1%'", i); if (opArgs.empty()) @@ -223,7 +223,7 @@ static void opPrintFixedPath(Strings opFlags, Strings opArgs) auto method = FileIngestionMethod::Flat; for (auto i : opFlags) - if (i == "--recursive") method = FileIngestionMethod::Recursive; + if (i == "--recursive") method = FileIngestionMethod::NixArchive; else throw UsageError("unknown flag '%1%'", i); if (opArgs.size() != 3) @@ -563,7 +563,7 @@ static void registerValidity(bool reregister, bool hashGiven, bool canonicalise) if (!hashGiven) { HashResult hash = hashPath( {store->getFSAccessor(false), CanonPath { store->printStorePath(info->path) }}, - FileSerialisationMethod::Recursive, HashAlgorithm::SHA256); + FileSerialisationMethod::NixArchive, HashAlgorithm::SHA256); info->narHash = hash.first; info->narSize = hash.second; } diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index af6743375..ed46254f3 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -12,7 +12,7 @@ struct CmdAddToStore : MixDryRun, StoreCommand { Path path; std::optional namePart; - ContentAddressMethod caMethod = FileIngestionMethod::Recursive; + ContentAddressMethod caMethod = FileIngestionMethod::NixArchive; HashAlgorithm hashAlgo = HashAlgorithm::SHA256; CmdAddToStore() diff --git a/src/nix/hash.cc b/src/nix/hash.cc index f57b224d2..62266fda1 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -68,7 +68,7 @@ struct CmdHashBase : Command switch (mode) { case FileIngestionMethod::Flat: return "print cryptographic hash of a regular file"; - case FileIngestionMethod::Recursive: + case FileIngestionMethod::NixArchive: return "print cryptographic hash of the NAR serialisation of a path"; case FileIngestionMethod::Git: return "print cryptographic hash of the Git serialisation of a path"; @@ -91,7 +91,7 @@ struct CmdHashBase : Command Hash h { HashAlgorithm::SHA256 }; // throwaway def to appease C++ switch (mode) { case FileIngestionMethod::Flat: - case FileIngestionMethod::Recursive: + case FileIngestionMethod::NixArchive: { auto hashSink = makeSink(); dumpPath(path2, *hashSink, (FileSerialisationMethod) mode); @@ -126,7 +126,7 @@ struct CmdHashBase : Command struct CmdHashPath : CmdHashBase { CmdHashPath() - : CmdHashBase(FileIngestionMethod::Recursive) + : CmdHashBase(FileIngestionMethod::NixArchive) { addFlag(flag::hashAlgo("algo", &hashAlgo)); addFlag(flag::fileIngestionMethod(&mode)); @@ -311,7 +311,7 @@ static int compatNixHash(int argc, char * * argv) }); if (op == opHash) { - CmdHashBase cmd(flat ? FileIngestionMethod::Flat : FileIngestionMethod::Recursive); + CmdHashBase cmd(flat ? FileIngestionMethod::Flat : FileIngestionMethod::NixArchive); if (!hashAlgo.has_value()) hashAlgo = HashAlgorithm::MD5; cmd.hashAlgo = hashAlgo.value(); cmd.hashFormat = hashFormat; diff --git a/src/nix/prefetch.cc b/src/nix/prefetch.cc index 3ce52acc5..143935572 100644 --- a/src/nix/prefetch.cc +++ b/src/nix/prefetch.cc @@ -57,7 +57,7 @@ std::tuple prefetchFile( bool unpack, bool executable) { - auto ingestionMethod = unpack || executable ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; + auto ingestionMethod = unpack || executable ? FileIngestionMethod::NixArchive : FileIngestionMethod::Flat; /* Figure out a name in the Nix store. */ if (!name) { diff --git a/src/nix/profile.cc b/src/nix/profile.cc index a5a40e4f6..c89b8c9bd 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -258,7 +258,7 @@ struct ProfileManifest *store, "profile", FixedOutputInfo { - .method = FileIngestionMethod::Recursive, + .method = FileIngestionMethod::NixArchive, .hash = narHash, .references = { .others = std::move(references), diff --git a/src/perl/lib/Nix/Store.xs b/src/perl/lib/Nix/Store.xs index 15eb5c4f8..e8e209d97 100644 --- a/src/perl/lib/Nix/Store.xs +++ b/src/perl/lib/Nix/Store.xs @@ -259,7 +259,7 @@ hashPath(char * algo, int base32, char * path) try { Hash h = hashPath( PosixSourceAccessor::createAtRoot(path), - FileIngestionMethod::Recursive, parseHashAlgo(algo)).first; + FileIngestionMethod::NixArchive, parseHashAlgo(algo)).first; auto s = h.to_string(base32 ? HashFormat::Nix32 : HashFormat::Base16, false); XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0))); } catch (Error & e) { @@ -335,7 +335,7 @@ SV * StoreWrapper::addToStore(char * srcPath, int recursive, char * algo) PPCODE: try { - auto method = recursive ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; + auto method = recursive ? FileIngestionMethod::NixArchive : FileIngestionMethod::Flat; auto path = THIS->store->addToStore( std::string(baseNameOf(srcPath)), PosixSourceAccessor::createAtRoot(srcPath), @@ -351,7 +351,7 @@ StoreWrapper::makeFixedOutputPath(int recursive, char * algo, char * hash, char PPCODE: try { auto h = Hash::parseAny(hash, parseHashAlgo(algo)); - auto method = recursive ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; + auto method = recursive ? FileIngestionMethod::NixArchive : FileIngestionMethod::Flat; auto path = THIS->store->makeFixedOutputPath(name, FixedOutputInfo { .method = method, .hash = h, diff --git a/tests/unit/libstore/common-protocol.cc b/tests/unit/libstore/common-protocol.cc index d23805fc3..2c629b601 100644 --- a/tests/unit/libstore/common-protocol.cc +++ b/tests/unit/libstore/common-protocol.cc @@ -91,7 +91,7 @@ CHARACTERIZATION_TEST( .hash = hashString(HashAlgorithm::SHA1, "blob blob..."), }, ContentAddress { - .method = FileIngestionMethod::Recursive, + .method = FileIngestionMethod::NixArchive, .hash = hashString(HashAlgorithm::SHA256, "(...)"), }, })) diff --git a/tests/unit/libstore/content-address.cc b/tests/unit/libstore/content-address.cc index cc1c7fcc6..f0806bd9a 100644 --- a/tests/unit/libstore/content-address.cc +++ b/tests/unit/libstore/content-address.cc @@ -12,7 +12,7 @@ TEST(ContentAddressMethod, testRoundTripPrintParse_1) { for (const ContentAddressMethod & cam : { ContentAddressMethod { TextIngestionMethod {} }, ContentAddressMethod { FileIngestionMethod::Flat }, - ContentAddressMethod { FileIngestionMethod::Recursive }, + ContentAddressMethod { FileIngestionMethod::NixArchive }, ContentAddressMethod { FileIngestionMethod::Git }, }) { EXPECT_EQ(ContentAddressMethod::parse(cam.render()), cam); diff --git a/tests/unit/libstore/derivation.cc b/tests/unit/libstore/derivation.cc index 7a4b1403a..210813137 100644 --- a/tests/unit/libstore/derivation.cc +++ b/tests/unit/libstore/derivation.cc @@ -117,7 +117,7 @@ TEST_JSON(DerivationTest, caFixedFlat, TEST_JSON(DerivationTest, caFixedNAR, (DerivationOutput::CAFixed { .ca = { - .method = FileIngestionMethod::Recursive, + .method = FileIngestionMethod::NixArchive, .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), }, }), @@ -133,7 +133,7 @@ TEST_JSON(DynDerivationTest, caFixedText, TEST_JSON(CaDerivationTest, caFloating, (DerivationOutput::CAFloating { - .method = FileIngestionMethod::Recursive, + .method = FileIngestionMethod::NixArchive, .hashAlgo = HashAlgorithm::SHA256, }), "drv-name", "output-name") @@ -144,7 +144,7 @@ TEST_JSON(DerivationTest, deferred, TEST_JSON(ImpureDerivationTest, impure, (DerivationOutput::Impure { - .method = FileIngestionMethod::Recursive, + .method = FileIngestionMethod::NixArchive, .hashAlgo = HashAlgorithm::SHA256, }), "drv-name", "output-name") diff --git a/tests/unit/libstore/nar-info.cc b/tests/unit/libstore/nar-info.cc index bd10602e7..a6cb62de4 100644 --- a/tests/unit/libstore/nar-info.cc +++ b/tests/unit/libstore/nar-info.cc @@ -25,7 +25,7 @@ static NarInfo makeNarInfo(const Store & store, bool includeImpureInfo) { store, "foo", FixedOutputInfo { - .method = FileIngestionMethod::Recursive, + .method = FileIngestionMethod::NixArchive, .hash = hashString(HashAlgorithm::SHA256, "(...)"), .references = { diff --git a/tests/unit/libstore/path-info.cc b/tests/unit/libstore/path-info.cc index 06c662b74..7637cb366 100644 --- a/tests/unit/libstore/path-info.cc +++ b/tests/unit/libstore/path-info.cc @@ -32,7 +32,7 @@ static UnkeyedValidPathInfo makeFull(const Store & store, bool includeImpureInfo store, "foo", FixedOutputInfo { - .method = FileIngestionMethod::Recursive, + .method = FileIngestionMethod::NixArchive, .hash = hashString(HashAlgorithm::SHA256, "(...)"), .references = { diff --git a/tests/unit/libstore/serve-protocol.cc b/tests/unit/libstore/serve-protocol.cc index ebf0c52b0..17d7153bb 100644 --- a/tests/unit/libstore/serve-protocol.cc +++ b/tests/unit/libstore/serve-protocol.cc @@ -63,7 +63,7 @@ VERSIONED_CHARACTERIZATION_TEST( .hash = hashString(HashAlgorithm::SHA1, "blob blob..."), }, ContentAddress { - .method = FileIngestionMethod::Recursive, + .method = FileIngestionMethod::NixArchive, .hash = hashString(HashAlgorithm::SHA256, "(...)"), }, })) @@ -280,7 +280,7 @@ VERSIONED_CHARACTERIZATION_TEST( *LibStoreTest::store, "foo", FixedOutputInfo { - .method = FileIngestionMethod::Recursive, + .method = FileIngestionMethod::NixArchive, .hash = hashString(HashAlgorithm::SHA256, "(...)"), .references = { .others = { diff --git a/tests/unit/libstore/worker-protocol.cc b/tests/unit/libstore/worker-protocol.cc index 70e03a8ab..8d81717c9 100644 --- a/tests/unit/libstore/worker-protocol.cc +++ b/tests/unit/libstore/worker-protocol.cc @@ -64,7 +64,7 @@ VERSIONED_CHARACTERIZATION_TEST( .hash = hashString(HashAlgorithm::SHA1, "blob blob..."), }, ContentAddress { - .method = FileIngestionMethod::Recursive, + .method = FileIngestionMethod::NixArchive, .hash = hashString(HashAlgorithm::SHA256, "(...)"), }, })) @@ -512,7 +512,7 @@ VERSIONED_CHARACTERIZATION_TEST( *LibStoreTest::store, "foo", FixedOutputInfo { - .method = FileIngestionMethod::Recursive, + .method = FileIngestionMethod::NixArchive, .hash = hashString(HashAlgorithm::SHA256, "(...)"), .references = { .others = { diff --git a/tests/unit/libutil/file-content-address.cc b/tests/unit/libutil/file-content-address.cc index 294e39806..27d926a87 100644 --- a/tests/unit/libutil/file-content-address.cc +++ b/tests/unit/libutil/file-content-address.cc @@ -11,7 +11,7 @@ namespace nix { TEST(FileSerialisationMethod, testRoundTripPrintParse_1) { for (const FileSerialisationMethod fim : { FileSerialisationMethod::Flat, - FileSerialisationMethod::Recursive, + FileSerialisationMethod::NixArchive, }) { EXPECT_EQ(parseFileSerialisationMethod(renderFileSerialisationMethod(fim)), fim); } @@ -37,7 +37,7 @@ TEST(FileSerialisationMethod, testParseFileSerialisationMethodOptException) { TEST(FileIngestionMethod, testRoundTripPrintParse_1) { for (const FileIngestionMethod fim : { FileIngestionMethod::Flat, - FileIngestionMethod::Recursive, + FileIngestionMethod::NixArchive, FileIngestionMethod::Git, }) { EXPECT_EQ(parseFileIngestionMethod(renderFileIngestionMethod(fim)), fim); From b51e161af57e92691b9d74413e24050ddf9ed2c5 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 16 May 2024 19:08:28 -0400 Subject: [PATCH 18/18] Cleanup `ContentAddressMethod` to match docs The old `std::variant` is bad because we aren't adding a new case to `FileIngestionMethod` so much as we are defining a separate concept --- store object content addressing rather than file system object content addressing. As such, it is more correct to just create a fresh enumeration. Co-authored-by: Robert Hensing --- src/libexpr/eval.cc | 2 +- src/libexpr/primops.cc | 22 +-- src/libfetchers/fetch-to-store.hh | 2 +- src/libfetchers/mercurial.cc | 2 +- src/libstore/content-address.cc | 217 +++++++++++++++---------- src/libstore/content-address.hh | 76 +++++---- src/libstore/daemon.cc | 12 +- src/libstore/derivations.cc | 6 +- src/libstore/local-store.cc | 2 +- src/libstore/path-info.cc | 32 ++-- src/libstore/remote-store.cc | 19 ++- src/libstore/store-api.cc | 6 +- src/libstore/store-api.hh | 6 +- src/libutil/file-content-address.hh | 2 + src/nix-env/user-env.cc | 2 +- src/nix-store/nix-store.cc | 4 +- src/nix/add-to-store.cc | 4 +- src/nix/develop.cc | 2 +- src/nix/prefetch.cc | 15 +- src/perl/lib/Nix/Store.xs | 2 +- tests/unit/libstore/common-protocol.cc | 8 +- tests/unit/libstore/content-address.cc | 10 +- tests/unit/libstore/derivation.cc | 9 +- tests/unit/libstore/serve-protocol.cc | 8 +- tests/unit/libstore/worker-protocol.cc | 8 +- 25 files changed, 275 insertions(+), 203 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index fc211e694..0c45a7436 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2303,7 +2303,7 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat path.resolveSymlinks(), settings.readOnlyMode ? FetchMode::DryRun : FetchMode::Copy, path.baseName(), - FileIngestionMethod::NixArchive, + ContentAddressMethod::Raw::NixArchive, nullptr, repair); allowPath(dstPath); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 02b970112..8a71359de 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1209,7 +1209,7 @@ static void derivationStrictInternal( auto handleHashMode = [&](const std::string_view s) { if (s == "recursive") { // back compat, new name is "nar" - ingestionMethod = FileIngestionMethod::NixArchive; + ingestionMethod = ContentAddressMethod::Raw::NixArchive; } else try { ingestionMethod = ContentAddressMethod::parse(s); } catch (UsageError &) { @@ -1217,9 +1217,9 @@ static void derivationStrictInternal( "invalid value '%s' for 'outputHashMode' attribute", s ).atPos(v).debugThrow(); } - if (ingestionMethod == TextIngestionMethod {}) + if (ingestionMethod == ContentAddressMethod::Raw::Text) experimentalFeatureSettings.require(Xp::DynamicDerivations); - if (ingestionMethod == FileIngestionMethod::Git) + if (ingestionMethod == ContentAddressMethod::Raw::Git) experimentalFeatureSettings.require(Xp::GitHashing); }; @@ -1391,7 +1391,7 @@ static void derivationStrictInternal( /* Check whether the derivation name is valid. */ if (isDerivation(drvName) && - !(ingestionMethod == ContentAddressMethod { TextIngestionMethod { } } && + !(ingestionMethod == ContentAddressMethod::Raw::Text && outputs.size() == 1 && *(outputs.begin()) == "out")) { @@ -1413,7 +1413,7 @@ static void derivationStrictInternal( auto h = newHashAllowEmpty(*outputHash, outputHashAlgo); - auto method = ingestionMethod.value_or(FileIngestionMethod::Flat); + auto method = ingestionMethod.value_or(ContentAddressMethod::Raw::Flat); DerivationOutput::CAFixed dof { .ca = ContentAddress { @@ -1432,7 +1432,7 @@ static void derivationStrictInternal( .atPos(v).debugThrow(); auto ha = outputHashAlgo.value_or(HashAlgorithm::SHA256); - auto method = ingestionMethod.value_or(FileIngestionMethod::NixArchive); + auto method = ingestionMethod.value_or(ContentAddressMethod::Raw::NixArchive); for (auto & i : outputs) { drv.env[i] = hashPlaceholder(i); @@ -2208,7 +2208,7 @@ static void prim_toFile(EvalState & state, const PosIdx pos, Value * * args, Val }) : ({ StringSource s { contents }; - state.store->addToStoreFromDump(s, name, FileSerialisationMethod::Flat, TextIngestionMethod {}, HashAlgorithm::SHA256, refs, state.repair); + state.store->addToStoreFromDump(s, name, FileSerialisationMethod::Flat, ContentAddressMethod::Raw::Text, HashAlgorithm::SHA256, refs, state.repair); }); /* Note: we don't need to add `context' to the context of the @@ -2391,7 +2391,7 @@ static void prim_filterSource(EvalState & state, const PosIdx pos, Value * * arg "while evaluating the second argument (the path to filter) passed to 'builtins.filterSource'"); state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.filterSource"); - addPath(state, pos, path.baseName(), path, args[0], FileIngestionMethod::NixArchive, std::nullopt, v, context); + addPath(state, pos, path.baseName(), path, args[0], ContentAddressMethod::Raw::NixArchive, std::nullopt, v, context); } static RegisterPrimOp primop_filterSource({ @@ -2454,7 +2454,7 @@ static void prim_path(EvalState & state, const PosIdx pos, Value * * args, Value std::optional path; std::string name; Value * filterFun = nullptr; - ContentAddressMethod method = FileIngestionMethod::NixArchive; + auto method = ContentAddressMethod::Raw::NixArchive; std::optional expectedHash; NixStringContext context; @@ -2470,8 +2470,8 @@ static void prim_path(EvalState & state, const PosIdx pos, Value * * args, Value state.forceFunction(*(filterFun = attr.value), attr.pos, "while evaluating the `filter` parameter passed to builtins.path"); else if (n == "recursive") method = state.forceBool(*attr.value, attr.pos, "while evaluating the `recursive` attribute passed to builtins.path") - ? FileIngestionMethod::NixArchive - : FileIngestionMethod::Flat; + ? ContentAddressMethod::Raw::NixArchive + : ContentAddressMethod::Raw::Flat; else if (n == "sha256") expectedHash = newHashAllowEmpty(state.forceStringNoCtx(*attr.value, attr.pos, "while evaluating the `sha256` attribute passed to builtins.path"), HashAlgorithm::SHA256); else diff --git a/src/libfetchers/fetch-to-store.hh b/src/libfetchers/fetch-to-store.hh index 95d1e6b01..c762629f3 100644 --- a/src/libfetchers/fetch-to-store.hh +++ b/src/libfetchers/fetch-to-store.hh @@ -18,7 +18,7 @@ StorePath fetchToStore( const SourcePath & path, FetchMode mode, std::string_view name = "source", - ContentAddressMethod method = FileIngestionMethod::NixArchive, + ContentAddressMethod method = ContentAddressMethod::Raw::NixArchive, PathFilter * filter = nullptr, RepairFlag repair = NoRepair); diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 4b80cbe9b..198795caa 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -213,7 +213,7 @@ struct MercurialInputScheme : InputScheme auto storePath = store->addToStore( input.getName(), {getFSSourceAccessor(), CanonPath(actualPath)}, - FileIngestionMethod::NixArchive, HashAlgorithm::SHA256, {}, + ContentAddressMethod::Raw::NixArchive, HashAlgorithm::SHA256, {}, filter); return storePath; diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index fa06c4aa3..e1cdfece6 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -8,6 +8,7 @@ std::string_view makeFileIngestionPrefix(FileIngestionMethod m) { switch (m) { case FileIngestionMethod::Flat: + // Not prefixed for back compat return ""; case FileIngestionMethod::NixArchive: return "r:"; @@ -15,91 +16,128 @@ std::string_view makeFileIngestionPrefix(FileIngestionMethod m) experimentalFeatureSettings.require(Xp::GitHashing); return "git:"; default: - throw Error("impossible, caught both cases"); + assert(false); } } std::string_view ContentAddressMethod::render() const { - return std::visit(overloaded { - [](TextIngestionMethod) -> std::string_view { return "text"; }, - [](FileIngestionMethod m2) { - /* Not prefixed for back compat with things that couldn't produce text before. */ - return renderFileIngestionMethod(m2); - }, - }, raw); + switch (raw) { + case ContentAddressMethod::Raw::Text: + return "text"; + case ContentAddressMethod::Raw::Flat: + case ContentAddressMethod::Raw::NixArchive: + case ContentAddressMethod::Raw::Git: + return renderFileIngestionMethod(getFileIngestionMethod()); + default: + assert(false); + } +} + +/** + * **Not surjective** + * + * This is not exposed because `FileIngestionMethod::Flat` maps to + * `ContentAddressMethod::Raw::Flat` and + * `ContentAddressMethod::Raw::Text` alike. We can thus only safely use + * this when the latter is ruled out (e.g. because it is already + * handled). + */ +static ContentAddressMethod fileIngestionMethodToContentAddressMethod(FileIngestionMethod m) +{ + switch (m) { + case FileIngestionMethod::Flat: + return ContentAddressMethod::Raw::Flat; + case FileIngestionMethod::NixArchive: + return ContentAddressMethod::Raw::NixArchive; + case FileIngestionMethod::Git: + return ContentAddressMethod::Raw::Git; + default: + assert(false); + } } ContentAddressMethod ContentAddressMethod::parse(std::string_view m) { if (m == "text") - return TextIngestionMethod {}; + return ContentAddressMethod::Raw::Text; else - return parseFileIngestionMethod(m); + return fileIngestionMethodToContentAddressMethod( + parseFileIngestionMethod(m)); } std::string_view ContentAddressMethod::renderPrefix() const { - return std::visit(overloaded { - [](TextIngestionMethod) -> std::string_view { return "text:"; }, - [](FileIngestionMethod m2) { - /* Not prefixed for back compat with things that couldn't produce text before. */ - return makeFileIngestionPrefix(m2); - }, - }, raw); + switch (raw) { + case ContentAddressMethod::Raw::Text: + return "text:"; + case ContentAddressMethod::Raw::Flat: + case ContentAddressMethod::Raw::NixArchive: + case ContentAddressMethod::Raw::Git: + return makeFileIngestionPrefix(getFileIngestionMethod()); + default: + assert(false); + } } ContentAddressMethod ContentAddressMethod::parsePrefix(std::string_view & m) { if (splitPrefix(m, "r:")) { - return FileIngestionMethod::NixArchive; + return ContentAddressMethod::Raw::NixArchive; } else if (splitPrefix(m, "git:")) { experimentalFeatureSettings.require(Xp::GitHashing); - return FileIngestionMethod::Git; + return ContentAddressMethod::Raw::Git; } else if (splitPrefix(m, "text:")) { - return TextIngestionMethod {}; + return ContentAddressMethod::Raw::Text; + } + return ContentAddressMethod::Raw::Flat; +} + +/** + * This is slightly more mindful of forward compat in that it uses `fixed:` + * rather than just doing a raw empty prefix or `r:`, which doesn't "save room" + * for future changes very well. + */ +static std::string renderPrefixModern(const ContentAddressMethod & ca) +{ + switch (ca.raw) { + case ContentAddressMethod::Raw::Text: + return "text:"; + case ContentAddressMethod::Raw::Flat: + case ContentAddressMethod::Raw::NixArchive: + case ContentAddressMethod::Raw::Git: + return "fixed:" + makeFileIngestionPrefix(ca.getFileIngestionMethod()); + default: + assert(false); } - return FileIngestionMethod::Flat; } std::string ContentAddressMethod::renderWithAlgo(HashAlgorithm ha) const { - return std::visit(overloaded { - [&](const TextIngestionMethod & th) { - return std::string{"text:"} + printHashAlgo(ha); - }, - [&](const FileIngestionMethod & fim) { - return "fixed:" + makeFileIngestionPrefix(fim) + printHashAlgo(ha); - } - }, raw); + return renderPrefixModern(*this) + printHashAlgo(ha); } FileIngestionMethod ContentAddressMethod::getFileIngestionMethod() const { - return std::visit(overloaded { - [&](const TextIngestionMethod & th) { - return FileIngestionMethod::Flat; - }, - [&](const FileIngestionMethod & fim) { - return fim; - } - }, raw); + switch (raw) { + case ContentAddressMethod::Raw::Flat: + return FileIngestionMethod::Flat; + case ContentAddressMethod::Raw::NixArchive: + return FileIngestionMethod::NixArchive; + case ContentAddressMethod::Raw::Git: + return FileIngestionMethod::Git; + case ContentAddressMethod::Raw::Text: + return FileIngestionMethod::Flat; + default: + assert(false); + } } std::string ContentAddress::render() const { - return std::visit(overloaded { - [](const TextIngestionMethod &) -> std::string { - return "text:"; - }, - [](const FileIngestionMethod & method) { - return "fixed:" - + makeFileIngestionPrefix(method); - }, - }, method.raw) - + this->hash.to_string(HashFormat::Nix32, true); + return renderPrefixModern(method) + this->hash.to_string(HashFormat::Nix32, true); } /** @@ -130,17 +168,17 @@ static std::pair parseContentAddressMethodP // No parsing of the ingestion method, "text" only support flat. HashAlgorithm hashAlgo = parseHashAlgorithm_(); return { - TextIngestionMethod {}, + ContentAddressMethod::Raw::Text, std::move(hashAlgo), }; } else if (prefix == "fixed") { // Parse method - auto method = FileIngestionMethod::Flat; + auto method = ContentAddressMethod::Raw::Flat; if (splitPrefix(rest, "r:")) - method = FileIngestionMethod::NixArchive; + method = ContentAddressMethod::Raw::NixArchive; else if (splitPrefix(rest, "git:")) { experimentalFeatureSettings.require(Xp::GitHashing); - method = FileIngestionMethod::Git; + method = ContentAddressMethod::Raw::Git; } HashAlgorithm hashAlgo = parseHashAlgorithm_(); return { @@ -201,57 +239,58 @@ size_t StoreReferences::size() const ContentAddressWithReferences ContentAddressWithReferences::withoutRefs(const ContentAddress & ca) noexcept { - return std::visit(overloaded { - [&](const TextIngestionMethod &) -> ContentAddressWithReferences { - return TextInfo { - .hash = ca.hash, - .references = {}, - }; - }, - [&](const FileIngestionMethod & method) -> ContentAddressWithReferences { - return FixedOutputInfo { - .method = method, - .hash = ca.hash, - .references = {}, - }; - }, - }, ca.method.raw); + switch (ca.method.raw) { + case ContentAddressMethod::Raw::Text: + return TextInfo { + .hash = ca.hash, + .references = {}, + }; + case ContentAddressMethod::Raw::Flat: + case ContentAddressMethod::Raw::NixArchive: + case ContentAddressMethod::Raw::Git: + return FixedOutputInfo { + .method = ca.method.getFileIngestionMethod(), + .hash = ca.hash, + .references = {}, + }; + default: + assert(false); + } } ContentAddressWithReferences ContentAddressWithReferences::fromParts( ContentAddressMethod method, Hash hash, StoreReferences refs) { - return std::visit(overloaded { - [&](TextIngestionMethod _) -> ContentAddressWithReferences { - if (refs.self) - throw Error("self-reference not allowed with text hashing"); - return ContentAddressWithReferences { - TextInfo { - .hash = std::move(hash), - .references = std::move(refs.others), - } - }; - }, - [&](FileIngestionMethod m2) -> ContentAddressWithReferences { - return ContentAddressWithReferences { - FixedOutputInfo { - .method = m2, - .hash = std::move(hash), - .references = std::move(refs), - } - }; - }, - }, method.raw); + switch (method.raw) { + case ContentAddressMethod::Raw::Text: + if (refs.self) + throw Error("self-reference not allowed with text hashing"); + return TextInfo { + .hash = std::move(hash), + .references = std::move(refs.others), + }; + case ContentAddressMethod::Raw::Flat: + case ContentAddressMethod::Raw::NixArchive: + case ContentAddressMethod::Raw::Git: + return FixedOutputInfo { + .method = method.getFileIngestionMethod(), + .hash = std::move(hash), + .references = std::move(refs), + }; + default: + assert(false); + } } ContentAddressMethod ContentAddressWithReferences::getMethod() const { return std::visit(overloaded { [](const TextInfo & th) -> ContentAddressMethod { - return TextIngestionMethod {}; + return ContentAddressMethod::Raw::Text; }, [](const FixedOutputInfo & fsh) -> ContentAddressMethod { - return fsh.method; + return fileIngestionMethodToContentAddressMethod( + fsh.method); }, }, raw); } diff --git a/src/libstore/content-address.hh b/src/libstore/content-address.hh index 5925f8e01..6cc3b7cd9 100644 --- a/src/libstore/content-address.hh +++ b/src/libstore/content-address.hh @@ -5,7 +5,6 @@ #include "hash.hh" #include "path.hh" #include "file-content-address.hh" -#include "comparator.hh" #include "variant-wrapper.hh" namespace nix { @@ -14,24 +13,6 @@ namespace nix { * Content addressing method */ -/* We only have one way to hash text with references, so this is a single-value - type, mainly useful with std::variant. -*/ - -/** - * The single way we can serialize "text" file system objects. - * - * Somewhat obscure, used by \ref Derivation derivations and - * `builtins.toFile` currently. - * - * TextIngestionMethod is identical to FileIngestionMethod::Fixed except that - * the former may not have self-references and is tagged `text:${algo}:${hash}` - * rather than `fixed:${algo}:${hash}`. The contents of the store path are - * ingested and hashed identically, aside from the slightly different tag and - * restriction on self-references. - */ -struct TextIngestionMethod : std::monostate { }; - /** * Compute the prefix to the hash algorithm which indicates how the * files were ingested. @@ -48,14 +29,51 @@ std::string_view makeFileIngestionPrefix(FileIngestionMethod m); */ struct ContentAddressMethod { - typedef std::variant< - TextIngestionMethod, - FileIngestionMethod - > Raw; + enum struct Raw { + /** + * Calculate a store path using the `FileIngestionMethod::Flat` + * hash of the file system objects, and references. + * + * See `store-object/content-address.md#method-flat` in the + * manual. + */ + Flat, + + /** + * Calculate a store path using the + * `FileIngestionMethod::NixArchive` hash of the file system + * objects, and references. + * + * See `store-object/content-address.md#method-flat` in the + * manual. + */ + NixArchive, + + /** + * Calculate a store path using the `FileIngestionMethod::Git` + * hash of the file system objects, and references. + * + * Part of `ExperimentalFeature::GitHashing`. + * + * See `store-object/content-address.md#method-git` in the + * manual. + */ + Git, + + /** + * Calculate a store path using the `FileIngestionMethod::Flat` + * hash of the file system objects, and references, but in a + * different way than `ContentAddressMethod::Raw::Flat`. + * + * See `store-object/content-address.md#method-text` in the + * manual. + */ + Text, + }; Raw raw; - GENERATE_CMP(ContentAddressMethod, me->raw); + auto operator <=>(const ContentAddressMethod &) const = default; MAKE_WRAPPER_CONSTRUCTOR(ContentAddressMethod); @@ -141,7 +159,7 @@ struct ContentAddress */ Hash hash; - GENERATE_CMP(ContentAddress, me->method, me->hash); + auto operator <=>(const ContentAddress &) const = default; /** * Compute the content-addressability assertion @@ -200,7 +218,7 @@ struct StoreReferences */ size_t size() const; - GENERATE_CMP(StoreReferences, me->self, me->others); + auto operator <=>(const StoreReferences &) const = default; }; // This matches the additional info that we need for makeTextPath @@ -217,7 +235,7 @@ struct TextInfo */ StorePathSet references; - GENERATE_CMP(TextInfo, me->hash, me->references); + auto operator <=>(const TextInfo &) const = default; }; struct FixedOutputInfo @@ -237,7 +255,7 @@ struct FixedOutputInfo */ StoreReferences references; - GENERATE_CMP(FixedOutputInfo, me->hash, me->references); + auto operator <=>(const FixedOutputInfo &) const = default; }; /** @@ -254,7 +272,7 @@ struct ContentAddressWithReferences Raw raw; - GENERATE_CMP(ContentAddressWithReferences, me->raw); + auto operator <=>(const ContentAddressWithReferences &) const = default; MAKE_WRAPPER_CONSTRUCTOR(ContentAddressWithReferences); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 788c5e2ea..40163a621 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -435,19 +435,21 @@ static void performOp(TunnelLogger * logger, ref store, } else { HashAlgorithm hashAlgo; std::string baseName; - FileIngestionMethod method; + ContentAddressMethod method; { bool fixed; uint8_t recursive; std::string hashAlgoRaw; from >> baseName >> fixed /* obsolete */ >> recursive >> hashAlgoRaw; - if (recursive > (uint8_t) FileIngestionMethod::NixArchive) + if (recursive > true) throw Error("unsupported FileIngestionMethod with value of %i; you may need to upgrade nix-daemon", recursive); - method = FileIngestionMethod { recursive }; + method = recursive + ? ContentAddressMethod::Raw::NixArchive + : ContentAddressMethod::Raw::Flat; /* Compatibility hack. */ if (!fixed) { hashAlgoRaw = "sha256"; - method = FileIngestionMethod::NixArchive; + method = ContentAddressMethod::Raw::NixArchive; } hashAlgo = parseHashAlgo(hashAlgoRaw); } @@ -500,7 +502,7 @@ static void performOp(TunnelLogger * logger, ref store, logger->startWork(); auto path = ({ StringSource source { s }; - store->addToStoreFromDump(source, suffix, FileSerialisationMethod::Flat, TextIngestionMethod {}, HashAlgorithm::SHA256, refs, NoRepair); + store->addToStoreFromDump(source, suffix, FileSerialisationMethod::Flat, ContentAddressMethod::Raw::Text, HashAlgorithm::SHA256, refs, NoRepair); }); logger->stopWork(); to << store->printStorePath(path); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 869880112..6dfcc408c 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -150,7 +150,7 @@ StorePath writeDerivation(Store & store, }) : ({ StringSource s { contents }; - store.addToStoreFromDump(s, suffix, FileSerialisationMethod::Flat, TextIngestionMethod {}, HashAlgorithm::SHA256, references, repair); + store.addToStoreFromDump(s, suffix, FileSerialisationMethod::Flat, ContentAddressMethod::Raw::Text, HashAlgorithm::SHA256, references, repair); }); } @@ -274,7 +274,7 @@ static DerivationOutput parseDerivationOutput( { if (hashAlgoStr != "") { ContentAddressMethod method = ContentAddressMethod::parsePrefix(hashAlgoStr); - if (method == TextIngestionMethod {}) + if (method == ContentAddressMethod::Raw::Text) xpSettings.require(Xp::DynamicDerivations); const auto hashAlgo = parseHashAlgo(hashAlgoStr); if (hashS == "impure") { @@ -1249,7 +1249,7 @@ DerivationOutput DerivationOutput::fromJSON( auto methodAlgo = [&]() -> std::pair { auto & method_ = getString(valueAt(json, "method")); ContentAddressMethod method = ContentAddressMethod::parse(method_); - if (method == TextIngestionMethod {}) + if (method == ContentAddressMethod::Raw::Text) xpSettings.require(Xp::DynamicDerivations); auto & hashAlgo_ = getString(valueAt(json, "hashAlgo")); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 07ace70d0..2b4e01eb3 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1253,7 +1253,7 @@ StorePath LocalStore::addToStoreFromDump( std::filesystem::path tempDir; AutoCloseFD tempDirFd; - bool methodsMatch = ContentAddressMethod(FileIngestionMethod(dumpMethod)) == hashMethod; + bool methodsMatch = static_cast(dumpMethod) == hashMethod.getFileIngestionMethod(); /* If the methods don't match, our streaming hash of the dump is the wrong sort, and we need to rehash. */ diff --git a/src/libstore/path-info.cc b/src/libstore/path-info.cc index ddd7f50d9..5c27182b7 100644 --- a/src/libstore/path-info.cc +++ b/src/libstore/path-info.cc @@ -48,15 +48,21 @@ std::optional ValidPathInfo::contentAddressWithRef if (! ca) return std::nullopt; - return std::visit(overloaded { - [&](const TextIngestionMethod &) -> ContentAddressWithReferences { + switch (ca->method.raw) { + case ContentAddressMethod::Raw::Text: + { assert(references.count(path) == 0); return TextInfo { .hash = ca->hash, .references = references, }; - }, - [&](const FileIngestionMethod & m2) -> ContentAddressWithReferences { + } + + case ContentAddressMethod::Raw::Flat: + case ContentAddressMethod::Raw::NixArchive: + case ContentAddressMethod::Raw::Git: + default: + { auto refs = references; bool hasSelfReference = false; if (refs.count(path)) { @@ -64,15 +70,15 @@ std::optional ValidPathInfo::contentAddressWithRef refs.erase(path); } return FixedOutputInfo { - .method = m2, + .method = ca->method.getFileIngestionMethod(), .hash = ca->hash, .references = { .others = std::move(refs), .self = hasSelfReference, }, }; - }, - }, ca->method.raw); + } + } } bool ValidPathInfo::isContentAddressed(const Store & store) const @@ -127,22 +133,18 @@ ValidPathInfo::ValidPathInfo( : UnkeyedValidPathInfo(narHash) , path(store.makeFixedOutputPathFromCA(name, ca)) { + this->ca = ContentAddress { + .method = ca.getMethod(), + .hash = ca.getHash(), + }; std::visit(overloaded { [this](TextInfo && ti) { this->references = std::move(ti.references); - this->ca = ContentAddress { - .method = TextIngestionMethod {}, - .hash = std::move(ti.hash), - }; }, [this](FixedOutputInfo && foi) { this->references = std::move(foi.references.others); if (foi.references.self) this->references.insert(path); - this->ca = ContentAddress { - .method = std::move(foi.method), - .hash = std::move(foi.hash), - }; }, }, std::move(ca).raw); } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 9adad9c2a..d749ccd0a 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -392,8 +392,9 @@ ref RemoteStore::addCAToStore( else { if (repair) throw Error("repairing is not supported when building through the Nix daemon protocol < 1.25"); - std::visit(overloaded { - [&](const TextIngestionMethod & thm) -> void { + switch (caMethod.raw) { + case ContentAddressMethod::Raw::Text: + { if (hashAlgo != HashAlgorithm::SHA256) throw UnimplementedError("When adding text-hashed data called '%s', only SHA-256 is supported but '%s' was given", name, printHashAlgo(hashAlgo)); @@ -401,8 +402,14 @@ ref RemoteStore::addCAToStore( conn->to << WorkerProto::Op::AddTextToStore << name << s; WorkerProto::write(*this, *conn, references); conn.processStderr(); - }, - [&](const FileIngestionMethod & fim) -> void { + break; + } + case ContentAddressMethod::Raw::Flat: + case ContentAddressMethod::Raw::NixArchive: + case ContentAddressMethod::Raw::Git: + default: + { + auto fim = caMethod.getFileIngestionMethod(); conn->to << WorkerProto::Op::AddToStore << name @@ -432,9 +439,9 @@ ref RemoteStore::addCAToStore( } catch (EndOfFile & e) { } throw; } - + break; } - }, caMethod.raw); + } auto path = parseStorePath(readString(conn->from)); // Release our connection to prevent a deadlock in queryPathInfo(). conn_.reset(); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 8c957ff1a..05c4e1c5e 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -356,7 +356,7 @@ ValidPathInfo Store::addToStoreSlow( RegularFileSink fileSink { caHashSink }; TeeSink unusualHashTee { narHashSink, caHashSink }; - auto & narSink = method == FileIngestionMethod::NixArchive && hashAlgo != HashAlgorithm::SHA256 + auto & narSink = method == ContentAddressMethod::Raw::NixArchive && hashAlgo != HashAlgorithm::SHA256 ? static_cast(unusualHashTee) : narHashSink; @@ -384,9 +384,9 @@ ValidPathInfo Store::addToStoreSlow( finish. */ auto [narHash, narSize] = narHashSink.finish(); - auto hash = method == FileIngestionMethod::NixArchive && hashAlgo == HashAlgorithm::SHA256 + auto hash = method == ContentAddressMethod::Raw::NixArchive && hashAlgo == HashAlgorithm::SHA256 ? narHash - : method == FileIngestionMethod::Git + : method == ContentAddressMethod::Raw::Git ? git::dumpHash(hashAlgo, srcPath).hash : caHashSink.finish().first; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index e719f9bf9..a5effb4c1 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -441,7 +441,7 @@ public: virtual StorePath addToStore( std::string_view name, const SourcePath & path, - ContentAddressMethod method = FileIngestionMethod::NixArchive, + ContentAddressMethod method = ContentAddressMethod::Raw::NixArchive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, const StorePathSet & references = StorePathSet(), PathFilter & filter = defaultPathFilter, @@ -455,7 +455,7 @@ public: ValidPathInfo addToStoreSlow( std::string_view name, const SourcePath & path, - ContentAddressMethod method = FileIngestionMethod::NixArchive, + ContentAddressMethod method = ContentAddressMethod::Raw::NixArchive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, const StorePathSet & references = StorePathSet(), std::optional expectedCAHash = {}); @@ -481,7 +481,7 @@ public: Source & dump, std::string_view name, FileSerialisationMethod dumpMethod = FileSerialisationMethod::NixArchive, - ContentAddressMethod hashMethod = FileIngestionMethod::NixArchive, + ContentAddressMethod hashMethod = ContentAddressMethod::Raw::NixArchive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, const StorePathSet & references = StorePathSet(), RepairFlag repair = NoRepair) = 0; diff --git a/src/libutil/file-content-address.hh b/src/libutil/file-content-address.hh index f3f5589de..4c7218f19 100644 --- a/src/libutil/file-content-address.hh +++ b/src/libutil/file-content-address.hh @@ -117,6 +117,8 @@ enum struct FileIngestionMethod : uint8_t { /** * Git hashing. * + * Part of `ExperimentalFeature::GitHashing`. + * * See `file-system-object/content-address.md#serial-git` in the * manual. */ diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index 5246b03e4..a24dd11d6 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -115,7 +115,7 @@ bool createUserEnv(EvalState & state, PackageInfos & elems, std::string str2 = str.str(); StringSource source { str2 }; state.store->addToStoreFromDump( - source, "env-manifest.nix", FileSerialisationMethod::Flat, TextIngestionMethod {}, HashAlgorithm::SHA256, references); + source, "env-manifest.nix", FileSerialisationMethod::Flat, ContentAddressMethod::Raw::Text, HashAlgorithm::SHA256, references); }); /* Get the environment builder expression. */ diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 178702f91..d0840a02e 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -194,10 +194,10 @@ static void opAdd(Strings opFlags, Strings opArgs) store. */ static void opAddFixed(Strings opFlags, Strings opArgs) { - auto method = FileIngestionMethod::Flat; + ContentAddressMethod method = ContentAddressMethod::Raw::Flat; for (auto & i : opFlags) - if (i == "--recursive") method = FileIngestionMethod::NixArchive; + if (i == "--recursive") method = ContentAddressMethod::Raw::NixArchive; else throw UsageError("unknown flag '%1%'", i); if (opArgs.empty()) diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index ed46254f3..5c08f7616 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -12,7 +12,7 @@ struct CmdAddToStore : MixDryRun, StoreCommand { Path path; std::optional namePart; - ContentAddressMethod caMethod = FileIngestionMethod::NixArchive; + ContentAddressMethod caMethod = ContentAddressMethod::Raw::NixArchive; HashAlgorithm hashAlgo = HashAlgorithm::SHA256; CmdAddToStore() @@ -68,7 +68,7 @@ struct CmdAddFile : CmdAddToStore { CmdAddFile() { - caMethod = FileIngestionMethod::Flat; + caMethod = ContentAddressMethod::Raw::Flat; } std::string description() override diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 27287a1a8..80510dc78 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -238,7 +238,7 @@ static StorePath getDerivationEnvironment(ref store, ref evalStore auto getEnvShPath = ({ StringSource source { getEnvSh }; evalStore->addToStoreFromDump( - source, "get-env.sh", FileSerialisationMethod::Flat, TextIngestionMethod {}, HashAlgorithm::SHA256, {}); + source, "get-env.sh", FileSerialisationMethod::Flat, ContentAddressMethod::Raw::Text, HashAlgorithm::SHA256, {}); }); drv.args = {store->printStorePath(getEnvShPath)}; diff --git a/src/nix/prefetch.cc b/src/nix/prefetch.cc index 143935572..6bbf2578e 100644 --- a/src/nix/prefetch.cc +++ b/src/nix/prefetch.cc @@ -57,7 +57,9 @@ std::tuple prefetchFile( bool unpack, bool executable) { - auto ingestionMethod = unpack || executable ? FileIngestionMethod::NixArchive : FileIngestionMethod::Flat; + ContentAddressMethod method = unpack || executable + ? ContentAddressMethod::Raw::NixArchive + : ContentAddressMethod::Raw::Flat; /* Figure out a name in the Nix store. */ if (!name) { @@ -73,11 +75,10 @@ std::tuple prefetchFile( the store. */ if (expectedHash) { hashAlgo = expectedHash->algo; - storePath = store->makeFixedOutputPath(*name, FixedOutputInfo { - .method = ingestionMethod, - .hash = *expectedHash, - .references = {}, - }); + storePath = store->makeFixedOutputPathFromCA(*name, ContentAddressWithReferences::fromParts( + method, + *expectedHash, + {})); if (store->isValidPath(*storePath)) hash = expectedHash; else @@ -128,7 +129,7 @@ std::tuple prefetchFile( auto info = store->addToStoreSlow( *name, PosixSourceAccessor::createAtRoot(tmpFile), - ingestionMethod, hashAlgo, {}, expectedHash); + method, hashAlgo, {}, expectedHash); storePath = info.path; assert(info.ca); hash = info.ca->hash; diff --git a/src/perl/lib/Nix/Store.xs b/src/perl/lib/Nix/Store.xs index e8e209d97..acce25f3a 100644 --- a/src/perl/lib/Nix/Store.xs +++ b/src/perl/lib/Nix/Store.xs @@ -335,7 +335,7 @@ SV * StoreWrapper::addToStore(char * srcPath, int recursive, char * algo) PPCODE: try { - auto method = recursive ? FileIngestionMethod::NixArchive : FileIngestionMethod::Flat; + auto method = recursive ? ContentAddressMethod::Raw::NixArchive : ContentAddressMethod::Raw::Flat; auto path = THIS->store->addToStore( std::string(baseNameOf(srcPath)), PosixSourceAccessor::createAtRoot(srcPath), diff --git a/tests/unit/libstore/common-protocol.cc b/tests/unit/libstore/common-protocol.cc index 2c629b601..c8f6dd002 100644 --- a/tests/unit/libstore/common-protocol.cc +++ b/tests/unit/libstore/common-protocol.cc @@ -83,15 +83,15 @@ CHARACTERIZATION_TEST( "content-address", (std::tuple { ContentAddress { - .method = TextIngestionMethod {}, + .method = ContentAddressMethod::Raw::Text, .hash = hashString(HashAlgorithm::SHA256, "Derive(...)"), }, ContentAddress { - .method = FileIngestionMethod::Flat, + .method = ContentAddressMethod::Raw::Flat, .hash = hashString(HashAlgorithm::SHA1, "blob blob..."), }, ContentAddress { - .method = FileIngestionMethod::NixArchive, + .method = ContentAddressMethod::Raw::NixArchive, .hash = hashString(HashAlgorithm::SHA256, "(...)"), }, })) @@ -178,7 +178,7 @@ CHARACTERIZATION_TEST( std::nullopt, std::optional { ContentAddress { - .method = FileIngestionMethod::Flat, + .method = ContentAddressMethod::Raw::Flat, .hash = hashString(HashAlgorithm::SHA1, "blob blob..."), }, }, diff --git a/tests/unit/libstore/content-address.cc b/tests/unit/libstore/content-address.cc index f0806bd9a..72eb84fec 100644 --- a/tests/unit/libstore/content-address.cc +++ b/tests/unit/libstore/content-address.cc @@ -9,11 +9,11 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(ContentAddressMethod, testRoundTripPrintParse_1) { - for (const ContentAddressMethod & cam : { - ContentAddressMethod { TextIngestionMethod {} }, - ContentAddressMethod { FileIngestionMethod::Flat }, - ContentAddressMethod { FileIngestionMethod::NixArchive }, - ContentAddressMethod { FileIngestionMethod::Git }, + for (ContentAddressMethod cam : { + ContentAddressMethod::Raw::Text, + ContentAddressMethod::Raw::Flat, + ContentAddressMethod::Raw::NixArchive, + ContentAddressMethod::Raw::Git, }) { EXPECT_EQ(ContentAddressMethod::parse(cam.render()), cam); } diff --git a/tests/unit/libstore/derivation.cc b/tests/unit/libstore/derivation.cc index 210813137..71979f885 100644 --- a/tests/unit/libstore/derivation.cc +++ b/tests/unit/libstore/derivation.cc @@ -108,7 +108,7 @@ TEST_JSON(DerivationTest, inputAddressed, TEST_JSON(DerivationTest, caFixedFlat, (DerivationOutput::CAFixed { .ca = { - .method = FileIngestionMethod::Flat, + .method = ContentAddressMethod::Raw::Flat, .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), }, }), @@ -117,7 +117,7 @@ TEST_JSON(DerivationTest, caFixedFlat, TEST_JSON(DerivationTest, caFixedNAR, (DerivationOutput::CAFixed { .ca = { - .method = FileIngestionMethod::NixArchive, + .method = ContentAddressMethod::Raw::NixArchive, .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), }, }), @@ -126,6 +126,7 @@ TEST_JSON(DerivationTest, caFixedNAR, TEST_JSON(DynDerivationTest, caFixedText, (DerivationOutput::CAFixed { .ca = { + .method = ContentAddressMethod::Raw::Text, .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), }, }), @@ -133,7 +134,7 @@ TEST_JSON(DynDerivationTest, caFixedText, TEST_JSON(CaDerivationTest, caFloating, (DerivationOutput::CAFloating { - .method = FileIngestionMethod::NixArchive, + .method = ContentAddressMethod::Raw::NixArchive, .hashAlgo = HashAlgorithm::SHA256, }), "drv-name", "output-name") @@ -144,7 +145,7 @@ TEST_JSON(DerivationTest, deferred, TEST_JSON(ImpureDerivationTest, impure, (DerivationOutput::Impure { - .method = FileIngestionMethod::NixArchive, + .method = ContentAddressMethod::Raw::NixArchive, .hashAlgo = HashAlgorithm::SHA256, }), "drv-name", "output-name") diff --git a/tests/unit/libstore/serve-protocol.cc b/tests/unit/libstore/serve-protocol.cc index 17d7153bb..2505c5a9a 100644 --- a/tests/unit/libstore/serve-protocol.cc +++ b/tests/unit/libstore/serve-protocol.cc @@ -55,15 +55,15 @@ VERSIONED_CHARACTERIZATION_TEST( defaultVersion, (std::tuple { ContentAddress { - .method = TextIngestionMethod {}, + .method = ContentAddressMethod::Raw::Text, .hash = hashString(HashAlgorithm::SHA256, "Derive(...)"), }, ContentAddress { - .method = FileIngestionMethod::Flat, + .method = ContentAddressMethod::Raw::Flat, .hash = hashString(HashAlgorithm::SHA1, "blob blob..."), }, ContentAddress { - .method = FileIngestionMethod::NixArchive, + .method = ContentAddressMethod::Raw::NixArchive, .hash = hashString(HashAlgorithm::SHA256, "(...)"), }, })) @@ -398,7 +398,7 @@ VERSIONED_CHARACTERIZATION_TEST( std::nullopt, std::optional { ContentAddress { - .method = FileIngestionMethod::Flat, + .method = ContentAddressMethod::Raw::Flat, .hash = hashString(HashAlgorithm::SHA1, "blob blob..."), }, }, diff --git a/tests/unit/libstore/worker-protocol.cc b/tests/unit/libstore/worker-protocol.cc index 8d81717c9..c15120010 100644 --- a/tests/unit/libstore/worker-protocol.cc +++ b/tests/unit/libstore/worker-protocol.cc @@ -56,15 +56,15 @@ VERSIONED_CHARACTERIZATION_TEST( defaultVersion, (std::tuple { ContentAddress { - .method = TextIngestionMethod {}, + .method = ContentAddressMethod::Raw::Text, .hash = hashString(HashAlgorithm::SHA256, "Derive(...)"), }, ContentAddress { - .method = FileIngestionMethod::Flat, + .method = ContentAddressMethod::Raw::Flat, .hash = hashString(HashAlgorithm::SHA1, "blob blob..."), }, ContentAddress { - .method = FileIngestionMethod::NixArchive, + .method = ContentAddressMethod::Raw::NixArchive, .hash = hashString(HashAlgorithm::SHA256, "(...)"), }, })) @@ -598,7 +598,7 @@ VERSIONED_CHARACTERIZATION_TEST( std::nullopt, std::optional { ContentAddress { - .method = FileIngestionMethod::Flat, + .method = ContentAddressMethod::Raw::Flat, .hash = hashString(HashAlgorithm::SHA1, "blob blob..."), }, },