Put worker protocol items inside a `WorkerProto` struct

See API docs on that struct for why. The pasing as as template argument
doesn't yet happen in that commit, but will instead happen in later
commit.

Also make `WorkerOp` (now `Op`) and enum struct. This led us to catch
that two operations were not handled!

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>

1 files changed, 118 insertions, 68 deletions

diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh
index 18a4e11b2..cd6801290 100644
--- a/src/libstore/worker-protocol.hh
+++ b/src/libstore/worker-protocol.hh
@@ -14,57 +14,6 @@ namespace nix {
 #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff)
 
 
-/**
- * Enumeration of all the request types for the "worker protocol", used
- * by unix:// and ssh-ng:// stores.
- */
-typedef enum {
-    wopIsValidPath = 1,
-    wopHasSubstitutes = 3,
-    wopQueryPathHash = 4, // obsolete
-    wopQueryReferences = 5, // obsolete
-    wopQueryReferrers = 6,
-    wopAddToStore = 7,
-    wopAddTextToStore = 8, // obsolete since 1.25, Nix 3.0. Use wopAddToStore
-    wopBuildPaths = 9,
-    wopEnsurePath = 10,
-    wopAddTempRoot = 11,
-    wopAddIndirectRoot = 12,
-    wopSyncWithGC = 13,
-    wopFindRoots = 14,
-    wopExportPath = 16, // obsolete
-    wopQueryDeriver = 18, // obsolete
-    wopSetOptions = 19,
-    wopCollectGarbage = 20,
-    wopQuerySubstitutablePathInfo = 21,
-    wopQueryDerivationOutputs = 22, // obsolete
-    wopQueryAllValidPaths = 23,
-    wopQueryFailedPaths = 24,
-    wopClearFailedPaths = 25,
-    wopQueryPathInfo = 26,
-    wopImportPaths = 27, // obsolete
-    wopQueryDerivationOutputNames = 28, // obsolete
-    wopQueryPathFromHashPart = 29,
-    wopQuerySubstitutablePathInfos = 30,
-    wopQueryValidPaths = 31,
-    wopQuerySubstitutablePaths = 32,
-    wopQueryValidDerivers = 33,
-    wopOptimiseStore = 34,
-    wopVerifyStore = 35,
-    wopBuildDerivation = 36,
-    wopAddSignatures = 37,
-    wopNarFromPath = 38,
-    wopAddToStoreNar = 39,
-    wopQueryMissing = 40,
-    wopQueryDerivationOutputMap = 41,
-    wopRegisterDrvOutput = 42,
-    wopQueryRealisation = 43,
-    wopAddMultipleToStore = 44,
-    wopAddBuildLog = 45,
-    wopBuildPathsWithResults = 46,
-} WorkerOp;
-
-
 #define STDERR_NEXT  0x6f6c6d67
 #define STDERR_READ  0x64617461 // data needed from source
 #define STDERR_WRITE 0x64617416 // data for sink
@@ -78,7 +27,7 @@ typedef enum {
 class Store;
 struct Source;
 
-// items being serialized
+// items being serialised
 struct DerivedPath;
 struct DrvOutput;
 struct Realisation;
@@ -88,31 +37,132 @@ enum TrustedFlag : bool;
 
 
 /**
- * Data type for canonical pairs of serializers for the worker protocol.
+ * The "worker protocol", used by unix:// and ssh-ng:// stores.
  *
- * See https://en.cppreference.com/w/cpp/language/adl for the broader
- * concept of what is going on here.
+ * This `struct` is basically just a `namespace`; We use a type rather
+ * than a namespace just so we can use it as a template argument.
  */
-template<typename T>
-struct WorkerProto {
-    static T read(const Store & store, Source & from);
-    static void write(const Store & store, Sink & out, const T & t);
+struct WorkerProto
+{
+    /**
+     * Enumeration of all the request types for the protocol.
+     */
+    enum struct Op : uint64_t;
+
+    /**
+     * Data type for canonical pairs of serialisers for the worker protocol.
+     *
+     * See https://en.cppreference.com/w/cpp/language/adl for the broader
+     * concept of what is going on here.
+     */
+    template<typename T>
+    struct Serialise;
+    // This is the definition of `Serialise` we *want* to put here, but
+    // do not do so.
+    //
+    // The problem is that if we do so, C++ will think we have
+    // seralisers for *all* types. We don't, of course, but that won't
+    // cause an error until link time. That makes for long debug cycles
+    // when there is a missing serialiser.
+    //
+    // By not defining it globally, and instead letting individual
+    // serialisers specialise the type, we get back the compile-time
+    // errors we would like. When no serialiser exists, C++ sees an
+    // abstract "incomplete" type with no definition, and any attempt to
+    // use `to` or `from` static methods is a compile-time error because
+    // they don't exist on an incomplete type.
+    //
+    // This makes for a quicker debug cycle, as desired.
+#if 0
+    {
+        static T read(const Store & store, Source & from);
+        static void write(const Store & store, Sink & out, const T & t);
+    };
+#endif
+
+    /**
+     * Wrapper function around `WorkerProto::Serialise<T>::write` that allows us to
+     * infer the type instead of having to write it down explicitly.
+     */
+    template<typename T>
+    static void write(const Store & store, Sink & out, const T & t)
+    {
+        WorkerProto::Serialise<T>::write(store, out, t);
+    }
+};
+
+enum struct WorkerProto::Op : uint64_t
+{
+    IsValidPath = 1,
+    HasSubstitutes = 3,
+    QueryPathHash = 4, // obsolete
+    QueryReferences = 5, // obsolete
+    QueryReferrers = 6,
+    AddToStore = 7,
+    AddTextToStore = 8, // obsolete since 1.25, Nix 3.0. Use WorkerProto::Op::AddToStore
+    BuildPaths = 9,
+    EnsurePath = 10,
+    AddTempRoot = 11,
+    AddIndirectRoot = 12,
+    SyncWithGC = 13,
+    FindRoots = 14,
+    ExportPath = 16, // obsolete
+    QueryDeriver = 18, // obsolete
+    SetOptions = 19,
+    CollectGarbage = 20,
+    QuerySubstitutablePathInfo = 21,
+    QueryDerivationOutputs = 22, // obsolete
+    QueryAllValidPaths = 23,
+    QueryFailedPaths = 24,
+    ClearFailedPaths = 25,
+    QueryPathInfo = 26,
+    ImportPaths = 27, // obsolete
+    QueryDerivationOutputNames = 28, // obsolete
+    QueryPathFromHashPart = 29,
+    QuerySubstitutablePathInfos = 30,
+    QueryValidPaths = 31,
+    QuerySubstitutablePaths = 32,
+    QueryValidDerivers = 33,
+    OptimiseStore = 34,
+    VerifyStore = 35,
+    BuildDerivation = 36,
+    AddSignatures = 37,
+    NarFromPath = 38,
+    AddToStoreNar = 39,
+    QueryMissing = 40,
+    QueryDerivationOutputMap = 41,
+    RegisterDrvOutput = 42,
+    QueryRealisation = 43,
+    AddMultipleToStore = 44,
+    AddBuildLog = 45,
+    BuildPathsWithResults = 46,
 };
 
 /**
- * Wrapper function around `WorkerProto<T>::write` that allows us to
- * infer the type instead of having to write it down explicitly.
+ * Convenience for sending operation codes.
+ *
+ * @todo Switch to using `WorkerProto::Serialise` instead probably. But
+ * this was not done at this time so there would be less churn.
  */
-template<typename T>
-void workerProtoWrite(const Store & store, Sink & out, const T & t)
+inline Sink & operator << (Sink & sink, WorkerProto::Op op)
+{
+    return sink << (uint64_t) op;
+}
+
+/**
+ * Convenience for debugging.
+ *
+ * @todo Perhaps render known opcodes more nicely.
+ */
+inline std::ostream & operator << (std::ostream & s, WorkerProto::Op op)
 {
-    WorkerProto<T>::write(store, out, t);
+    return s << (uint64_t) op;
 }
 
 /**
- * Declare a canonical serializer pair for the worker protocol.
+ * Declare a canonical serialiser pair for the worker protocol.
  *
- * We specialize the struct merely to indicate that we are implementing
+ * We specialise the struct merely to indicate that we are implementing
  * the function for the given type.
  *
  * Some sort of `template<...>` must be used with the caller for this to
@@ -120,7 +170,7 @@ void workerProtoWrite(const Store & store, Sink & out, const T & t)
  * practice.
  */
 #define MAKE_WORKER_PROTO(T) \
-    struct WorkerProto< T > { \
+    struct WorkerProto::Serialise< T > { \
         static T read(const Store & store, Source & from); \
         static void write(const Store & store, Sink & out, const T & t); \
     };
@@ -156,7 +206,7 @@ MAKE_WORKER_PROTO(X_);
 
 /**
  * These use the empty string for the null case, relying on the fact
- * that the underlying types never serialize to the empty string.
+ * that the underlying types never serialise to the empty string.
  *
  * We do this instead of a generic std::optional<T> instance because
  * ordinal tags (0 or 1, here) are a bit of a compatability hazard. For