Separate SystemError from SysError

Most of this is a `catch SysError` -> `catch SystemError` sed. This
is a rather pure-churn change I would like to get out of the way. **The
intersting part is `src/libutil/error.hh`.**

On Unix, we will only throw the `SysError` concrete class, which has
the same constructors that `SystemError` used to have.

On Windows, we will throw `WinError` *and* `SysError`. `WinError`
(which will be created in a later PR), will use a `DWORD` instead of
`int` error value, and `GetLastError()`, which is the Windows equivalent
of the `errno` machinery. Windows will *also* use `SysError` because
Window's "libc" (MSVCRT) implements the POSIX interface, and we use it
too.

As the docs describe, while we *throw* one of the 3 choices above (2
concrete classes or the alias), we should always *catch* `SystemError`.
This ensures no matter how the implementation changes for Windows (e.g.
between `SysError` and `WinError`) the catching logic stays the same
and stays correct.

Co-Authored-By volth <volth@volth.com>
Co-Authored-By Eugene Butler <eugene@eugene4.com>

src/libcmd/repl.cc

@@ -254,7 +254,7 @@ void NixRepl::mainLoop()
     rl_readline_name = "nix-repl";
     try {
         createDirs(dirOf(historyFile));
-    } catch (SysError & e) {
+    } catch (SystemError & e) {
         logWarning(e.info());
     }
 #ifndef USE_READLINE

src/libstore/build/local-derivation-goal.cc

@@ -1495,7 +1495,7 @@ void LocalDerivationGoal::startDaemon()
                     daemon::processConnection(store, from, to,
                         NotTrusted, daemon::Recursive);
                     debug("terminated daemon connection");
-                } catch (SysError &) {
+                } catch (SystemError &) {
                     ignoreException();
                 }
             });
@@ -1707,7 +1707,7 @@ void LocalDerivationGoal::runChild()
         try {
             if (drv->isBuiltin() && drv->builder == "builtin:fetchurl")
                 netrcData = readFile(settings.netrcFile);
-        } catch (SysError &) { }
+        } catch (SystemError &) { }
 
 #if __linux__
         if (useChroot) {

src/libstore/gc.cc

@@ -413,7 +413,7 @@ void LocalStore::findRuntimeRoots(Roots & roots, bool censor)
                     auto env_end = std::sregex_iterator{};
                     for (auto i = std::sregex_iterator{envString.begin(), envString.end(), storePathRegex}; i != env_end; ++i)
                         unchecked[i->str()].emplace(envFile);
-                } catch (SysError & e) {
+                } catch (SystemError & e) {
                     if (errno == ENOENT || errno == EACCES || errno == ESRCH)
                         continue;
                     throw;

src/libstore/globals.cc

@@ -118,7 +118,7 @@ void loadConfFile()
         try {
             std::string contents = readFile(path);
             globalConfig.applyConfig(contents, path);
-        } catch (SysError &) { }
+        } catch (SystemError &) { }
     };
 
     applyConfigFile(settings.nixConfDir + "/nix.conf");

src/libstore/keys.cc

@@ -19,7 +19,7 @@ PublicKeys getDefaultPublicKeys()
         try {
             SecretKey secretKey(readFile(secretKeyFile));
             publicKeys.emplace(secretKey.name, secretKey.toPublicKey());
-        } catch (SysError & e) {
+        } catch (SystemError & e) {
             /* Ignore unreadable key files. That's normal in a
                multi-user installation. */
         }

src/libstore/local-store.cc

@@ -276,7 +276,7 @@ LocalStore::LocalStore(const Params & params)
                 [[gnu::unused]] auto res2 = ftruncate(fd.get(), settings.reservedSize);
             }
         }
