about summary refs log tree commit diff
diff options
context:
space:
mode:
authorGriffin Smith <grfn@gws.fyi>2020-09-07T17·01-0400
committerglittershark <grfn@gws.fyi>2020-09-14T21·38+0000
commit381ce8a66658ac9d02c44e96c860cd05bcb6a5f8 (patch)
treee475adb3f06e4bfa55af5ced0c16f9591734544d
parent31b06516f315241c40ae0e6c3dd9dc7e641ea8dc (diff)
refactor(tvix): Make static strings constexpr string_views r/1791
Make all static std::strings constexpr std::string_views, and replace
concatenation with absl::StrCat where necessary.

Technically all of these are constant, so they really don't need to be
top-level statics - and since I'm trying to get rid of as much global
state as possible in preparation for making the nix daemon properly
multithreaded I figured I'd knock these out while I was at it.

Change-Id: Ibd3ad9ef68f0a0eacb135541b39fdb13dae042e1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1939
Tested-by: BuildkiteCI
Reviewed-by: tazjin <mail@tazj.in>
-rw-r--r--third_party/nix/src/libstore/binary-cache-store.cc2
-rw-r--r--third_party/nix/src/libstore/build.cc6
-rw-r--r--third_party/nix/src/libstore/gc.cc25
-rw-r--r--third_party/nix/src/libstore/legacy-ssh-store.cc10
-rw-r--r--third_party/nix/src/libstore/rpc-store.cc4
-rw-r--r--third_party/nix/src/libstore/ssh-store.cc23
-rw-r--r--third_party/nix/src/libutil/archive.cc16
-rw-r--r--third_party/nix/src/libutil/archive.hh2
8 files changed, 46 insertions, 42 deletions
diff --git a/third_party/nix/src/libstore/binary-cache-store.cc b/third_party/nix/src/libstore/binary-cache-store.cc
index 91da7e2265..e5b7ef2cdc 100644
--- a/third_party/nix/src/libstore/binary-cache-store.cc
+++ b/third_party/nix/src/libstore/binary-cache-store.cc
@@ -30,7 +30,7 @@ BinaryCacheStore::BinaryCacheStore(const Params& params) : Store(params) {
   }
 
   StringSink sink;
-  sink << narVersionMagic1;
+  sink << std::string(kNarVersionMagic1);
   narMagic = *sink.s;
 }
 
