From 2892b758b326a6163e2925e45eaae47288f839aa Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Wed, 9 Apr 2025 12:31:33 -0400
Subject: [PATCH] Fix `;` and `#` bug in machine file parsing

Comments go to the end of the line, not merely the next ; *or* \n. Fix
by splitting on `;` *within* lines, and test.

(cherry picked from commit f8b13cce19538796a881cc30fe449436d45cdbb6)
---
 src/libstore-tests/machines.cc | 12 ++++++++++++
 src/libstore/machines.cc       | 36 ++++++++++++++++++----------------
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/src/libstore-tests/machines.cc b/src/libstore-tests/machines.cc
index 1d574ceeb..3d8570946 100644
--- a/src/libstore-tests/machines.cc
+++ b/src/libstore-tests/machines.cc
@@ -73,6 +73,18 @@ TEST(machines, getMachinesWithSemicolonSeparator) {
     EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("nix@itchy.labs.cs.uu.nl"))));
 }
 
+TEST(machines, getMachinesWithCommentsAndSemicolonSeparator) {
+    auto actual = Machine::parseConfig({},
+        "# This is a comment ; this is still that comment\n"
+        "nix@scratchy.labs.cs.uu.nl ; nix@itchy.labs.cs.uu.nl\n"
+        "# This is also a comment ; this also is still that comment\n"
+        "nix@scabby.labs.cs.uu.nl\n");
+    EXPECT_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"))));
+    EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("nix@scabby.labs.cs.uu.nl"))));
+}
+
 TEST(machines, getMachinesWithCorrectCompleteSingleBuilder) {
     auto actual = Machine::parseConfig({},
         "nix@scratchy.labs.cs.uu.nl     i686-linux      "
diff --git a/src/libstore/machines.cc b/src/libstore/machines.cc
index 7c077239d..6ed4ac8b6 100644
--- a/src/libstore/machines.cc
+++ b/src/libstore/machines.cc
@@ -105,28 +105,30 @@ ref<Store> Machine::openStore() const
 static std::vector<std::string> expandBuilderLines(const std::string & builders)
 {
     std::vector<std::string> result;
-    for (auto line : tokenizeString<std::vector<std::string>>(builders, "\n;")) {
+    for (auto line : tokenizeString<std::vector<std::string>>(builders, "\n")) {
         trim(line);
         line.erase(std::find(line.begin(), line.end(), '#'), line.end());
-        if (line.empty()) continue;
+        for (auto entry : tokenizeString<std::vector<std::string>>(line, ";")) {
+            if (entry.empty()) continue;
 
-        if (line[0] == '@') {
-            const std::string path = trim(std::string(line, 1));
-            std::string text;
-            try {
-                text = readFile(path);
-            } catch (const SysError & e) {
-                if (e.errNo != ENOENT)
-                    throw;
-                debug("cannot find machines file '%s'", path);
+            if (entry[0] == '@') {
+                const std::string path = trim(std::string(entry, 1));
+                std::string text;
+                try {
+                    text = readFile(path);
+                } catch (const SysError & e) {
+                    if (e.errNo != ENOENT)
+                        throw;
+                    debug("cannot find machines file '%s'", path);
+                    continue;
+                }
+
+                const auto entrys = expandBuilderLines(text);
+                result.insert(end(result), begin(entrys), end(entrys));
+            } else {
+                result.emplace_back(entry);
             }
-
-            const auto lines = expandBuilderLines(text);
-            result.insert(end(result), begin(lines), end(lines));
-            continue;
         }
-
-        result.emplace_back(line);
     }
     return result;
 }