-    } catch (SysError & e) { /* don't care about errors */
+    } catch (SystemError & e) { /* don't care about errors */
     }
 
     /* Acquire the big fat lock in shared mode to make sure that no

src/libstore/optimise-store.cc

@@ -242,7 +242,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
     /* Atomically replace the old file with the new hard link. */
     try {
         renameFile(tempLink, path);
-    } catch (SysError & e) {
+    } catch (SystemError & e) {
         if (unlink(tempLink.c_str()) == -1)
             printError("unable to unlink '%1%'", tempLink);
         if (errno == EMLINK) {

src/libstore/remote-fs-accessor.cc

@@ -87,13 +87,13 @@ std::pair<ref<SourceAccessor>, CanonPath> RemoteFSAccessor::fetch(const CanonPat
             nars.emplace(storePath.hashPart(), narAccessor);
             return {narAccessor, restPath};
 
-        } catch (SysError &) { }
+        } catch (SystemError &) { }
 
         try {
             auto narAccessor = makeNarAccessor(nix::readFile(cacheFile));
             nars.emplace(storePath.hashPart(), narAccessor);
             return {narAccessor, restPath};
-        } catch (SysError &) { }
+        } catch (SystemError &) { }
     }
 
     StringSink sink;

src/libutil/args.cc

@@ -304,7 +304,7 @@ void RootArgs::parseCmdline(const Strings & _cmdline, bool allowShebang)
                 for (auto pos = savedArgs.begin(); pos != savedArgs.end();pos++)
                     cmdline.push_back(*pos);
             }
-        } catch (SysError &) { }
+        } catch (SystemError &) { }
     }
     for (auto pos = cmdline.begin(); pos != cmdline.end(); ) {
 

src/libutil/cgroup.cc

@@ -95,7 +95,7 @@ static CgroupStats destroyCgroup(const Path & cgroup, bool returnStats)
                     using namespace std::string_literals;
                     warn("killing stray builder process %d (%s)...",
                         pid, trim(replaceStrings(cmdline, "\0"s, " ")));
-                } catch (SysError &) {
+                } catch (SystemError &) {
                 }
             }
             // FIXME: pid wraparound

src/libutil/config.cc

@@ -124,7 +124,7 @@ static void applyConfigInner(const std::string & contents, const std::string & p
                 try {
                     std::string includedContents = readFile(path);
                     applyConfigInner(includedContents, p, parsedContents);
-                } catch (SysError &) {
+                } catch (SystemError &) {
                     // TODO: Do we actually want to ignore this? Or is it better to fail?
                 }
             } else if (!ignoreMissing) {

src/libutil/error.hh

@@ -178,28 +178,55 @@ MakeError(Error, BaseError);
 MakeError(UsageError, Error);
 MakeError(UnimplementedError, Error);
 
-class SysError : public Error
+/**
+ * To use in catch-blocks.
+ */
+MakeError(SystemError, Error);
+
+/**
+ * POSIX system error, created using `errno`, `strerror` friends.
+ *
+ * Throw this, but do not catch this! Catch `SystemError` instead. This
+ * allows implementations to freely switch between this and `WinError`
+ * without breaking catch blocks.
+ *
+ * @todo Rename this to `PosixError` or similar. At this point Windows
+ * support is too WIP to justify the code churn, but if it is finished
+ * then a better identifier becomes moe worth it.
+ */
+class SysError : public SystemError
 {
 public:
     int errNo;
 
+    /**
+     * Construct using the explicitly-provided error number. `strerror`
+     * will be used to try to add additional information to the message.
+     */
     template<typename... Args>
-    SysError(int errNo_, const Args & ... args)
-        : Error("")
+    SysError(int errNo, const Args & ... args)
+        : SystemError(""), errNo(errNo)
     {
-        errNo = errNo_;
         auto hf = hintfmt(args...);
         err.msg = hintfmt("%1%: %2%", normaltxt(hf.str()), strerror(errNo));
     }
 
+    /**
+     * Construct using the ambient `errno`.
+     *
+     * Be sure to not perform another `errno`-modifying operation before
+     * calling this constructor!
+     */
     template<typename... Args>
     SysError(const Args & ... args)
         : SysError(errno, args ...)
     {
     }
 };
 
-/** Throw an exception for the purpose of checking that exception handling works; see 'initLibUtil()'.
+/**
+ * Throw an exception for the purpose of checking that exception
+ * handling works; see 'initLibUtil()'.
  */
 void throwExceptionSelfCheck();
 

src/libutil/file-descriptor.cc

@@ -231,7 +231,7 @@ void closeMostFDs(const std::set<int> & exceptions)
             }
         }
         return;
