From e989c83b44d7c4d8ffe2e5f4231e4861c5f7732f Mon Sep 17 00:00:00 2001 From: Alexey Novikov Date: Tue, 12 Oct 2021 16:18:44 +0400 Subject: Add error reporting to machine spec paser Currently machine specification (`/etc/nix/machine`) parser fails with a vague exception if the file had incorrect format. This commit adds verbose exceptions and unit-tests for the parser. --- src/libstore/machines.cc | 78 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 54 insertions(+), 24 deletions(-) (limited to 'src/libstore/machines.cc') diff --git a/src/libstore/machines.cc b/src/libstore/machines.cc index 9843ccf04..910e32a76 100644 --- a/src/libstore/machines.cc +++ b/src/libstore/machines.cc @@ -83,53 +83,83 @@ ref Machine::openStore() const { return nix::openStore(storeUri, storeParams); } -void parseMachines(const std::string & s, Machines & machines) -{ - for (auto line : tokenizeString>(s, "\n;")) { +static std::vector expandBuilderLines(const std::string& builders) { + std::vector result; + for (auto line : tokenizeString>(builders, "\n;")) { trim(line); line.erase(std::find(line.begin(), line.end(), '#'), line.end()); if (line.empty()) continue; if (line[0] == '@') { - auto file = trim(std::string(line, 1)); + const std::string path = trim(std::string(line, 1)); + std::string text; try { - parseMachines(readFile(file), machines); + text = readFile(path); } catch (const SysError & e) { if (e.errNo != ENOENT) throw; - debug("cannot find machines file '%s'", file); + debug("cannot find machines file '%s'", path); } + + const auto lines = expandBuilderLines(text); + result.insert(end(result), begin(lines), end(lines)); continue; } - auto tokens = tokenizeString>(line); - auto sz = tokens.size(); - if (sz < 1) - throw FormatError("bad machine specification '%s'", line); + result.emplace_back(line); + } + return result; +} - auto isSet = [&](size_t n) { - return tokens.size() > n && tokens[n] != "" && tokens[n] != "-"; - }; +static Machine parseBuilderLine(const std::string& line) { + const auto tokens = tokenizeString>(line); + + auto isSet = [&](size_t fieldIndex) { + return tokens.size() > fieldIndex && tokens[fieldIndex] != "" && tokens[fieldIndex] != "-"; + }; + + auto parseUnsignedIntField = [&](size_t fieldIndex) { + const auto result = string2Int(tokens[fieldIndex]); + if (!result) { + throw FormatError("bad machine specification: failed to convert column #%lu in a row: '%s' to 'unsigned int'", fieldIndex, line); + } + return result.value(); + }; + + auto ensureBase64 = [&](size_t fieldIndex) { + const auto& str = tokens[fieldIndex]; + try { + base64Decode(str); + } catch (const Error& e) { + throw FormatError("bad machine specification: a column #%lu in a row: '%s' is not valid base64 string: %s", fieldIndex, line, e.what()); + } + return str; + }; + + if (!isSet(0)) { + throw FormatError("bad machine specification: store URI was not found at the first column of a row: '%s'", line); + } - machines.emplace_back(tokens[0], + return {tokens[0], isSet(1) ? tokenizeString>(tokens[1], ",") : std::vector{settings.thisSystem}, isSet(2) ? tokens[2] : "", - isSet(3) ? std::stoull(tokens[3]) : 1LL, - isSet(4) ? std::stoull(tokens[4]) : 1LL, + isSet(3) ? parseUnsignedIntField(3) : 1U, + isSet(4) ? parseUnsignedIntField(4) : 1U, isSet(5) ? tokenizeString>(tokens[5], ",") : std::set{}, isSet(6) ? tokenizeString>(tokens[6], ",") : std::set{}, - isSet(7) ? tokens[7] : ""); - } + isSet(7) ? ensureBase64(7) : ""}; +} + +static Machines parseBuilderLines(const std::vector& builders) { + Machines result; + std::transform(builders.begin(), builders.end(), std::back_inserter(result), parseBuilderLine); + return result; } Machines getMachines() { - static auto machines = [&]() { - Machines machines; - parseMachines(settings.builders, machines); - return machines; - }(); - return machines; + const auto builderLines = expandBuilderLines(settings.builders); + return parseBuilderLines(builderLines); } } -- cgit v1.2.3 From e6795c43504b763a03b21dccd6814b8703af357e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 29 Oct 2021 14:45:13 +0200 Subject: Style --- src/libstore/machines.cc | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) (limited to 'src/libstore/machines.cc') diff --git a/src/libstore/machines.cc b/src/libstore/machines.cc index 910e32a76..b6270a81b 100644 --- a/src/libstore/machines.cc +++ b/src/libstore/machines.cc @@ -39,7 +39,8 @@ Machine::Machine(decltype(storeUri) storeUri, sshPublicHostKey(sshPublicHostKey) {} -bool Machine::allSupported(const std::set & features) const { +bool Machine::allSupported(const std::set & features) const +{ return std::all_of(features.begin(), features.end(), [&](const string & feature) { return supportedFeatures.count(feature) || @@ -47,14 +48,16 @@ bool Machine::allSupported(const std::set & features) const { }); } -bool Machine::mandatoryMet(const std::set & features) const { +bool Machine::mandatoryMet(const std::set & features) const +{ return std::all_of(mandatoryFeatures.begin(), mandatoryFeatures.end(), [&](const string & feature) { return features.count(feature); }); } -ref Machine::openStore() const { +ref Machine::openStore() const +{ Store::Params storeParams; if (hasPrefix(storeUri, "ssh://")) { storeParams["max-connections"] = "1"; @@ -83,7 +86,8 @@ ref Machine::openStore() const { return nix::openStore(storeUri, storeParams); } -static std::vector expandBuilderLines(const std::string& builders) { +static std::vector expandBuilderLines(const std::string & builders) +{ std::vector result; for (auto line : tokenizeString>(builders, "\n;")) { trim(line); @@ -111,7 +115,8 @@ static std::vector expandBuilderLines(const std::string& builders) return result; } -static Machine parseBuilderLine(const std::string& line) { +static Machine parseBuilderLine(const std::string & line) +{ const auto tokens = tokenizeString>(line); auto isSet = [&](size_t fieldIndex) { @@ -127,27 +132,28 @@ static Machine parseBuilderLine(const std::string& line) { }; auto ensureBase64 = [&](size_t fieldIndex) { - const auto& str = tokens[fieldIndex]; + const auto & str = tokens[fieldIndex]; try { base64Decode(str); - } catch (const Error& e) { + } catch (const Error & e) { throw FormatError("bad machine specification: a column #%lu in a row: '%s' is not valid base64 string: %s", fieldIndex, line, e.what()); } return str; }; - if (!isSet(0)) { - throw FormatError("bad machine specification: store URI was not found at the first column of a row: '%s'", line); - } - - return {tokens[0], - isSet(1) ? tokenizeString>(tokens[1], ",") : std::vector{settings.thisSystem}, - isSet(2) ? tokens[2] : "", - isSet(3) ? parseUnsignedIntField(3) : 1U, - isSet(4) ? parseUnsignedIntField(4) : 1U, - isSet(5) ? tokenizeString>(tokens[5], ",") : std::set{}, - isSet(6) ? tokenizeString>(tokens[6], ",") : std::set{}, - isSet(7) ? ensureBase64(7) : ""}; + if (!isSet(0)) + throw FormatError("bad machine specification: store URL was not found at the first column of a row: '%s'", line); + + return { + tokens[0], + isSet(1) ? tokenizeString>(tokens[1], ",") : std::vector{settings.thisSystem}, + isSet(2) ? tokens[2] : "", + isSet(3) ? parseUnsignedIntField(3) : 1U, + isSet(4) ? parseUnsignedIntField(4) : 1U, + isSet(5) ? tokenizeString>(tokens[5], ",") : std::set{}, + isSet(6) ? tokenizeString>(tokens[6], ",") : std::set{}, + isSet(7) ? ensureBase64(7) : "" + }; } static Machines parseBuilderLines(const std::vector& builders) { -- cgit v1.2.3 From df552ff53e68dff8ca360adbdbea214ece1d08ee Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Feb 2022 16:00:00 +0100 Subject: Remove std::string alias (for real this time) Also use std::string_view in a few more places. --- src/libstore/machines.cc | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'src/libstore/machines.cc') diff --git a/src/libstore/machines.cc b/src/libstore/machines.cc index b6270a81b..e87f46980 100644 --- a/src/libstore/machines.cc +++ b/src/libstore/machines.cc @@ -39,19 +39,19 @@ Machine::Machine(decltype(storeUri) storeUri, sshPublicHostKey(sshPublicHostKey) {} -bool Machine::allSupported(const std::set & features) const +bool Machine::allSupported(const std::set & features) const { return std::all_of(features.begin(), features.end(), - [&](const string & feature) { + [&](const std::string & feature) { return supportedFeatures.count(feature) || mandatoryFeatures.count(feature); }); } -bool Machine::mandatoryMet(const std::set & features) const +bool Machine::mandatoryMet(const std::set & features) const { return std::all_of(mandatoryFeatures.begin(), mandatoryFeatures.end(), - [&](const string & feature) { + [&](const std::string & feature) { return features.count(feature); }); } @@ -89,7 +89,7 @@ ref Machine::openStore() const static std::vector expandBuilderLines(const std::string & builders) { std::vector result; - for (auto line : tokenizeString>(builders, "\n;")) { + for (auto line : tokenizeString>(builders, "\n;")) { trim(line); line.erase(std::find(line.begin(), line.end(), '#'), line.end()); if (line.empty()) continue; @@ -117,7 +117,7 @@ static std::vector expandBuilderLines(const std::string & builders) static Machine parseBuilderLine(const std::string & line) { - const auto tokens = tokenizeString>(line); + const auto tokens = tokenizeString>(line); auto isSet = [&](size_t fieldIndex) { return tokens.size() > fieldIndex && tokens[fieldIndex] != "" && tokens[fieldIndex] != "-"; @@ -146,17 +146,18 @@ static Machine parseBuilderLine(const std::string & line) return { tokens[0], - isSet(1) ? tokenizeString>(tokens[1], ",") : std::vector{settings.thisSystem}, + isSet(1) ? tokenizeString>(tokens[1], ",") : std::vector{settings.thisSystem}, isSet(2) ? tokens[2] : "", isSet(3) ? parseUnsignedIntField(3) : 1U, isSet(4) ? parseUnsignedIntField(4) : 1U, - isSet(5) ? tokenizeString>(tokens[5], ",") : std::set{}, - isSet(6) ? tokenizeString>(tokens[6], ",") : std::set{}, + isSet(5) ? tokenizeString>(tokens[5], ",") : std::set{}, + isSet(6) ? tokenizeString>(tokens[6], ",") : std::set{}, isSet(7) ? ensureBase64(7) : "" }; } -static Machines parseBuilderLines(const std::vector& builders) { +static Machines parseBuilderLines(const std::vector & builders) +{ Machines result; std::transform(builders.begin(), builders.end(), std::back_inserter(result), parseBuilderLine); return result; -- cgit v1.2.3