Merge pull request #3720 from obsidiansystems/fix-url-format

Avoid `fmt` when constructor already does it

8 files changed, 80 insertions, 51 deletions

diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc
index 1beb8b944..5c5671681 100644
--- a/src/libfetchers/mercurial.cc
+++ b/src/libfetchers/mercurial.cc
@@ -36,7 +36,7 @@ static std::string runHg(const Strings & args, const std::optional<std::string>
     auto res = runProgram(std::move(opts));
 
     if (!statusOk(res.first))
-        throw ExecError(res.first, fmt("hg %1%", statusToString(res.first)));
+        throw ExecError(res.first, "hg %1%", statusToString(res.first));
 
     return res.second;
 }
@@ -273,7 +273,7 @@ struct MercurialInputScheme : InputScheme
                         runHg({ "recover", "-R", cacheDir });
                         runHg({ "pull", "-R", cacheDir, "--", actualUrl });
                     } else {
-                        throw ExecError(e.status, fmt("'hg pull' %s", statusToString(e.status)));
+                        throw ExecError(e.status, "'hg pull' %s", statusToString(e.status));
                     }
                 }
             } else {
diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc
index 562d1b414..31454e49d 100644
--- a/src/libmain/shared.cc
+++ b/src/libmain/shared.cc
@@ -60,37 +60,37 @@ void printMissing(ref<Store> store, const StorePathSet & willBuild,
 {
     if (!willBuild.empty()) {
         if (willBuild.size() == 1)
-            printMsg(lvl, fmt("this derivation will be built:"));
+            printMsg(lvl, "this derivation will be built:");
         else
-            printMsg(lvl, fmt("these %d derivations will be built:", willBuild.size()));
+            printMsg(lvl, "these %d derivations will be built:", willBuild.size());
         auto sorted = store->topoSortPaths(willBuild);
         reverse(sorted.begin(), sorted.end());
         for (auto & i : sorted)
-            printMsg(lvl, fmt("  %s", store->printStorePath(i)));
+            printMsg(lvl, "  %s", store->printStorePath(i));
     }
 
     if (!willSubstitute.empty()) {
         const float downloadSizeMiB = downloadSize / (1024.f * 1024.f);
         const float narSizeMiB = narSize / (1024.f * 1024.f);
         if (willSubstitute.size() == 1) {
-            printMsg(lvl, fmt("this path will be fetched (%.2f MiB download, %.2f MiB unpacked):",
+            printMsg(lvl, "this path will be fetched (%.2f MiB download, %.2f MiB unpacked):",
                 downloadSizeMiB,
-                narSizeMiB));
+                narSizeMiB);
         } else {
-            printMsg(lvl, fmt("these %d paths will be fetched (%.2f MiB download, %.2f MiB unpacked):",
+            printMsg(lvl, "these %d paths will be fetched (%.2f MiB download, %.2f MiB unpacked):",
                 willSubstitute.size(),
                 downloadSizeMiB,
-                narSizeMiB));
+                narSizeMiB);
         }
         for (auto & i : willSubstitute)
-            printMsg(lvl, fmt("  %s", store->printStorePath(i)));
+            printMsg(lvl, "  %s", store->printStorePath(i));
     }
 
     if (!unknown.empty()) {
-        printMsg(lvl, fmt("don't know how to build these paths%s:",
-                (settings.readOnlyMode ? " (may be caused by read-only store access)" : "")));
+        printMsg(lvl, "don't know how to build these paths%s:",
+                (settings.readOnlyMode ? " (may be caused by read-only store access)" : ""));
         for (auto & i : unknown)
-            printMsg(lvl, fmt("  %s", store->printStorePath(i)));
+            printMsg(lvl, "  %s", store->printStorePath(i));
     }
 }
 
diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc
index c46262299..529a41891 100644
--- a/src/libstore/filetransfer.cc
+++ b/src/libstore/filetransfer.cc
@@ -443,14 +443,13 @@ struct curlFileTransfer : public FileTransfer
                     : httpStatus != 0
                     ? FileTransferError(err,
                         std::move(response),
-                        fmt("unable to %s '%s': HTTP error %d ('%s')",
-                            request.verb(), request.uri, httpStatus, statusMsg)
-                        + (code == CURLE_OK ? "" : fmt(" (curl error: %s)", curl_easy_strerror(code)))
-                        )
+                        "unable to %s '%s': HTTP error %d%s",
+                        request.verb(), request.uri, httpStatus,
+                        code == CURLE_OK ? "" : fmt(" (curl error: %s)", curl_easy_strerror(code)))
                     : FileTransferError(err,
                         std::move(response),
-                        fmt("unable to %s '%s': %s (%d)",
-                            request.verb(), request.uri, curl_easy_strerror(code), code));
+                        "unable to %s '%s': %s (%d)",
+                        request.verb(), request.uri, curl_easy_strerror(code), code);
 
                 /* If this is a transient error, then maybe retry the
                    download after a while. If we're writing to a
@@ -704,7 +703,7 @@ struct curlFileTransfer : public FileTransfer
                 auto s3Res = s3Helper.getObject(bucketName, key);
                 FileTransferResult res;
                 if (!s3Res.data)
-                    throw FileTransferError(NotFound, {}, "S3 object '%s' does not exist", request.uri);
+                    throw FileTransferError(NotFound, "S3 object '%s' does not exist", request.uri);
                 res.data = std::move(*s3Res.data);
                 callback(std::move(res));
 #else
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index d77fff963..ece5bb5ef 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -482,18 +482,18 @@ void LocalStore::openDB(State & state, bool create)
         SQLiteStmt stmt;
         stmt.create(db, "pragma main.journal_mode;");
         if (sqlite3_step(stmt) != SQLITE_ROW)
-            throwSQLiteError(db, "querying journal mode");
+            SQLiteError::throw_(db, "querying journal mode");
         prevMode = std::string((const char *) sqlite3_column_text(stmt, 0));
     }
     if (prevMode != mode &&
         sqlite3_exec(db, ("pragma main.journal_mode = " + mode + ";").c_str(), 0, 0, 0) != SQLITE_OK)
-        throwSQLiteError(db, "setting journal mode");
+        SQLiteError::throw_(db, "setting journal mode");
 
     /* Increase the auto-checkpoint interval to 40000 pages.  This
        seems enough to ensure that instantiating the NixOS system
        derivation is done in a single fsync(). */
     if (mode == "wal" && sqlite3_exec(db, "pragma wal_autocheckpoint = 40000;", 0, 0, 0) != SQLITE_OK)
