Merge remote-tracking branch 'upstream/master' into trustless-remote-builder-simple

9 files changed, 92 insertions, 150 deletions

diff --git a/src/libutil/args.cc b/src/libutil/args.cc
index 61f9503ec..fb5cb80fb 100644
--- a/src/libutil/args.cc
+++ b/src/libutil/args.cc
@@ -254,6 +254,8 @@ nlohmann::json Args::toJSON()
     res["description"] = description();
     res["flags"] = std::move(flags);
     res["args"] = std::move(args);
+    auto s = doc();
+    if (s != "") res.emplace("doc", stripIndentation(s));
     return res;
 }
 
@@ -351,38 +353,6 @@ void printTable(std::ostream & out, const Table2 & table)
     }
 }
 
-void Command::printHelp(const string & programName, std::ostream & out)
-{
-    Args::printHelp(programName, out);
-
-    auto exs = examples();
-    if (!exs.empty()) {
-        out << "\n" ANSI_BOLD "Examples:" ANSI_NORMAL "\n";
-        for (auto & ex : exs)
-            out << "\n"
-                << "  " << ex.description << "\n" // FIXME: wrap
-                << "  $ " << ex.command << "\n";
-    }
-}
-
-nlohmann::json Command::toJSON()
-{
-    auto exs = nlohmann::json::array();
-
-    for (auto & example : examples()) {
-        auto ex = nlohmann::json::object();
-        ex["description"] = example.description;
-        ex["command"] = chomp(stripIndentation(example.command));
-        exs.push_back(std::move(ex));
-    }
-
-    auto res = Args::toJSON();
-    res["examples"] = std::move(exs);
-    auto s = doc();
-    if (s != "") res.emplace("doc", stripIndentation(s));
-    return res;
-}
-
 MultiCommand::MultiCommand(const Commands & commands)
     : commands(commands)
 {
diff --git a/src/libutil/args.hh b/src/libutil/args.hh
index 8069fd70f..3783bc84f 100644
--- a/src/libutil/args.hh
+++ b/src/libutil/args.hh
@@ -25,6 +25,9 @@ public:
     /* Return a short one-line description of the command. */
     virtual std::string description() { return ""; }
 
+    /* Return documentation about this command, in Markdown format. */
+    virtual std::string doc() { return ""; }
+
 protected:
 
     static const size_t ArityAny = std::numeric_limits<size_t>::max();
@@ -65,8 +68,12 @@ protected:
             , arity(ArityAny)
         { }
 
-        template<class T>
-        Handler(T * dest)
+        Handler(std::string * dest)
+            : fun([=](std::vector<std::string> ss) { *dest = ss[0]; })
+            , arity(1)
+        { }
+
+        Handler(std::optional<std::string> * dest)
             : fun([=](std::vector<std::string> ss) { *dest = ss[0]; })
             , arity(1)
         { }
@@ -76,6 +83,14 @@ protected:
             : fun([=](std::vector<std::string> ss) { *dest = val; })
             , arity(0)
         { }
+
+        template<class I>
+        Handler(I * dest)
+            : fun([=](std::vector<std::string> ss) {
+                *dest = string2IntWithUnitPrefix<I>(ss[0]);
+              })
+            , arity(1)
+        { }
     };
 
     /* Flags. */
@@ -127,19 +142,6 @@ public:
     /* Helper functions for constructing flags / positional
        arguments. */
 
-    void mkFlag1(char shortName, const std::string & longName,
-        const std::string & label, const std::string & description,
-        std::function<void(std::string)> fun)
-    {
-        addFlag({
-            .longName = longName,
-            .shortName = shortName,
-            .description = description,
-            .labels = {label},
-            .handler = {[=](std::string s) { fun(s); }}
-        });
-    }
-
     void mkFlag(char shortName, const std::string & name,
         const std::string & description, bool * dest)
     {
@@ -158,33 +160,6 @@ public:
         });
     }
 
