Restore exposing machine file parsing

This was accidentally removed in
e989c83b44. I restored it and also did a
few other cleanups:

- Make a static method for namespacing purposes

- Put the test files in the data dir with the other test data

- Avoid mutating globals in the machine config tests

This will be used by Hydra.

src/libstore/machines.cc

@@ -131,7 +131,7 @@ static std::vector<std::string> expandBuilderLines(const std::string & builders)
     return result;
 }
 
-static Machine parseBuilderLine(const std::string & line)
+static Machine parseBuilderLine(const std::set<std::string> & defaultSystems, const std::string & line)
 {
     const auto tokens = tokenizeString<std::vector<std::string>>(line);
 
@@ -170,7 +170,7 @@ static Machine parseBuilderLine(const std::string & line)
 
     return {
         tokens[0],
-        isSet(1) ? tokenizeString<std::set<std::string>>(tokens[1], ",") : std::set<std::string>{settings.thisSystem},
+        isSet(1) ? tokenizeString<std::set<std::string>>(tokens[1], ",") : defaultSystems,
         isSet(2) ? tokens[2] : "",
         isSet(3) ? parseUnsignedIntField(3) : 1U,
         isSet(4) ? parseFloatField(4) : 1.0f,
@@ -180,17 +180,24 @@ static Machine parseBuilderLine(const std::string & line)
     };
 }
 
-static Machines parseBuilderLines(const std::vector<std::string> & builders)
+static Machines parseBuilderLines(const std::set<std::string> & defaultSystems, const std::vector<std::string> & builders)
 {
     Machines result;
-    std::transform(builders.begin(), builders.end(), std::back_inserter(result), parseBuilderLine);
+    std::transform(
+        builders.begin(), builders.end(), std::back_inserter(result),
+        [&](auto && line) { return parseBuilderLine(defaultSystems, line); });
     return result;
 }
 
+Machines Machine::parseConfig(const std::set<std::string> & defaultSystems, const std::string & s)
+{
+    const auto builderLines = expandBuilderLines(s);
+    return parseBuilderLines(defaultSystems, builderLines);
+}
+
 Machines getMachines()
 {
-    const auto builderLines = expandBuilderLines(settings.builders);
+    return Machine::parseConfig({settings.thisSystem}, settings.builders);
-    return parseBuilderLines(builderLines);
 }
 
 }

src/libstore/machines.hh