-        throwSQLiteError(db, "setting autocheckpoint interval");
+        SQLiteError::throw_(db, "setting autocheckpoint interval");
 
     /* Initialise the database schema, if necessary. */
     if (create) {
diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc
index 1d82b4ab1..2090beabd 100644
--- a/src/libstore/sqlite.cc
+++ b/src/libstore/sqlite.cc
@@ -8,22 +8,32 @@
 
 namespace nix {
 
-[[noreturn]] void throwSQLiteError(sqlite3 * db, const FormatOrString & fs)
+SQLiteError::SQLiteError(const char *path, int errNo, int extendedErrNo, hintformat && hf)
+  : Error(""), path(path), errNo(errNo), extendedErrNo(extendedErrNo)
+{
+    err.msg = hintfmt("%s: %s (in '%s')",
+        normaltxt(hf.str()),
+        sqlite3_errstr(extendedErrNo),
+        path ? path : "(in-memory)");
+}
+
+[[noreturn]] void SQLiteError::throw_(sqlite3 * db, hintformat && hf)
 {
     int err = sqlite3_errcode(db);
     int exterr = sqlite3_extended_errcode(db);
 
     auto path = sqlite3_db_filename(db, nullptr);
-    if (!path) path = "(in-memory)";
 
     if (err == SQLITE_BUSY || err == SQLITE_PROTOCOL) {
-        throw SQLiteBusy(
+        auto exp = SQLiteBusy(path, err, exterr, std::move(hf));
+        exp.err.msg = hintfmt(
             err == SQLITE_PROTOCOL
-            ? fmt("SQLite database '%s' is busy (SQLITE_PROTOCOL)", path)
-            : fmt("SQLite database '%s' is busy", path));
-    }
-    else
-        throw SQLiteError("%s: %s (in '%s')", fs.s, sqlite3_errstr(exterr), path);
+                ? "SQLite database '%s' is busy (SQLITE_PROTOCOL)"
+                : "SQLite database '%s' is busy",
+            path ? path : "(in-memory)");
+        throw exp;
+    } else
+        throw SQLiteError(path, err, exterr, std::move(hf));
 }
 
 SQLite::SQLite(const Path & path, bool create)
@@ -37,7 +47,7 @@ SQLite::SQLite(const Path & path, bool create)
         throw Error("cannot open SQLite database '%s'", path);
 
     if (sqlite3_busy_timeout(db, 60 * 60 * 1000) != SQLITE_OK)
-        throwSQLiteError(db, "setting timeout");
+        SQLiteError::throw_(db, "setting timeout");
 
     exec("pragma foreign_keys = 1");
 }