diff --git a/third_party/nix/src/libstore/build.cc b/third_party/nix/src/libstore/build.cc
index a009651c9d..e50f4cfa99 100644
--- a/third_party/nix/src/libstore/build.cc
+++ b/third_party/nix/src/libstore/build.cc
@@ -81,7 +81,7 @@
 
 namespace nix {
 
-static std::string pathNullDevice = "/dev/null";
+constexpr std::string_view kPathNullDevice = "/dev/null";
 
 /* Forward definition. */
 class Worker;
@@ -448,9 +448,9 @@ static void commonChildInit(Pipe& logPipe) {
   }
 
   /* Reroute stdin to /dev/null. */
-  int fdDevNull = open(pathNullDevice.c_str(), O_RDWR);
+  int fdDevNull = open(kPathNullDevice.begin(), O_RDWR);
   if (fdDevNull == -1) {
-    throw SysError(format("cannot open '%1%'") % pathNullDevice);
+    throw SysError(format("cannot open '%1%'") % kPathNullDevice);
   }
   if (dup2(fdDevNull, STDIN_FILENO) == -1) {
     throw SysError("cannot dup null device into stdin");
diff --git a/third_party/nix/src/libstore/gc.cc b/third_party/nix/src/libstore/gc.cc
index 50c52bb009..07dc10629a 100644
--- a/third_party/nix/src/libstore/gc.cc
+++ b/third_party/nix/src/libstore/gc.cc
@@ -7,6 +7,7 @@
 #include <regex>
 
 #include <absl/strings/match.h>
+#include <absl/strings/str_cat.h>
 #include <absl/strings/str_split.h>
 #include <fcntl.h>
 #include <glog/logging.h>
@@ -22,8 +23,8 @@
 
 namespace nix {
 
-static std::string gcLockName = "gc.lock";
-static std::string gcRootsDir = "gcroots";
+constexpr std::string_view kGcLockName = "gc.lock";
+constexpr std::string_view kGcRootsDir = "gcroots";
 
 /* Acquire the global GC lock.  This is used to prevent new Nix
    processes from starting after the temporary root files have been
@@ -31,7 +32,7 @@ static std::string gcRootsDir = "gcroots";
    file, they will block until the garbage collector has finished /
    yielded the GC lock. */
 AutoCloseFD LocalStore::openGCLock(LockType lockType) {
-  Path fnGCLock = (format("%1%/%2%") % stateDir % gcLockName).str();
+  Path fnGCLock = absl::StrCat(stateDir.get(), "/", kGcLockName);
 
   DLOG(INFO) << "acquiring global GC lock " << fnGCLock;
 
@@ -73,8 +74,8 @@ void LocalStore::syncWithGC() { AutoCloseFD fdGCLock = openGCLock(ltRead); }
 
 void LocalStore::addIndirectRoot(const Path& path) {
   std::string hash = hashString(htSHA1, path).to_string(Base32, false);
-  Path realRoot = canonPath(
-      (format("%1%/%2%/auto/%3%") % stateDir % gcRootsDir % hash).str());
+  Path realRoot =
+      canonPath(absl::StrCat(stateDir.get(), "/", kGcRootsDir, "/auto/", hash));
   makeSymlink(realRoot, path);
 }
 
@@ -105,8 +106,7 @@ Path LocalFSStore::addPermRoot(const Path& _storePath, const Path& _gcRoot,
 
   else {
     if (!allowOutsideRootsDir) {
-      Path rootsDir =
-          canonPath((format("%1%/%2%") % stateDir % gcRootsDir).str());
+      Path rootsDir = canonPath(absl::StrCat(stateDir.get(), "/", kGcRootsDir));
 
       if (std::string(gcRoot, 0, rootsDir.size() + 1) != rootsDir + "/") {
         throw Error(format("path '%1%' is not a valid garbage collector root; "
@@ -195,7 +195,7 @@ void LocalStore::addTempRoot(const Path& path) {
   lockFile(state->fdTempRoots.get(), ltRead, true);
 }
 
-static std::string censored = "{censored}";
+constexpr std::string_view kCensored = "{censored}";
 
 void LocalStore::findTempRoots(FDs& fds, Roots& tempRoots, bool censor) {
   /* Read the `temproots' directory for per-process temporary root
@@ -247,7 +247,7 @@ void LocalStore::findTempRoots(FDs& fds, Roots& tempRoots, bool censor) {
       Path root(contents, pos, end - pos);
       DLOG(INFO) << "got temporary root " << root;
       assertStorePath(root);
-      tempRoots[root].emplace(censor ? censored : fmt("{temp:%d}", pid));
+      tempRoots[root].emplace(censor ? kCensored : fmt("{temp:%d}", pid));
       pos = end + 1;
     }
 
@@ -287,7 +287,8 @@ void LocalStore::findRoots(const Path& path, unsigned char type, Roots& roots) {
       else {
         target = absPath(target, dirOf(path));
         if (!pathExists(target)) {
-          if (isInDir(path, stateDir + "/" + gcRootsDir + "/auto")) {
+          if (isInDir(path, absl::StrCat(stateDir.get(), "/", kGcRootsDir,
+                                         "/auto"))) {
             LOG(INFO) << "removing stale link from '" << path << "' to '"
                       << target << "'";
             unlink(path.c_str());
@@ -326,7 +327,7 @@ void LocalStore::findRoots(const Path& path, unsigned char type, Roots& roots) {
 
 void LocalStore::findRootsNoTemp(Roots& roots, bool censor) {
   /* Process direct roots in {gcroots,profiles}. */
-  findRoots(stateDir + "/" + gcRootsDir, DT_UNKNOWN, roots);
+  findRoots(absl::StrCat(stateDir.get(), "/", kGcRootsDir), DT_UNKNOWN, roots);
   findRoots(stateDir + "/profiles", DT_UNKNOWN, roots);
 
   /* Add additional roots returned by different platforms-specific
@@ -465,7 +466,7 @@ void LocalStore::findRuntimeRoots(Roots& roots, bool censor) {
       if (isStorePath(path) && isValidPath(path)) {
         DLOG(INFO) << "got additional root " << path;
         if (censor) {
-          roots[path].insert(censored);
+          roots[path].insert(std::string(kCensored));
         } else {
           roots[path].insert(links.begin(), links.end());
         }
diff --git a/third_party/nix/src/libstore/legacy-ssh-store.cc b/third_party/nix/src/libstore/legacy-ssh-store.cc
index 343801870a..19603115a8 100644
--- a/third_party/nix/src/libstore/legacy-ssh-store.cc
+++ b/third_party/nix/src/libstore/legacy-ssh-store.cc
@@ -1,3 +1,5 @@
+#include <absl/strings/match.h>
+#include <absl/strings/str_cat.h>
 #include <glog/logging.h>
 
 #include "libstore/derivations.hh"
@@ -11,7 +13,7 @@
 
 namespace nix {
 
-static std::string uriScheme = "ssh://";
+constexpr std::string_view kUriScheme = "ssh://";
 
 struct LegacySSHStore : public Store {
   const Setting<int> maxConnections{
@@ -86,7 +88,7 @@ struct LegacySSHStore : public Store {
     return conn;
   };
 
-  std::string getUri() override { return uriScheme + host; }
+  std::string getUri() override { return absl::StrCat(kUriScheme, host); }
 
   void queryPathInfoUncached(
       const Path& path,
@@ -269,11 +271,11 @@ struct LegacySSHStore : public Store {
 static RegisterStoreImplementation regStore(
     [](const std::string& uri,
        const Store::Params& params) -> std::shared_ptr<Store> {
-      if (std::string(uri, 0, uriScheme.size()) != uriScheme) {
+      if (!absl::StartsWith(uri, kUriScheme)) {
         return nullptr;
       }
       return std::make_shared<LegacySSHStore>(
-          std::string(uri, uriScheme.size()), params);
+          std::string(uri, kUriScheme.size()), params);
     });
 
 }  // namespace nix
diff --git a/third_party/nix/src/libstore/rpc-store.cc b/third_party/nix/src/libstore/rpc-store.cc
index bc1fcc73d8..02975341fd 100644
--- a/third_party/nix/src/libstore/rpc-store.cc
+++ b/third_party/nix/src/libstore/rpc-store.cc
@@ -531,13 +531,13 @@ unsigned int RpcStore::getProtocol() { return PROTOCOL_VERSION; }
 
 }  // namespace store
 
-static std::string uriScheme = "unix://";
+constexpr std::string_view kUriScheme = "unix://";
 
 // TODO(grfn): Make this a function that we call from main rather than... this
 static RegisterStoreImplementation regStore([](const std::string& uri,
                                                const Store::Params& params)
                                                 -> std::shared_ptr<Store> {
-  if (std::string(uri, 0, uriScheme.size()) != uriScheme) {
+  if (std::string(uri, 0, kUriScheme.size()) != kUriScheme) {
     return nullptr;
   }
   auto channel = grpc::CreateChannel(uri, grpc::InsecureChannelCredentials());
diff --git a/third_party/nix/src/libstore/ssh-store.cc b/third_party/nix/src/libstore/ssh-store.cc
index 48fea858a3..96adb3660d 100644
--- a/third_party/nix/src/libstore/ssh-store.cc
+++ b/third_party/nix/src/libstore/ssh-store.cc
@@ -1,3 +1,5 @@
+#include <absl/strings/str_cat.h>
+
 #include "libstore/remote-fs-accessor.hh"
 #include "libstore/remote-store.hh"
 #include "libstore/ssh.hh"
@@ -8,7 +10,7 @@
 
 namespace nix {
 
-static std::string uriScheme = "ssh-ng://";
+constexpr std::string_view kUriScheme = "ssh-ng://";
 
 class SSHStore : public RemoteStore {
  public:
@@ -25,7 +27,7 @@ class SSHStore : public RemoteStore {
                // Use SSH master only if using more than 1 connection.
                connections->capacity() > 1, compress) {}
 
-  std::string getUri() override { return uriScheme + host; }
+  std::string getUri() override { return absl::StrCat(kUriScheme, host); }
 
   bool sameMachine() override { return false; }
 
@@ -74,13 +76,14 @@ ref<RemoteStore::Connection> SSHStore::openConnection() {
   return conn;
 }
 
-static RegisterStoreImplementation regStore([](const std::string& uri,
-                                               const Store::Params& params)
-                                                -> std::shared_ptr<Store> {
-  if (std::string(uri, 0, uriScheme.size()) != uriScheme) {
-    return nullptr;
-  }
-  return std::make_shared<SSHStore>(std::string(uri, uriScheme.size()), params);
-});
+static RegisterStoreImplementation regStore(
+    [](const std::string& uri,
+       const Store::Params& params) -> std::shared_ptr<Store> {
+      if (std::string(uri, 0, kUriScheme.size()) != kUriScheme) {
+        return nullptr;
+      }
+      return std::make_shared<SSHStore>(std::string(uri, kUriScheme.size()),
+                                        params);
+    });
 
 }  // namespace nix
diff --git a/third_party/nix/src/libutil/archive.cc b/third_party/nix/src/libutil/archive.cc
index e3233d9ee4..e470ad7be6 100644
--- a/third_party/nix/src/libutil/archive.cc
+++ b/third_party/nix/src/libutil/archive.cc
@@ -36,9 +36,7 @@ static ArchiveSettings archiveSettings;
 
 static GlobalConfig::Register r1(&archiveSettings);
 
-const std::string narVersionMagic1 = "nix-archive-1";
-
-static std::string caseHackSuffix = "~nix~case~hack~";
+constexpr std::string_view kCaseHackSuffix = "~nix~case~hack~";
 
 PathFilter defaultPathFilter = [](const Path& /*unused*/) { return true; };
 
@@ -93,7 +91,7 @@ static void dump(const Path& path, Sink& sink, PathFilter& filter) {
     for (auto& i : readDirectory(path)) {
       if (archiveSettings.useCaseHack) {
         std::string name(i.name);
-        size_t pos = i.name.find(caseHackSuffix);
+        size_t pos = i.name.find(kCaseHackSuffix);
         if (pos != std::string::npos) {
           DLOG(INFO) << "removing case hack suffix from " << path << "/"
                      << i.name;
@@ -134,12 +132,12 @@ static void dump(const Path& path, Sink& sink, PathFilter& filter) {
 }
 
 void dumpPath(const Path& path, Sink& sink, PathFilter& filter) {
-  sink << narVersionMagic1;
+  sink << std::string(kNarVersionMagic1);
   dump(path, sink, filter);
 }
 
 void dumpString(const std::string& s, Sink& sink) {
-  sink << narVersionMagic1 << "("
+  sink << std::string(kNarVersionMagic1) << "("
        << "type"
        << "regular"
        << "contents" << s << ")";
@@ -279,7 +277,7 @@ static void parse(ParseSink& sink, Source& source, const Path& path) {
             if (i != names.end()) {
               DLOG(INFO) << "case collision between '" << i->first << "' and '"
                          << name << "'";
-              name += caseHackSuffix;
+              name += kCaseHackSuffix;
               name += std::to_string(++i->second);
             } else {
               names[name] = 0;
@@ -310,12 +308,12 @@ static void parse(ParseSink& sink, Source& source, const Path& path) {
 void parseDump(ParseSink& sink, Source& source) {
   std::string version;
   try {
-    version = readString(source, narVersionMagic1.size());
+    version = readString(source, kNarVersionMagic1.size());
   } catch (SerialisationError& e) {
     /* This generally means the integer at the start couldn't be
        decoded.  Ignore and throw the exception below. */
   }
-  if (version != narVersionMagic1) {
+  if (version != kNarVersionMagic1) {
     throw badArchive("input doesn't look like a Nix archive");
   }
   parse(sink, source, "");
diff --git a/third_party/nix/src/libutil/archive.hh b/third_party/nix/src/libutil/archive.hh
index 194d31d078..3966278785 100644
--- a/third_party/nix/src/libutil/archive.hh
+++ b/third_party/nix/src/libutil/archive.hh
@@ -72,6 +72,6 @@ void restorePath(const Path& path, Source& source);
 /* Read a NAR from 'source' and write it to 'sink'. */
 void copyNAR(Source& source, Sink& sink);
 
-extern const std::string narVersionMagic1;
+constexpr std::string_view kNarVersionMagic1 = "nix-archive-1";
 
 }  // namespace nix