@@ -8,6 +8,10 @@ namespace nix {
 
 class Store;
 
+struct Machine;
+
+typedef std::vector<Machine> Machines;
+
 struct Machine {
 
     const StoreReference storeUri;
@@ -63,12 +67,22 @@ struct Machine {
      * ```
      */
     ref<Store> openStore() const;
+
+    /**
+     * Parse a machine configuration.
+     *
+     * Every machine is specified on its own line, and lines beginning
+     * with `@` are interpreted as paths to other configuration files in
+     * the same format.
+     */
+    static Machines parseConfig(const std::set<std::string> & defaultSystems, const std::string & config);
 };
 
-typedef std::vector<Machine> Machines;
+/**
-
+ * Parse machines from the global config
-void parseMachines(const std::string & s, Machines & machines);
+ *
-
+ * @todo Remove, globals are bad.
+ */
 Machines getMachines();
 
 }

tests/unit/libstore/machines.cc

@@ -1,8 +1,9 @@
 #include "machines.hh"
-#include "globals.hh"
 #include "file-system.hh"
 #include "util.hh"
 
+#include "tests/characterization.hh"
+
 #include <gtest/gtest.h>
 #include <gmock/gmock-matchers.h>
 
@@ -14,23 +15,13 @@ using testing::SizeIs;
 
 using namespace nix;
 
-class Environment : public ::testing::Environment {
-  public:
-    void SetUp() override { settings.thisSystem = "TEST_ARCH-TEST_OS"; }
-};
-
-testing::Environment* const foo_env =
-    testing::AddGlobalTestEnvironment(new Environment);
-
 TEST(machines, getMachinesWithEmptyBuilders) {
-    settings.builders = "";
+    auto actual = Machine::parseConfig({}, "");
-    Machines actual = getMachines();
     ASSERT_THAT(actual, SizeIs(0));
 }
 
 TEST(machines, getMachinesUriOnly) {
-    settings.builders = "nix@scratchy.labs.cs.uu.nl";
+    auto actual = Machine::parseConfig({"TEST_ARCH-TEST_OS"}, "nix@scratchy.labs.cs.uu.nl");
-    Machines actual = getMachines();
     ASSERT_THAT(actual, SizeIs(1));
     EXPECT_THAT(actual[0], Field(&Machine::storeUri, Eq(StoreReference::parse("ssh://nix@scratchy.labs.cs.uu.nl"))));
     EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("TEST_ARCH-TEST_OS")));
@@ -43,8 +34,7 @@ TEST(machines, getMachinesUriOnly) {
 }
 
 TEST(machines, getMachinesDefaults) {
-    settings.builders = "nix@scratchy.labs.cs.uu.nl - - - - - - -";
+    auto actual = Machine::parseConfig({"TEST_ARCH-TEST_OS"}, "nix@scratchy.labs.cs.uu.nl - - - - - - -");
-    Machines actual = getMachines();
     ASSERT_THAT(actual, SizeIs(1));
     EXPECT_THAT(actual[0], Field(&Machine::storeUri, Eq(StoreReference::parse("ssh://nix@scratchy.labs.cs.uu.nl"))));
     EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("TEST_ARCH-TEST_OS")));
@@ -68,26 +58,24 @@ MATCHER_P(AuthorityMatches, authority, "") {
 }
 
 TEST(machines, getMachinesWithNewLineSeparator) {
-    settings.builders = "nix@scratchy.labs.cs.uu.nl\nnix@itchy.labs.cs.uu.nl";
+    auto actual = Machine::parseConfig({}, "nix@scratchy.labs.cs.uu.nl\nnix@itchy.labs.cs.uu.nl");
-    Machines actual = getMachines();
     ASSERT_THAT(actual, SizeIs(2));
     EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("nix@scratchy.labs.cs.uu.nl"))));
     EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("nix@itchy.labs.cs.uu.nl"))));
 }
 
 TEST(machines, getMachinesWithSemicolonSeparator) {
-    settings.builders = "nix@scratchy.labs.cs.uu.nl ; nix@itchy.labs.cs.uu.nl";
+    auto actual = Machine::parseConfig({}, "nix@scratchy.labs.cs.uu.nl ; nix@itchy.labs.cs.uu.nl");
-    Machines actual = getMachines();
     EXPECT_THAT(actual, SizeIs(2));
     EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("nix@scratchy.labs.cs.uu.nl"))));
     EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("nix@itchy.labs.cs.uu.nl"))));
 }
 
 TEST(machines, getMachinesWithCorrectCompleteSingleBuilder) {
-    settings.builders = "nix@scratchy.labs.cs.uu.nl     i686-linux      "
+    auto actual = Machine::parseConfig({},
+        "nix@scratchy.labs.cs.uu.nl     i686-linux      "
         "/home/nix/.ssh/id_scratchy_auto        8 3 kvm "
-                        "benchmark SSH+HOST+PUBLIC+KEY+BASE64+ENCODED==";
+        "benchmark SSH+HOST+PUBLIC+KEY+BASE64+ENCODED==");
-    Machines actual = getMachines();
     ASSERT_THAT(actual, SizeIs(1));
     EXPECT_THAT(actual[0], Field(&Machine::storeUri, AuthorityMatches("nix@scratchy.labs.cs.uu.nl")));
     EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("i686-linux")));
@@ -101,11 +89,10 @@ TEST(machines, getMachinesWithCorrectCompleteSingleBuilder) {
 
 TEST(machines,
      getMachinesWithCorrectCompleteSingleBuilderWithTabColumnDelimiter) {
-    settings.builders =
+    auto actual = Machine::parseConfig({},
         "nix@scratchy.labs.cs.uu.nl\ti686-linux\t/home/nix/.ssh/"
         "id_scratchy_auto\t8\t3\tkvm\tbenchmark\tSSH+HOST+PUBLIC+"
-        "KEY+BASE64+ENCODED==";
+        "KEY+BASE64+ENCODED==");
-    Machines actual = getMachines();
     ASSERT_THAT(actual, SizeIs(1));
     EXPECT_THAT(actual[0], Field(&Machine::storeUri, AuthorityMatches("nix@scratchy.labs.cs.uu.nl")));
     EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("i686-linux")));
@@ -118,10 +105,10 @@ TEST(machines,
 }
 
 TEST(machines, getMachinesWithMultiOptions) {
-    settings.builders = "nix@scratchy.labs.cs.uu.nl Arch1,Arch2 - - - "
+    auto actual = Machine::parseConfig({},
+        "nix@scratchy.labs.cs.uu.nl Arch1,Arch2 - - - "
         "SupportedFeature1,SupportedFeature2 "
-                        "MandatoryFeature1,MandatoryFeature2";
+        "MandatoryFeature1,MandatoryFeature2");
-    Machines actual = getMachines();
     ASSERT_THAT(actual, SizeIs(1));
     EXPECT_THAT(actual[0], Field(&Machine::storeUri, AuthorityMatches("nix@scratchy.labs.cs.uu.nl")));
     EXPECT_THAT(actual[0], Field(&Machine::systemTypes, ElementsAre("Arch1", "Arch2")));
@@ -130,24 +117,28 @@ TEST(machines, getMachinesWithMultiOptions) {
 }
 
 TEST(machines, getMachinesWithIncorrectFormat) {
-    settings.builders = "nix@scratchy.labs.cs.uu.nl - - eight";
+    EXPECT_THROW(
-    EXPECT_THROW(getMachines(), FormatError);
+        Machine::parseConfig({}, "nix@scratchy.labs.cs.uu.nl - - eight"),
-    settings.builders = "nix@scratchy.labs.cs.uu.nl - - -1";
+        FormatError);
-    EXPECT_THROW(getMachines(), FormatError);
+    EXPECT_THROW(
-    settings.builders = "nix@scratchy.labs.cs.uu.nl - - 8 three";
+        Machine::parseConfig({}, "nix@scratchy.labs.cs.uu.nl - - -1"),
-    EXPECT_THROW(getMachines(), FormatError);
+        FormatError);
-    settings.builders = "nix@scratchy.labs.cs.uu.nl - - 8 -3";
+    EXPECT_THROW(
-    EXPECT_THROW(getMachines(), UsageError);
+        Machine::parseConfig({}, "nix@scratchy.labs.cs.uu.nl - - 8 three"),
-    settings.builders = "nix@scratchy.labs.cs.uu.nl - - 8 3 - - BAD_BASE64";
+        FormatError);
-    EXPECT_THROW(getMachines(), FormatError);
+    EXPECT_THROW(
+        Machine::parseConfig({}, "nix@scratchy.labs.cs.uu.nl - - 8 -3"),
+        UsageError);
+    EXPECT_THROW(
+        Machine::parseConfig({}, "nix@scratchy.labs.cs.uu.nl - - 8 3 - - BAD_BASE64"),
+        FormatError);
 }
 
 TEST(machines, getMachinesWithCorrectFileReference) {
-    auto path = absPath("tests/unit/libstore/test-data/machines.valid");
+    auto path = absPath(getUnitTestData() + "/machines/valid");
     ASSERT_TRUE(pathExists(path));
 
-    settings.builders = std::string("@") + path;
+    auto actual = Machine::parseConfig({}, "@" + path);
-    Machines actual = getMachines();
     ASSERT_THAT(actual, SizeIs(3));
     EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("nix@scratchy.labs.cs.uu.nl"))));
     EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("nix@itchy.labs.cs.uu.nl"))));
@@ -158,18 +149,17 @@ TEST(machines, getMachinesWithCorrectFileReferenceToEmptyFile) {
     auto path = "/dev/null";
     ASSERT_TRUE(pathExists(path));
 
-    settings.builders = std::string("@") + path;
+    auto actual = Machine::parseConfig({}, std::string{"@"} + path);
-    Machines actual = getMachines();
     ASSERT_THAT(actual, SizeIs(0));
 }
 
 TEST(machines, getMachinesWithIncorrectFileReference) {
-    settings.builders = std::string("@") + absPath("/not/a/file");
+    auto actual = Machine::parseConfig({}, "@" + absPath("/not/a/file"));
-    Machines actual = getMachines();
     ASSERT_THAT(actual, SizeIs(0));
 }
 
 TEST(machines, getMachinesWithCorrectFileReferenceToIncorrectFile) {
-    settings.builders = std::string("@") + absPath("tests/unit/libstore/test-data/machines.bad_format");
+    EXPECT_THROW(
-    EXPECT_THROW(getMachines(), FormatError);
+        Machine::parseConfig({}, "@" + absPath(getUnitTestData() + "/machines/bad_format")),
+        FormatError);
 }

tests/unit/libutil-support/tests/characterization.hh

@@ -12,7 +12,7 @@ namespace nix {
  * The path to the unit test data directory. See the contributing guide
  * in the manual for further details.
  */
-static Path getUnitTestData() {
+static inline Path getUnitTestData() {
     return getEnv("_NIX_TEST_UNIT_DATA").value();
 }
 
@@ -21,7 +21,7 @@ static Path getUnitTestData() {
  * against them. See the contributing guide in the manual for further
  * details.
  */
-static bool testAccept() {
+static inline bool testAccept() {
     return getEnv("_NIX_TEST_ACCEPT") == "1";
 }