@@ -46,7 +56,7 @@ SQLite::~SQLite()
 {
     try {
         if (db && sqlite3_close(db) != SQLITE_OK)
-            throwSQLiteError(db, "closing database");
+            SQLiteError::throw_(db, "closing database");
     } catch (...) {
         ignoreException();
     }
@@ -62,7 +72,7 @@ void SQLite::exec(const std::string & stmt)
 {
     retrySQLite<void>([&]() {
         if (sqlite3_exec(db, stmt.c_str(), 0, 0, 0) != SQLITE_OK)
-            throwSQLiteError(db, format("executing SQLite statement '%s'") % stmt);
+            SQLiteError::throw_(db, "executing SQLite statement '%s'", stmt);
     });
 }
 
@@ -76,7 +86,7 @@ void SQLiteStmt::create(sqlite3 * db, const std::string & sql)
     checkInterrupt();
     assert(!stmt);
     if (sqlite3_prepare_v2(db, sql.c_str(), -1, &stmt, 0) != SQLITE_OK)
-        throwSQLiteError(db, fmt("creating statement '%s'", sql));
+        SQLiteError::throw_(db, "creating statement '%s'", sql);
     this->db = db;
     this->sql = sql;
 }
@@ -85,7 +95,7 @@ SQLiteStmt::~SQLiteStmt()
 {
     try {
         if (stmt && sqlite3_finalize(stmt) != SQLITE_OK)
-            throwSQLiteError(db, fmt("finalizing statement '%s'", sql));
+            SQLiteError::throw_(db, "finalizing statement '%s'", sql);
     } catch (...) {
         ignoreException();
     }
@@ -109,7 +119,7 @@ SQLiteStmt::Use & SQLiteStmt::Use::operator () (std::string_view value, bool not
 {
     if (notNull) {
         if (sqlite3_bind_text(stmt, curArg++, value.data(), -1, SQLITE_TRANSIENT) != SQLITE_OK)
-            throwSQLiteError(stmt.db, "binding argument");
+            SQLiteError::throw_(stmt.db, "binding argument");
     } else
         bind();
     return *this;
@@ -119,7 +129,7 @@ SQLiteStmt::Use & SQLiteStmt::Use::operator () (const unsigned char * data, size
 {
     if (notNull) {
         if (sqlite3_bind_blob(stmt, curArg++, data, len, SQLITE_TRANSIENT) != SQLITE_OK)
-            throwSQLiteError(stmt.db, "binding argument");
+            SQLiteError::throw_(stmt.db, "binding argument");
     } else
         bind();
     return *this;
@@ -129,7 +139,7 @@ SQLiteStmt::Use & SQLiteStmt::Use::operator () (int64_t value, bool notNull)
 {
     if (notNull) {
         if (sqlite3_bind_int64(stmt, curArg++, value) != SQLITE_OK)
-            throwSQLiteError(stmt.db, "binding argument");
+            SQLiteError::throw_(stmt.db, "binding argument");
     } else
         bind();
     return *this;
@@ -138,7 +148,7 @@ SQLiteStmt::Use & SQLiteStmt::Use::operator () (int64_t value, bool notNull)
 SQLiteStmt::Use & SQLiteStmt::Use::bind()
 {
     if (sqlite3_bind_null(stmt, curArg++) != SQLITE_OK)
-        throwSQLiteError(stmt.db, "binding argument");
+        SQLiteError::throw_(stmt.db, "binding argument");
     return *this;
 }
 
@@ -152,14 +162,14 @@ void SQLiteStmt::Use::exec()
     int r = step();
     assert(r != SQLITE_ROW);
     if (r != SQLITE_DONE)
-        throwSQLiteError(stmt.db, fmt("executing SQLite statement '%s'", sqlite3_expanded_sql(stmt.stmt)));
+        SQLiteError::throw_(stmt.db, fmt("executing SQLite statement '%s'", sqlite3_expanded_sql(stmt.stmt)));
 }
 
 bool SQLiteStmt::Use::next()
 {
     int r = step();
     if (r != SQLITE_DONE && r != SQLITE_ROW)
-        throwSQLiteError(stmt.db, fmt("executing SQLite query '%s'", sqlite3_expanded_sql(stmt.stmt)));
+        SQLiteError::throw_(stmt.db, fmt("executing SQLite query '%s'", sqlite3_expanded_sql(stmt.stmt)));
     return r == SQLITE_ROW;
 }
 
@@ -185,14 +195,14 @@ SQLiteTxn::SQLiteTxn(sqlite3 * db)
 {
     this->db = db;
     if (sqlite3_exec(db, "begin;", 0, 0, 0) != SQLITE_OK)
-        throwSQLiteError(db, "starting transaction");
+        SQLiteError::throw_(db, "starting transaction");
     active = true;
 }
 
 void SQLiteTxn::commit()
 {
     if (sqlite3_exec(db, "commit;", 0, 0, 0) != SQLITE_OK)
-        throwSQLiteError(db, "committing transaction");
+        SQLiteError::throw_(db, "committing transaction");
     active = false;
 }
 
@@ -200,7 +210,7 @@ SQLiteTxn::~SQLiteTxn()
 {
     try {
         if (active && sqlite3_exec(db, "rollback;", 0, 0, 0) != SQLITE_OK)
-            throwSQLiteError(db, "aborting transaction");
+            SQLiteError::throw_(db, "aborting transaction");
     } catch (...) {
         ignoreException();
     }
diff --git a/src/libstore/sqlite.hh b/src/libstore/sqlite.hh
index 99f0d56ce..3a4ad8633 100644
--- a/src/libstore/sqlite.hh
+++ b/src/libstore/sqlite.hh
@@ -96,10 +96,30 @@ struct SQLiteTxn
 };
 
 
-MakeError(SQLiteError, Error);
-MakeError(SQLiteBusy, SQLiteError);
+struct SQLiteError : Error
+{
+    const char *path;
+    int errNo, extendedErrNo;
+
+    template<typename... Args>
+    [[noreturn]] static void throw_(sqlite3 * db, const std::string & fs, const Args & ... args) {
+        throw_(db, hintfmt(fs, args...));
+    }
+
+protected:
+
+    SQLiteError(const char *path, int errNo, int extendedErrNo, hintformat && hf);
 
-[[noreturn]] void throwSQLiteError(sqlite3 * db, const FormatOrString & fs);
+    template<typename... Args>
+    SQLiteError(const char *path, int errNo, int extendedErrNo, const std::string & fs, const Args & ... args)
+      : SQLiteError(path, errNo, extendedErrNo, hintfmt(fs, args...))
+    { }
+
+    [[noreturn]] static void throw_(sqlite3 * db, hintformat && hf);
+
+};
+
+MakeError(SQLiteBusy, SQLiteError);
 
 void handleSQLiteBusy(const SQLiteBusy & e);
 
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index 656804007..b49c1e466 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -1082,7 +1082,7 @@ std::string runProgram(Path program, bool searchPath, const Strings & args,
     auto res = runProgram(RunOptions {.program = program, .searchPath = searchPath, .args = args, .input = input});
 
     if (!statusOk(res.first))
-        throw ExecError(res.first, fmt("program '%1%' %2%", program, statusToString(res.first)));
+        throw ExecError(res.first, "program '%1%' %2%", program, statusToString(res.first));
 
     return res.second;
 }
@@ -1210,7 +1210,7 @@ void runProgram2(const RunOptions & options)
     if (source) promise.get_future().get();
 
     if (status)
-        throw ExecError(status, fmt("program '%1%' %2%", options.program, statusToString(status)));
+        throw ExecError(status, "program '%1%' %2%", options.program, statusToString(status));
 }
 
 
diff --git a/src/nix/repl.cc b/src/nix/repl.cc
index b055698b3..0eb037858 100644
--- a/src/nix/repl.cc
+++ b/src/nix/repl.cc
@@ -120,7 +120,7 @@ std::string runNix(Path program, const Strings & args,
     });
 
     if (!statusOk(res.first))
-        throw ExecError(res.first, fmt("program '%1%' %2%", program, statusToString(res.first)));
+        throw ExecError(res.first, "program '%1%' %2%", program, statusToString(res.first));
 
     return res.second;
 }