-    } catch (SysError &) {
+    } catch (SystemError &) {
     }
 #endif
 

src/libutil/logging.cc

@@ -116,7 +116,7 @@ void writeToStderr(std::string_view s)
 {
     try {
         writeFull(STDERR_FILENO, s, false);
-    } catch (SysError & e) {
+    } catch (SystemError & e) {
         /* Ignore failing writes to stderr.  We need to ignore write
            errors to ensure that cleanup code that logs to stderr runs
            to completion if the other side of stderr has been closed

src/libutil/namespaces.cc

@@ -52,7 +52,7 @@ bool userNamespacesSupported()
 
             auto r = pid.wait();
             assert(!r);
-        } catch (SysError & e) {
+        } catch (SystemError & e) {
             debug("user namespaces do not work on this system: %s", e.msg());
             return false;
         }
@@ -91,7 +91,7 @@ bool mountAndPidNamespacesSupported()
                 return false;
             }
 
-        } catch (SysError & e) {
+        } catch (SystemError & e) {
             debug("mount namespaces do not work on this system: %s", e.msg());
             return false;
         }

src/libutil/serialise.cc

@@ -53,7 +53,7 @@ void FdSink::writeUnbuffered(std::string_view data)
     written += data.size();
     try {
         writeFull(fd, data);
-    } catch (SysError & e) {
+    } catch (SystemError & e) {
         _good = false;
         throw;
     }

src/libutil/util.cc

@@ -20,7 +20,7 @@ void initLibUtil() {
     // When exception handling fails, the message tends to be printed by the
     // C++ runtime, followed by an abort.
     // For example on macOS we might see an error such as
-    // libc++abi: terminating with uncaught exception of type nix::SysError: error: C++ exception handling is broken. This would appear to be a problem with the way Nix was compiled and/or linked and/or loaded.
+    // libc++abi: terminating with uncaught exception of type nix::SystemError: error: C++ exception handling is broken. This would appear to be a problem with the way Nix was compiled and/or linked and/or loaded.
     bool caught = false;
     try {
         throwExceptionSelfCheck();

src/nix-build/nix-build.cc

@@ -148,7 +148,7 @@ static void main_nix_build(int argc, char * * argv)
                             args.push_back(word);
                 }
             }
-        } catch (SysError &) { }
+        } catch (SystemError &) { }
     }
 
     struct MyArgs : LegacyArgs, MixEvalArgs

src/nix/config-check.cc

@@ -107,7 +107,7 @@ struct CmdConfigCheck : StoreCommand
                     if (profileDir.find("/profiles/") == std::string::npos)
                         dirs.insert(dir);
                 }
-            } catch (SysError &) {}
+            } catch (SystemError &) {}
         }
 
         if (!dirs.empty()) {

tests/unit/libutil/logging.cc

@@ -73,20 +73,20 @@ namespace nix {
 
     }
 
-    TEST(logEI, picksUpSysErrorExitCode) {
+    TEST(logEI, picksUpSystemErrorExitCode) {
 
         MakeError(TestError, Error);
         ErrorInfo::programName = std::optional("error-unit-test");
 
         try {
             auto x = readFile(-1);
         }
-        catch (SysError &e) {
+        catch (SystemError &e) {
             testing::internal::CaptureStderr();
             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\nstatting file: \x1B[33;1mBad file descriptor\x1B[0m\n");
+            ASSERT_STREQ(str.c_str(), "\x1B[31;1merror:\x1B[0m\x1B[34;1m --- SystemError --- error-unit-test\x1B[0m\nstatting file: \x1B[33;1mBad file descriptor\x1B[0m\n");
         }
     }