-    template<class I>
-    void mkIntFlag(char shortName, const std::string & longName,
-        const std::string & description, I * dest)
-    {
-        mkFlag<I>(shortName, longName, description, [=](I n) {
-            *dest = n;
-        });
-    }
-
-    template<class I>
-    void mkFlag(char shortName, const std::string & longName,
-        const std::string & description, std::function<void(I)> fun)
-    {
-        addFlag({
-            .longName = longName,
-            .shortName = shortName,
-            .description = description,
-            .labels = {"N"},
-            .handler = {[=](std::string s) {
-                I n;
-                if (!string2Int(s, n))
-                    throw UsageError("flag '--%s' requires a integer argument", longName);
-                fun(n);
-            }}
-        });
-    }
-
     void expectArgs(ExpectedArg && arg)
     {
         expectedArgs.emplace_back(std::move(arg));
@@ -225,28 +200,11 @@ struct Command : virtual Args
     virtual void prepare() { };
     virtual void run() = 0;
 
-    /* Return documentation about this command, in Markdown format. */
-    virtual std::string doc() { return ""; }
-
-    struct Example
-    {
-        std::string description;
-        std::string command;
-    };
-
-    typedef std::list<Example> Examples;
-
-    virtual Examples examples() { return Examples(); }
-
     typedef int Category;
 
     static constexpr Category catDefault = 0;
 
     virtual Category category() { return catDefault; }
-
-    void printHelp(const string & programName, std::ostream & out) override;
-
-    nlohmann::json toJSON() override;
 };
 
 typedef std::map<std::string, std::function<ref<Command>()>> Commands;
diff --git a/src/libutil/config.cc b/src/libutil/config.cc
index 7af3e7883..7467e5ac0 100644
--- a/src/libutil/config.cc
+++ b/src/libutil/config.cc
@@ -230,7 +230,9 @@ template<typename T>
 void BaseSetting<T>::set(const std::string & str, bool append)
 {
     static_assert(std::is_integral<T>::value, "Integer required.");
-    if (!string2Int(str, value))
+    if (auto n = string2Int<T>(str))
+        value = *n;
+    else
         throw UsageError("setting '%s' has invalid value '%s'", name, str);
 }
 
