From cbc43442977a6fbbd046a98fcb09362a60323b5d Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 12 Aug 2020 03:47:36 +0000 Subject: Trustless remote building Co-authored-by: Matthew Bauer --- src/build-remote/build-remote.cc | 22 +++++++++++++++------- src/libstore/daemon.cc | 2 ++ src/libstore/store-api.cc | 18 +++++++++++++++++- src/libstore/store-api.hh | 4 +++- src/nix-daemon/nix-daemon.cc | 19 +++++++++++++++++-- 5 files changed, 54 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index ce5127113..8ae1cabfe 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -247,6 +247,9 @@ static int _main(int argc, char * * argv) connected: close(5); + assert(sshStore); + auto sshStore2 = ref(sshStore); + std::cerr << "# accept\n" << storeUri << "\n"; auto inputs = readStrings(source); @@ -269,18 +272,23 @@ connected: { Activity act(*logger, lvlTalkative, actUnknown, fmt("copying dependencies to '%s'", storeUri)); - copyPaths(store, ref(sshStore), store->parseStorePathSet(inputs), NoRepair, NoCheckSigs, substitute); + copyPaths(store, sshStore2, store->parseStorePathSet(inputs), NoRepair, NoCheckSigs, substitute); } uploadLock = -1; - auto drv = store->readDerivation(*drvPath); - drv.inputSrcs = store->parseStorePathSet(inputs); + BasicDerivation drv = store->readDerivation(*drvPath); - auto result = sshStore->buildDerivation(*drvPath, drv); + if (sshStore2->isTrusting || derivationIsCA(drv.type())) { + drv.inputSrcs = store->parseStorePathSet(inputs); + auto result = sshStore2->buildDerivation(*drvPath, drv); + if (!result.success()) + throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri, result.errorMsg); + } else { + copyPaths(store, sshStore2, {*drvPath}, NoRepair, NoCheckSigs, substitute); + sshStore2->buildPaths({{*drvPath}}); + } - if (!result.success()) - throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri, result.errorMsg); StorePathSet missing; for (auto & path : outputs) @@ -290,7 +298,7 @@ connected: Activity act(*logger, lvlTalkative, actUnknown, fmt("copying outputs from '%s'", storeUri)); for (auto & i : missing) store->locksHeld.insert(store->printStorePath(i)); /* FIXME: ugly */ - copyPaths(ref(sshStore), store, missing, NoRepair, NoCheckSigs, NoSubstitute); + copyPaths(sshStore2, store, missing, NoRepair, NoCheckSigs, NoSubstitute); } return 0; diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 45e81c8da..ffa7e0dcd 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -914,6 +914,8 @@ void processConnection( opCount++; + debug("performing daemon worker op: %d", op); + try { performOp(tunnelLogger, store, trusted, recursive, clientVersion, from, to, op); } catch (Error & e) { diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 3d07e2d38..360820b1b 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -834,7 +834,23 @@ std::map copyPaths(ref srcStore, ref dstStor MaintainCount mc(nrRunning); showProgress(); try { - copyStorePath(srcStore, dstStore, storePath, repair, checkSigs); + if (dstStore->isTrusting || info->ca) { + copyStorePath(srcStore, dstStore, storePath, repair, checkSigs); + } else if (info->deriver && dstStore->storeDir == srcStore->storeDir) { + auto drvPath = *info->deriver; + auto outputMap = srcStore->queryDerivationOutputMap(drvPath); + auto p = std::find_if(outputMap.begin(), outputMap.end(), [&](auto & i) { + return i.second == storePath; + }); + // drv file is always CA + copyStorePath(srcStore, dstStore, drvPath, repair, checkSigs); + dstStore->buildPaths({{ + drvPath, + p != outputMap.end() ? StringSet { p->first } : StringSet {}, + }}); + } else { + dstStore->ensurePath(storePath); + } } catch (Error &e) { nrFailed++; if (!settings.keepGoing) diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 1e940e6a8..4ba51a9e5 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -158,7 +158,9 @@ public: const Setting pathInfoCacheSize{this, 65536, "path-info-cache-size", "size of the in-memory store path information cache"}; - const Setting isTrusted{this, false, "trusted", "whether paths from this store can be used as substitutes even when they lack trusted signatures"}; + const Setting isTrusted{this, false, "trusted", "whether paths from this store can be used as substitutes even when they lack trusted signatures. Compare \"trusting\""}; + + Setting isTrusting{this, true, "trusting", "whether (we think) paths can be added to this store even when they lack trusted signatures. Compare \"trusted\""}; Setting priority{this, 0, "priority", "priority of this substituter (lower value means higher priority)"}; diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index cfa634a44..ce963acdc 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -268,6 +268,7 @@ static int _main(int argc, char * * argv) { { auto stdio = false; + std::optional isTrustedOpt; parseCmdLine(argc, argv, [&](Strings::iterator & arg, const Strings::iterator & end) { if (*arg == "--daemon") @@ -278,14 +279,26 @@ static int _main(int argc, char * * argv) printVersion("nix-daemon"); else if (*arg == "--stdio") stdio = true; - else return false; + else if (*arg == "--trust") { + settings.requireExperimentalFeature("nix-testing"); + isTrustedOpt = Trusted; + } else if (*arg == "--no-trust") { + settings.requireExperimentalFeature("nix-testing"); + isTrustedOpt = NotTrusted; + } else return false; return true; }); initPlugins(); + auto ensureNoTrustedFlag = [&]() { + if (isTrustedOpt) + throw Error("--trust and --no-trust flags are only for use with --stdio when this nix-daemon process is not proxying another"); + }; + if (stdio) { if (getStoreType() == tDaemon) { + ensureNoTrustedFlag(); // Forward on this connection to the real daemon auto socketPath = settings.nixDaemonSocketFile; auto s = socket(PF_UNIX, SOCK_STREAM, 0); @@ -335,9 +348,11 @@ static int _main(int argc, char * * argv) /* Auth hook is empty because in this mode we blindly trust the standard streams. Limitting access to thoses is explicitly not `nix-daemon`'s responsibility. */ - processConnection(openUncachedStore(), from, to, Trusted, NotRecursive, [&](Store & _){}); + auto isTrusted = isTrustedOpt.value_or(Trusted); + processConnection(openUncachedStore(), from, to, isTrusted, NotRecursive, [&](Store & _){}); } } else { + ensureNoTrustedFlag(); daemonLoop(argv); } -- cgit v1.2.3 From 6c7b81047f8ead0bb2f8dd588dfcb5f50d1554a9 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 9 Sep 2020 17:35:54 +0000 Subject: Make sure srcStore has path before coppying --- src/libstore/store-api.cc | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index aed43543f..8075b3ff9 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -847,6 +847,7 @@ std::map copyPaths(ref srcStore, ref dstStor return i.second == storePath; }); // drv file is always CA + srcStore->ensurePath(drvPath); copyStorePath(srcStore, dstStore, drvPath, repair, checkSigs); dstStore->buildPaths({{ drvPath, -- cgit v1.2.3 From 141cb9a706f73725fd4f8d62569f45bbbf9b6c3b Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 22 Jan 2021 17:56:28 +0000 Subject: Make regular `copyPaths` only copy again The is new function parameter so just the build hook can opt into the remote-side building. --- src/build-remote/build-remote.cc | 7 ++++-- src/libstore/store-api.cc | 53 ++++++++++++++++++++++++++-------------- src/libstore/store-api.hh | 20 +++++++++++++-- 3 files changed, 57 insertions(+), 23 deletions(-) (limited to 'src') diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 402754c3c..ef943ee11 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -270,9 +270,11 @@ connected: auto substitute = settings.buildersUseSubstitutes ? Substitute : NoSubstitute; + auto copyStorePathImpl = sshStore2->isTrusting ? copyStorePathAdapter : copyOrBuildStorePath; + { Activity act(*logger, lvlTalkative, actUnknown, fmt("copying dependencies to '%s'", storeUri)); - copyPaths(store, sshStore2, store->parseStorePathSet(inputs), NoRepair, NoCheckSigs, substitute); + copyPaths(store, sshStore2, store->parseStorePathSet(inputs), NoRepair, NoCheckSigs, substitute, copyStorePathImpl); } uploadLock = -1; @@ -285,7 +287,7 @@ connected: if (!result.success()) throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri, result.errorMsg); } else { - copyPaths(store, sshStore2, {*drvPath}, NoRepair, NoCheckSigs, substitute); + copyPaths(store, sshStore2, {*drvPath}, NoRepair, NoCheckSigs, substitute, copyStorePathImpl); sshStore2->buildPaths({{*drvPath}}); } @@ -298,6 +300,7 @@ connected: Activity act(*logger, lvlTalkative, actUnknown, fmt("copying outputs from '%s'", storeUri)); for (auto & i : missing) store->locksHeld.insert(store->printStorePath(i)); /* FIXME: ugly */ + /* No `copyStorePathImpl` because we always trust ourselves. */ copyPaths(sshStore2, store, missing, NoRepair, NoCheckSigs, NoSubstitute); } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index cce95e4cf..ad773bb0f 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -819,8 +819,40 @@ void copyStorePath(ref srcStore, ref dstStore, } +void copyStorePathAdapter(ref srcStore, ref dstStore, + const ValidPathInfo & info, RepairFlag repair, CheckSigsFlag checkSigs) +{ + copyStorePath(srcStore, dstStore, info.path, repair, checkSigs); +} + +void copyOrBuildStorePath(ref srcStore, ref dstStore, + const ValidPathInfo & info, RepairFlag repair, CheckSigsFlag checkSigs) +{ + auto storePath = info.path; + if (dstStore->isTrusting || info.ca) { + copyStorePath(srcStore, dstStore, storePath, repair, checkSigs); + } else if (info.deriver && dstStore->storeDir == srcStore->storeDir) { + auto drvPath = *info.deriver; + auto outputMap = srcStore->queryDerivationOutputMap(drvPath); + auto p = std::find_if(outputMap.begin(), outputMap.end(), [&](auto & i) { + return i.second == storePath; + }); + // drv file is always CA + srcStore->ensurePath(drvPath); + copyStorePath(srcStore, dstStore, drvPath, repair, checkSigs); + dstStore->buildPaths({{ + drvPath, + p != outputMap.end() ? StringSet { p->first } : StringSet {}, + }}); + } else { + dstStore->ensurePath(storePath); + } +} + + std::map copyPaths(ref srcStore, ref dstStore, const StorePathSet & storePaths, - RepairFlag repair, CheckSigsFlag checkSigs, SubstituteFlag substitute) + RepairFlag repair, CheckSigsFlag checkSigs, SubstituteFlag substitute, + std::function, ref, const ValidPathInfo &, RepairFlag, CheckSigsFlag)> copyStorePathImpl) { auto valid = dstStore->queryValidPaths(storePaths, substitute); @@ -893,24 +925,7 @@ std::map copyPaths(ref srcStore, ref dstStor MaintainCount mc(nrRunning); showProgress(); try { - if (dstStore->isTrusting || info->ca) { - copyStorePath(srcStore, dstStore, storePath, repair, checkSigs); - } else if (info->deriver && dstStore->storeDir == srcStore->storeDir) { - auto drvPath = *info->deriver; - auto outputMap = srcStore->queryDerivationOutputMap(drvPath); - auto p = std::find_if(outputMap.begin(), outputMap.end(), [&](auto & i) { - return i.second == storePath; - }); - // drv file is always CA - srcStore->ensurePath(drvPath); - copyStorePath(srcStore, dstStore, drvPath, repair, checkSigs); - dstStore->buildPaths({{ - drvPath, - p != outputMap.end() ? StringSet { p->first } : StringSet {}, - }}); - } else { - dstStore->ensurePath(storePath); - } + copyStorePathImpl(srcStore, dstStore, *info, repair, checkSigs); } catch (Error &e) { nrFailed++; if (!settings.keepGoing) diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 696a2d3fd..5597ebb22 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -738,17 +738,33 @@ void copyStorePath(ref srcStore, ref dstStore, const StorePath & storePath, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs); +/* copyStorePath wrapped to be used with `copyPaths`. */ +void copyStorePathAdapter(ref srcStore, ref dstStore, + const ValidPathInfo & info, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs); + +/* The more liberal alternative to `copyStorePathAdapter`, useful for remote + stores that do not trust us. */ +void copyOrBuildStorePath(ref srcStore, ref dstStore, + const ValidPathInfo & info, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs); + /* Copy store paths from one store to another. The paths may be copied in parallel. They are copied in a topologically sorted order (i.e. if A is a reference of B, then A is copied before B), but the set of store paths is not automatically closed; use copyClosure() for that. Returns a map of what each path was copied to the dstStore - as. */ + as. + + The `copyStorePathImpl` parameter allows doing something other than just + copying. For example, this is used with the build hook to allow the other + side to build dependencies we don't have permission to copy. This behavior + isn't just the default that way `nix copy` etc. still can be relied upon to + not build anything. */ std::map copyPaths(ref srcStore, ref dstStore, const StorePathSet & storePaths, RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs, - SubstituteFlag substitute = NoSubstitute); + SubstituteFlag substitute = NoSubstitute, + std::function, ref, const ValidPathInfo &, RepairFlag, CheckSigsFlag)> copyStorePathImpl = copyStorePathAdapter); /* Copy the closure of the specified paths from one store to another. */ -- cgit v1.2.3 From 6e1e15ffec69de8a3954100389f1d41494f8da8d Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 7 Apr 2023 11:13:23 -0400 Subject: Fix it! --- src/build-remote/build-remote.cc | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 649ad3e7c..3d4dbc3d6 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -290,14 +290,19 @@ connected: auto drv = store->readDerivation(*drvPath); std::optional optResult; - if (sshStore->isTrustedClient() || drv.type().isCA()) { - // Hijack the inputs paths of the derivation to include all the paths - // that come from the `inputDrvs` set. - // We don’t do that for the derivations whose `inputDrvs` is empty - // because + // If we don't know whether we are trusted (e.g. `ssh://` + // stores), we assume we are. This is neccessary for backwards + // compat. + if (std::optional trust = sshStore->isTrustedClient(); (!trust || *trust) || drv.type().isCA()) { + // Hijack the inputs paths of the derivation to include all + // the paths that come from the `inputDrvs` set. We don’t do + // that for the derivations whose `inputDrvs` is empty + // because: + // // 1. It’s not needed - // 2. Changing the `inputSrcs` set changes the associated output ids, - // which break CA derivations + // + // 2. Changing the `inputSrcs` set changes the associated + // output ids, which break CA derivations if (!drv.inputDrvs.empty()) drv.inputSrcs = store->parseStorePathSet(inputs); optResult = sshStore->buildDerivation(*drvPath, (const BasicDerivation &) drv); -- cgit v1.2.3 From e95db8f2b9aebbb4079805cb7ecfc751af41e0b4 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 17 Apr 2023 09:35:43 -0400 Subject: `nix-testing` -> `daemon-trust-override` And only enable in the tests that need it. This makes it less of a sledgehammer. --- src/libutil/experimental-features.cc | 11 ++++++----- src/libutil/experimental-features.hh | 2 +- src/nix/daemon.cc | 6 +++--- 3 files changed, 10 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index be5a2c088..bd1899662 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -190,12 +190,13 @@ constexpr std::array xpFeatureDetails = {{ )", }, { - .tag = Xp::NixTesting, - .name = "nix-testing", + .tag = Xp::DaemonTrustOverride, + .name = "daemon-trust-override", .description = R"( - A "permanent" experimental feature for extra features we just need - for testing. Not actually an "experiment" in the sense of being - prospective functionality for regular users. + Allow forcing trusting or not trusting clients with + `nix-daemon`. This is useful for testing, but possibly also + useful for various experiments with `nix-daemon --stdio` + networking. )", }, }}; diff --git a/src/libutil/experimental-features.hh b/src/libutil/experimental-features.hh index c41f73fa0..3c00bc4e5 100644 --- a/src/libutil/experimental-features.hh +++ b/src/libutil/experimental-features.hh @@ -28,7 +28,7 @@ enum struct ExperimentalFeature AutoAllocateUids, Cgroups, DiscardReferences, - NixTesting, + DaemonTrustOverride, }; /** diff --git a/src/nix/daemon.cc b/src/nix/daemon.cc index 4be93bb1c..35e8a5f87 100644 --- a/src/nix/daemon.cc +++ b/src/nix/daemon.cc @@ -472,13 +472,13 @@ static int main_nix_daemon(int argc, char * * argv) else if (*arg == "--stdio") stdio = true; else if (*arg == "--force-trusted") { - experimentalFeatureSettings.require(Xp::NixTesting); + experimentalFeatureSettings.require(Xp::DaemonTrustOverride); isTrustedOpt = Trusted; } else if (*arg == "--force-untrusted") { - experimentalFeatureSettings.require(Xp::NixTesting); + experimentalFeatureSettings.require(Xp::DaemonTrustOverride); isTrustedOpt = NotTrusted; } else if (*arg == "--default-trust") { - experimentalFeatureSettings.require(Xp::NixTesting); + experimentalFeatureSettings.require(Xp::DaemonTrustOverride); isTrustedOpt = std::nullopt; } else return false; return true; -- cgit v1.2.3 From 79ba0ba37ab35ac5ea94fa1db1fc46a5b7588ece Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 17 Apr 2023 09:56:32 -0400 Subject: Improve the build remote comment. --- src/build-remote/build-remote.cc | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 3d4dbc3d6..b0bc8a9ff 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -290,9 +290,30 @@ connected: auto drv = store->readDerivation(*drvPath); std::optional optResult; + + // Let's break this down + // + // ### Trust part + // + // ``` + // std::optional trust = sshStore->isTrustedClient(); (!trust || *trust) + // ``` + // // If we don't know whether we are trusted (e.g. `ssh://` - // stores), we assume we are. This is neccessary for backwards + // stores), we assume we are. This is necessary for backwards // compat. + // + // ### Content-addressing part + // + // ``` + // ...trustCond... || drv.type().isCA() + // ``` + // + // See the very large comment in `case wopBuildDerivation:` in + // `src/libstore/daemon.cc` that explains the trust model here. + // + // This condition mirrors that: that code enforces the "rules"; + // we do the best we can given those "rules". if (std::optional trust = sshStore->isTrustedClient(); (!trust || *trust) || drv.type().isCA()) { // Hijack the inputs paths of the derivation to include all // the paths that come from the `inputDrvs` set. We don’t do -- cgit v1.2.3 From 23ee2d79a90ace2fe012f32c37121f50afc35223 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 17 Apr 2023 09:59:14 -0400 Subject: Use `buildPathsWithResults` in build-remote.cc trustless path It handles failures more correctly; I am glad we have it now! --- src/build-remote/build-remote.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index b0bc8a9ff..403f0fb17 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -332,7 +332,10 @@ connected: throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri, result.errorMsg); } else { copyPaths(*store, *sshStore, StorePathSet {*drvPath}, NoRepair, NoCheckSigs, substitute); - sshStore->buildPaths({ DerivedPath::Built { *drvPath, OutputsSpec::All {} } }); + auto res = sshStore->buildPathsWithResults({ DerivedPath::Built { *drvPath, OutputsSpec::All {} } }); + // One path to build should mean one result back + assert(res.size() == 1); + optResult = std::move(res[0]); } -- cgit v1.2.3 From cd0d8e0bd50d7259057ac138a7227907f88b91d5 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 8 May 2023 09:57:05 -0400 Subject: Apply suggestions from code review Co-authored-by: Valentin Gagarin --- src/build-remote/build-remote.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 1467761c9..7366a7c27 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -312,9 +312,12 @@ connected: // See the very large comment in `case wopBuildDerivation:` in // `src/libstore/daemon.cc` that explains the trust model here. // - // This condition mirrors that: that code enforces the "rules"; + // This condition mirrors that: that code enforces the "rules" outlined there; // we do the best we can given those "rules". - if (std::optional trust = sshStore->isTrustedClient(); (!trust || *trust) || drv.type().isCA()) { + std::optional trusted = sshStore->isTrustedClient(); + // for backward compatibility (use existing comments here) + bool trustedOrLegacy = !trusted || *trusted; + if (trustedOrLegacy || drv.type().isCA()) { // Hijack the inputs paths of the derivation to include all // the paths that come from the `inputDrvs` set. We don’t do // that for the derivations whose `inputDrvs` is empty @@ -331,9 +334,9 @@ connected: if (!result.success()) throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri, result.errorMsg); } else { - copyPaths(*store, *sshStore, StorePathSet {*drvPath}, NoRepair, NoCheckSigs, substitute); + copyClosure(*store, *sshStore, StorePathSet {*drvPath}, NoRepair, NoCheckSigs, substitute); auto res = sshStore->buildPathsWithResults({ DerivedPath::Built { *drvPath, OutputsSpec::All {} } }); - // One path to build should mean one result back + // One path to build should produce exactly one build result assert(res.size() == 1); optResult = std::move(res[0]); } -- cgit v1.2.3 From df53a7d26868d7ce432e34e5d0c397886f7772f8 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 8 May 2023 10:08:01 -0400 Subject: Split comment, match with each variable --- src/build-remote/build-remote.cc | 23 +++++------------------ 1 file changed, 5 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 7366a7c27..323e04fdb 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -291,32 +291,19 @@ connected: std::optional optResult; - // Let's break this down - // - // ### Trust part - // - // ``` - // std::optional trust = sshStore->isTrustedClient(); (!trust || *trust) - // ``` - // // If we don't know whether we are trusted (e.g. `ssh://` // stores), we assume we are. This is necessary for backwards // compat. - // - // ### Content-addressing part - // - // ``` - // ...trustCond... || drv.type().isCA() - // ``` - // + bool trustedOrLegacy = ({ + std::optional trusted = sshStore->isTrustedClient(); + !trusted || *trusted; + }); + // See the very large comment in `case wopBuildDerivation:` in // `src/libstore/daemon.cc` that explains the trust model here. // // This condition mirrors that: that code enforces the "rules" outlined there; // we do the best we can given those "rules". - std::optional trusted = sshStore->isTrustedClient(); - // for backward compatibility (use existing comments here) - bool trustedOrLegacy = !trusted || *trusted; if (trustedOrLegacy || drv.type().isCA()) { // Hijack the inputs paths of the derivation to include all // the paths that come from the `inputDrvs` set. We don’t do -- cgit v1.2.3