Skip to content

Commit

Permalink
Merge pull request #9839 from obsidiansystems/more-machine-cleanup
Browse files Browse the repository at this point in the history 
Create `StoreReference` and use it in `Machine`
  • Loading branch information
@Ericson2314
Ericson2314 authored May 22, 2024
2 parents 859e55d + b3ebcc5 commit 4a19f4a
Show file tree
Hide file tree
Showing 26 changed files with 589 additions and 197 deletions.
4 changes: 3 additions & 1 deletion .clang-format
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ SpaceAfterCStyleCast: true
SpaceAfterTemplateKeyword: false
AccessModifierOffset: -4
AlignAfterOpenBracket: AlwaysBreak
AlignEscapedNewlines: DontAlign
AlignEscapedNewlines: Left
ColumnLimit: 120
BreakStringLiterals: false
BitFieldColonSpacing: None
Expand All @@ -30,3 +30,5 @@ BreakBeforeBinaryOperators: NonAssignment
AlwaysBreakBeforeMultilineStrings: true
IndentPPDirectives: AfterHash
PPIndentWidth: 2
BinPackArguments: false
BinPackParameters: false
66 changes: 65 additions & 1 deletion maintainers/flake-module.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
excludes = [
# We don't want to format test data
# ''tests/(?!nixos/).*\.nix''
''^tests/.*''
''^tests/functional/.*$''
''^tests/unit/[^/]*/data/.*$''

# Don't format vendored code
''^src/toml11/.*''
Expand Down Expand Up @@ -426,6 +427,69 @@
''^src/nix/upgrade-nix\.cc$''
''^src/nix/verify\.cc$''
''^src/nix/why-depends\.cc$''

''^tests/nixos/ca-fd-leak/sender\.c''
''^tests/nixos/ca-fd-leak/smuggler\.c''
''^tests/unit/libexpr-support/tests/libexpr\.hh''
''^tests/unit/libexpr-support/tests/value/context\.cc''
''^tests/unit/libexpr-support/tests/value/context\.hh''
''^tests/unit/libexpr/derived-path\.cc''
''^tests/unit/libexpr/error_traces\.cc''
''^tests/unit/libexpr/eval\.cc''
''^tests/unit/libexpr/flake/flakeref\.cc''
''^tests/unit/libexpr/flake/url-name\.cc''
''^tests/unit/libexpr/json\.cc''
''^tests/unit/libexpr/main\.cc''
''^tests/unit/libexpr/primops\.cc''
''^tests/unit/libexpr/search-path\.cc''
''^tests/unit/libexpr/trivial\.cc''
''^tests/unit/libexpr/value/context\.cc''
''^tests/unit/libexpr/value/print\.cc''
''^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''
''^tests/unit/libstore-support/tests/path\.cc''
''^tests/unit/libstore-support/tests/path\.hh''
''^tests/unit/libstore-support/tests/protocol\.hh''
''^tests/unit/libstore/common-protocol\.cc''
''^tests/unit/libstore/content-address\.cc''
''^tests/unit/libstore/derivation\.cc''
''^tests/unit/libstore/derived-path\.cc''
''^tests/unit/libstore/downstream-placeholder\.cc''
''^tests/unit/libstore/machines\.cc''
''^tests/unit/libstore/nar-info-disk-cache\.cc''
''^tests/unit/libstore/nar-info\.cc''
''^tests/unit/libstore/outputs-spec\.cc''
''^tests/unit/libstore/path-info\.cc''
''^tests/unit/libstore/path\.cc''
''^tests/unit/libstore/serve-protocol\.cc''
''^tests/unit/libstore/worker-protocol\.cc''
''^tests/unit/libutil-support/tests/characterization\.hh''
''^tests/unit/libutil-support/tests/hash\.cc''
''^tests/unit/libutil-support/tests/hash\.hh''
''^tests/unit/libutil/args\.cc''
''^tests/unit/libutil/canon-path\.cc''
''^tests/unit/libutil/chunked-vector\.cc''
''^tests/unit/libutil/closure\.cc''
''^tests/unit/libutil/compression\.cc''
''^tests/unit/libutil/config\.cc''
''^tests/unit/libutil/file-content-address\.cc''
''^tests/unit/libutil/git\.cc''
''^tests/unit/libutil/hash\.cc''
''^tests/unit/libutil/hilite\.cc''
''^tests/unit/libutil/json-utils\.cc''
''^tests/unit/libutil/logging\.cc''
''^tests/unit/libutil/lru-cache\.cc''
''^tests/unit/libutil/pool\.cc''
''^tests/unit/libutil/references\.cc''
''^tests/unit/libutil/suggestions\.cc''
''^tests/unit/libutil/tests\.cc''
''^tests/unit/libutil/url\.cc''
''^tests/unit/libutil/xml-writer\.cc''
];
};