diff --git a/src/libutil/error.cc b/src/libutil/error.cc
index 803a72953..2a67a730a 100644
--- a/src/libutil/error.cc
+++ b/src/libutil/error.cc
@@ -61,36 +61,31 @@ std::optional<LinesOfCode> getCodeLines(const ErrPos & errPos)
     if (errPos.origin == foFile) {
         LinesOfCode loc;
         try {
+            // FIXME: when running as the daemon, make sure we don't
+            // open a file to which the client doesn't have access.
             AutoCloseFD fd = open(errPos.file.c_str(), O_RDONLY | O_CLOEXEC);
-            if (!fd) {
-                logError(SysError("opening file '%1%'", errPos.file).info());
-                return std::nullopt;
-            }
-            else
+            if (!fd) return {};
+
+            // count the newlines.
+            int count = 0;
+            string line;
+            int pl = errPos.line - 1;
+            do
             {
-                // count the newlines.
-                int count = 0;
-                string line;
-                int pl = errPos.line - 1;
-                do
-                {
-                    line = readLine(fd.get());
-                    ++count;
-                    if (count < pl)
-                    {
-                        ;
-                    }
-                    else if (count == pl) {
-                        loc.prevLineOfCode = line;
-                    } else if (count == pl + 1) {
-                        loc.errLineOfCode = line;
-                    } else if (count == pl + 2) {
-                        loc.nextLineOfCode = line;
-                        break;
-                    }
-                } while (true);
-                return loc;
-            }
+                line = readLine(fd.get());
+                ++count;
+                if (count < pl)
+                    ;
+                else if (count == pl)
+                    loc.prevLineOfCode = line;
+                else if (count == pl + 1)
+                    loc.errLineOfCode = line;
+                else if (count == pl + 2) {
+                    loc.nextLineOfCode = line;
+                    break;
+                }
+            } while (true);
+            return loc;
         }
         catch (EndOfFile & eof) {
             if (loc.errLineOfCode.has_value())
@@ -99,7 +94,6 @@ std::optional<LinesOfCode> getCodeLines(const ErrPos & errPos)
                 return std::nullopt;
         }
         catch (std::exception & e) {
-            printError("error reading nix file: %s\n%s", errPos.file, e.what());
             return std::nullopt;
         }
     } else {
diff --git a/src/libutil/error.hh b/src/libutil/error.hh
index aa4fadfcc..1e0bde7ea 100644
--- a/src/libutil/error.hh
+++ b/src/libutil/error.hh
@@ -38,7 +38,7 @@ namespace nix {
    ErrorInfo structs are sent to the logger as part of an exception, or directly with the
    logError or logWarning macros.
 
-   See the error-demo.cc program for usage examples.
+   See libutil/tests/logging.cc for usage examples.
 
  */
 
diff --git a/src/libutil/tests/logging.cc b/src/libutil/tests/logging.cc
index 7e53f17c6..5b32c84a4 100644
--- a/src/libutil/tests/logging.cc
+++ b/src/libutil/tests/logging.cc
@@ -49,7 +49,7 @@ namespace nix {
             });
 
         auto str = testing::internal::GetCapturedStderr();
-        ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SysError --- error-unit-test\x1B[0m\nopening file '\x1B[33;1mrandom.nix\x1B[0m': \x1B[33;1mNo such file or directory\x1B[0m\n@nix {\"action\":\"msg\",\"column\":13,\"file\":\"random.nix\",\"level\":0,\"line\":2,\"msg\":\"\\u001b[31;1merror:\\u001b[0m\\u001b[34;1m --- error name --- error-unit-test\\u001b[0m\\n\\u001b[34;1mat: \\u001b[33;1m(2:13)\\u001b[34;1m in file: \\u001b[0mrandom.nix\\n\\nerror without any code lines.\\n\\nthis hint has \\u001b[33;1myellow\\u001b[0m templated \\u001b[33;1mvalues\\u001b[0m!!\",\"raw_msg\":\"this hint has \\u001b[33;1myellow\\u001b[0m templated \\u001b[33;1mvalues\\u001b[0m!!\"}\n");
+        ASSERT_STREQ(str.c_str(), "@nix {\"action\":\"msg\",\"column\":13,\"file\":\"random.nix\",\"level\":0,\"line\":2,\"msg\":\"\\u001b[31;1merror:\\u001b[0m\\u001b[34;1m --- error name --- error-unit-test\\u001b[0m\\n\\u001b[34;1mat: \\u001b[33;1m(2:13)\\u001b[34;1m in file: \\u001b[0mrandom.nix\\n\\nerror without any code lines.\\n\\nthis hint has \\u001b[33;1myellow\\u001b[0m templated \\u001b[33;1mvalues\\u001b[0m!!\",\"raw_msg\":\"this hint has \\u001b[33;1myellow\\u001b[0m templated \\u001b[33;1mvalues\\u001b[0m!!\"}\n");
     }
 
     TEST(logEI, appendingHintsToPreviousError) {
@@ -208,7 +208,7 @@ namespace nix {
             });
 
         auto str = testing::internal::GetCapturedStderr();
-        ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SysError --- error-unit-test\x1B[0m\nopening file '\x1B[33;1minvalid filename\x1B[0m': \x1B[33;1mNo such file or directory\x1B[0m\n\x1B[31;1merror:\x1B[0m\x1B[34;1m --- error name --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m in file: \x1B[0minvalid filename\n\nerror without any code lines.\n\nthis hint has \x1B[33;1myellow\x1B[0m templated \x1B[33;1mvalues\x1B[0m!!\n");
+        ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- error name --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m in file: \x1B[0minvalid filename\n\nerror without any code lines.\n\nthis hint has \x1B[33;1myellow\x1B[0m templated \x1B[33;1mvalues\x1B[0m!!\n");
     }
 
     TEST(logError, logErrorWithOnlyHintAndName) {
@@ -290,7 +290,7 @@ namespace nix {
         logError(e.info());
 
         auto str = testing::internal::GetCapturedStderr();
-        ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SysError --- error-unit-test\x1B[0m\nopening file '\x1B[33;1minvalid filename\x1B[0m': \x1B[33;1mNo such file or directory\x1B[0m\n\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from string\x1B[0m\n\nshow-traces\n\n     1| previous line of code\n     2| this is the problem line of code\n      |             \x1B[31;1m^\x1B[0m\n     3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n\x1B[34;1m---- show-trace ----\x1B[0m\n\x1B[34;1mtrace: \x1B[0mwhile trying to compute \x1B[33;1m42\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(1:19)\x1B[34;1m from stdin\x1B[0m\n\n     1| this is the other problem line of code\n      |                   \x1B[31;1m^\x1B[0m\n\n\x1B[34;1mtrace: \x1B[0mwhile doing something without a \x1B[33;1mpos\x1B[0m\n\x1B[34;1mtrace: \x1B[0mmissing \x1B[33;1mnix file\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(100:1)\x1B[34;1m in file: \x1B[0minvalid filename\n");
+        ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- AssertionError --- error-unit-test\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(2:13)\x1B[34;1m from string\x1B[0m\n\nshow-traces\n\n     1| previous line of code\n     2| this is the problem line of code\n      |             \x1B[31;1m^\x1B[0m\n     3| next line of code\n\nit has been \x1B[33;1mzero\x1B[0m days since our last error\n\x1B[34;1m---- show-trace ----\x1B[0m\n\x1B[34;1mtrace: \x1B[0mwhile trying to compute \x1B[33;1m42\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(1:19)\x1B[34;1m from stdin\x1B[0m\n\n     1| this is the other problem line of code\n      |                   \x1B[31;1m^\x1B[0m\n\n\x1B[34;1mtrace: \x1B[0mwhile doing something without a \x1B[33;1mpos\x1B[0m\n\x1B[34;1mtrace: \x1B[0mmissing \x1B[33;1mnix file\x1B[0m\n\x1B[34;1mat: \x1B[33;1m(100:1)\x1B[34;1m in file: \x1B[0minvalid filename\n");
     }
 
     TEST(addTrace, hideTracesWithoutShowTrace) {
diff --git a/src/libutil/tests/tests.cc b/src/libutil/tests/tests.cc
index 35a5d27bb..58df9c5ac 100644
--- a/src/libutil/tests/tests.cc
+++ b/src/libutil/tests/tests.cc
@@ -320,20 +320,15 @@ namespace nix {
      * --------------------------------------------------------------------------*/
 
     TEST(string2Float, emptyString) {
-        double n;
-        ASSERT_EQ(string2Float("", n), false);
+        ASSERT_EQ(string2Float<double>(""), std::nullopt);
     }
 
     TEST(string2Float, trivialConversions) {
-        double n;
-        ASSERT_EQ(string2Float("1.0", n), true);
-        ASSERT_EQ(n, 1.0);
+        ASSERT_EQ(string2Float<double>("1.0"), 1.0);
 
-        ASSERT_EQ(string2Float("0.0", n), true);
-        ASSERT_EQ(n, 0.0);
+        ASSERT_EQ(string2Float<double>("0.0"), 0.0);
 
-        ASSERT_EQ(string2Float("-100.25", n), true);
-        ASSERT_EQ(n, (-100.25));
+        ASSERT_EQ(string2Float<double>("-100.25"), -100.25);
     }
 
     /* ----------------------------------------------------------------------------
@@ -341,20 +336,15 @@ namespace nix {
      * --------------------------------------------------------------------------*/
 
     TEST(string2Int, emptyString) {
-        double n;
-        ASSERT_EQ(string2Int("", n), false);
+        ASSERT_EQ(string2Int<int>(""), std::nullopt);
     }
 
     TEST(string2Int, trivialConversions) {
-        double n;
-        ASSERT_EQ(string2Int("1", n), true);
-        ASSERT_EQ(n, 1);
+        ASSERT_EQ(string2Int<int>("1"), 1);
 
-        ASSERT_EQ(string2Int("0", n), true);
-        ASSERT_EQ(n, 0);
+        ASSERT_EQ(string2Int<int>("0"), 0);
 
-        ASSERT_EQ(string2Int("-100", n), true);
-        ASSERT_EQ(n, (-100));
+        ASSERT_EQ(string2Int<int>("-100"), -100);
     }
 
     /* ----------------------------------------------------------------------------
diff --git a/src/libutil/url-parts.hh b/src/libutil/url-parts.hh
index 5d21b8d1a..862d9fa6e 100644
--- a/src/libutil/url-parts.hh
+++ b/src/libutil/url-parts.hh
@@ -23,7 +23,7 @@ const static std::string absPathRegex = "(?:(?:/" + segmentRegex + ")*/?)";
 const static std::string pathRegex = "(?:" + segmentRegex + "(?:/" + segmentRegex + ")*/?)";
 
 // A Git ref (i.e. branch or tag name).
-const static std::string refRegexS = "[a-zA-Z0-9][a-zA-Z0-9_.-]*"; // FIXME: check
+const static std::string refRegexS = "[a-zA-Z0-9][a-zA-Z0-9_.\\/-]*"; // FIXME: check
 extern std::regex refRegex;
 
 // Instead of defining what a good Git Ref is, we define what a bad Git Ref is
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index 0f82bed78..ab0bd865a 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -397,21 +397,49 @@ bool statusOk(int status);
 
 
 /* Parse a string into an integer. */
-template<class N> bool string2Int(const string & s, N & n)
+template<class N>
+std::optional<N> string2Int(const std::string & s)
 {
-    if (string(s, 0, 1) == "-" && !std::numeric_limits<N>::is_signed)
-        return false;
+    if (s.substr(0, 1) == "-" && !std::numeric_limits<N>::is_signed)
+        return std::nullopt;
     std::istringstream str(s);
+    N n;
     str >> n;
-    return str && str.get() == EOF;
+    if (str && str.get() == EOF) return n;
+    return std::nullopt;
+}
+
+/* Like string2Int(), but support an optional suffix 'K', 'M', 'G' or
+   'T' denoting a binary unit prefix. */
+template<class N>
+N string2IntWithUnitPrefix(std::string s)
+{
+    N multiplier = 1;
+    if (!s.empty()) {
+        char u = std::toupper(*s.rbegin());
+        if (std::isalpha(u)) {
+            if (u == 'K') multiplier = 1ULL << 10;
+            else if (u == 'M') multiplier = 1ULL << 20;
+            else if (u == 'G') multiplier = 1ULL << 30;
+            else if (u == 'T') multiplier = 1ULL << 40;
+            else throw UsageError("invalid unit specifier '%1%'", u);
+            s.resize(s.size() - 1);
+        }
+    }
+    if (auto n = string2Int<N>(s))
+        return *n * multiplier;
+    throw UsageError("'%s' is not an integer", s);
 }
 
 /* Parse a string into a float. */
-template<class N> bool string2Float(const string & s, N & n)
+template<class N>
+std::optional<N> string2Float(const string & s)
 {
     std::istringstream str(s);
+    N n;
     str >> n;
-    return str && str.get() == EOF;
+    if (str && str.get() == EOF) return n;
+    return std::nullopt;
 }