Expand Down
22 changes: 11 additions & 11 deletions src/build-remote/build-remote.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ static std::string currentLoad;

static AutoCloseFD openSlotLock(const Machine & m, uint64_t slot)
{
return openLockFile(fmt("%s/%s-%d", currentLoad, escapeUri(m.storeUri), slot), true);
return openLockFile(fmt("%s/%s-%d", currentLoad, escapeUri(m.storeUri.render()), slot), true);
}

static bool allSupportedLocally(Store & store, const std::set<std::string>& requiredFeatures) {
Expand Down Expand Up @@ -99,7 +99,7 @@ static int main_build_remote(int argc, char * * argv)
}

std::optional<StorePath> drvPath;
std::string storeUri;
StoreReference storeUri;

while (true) {

Expand Down Expand Up @@ -135,7 +135,7 @@ static int main_build_remote(int argc, char * * argv)
Machine * bestMachine = nullptr;
uint64_t bestLoad = 0;
for (auto & m : machines) {
debug("considering building on remote machine '%s'", m.storeUri);
debug("considering building on remote machine '%s'", m.storeUri.render());

if (m.enabled &&
m.systemSupported(neededSystem) &&
Expand Down Expand Up @@ -233,7 +233,7 @@ static int main_build_remote(int argc, char * * argv)

try {

Activity act(*logger, lvlTalkative, actUnknown, fmt("connecting to '%s'", bestMachine->storeUri));
Activity act(*logger, lvlTalkative, actUnknown, fmt("connecting to '%s'", bestMachine->storeUri.render()));

sshStore = bestMachine->openStore();
sshStore->connect();
Expand All @@ -242,7 +242,7 @@ static int main_build_remote(int argc, char * * argv)
} catch (std::exception & e) {
auto msg = chomp(drainFD(5, false));
printError("cannot build on '%s': %s%s",
bestMachine->storeUri, e.what(),
bestMachine->storeUri.render(), e.what(),
msg.empty() ? "" : ": " + msg);
bestMachine->enabled = false;
continue;
Expand All @@ -257,15 +257,15 @@ static int main_build_remote(int argc, char * * argv)

assert(sshStore);

std::cerr << "# accept\n" << storeUri << "\n";
std::cerr << "# accept\n" << storeUri.render() << "\n";

auto inputs = readStrings<PathSet>(source);
auto wantedOutputs = readStrings<StringSet>(source);

AutoCloseFD uploadLock = openLockFile(currentLoad + "/" + escapeUri(storeUri) + ".upload-lock", true);
AutoCloseFD uploadLock = openLockFile(currentLoad + "/" + escapeUri(storeUri.render()) + ".upload-lock", true);

{
Activity act(*logger, lvlTalkative, actUnknown, fmt("waiting for the upload lock to '%s'", storeUri));
Activity act(*logger, lvlTalkative, actUnknown, fmt("waiting for the upload lock to '%s'", storeUri.render()));

auto old = signal(SIGALRM, handleAlarm);
alarm(15 * 60);
Expand All @@ -278,7 +278,7 @@ static int main_build_remote(int argc, char * * argv)
auto substitute = settings.buildersUseSubstitutes ? Substitute : NoSubstitute;

{
Activity act(*logger, lvlTalkative, actUnknown, fmt("copying dependencies to '%s'", storeUri));
Activity act(*logger, lvlTalkative, actUnknown, fmt("copying dependencies to '%s'", storeUri.render()));
copyPaths(*store, *sshStore, store->parseStorePathSet(inputs), NoRepair, NoCheckSigs, substitute);
}

Expand Down Expand Up @@ -316,7 +316,7 @@ static int main_build_remote(int argc, char * * argv)
optResult = sshStore->buildDerivation(*drvPath, (const BasicDerivation &) drv);
auto & result = *optResult;
if (!result.success())
throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri, result.errorMsg);
throw Error("build of '%s' on '%s' failed: %s", store->printStorePath(*drvPath), storeUri.render(), result.errorMsg);
} else {
copyClosure(*store, *sshStore, StorePathSet {*drvPath}, NoRepair, NoCheckSigs, substitute);
auto res = sshStore->buildPathsWithResults({
Expand Down Expand Up @@ -359,7 +359,7 @@ static int main_build_remote(int argc, char * * argv)
}

if (!missingPaths.empty()) {
Activity act(*logger, lvlTalkative, actUnknown, fmt("copying outputs from '%s'", storeUri));
Activity act(*logger, lvlTalkative, actUnknown, fmt("copying outputs from '%s'", storeUri.render()));
if (auto localStore = store.dynamic_pointer_cast<LocalStore>())
for (auto & path : missingPaths)
localStore->locksHeld.insert(store->printStorePath(path)); /* FIXME: ugly */
Expand Down
5 changes: 4 additions & 1 deletion src/libcmd/network-proxy.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ static StringSet getExcludingNoProxyVariables()
static const StringSet excludeVariables{"no_proxy", "NO_PROXY"};
StringSet variables;
std::set_difference(
networkProxyVariables.begin(), networkProxyVariables.end(), excludeVariables.begin(), excludeVariables.end(),
networkProxyVariables.begin(),
networkProxyVariables.end(),
excludeVariables.begin(),
excludeVariables.end(),
std::inserter(variables, variables.begin()));
return variables;
}
Expand Down
35 changes: 22 additions & 13 deletions src/libstore/machines.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@

namespace nix {

Machine::Machine(decltype(storeUri) storeUri,
Machine::Machine(
const std::string & storeUri,
decltype(systemTypes) systemTypes,
decltype(sshKey) sshKey,
decltype(maxJobs) maxJobs,
decltype(speedFactor) speedFactor,
decltype(supportedFeatures) supportedFeatures,
decltype(mandatoryFeatures) mandatoryFeatures,
decltype(sshPublicHostKey) sshPublicHostKey) :
storeUri(
storeUri(StoreReference::parse(
// Backwards compatibility: if the URI is schemeless, is not a path,
// and is not one of the special store connection words, prepend
// ssh://.
Expand All @@ -28,7 +29,7 @@ Machine::Machine(decltype(storeUri) storeUri,
|| hasPrefix(storeUri, "local?")
|| hasPrefix(storeUri, "?")
? storeUri
: "ssh://" + storeUri),
: "ssh://" + storeUri)),
systemTypes(systemTypes),
sshKey(sshKey),
maxJobs(maxJobs),
Expand Down Expand Up @@ -63,23 +64,26 @@ bool Machine::mandatoryMet(const std::set<std::string> & features) const
});
}

ref<Store> Machine::openStore() const
StoreReference Machine::completeStoreReference() const
{
Store::Params storeParams;
if (hasPrefix(storeUri, "ssh://")) {
storeParams["max-connections"] = "1";
storeParams["log-fd"] = "4";
auto storeUri = this->storeUri;

auto * generic = std::get_if<StoreReference::Specified>(&storeUri.variant);

if (generic && generic->scheme == "ssh") {
storeUri.params["max-connections"] = "1";
storeUri.params["log-fd"] = "4";
}

if (hasPrefix(storeUri, "ssh://") || hasPrefix(storeUri, "ssh-ng://")) {
if (generic && (generic->scheme == "ssh" || generic->scheme == "ssh-ng")) {
if (sshKey != "")
storeParams["ssh-key"] = sshKey;
storeUri.params["ssh-key"] = sshKey;
if (sshPublicHostKey != "")
storeParams["base64-ssh-public-host-key"] = sshPublicHostKey;
storeUri.params["base64-ssh-public-host-key"] = sshPublicHostKey;
}

{
auto & fs = storeParams["system-features"];
auto & fs = storeUri.params["system-features"];
auto append = [&](auto feats) {
for (auto & f : feats) {
if (fs.size() > 0) fs += ' ';
Expand All @@ -90,7 +94,12 @@ ref<Store> Machine::openStore() const
append(mandatoryFeatures);
}

return nix::openStore(storeUri, storeParams);
return storeUri;
}

ref<Store> Machine::openStore() const
{
return nix::openStore(completeStoreReference());
}

static std::vector<std::string> expandBuilderLines(const std::string & builders)
Expand Down
21 changes: 19 additions & 2 deletions src/libstore/machines.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@
///@file

#include "types.hh"
#include "store-reference.hh"

namespace nix {

class Store;

struct Machine {

const std::string storeUri;
const StoreReference storeUri;
const std::set<std::string> systemTypes;
const std::string sshKey;
const unsigned int maxJobs;
Expand All @@ -36,7 +37,8 @@ struct Machine {
*/
bool mandatoryMet(const std::set<std::string> & features) const;

Machine(decltype(storeUri) storeUri,
Machine(
const std::string & storeUri,
decltype(systemTypes) systemTypes,
decltype(sshKey) sshKey,
decltype(maxJobs) maxJobs,
Expand All @@ -45,6 +47,21 @@ struct Machine {
decltype(mandatoryFeatures) mandatoryFeatures,
decltype(sshPublicHostKey) sshPublicHostKey);

/**
* Elaborate `storeUri` into a complete store reference,
* incorporating information from the other fields of the `Machine`
* as applicable.
*/
StoreReference completeStoreReference() const;

/**
* Open a `Store` for this machine.
*
* Just a simple function composition:
* ```c++
* nix::openStore(completeStoreReference())
* ```
*/
ref<Store> openStore() const;
};

Expand Down
Loading

0 comments on commit 4a19f4a

Please sign in to comment.