Use "raw pattern" for content address types

We weren't because this ancient PR predated it! This is actually a new version of the pattern which addresses some issues identified in #7479.
2024-11-27 00:06:16 +02:00 · 2023-03-30 17:12:49 -04:00 · 2023-03-30 17:12:49 -04:00 · c51d554c93
commit c51d554c93
parent a6d00a7bfb
14 changed files with 118 additions and 87 deletions
--- a/src/libstore/build/local-derivation-goal.cc
+++ b/src/libstore/build/local-derivation-goal.cc
@ -2508,7 +2508,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
                /* Check wanted hash */
                const Hash & wanted = dof.hash.hash;
                assert(newInfo0.ca);
-                auto got = getContentAddressHash(*newInfo0.ca);
+                auto got = newInfo0.ca->getHash();
                if (wanted != got) {
                    /* Throw an error after registering the path as
                       valid. */
--- a/src/libstore/build/substitution-goal.cc
+++ b/src/libstore/build/substitution-goal.cc
@ -97,7 +97,7 @@ void PathSubstitutionGoal::tryNext()
    if (ca) {
        subPath = sub->makeFixedOutputPathFromCA(
            std::string { storePath.name() },
-            caWithoutRefs(*ca));
+            ContentAddressWithReferences::withoutRefs(*ca));
        if (sub->storeDir == worker.store.storeDir)
            assert(subPath == storePath);
    } else if (sub->storeDir != worker.store.storeDir) {
--- a/src/libstore/content-address.cc
+++ b/src/libstore/content-address.cc
@ -9,7 +9,6 @@ std::string FixedOutputHash::printMethodAlgo() const
    return makeFileIngestionPrefix(method) + printHashType(hash.type);
 }

-
 std::string makeFileIngestionPrefix(FileIngestionMethod m)
 {
    switch (m) {
@ -22,35 +21,35 @@ std::string makeFileIngestionPrefix(FileIngestionMethod m)
    }
 }

-std::string renderContentAddress(ContentAddress ca)
+std::string ContentAddress::render() const
 {
    return std::visit(overloaded {
-        [](TextHash & th) {
+        [](const TextHash & th) {
            return "text:"
                + th.hash.to_string(Base32, true);
        },
-        [](FixedOutputHash & fsh) {
+        [](const FixedOutputHash & fsh) {
            return "fixed:"
                + makeFileIngestionPrefix(fsh.method)
                + fsh.hash.to_string(Base32, true);
        }
-    }, ca);
+    }, raw);
 }

-std::string renderContentAddressMethod(ContentAddressMethod cam)
+std::string ContentAddressMethod::render() const
 {
    return std::visit(overloaded {
-        [](TextHashMethod & th) {
+        [](const TextHashMethod & th) {
            return std::string{"text:"} + printHashType(htSHA256);
        },
-        [](FixedOutputHashMethod & fshm) {
+        [](const FixedOutputHashMethod & fshm) {
            return "fixed:" + makeFileIngestionPrefix(fshm.fileIngestionMethod) + printHashType(fshm.hashType);
        }
-    }, cam);
+    }, raw);
 }

-/*
-  Parses content address strings up to the hash.
+/**
+ * Parses content address strings up to the hash.
 */
 static ContentAddressMethod parseContentAddressMethodPrefix(std::string_view & rest)
 {
@ -94,7 +93,7 @@ static ContentAddressMethod parseContentAddressMethodPrefix(std::string_view & r
        throw UsageError("content address prefix '%s' is unrecognized. Recogonized prefixes are 'text' or 'fixed'", prefix);
 }

-ContentAddress parseContentAddress(std::string_view rawCa) {
+ContentAddress ContentAddress::parse(std::string_view rawCa) {
    auto rest = rawCa;

    ContentAddressMethod caMethod = parseContentAddressMethodPrefix(rest);
@ -112,10 +111,10 @@ ContentAddress parseContentAddress(std::string_view rawCa) {
                    .hash = Hash::parseNonSRIUnprefixed(rest, std::move(fohMethod.hashType)),
                });
            },
-        }, caMethod);
+        }, caMethod.raw);
 }

-ContentAddressMethod parseContentAddressMethod(std::string_view caMethod)
+ContentAddressMethod ContentAddressMethod::parse(std::string_view caMethod)
 {
    std::string asPrefix = std::string{caMethod} + ":";
    // parseContentAddressMethodPrefix takes its argument by reference
@ -123,26 +122,28 @@ ContentAddressMethod parseContentAddressMethod(std::string_view caMethod)
    return parseContentAddressMethodPrefix(asPrefixView);
 }

-std::optional<ContentAddress> parseContentAddressOpt(std::string_view rawCaOpt)
+std::optional<ContentAddress> ContentAddress::parseOpt(std::string_view rawCaOpt)
 {
-    return rawCaOpt == "" ? std::optional<ContentAddress>() : parseContentAddress(rawCaOpt);
+    return rawCaOpt == ""
+        ? std::nullopt
+        : std::optional { ContentAddress::parse(rawCaOpt) };
 };

 std::string renderContentAddress(std::optional<ContentAddress> ca)
 {
-    return ca ? renderContentAddress(*ca) : "";
+    return ca ? ca->render() : "";
 }

-Hash getContentAddressHash(const ContentAddress & ca)
+const Hash & ContentAddress::getHash() const
 {
    return std::visit(overloaded {
-        [](const TextHash & th) {
+        [](const TextHash & th) -> auto & {
            return th.hash;
        },
-        [](const FixedOutputHash & fsh) {
+        [](const FixedOutputHash & fsh) -> auto & {
            return fsh.hash;
        },
-    }, ca);
+    }, raw);
 }

 bool StoreReferences::empty() const
@ -155,7 +156,7 @@ size_t StoreReferences::size() const
    return (self ? 1 : 0) + others.size();
 }

-ContentAddressWithReferences caWithoutRefs(const ContentAddress & ca) {
+ContentAddressWithReferences ContentAddressWithReferences::withoutRefs(const ContentAddress & ca) {
    return std::visit(overloaded {
        [&](const TextHash & h) -> ContentAddressWithReferences {
            return TextInfo {
@ -169,7 +170,7 @@ ContentAddressWithReferences caWithoutRefs(const ContentAddress & ca) {
                .references = {},
            };
        },
-    }, ca);
+    }, ca.raw);
 }

 }
--- a/src/libstore/content-address.hh
+++ b/src/libstore/content-address.hh
@ -39,17 +39,16 @@ enum struct FileIngestionMethod : uint8_t {
    Recursive = true
 };

-struct FixedOutputHashMethod {
-  FileIngestionMethod fileIngestionMethod;
-  HashType hashType;
-};
-
 /**
 * Compute the prefix to the hash algorithm which indicates how the
 * files were ingested.
 */
 std::string makeFileIngestionPrefix(FileIngestionMethod m);

+struct FixedOutputHashMethod {
+  FileIngestionMethod fileIngestionMethod;
+  HashType hashType;
+};

 /**
 * An enumeration of all the ways we can serialize file system objects.
@ -59,14 +58,25 @@ std::string makeFileIngestionPrefix(FileIngestionMethod m);
 * with info on references, and we have `ContentAddressWithReferences`,
 * as defined further below.
 */
-typedef std::variant<
+struct ContentAddressMethod
+{
+    typedef std::variant<
        TextHashMethod,
        FixedOutputHashMethod
-> ContentAddressMethod;
+    > Raw;

-ContentAddressMethod parseContentAddressMethod(std::string_view rawCaMethod);
+    Raw raw;
+
+    /* The moral equivalent of `using Raw::Raw;` */
+    ContentAddressMethod(auto &&... arg)
+        : raw(std::forward<decltype(arg)>(arg)...)
+    { }
+
+    static ContentAddressMethod parse(std::string_view rawCaMethod);
+
+    std::string render() const;
+};

-std::string renderContentAddressMethod(ContentAddressMethod caMethod);

 /*
 * Mini content address
@ -115,25 +125,41 @@ struct FixedOutputHash {
 * - ‘fixed:<r?>:<ht>:<h>’: For paths computed by
 *   Store::makeFixedOutputPath() / Store::addToStore().
 */
-typedef std::variant<
+struct ContentAddress
+{
+    typedef std::variant<
        TextHash,
        FixedOutputHash
-> ContentAddress;
+    > Raw;

-/**
+    Raw raw;
+
+    /* The moral equivalent of `using Raw::Raw;` */
+    ContentAddress(auto &&... arg)
+        : raw(std::forward<decltype(arg)>(arg)...)
+    { }
+
+    /**
     * Compute the content-addressability assertion (ValidPathInfo::ca) for
     * paths created by Store::makeFixedOutputPath() / Store::addToStore().
     */
-std::string renderContentAddress(ContentAddress ca);
+    std::string render() const;
+
+    static ContentAddress parse(std::string_view rawCa);
+
+    static std::optional<ContentAddress> parseOpt(std::string_view rawCaOpt);
+
+    const Hash & getHash() const;
+};

 std::string renderContentAddress(std::optional<ContentAddress> ca);

-ContentAddress parseContentAddress(std::string_view rawCa);
-
-std::optional<ContentAddress> parseContentAddressOpt(std::string_view rawCaOpt);
-
-Hash getContentAddressHash(const ContentAddress & ca);

+/*
+ * Full content address
+ *
+ * See the schema for store paths in store-api.cc
+ */

 /**
 * A set of references to other store objects.
@ -167,12 +193,6 @@ struct StoreReferences {
    GENERATE_CMP(StoreReferences, me->self, me->others);
 };

-/*
- * Full content address
- *
- * See the schema for store paths in store-api.cc
- */
-
 // This matches the additional info that we need for makeTextPath
 struct TextInfo {
    TextHash hash;
@ -200,15 +220,25 @@ struct FixedOutputInfo {
 *
 * A ContentAddress without a Hash.
 */
-typedef std::variant<
+struct ContentAddressWithReferences
+{
+    typedef std::variant<
        TextInfo,
        FixedOutputInfo
-> ContentAddressWithReferences;
+    > Raw;

-/**
+    Raw raw;
+
+    /* The moral equivalent of `using Raw::Raw;` */
+    ContentAddressWithReferences(auto &&... arg)
+        : raw(std::forward<decltype(arg)>(arg)...)
+    { }
+
+    /**
     * Create a ContentAddressWithReferences from a mere ContentAddress, by
     * assuming no references in all cases.
     */
-ContentAddressWithReferences caWithoutRefs(const ContentAddress &);
+    static ContentAddressWithReferences withoutRefs(const ContentAddress &);
+};

 }
--- a/src/libstore/daemon.cc
+++ b/src/libstore/daemon.cc
@ -401,21 +401,21 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
            logger->startWork();
            auto pathInfo = [&]() {
                // NB: FramedSource must be out of scope before logger->stopWork();
-                ContentAddressMethod contentAddressMethod = parseContentAddressMethod(camStr);
+                ContentAddressMethod contentAddressMethod = ContentAddressMethod::parse(camStr);
                FramedSource source(from);
                // TODO this is essentially RemoteStore::addCAToStore. Move it up to Store.
                return std::visit(overloaded {
-                    [&](TextHashMethod &) {
+                    [&](const TextHashMethod &) {
                        // We could stream this by changing Store
                        std::string contents = source.drain();
                        auto path = store->addTextToStore(name, contents, refs, repair);
                        return store->queryPathInfo(path);
                    },
-                    [&](FixedOutputHashMethod & fohm) {
+                    [&](const FixedOutputHashMethod & fohm) {
                        auto path = store->addToStoreFromDump(source, name, fohm.fileIngestionMethod, fohm.hashType, repair, refs);
                        return store->queryPathInfo(path);
                    },
-                }, contentAddressMethod);
+                }, contentAddressMethod.raw);
            }();
            logger->stopWork();

@ -880,7 +880,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
        info.references = worker_proto::read(*store, from, Phantom<StorePathSet> {});
        from >> info.registrationTime >> info.narSize >> info.ultimate;
        info.sigs = readStrings<StringSet>(from);
-        info.ca = parseContentAddressOpt(readString(from));
+        info.ca = ContentAddress::parseOpt(readString(from));
        from >> repair >> dontCheckSigs;
        if (!trusted && dontCheckSigs)
            dontCheckSigs = false;
--- a/src/libstore/legacy-ssh-store.cc
+++ b/src/libstore/legacy-ssh-store.cc
@ -156,7 +156,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
                    throw Error("NAR hash is now mandatory");
                info->narHash = Hash::parseAnyPrefixed(s);
            }
-            info->ca = parseContentAddressOpt(readString(conn->from));
+            info->ca = ContentAddress::parseOpt(readString(conn->from));
            info->sigs = readStrings<StringSet>(conn->from);

            auto s = readString(conn->from);
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@ -944,7 +944,7 @@ std::shared_ptr<const ValidPathInfo> LocalStore::queryPathInfoInternal(State & s
    if (s) info->sigs = tokenizeString<StringSet>(s, " ");

    s = (const char *) sqlite3_column_text(state.stmts->QueryPathInfo, 7);
-    if (s) info->ca = parseContentAddressOpt(s);
+    if (s) info->ca = ContentAddress::parseOpt(s);

    /* Get the references. */
    auto useQueryReferences(state.stmts->QueryReferences.use()(info->id));
@ -1150,7 +1150,7 @@ void LocalStore::querySubstitutablePathInfos(const StorePathCAMap & paths, Subst
            if (path.second) {
                subPath = makeFixedOutputPathFromCA(
                    path.first.name(),
-                    caWithoutRefs(*path.second));
+                    ContentAddressWithReferences::withoutRefs(*path.second));
                if (sub->storeDir == storeDir)
                    assert(subPath == path.first);
                if (subPath != path.first)
@ -1329,7 +1329,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
                    printStorePath(info.path), info.narSize, hashResult.second);

            if (info.ca) {
-                if (auto foHash = std::get_if<FixedOutputHash>(&*info.ca)) {
+                if (auto foHash = std::get_if<FixedOutputHash>(&info.ca->raw)) {
                    auto actualFoHash = hashCAPath(
                        foHash->method,
                        foHash->hash.type,
@ -1342,7 +1342,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
                            actualFoHash.hash.to_string(Base32, true));
                    }
                }
-                if (auto textHash = std::get_if<TextHash>(&*info.ca)) {
+                if (auto textHash = std::get_if<TextHash>(&info.ca->raw)) {
                    auto actualTextHash = hashString(htSHA256, readFile(realPath));
                    if (textHash->hash != actualTextHash) {
                        throw Error("ca hash mismatch importing path '%s';\n  specified: %s\n  got:       %s",
--- a/src/libstore/nar-info-disk-cache.cc
+++ b/src/libstore/nar-info-disk-cache.cc
@ -273,7 +273,7 @@ public:
                narInfo->deriver = StorePath(queryNAR.getStr(9));
            for (auto & sig : tokenizeString<Strings>(queryNAR.getStr(10), " "))
                narInfo->sigs.insert(sig);
-            narInfo->ca = parseContentAddressOpt(queryNAR.getStr(11));
+            narInfo->ca = ContentAddress::parseOpt(queryNAR.getStr(11));

            return {oValid, narInfo};
        });
--- a/src/libstore/nar-info.cc
+++ b/src/libstore/nar-info.cc
@ -74,7 +74,7 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string &
        else if (name == "CA") {
            if (ca) throw corrupt();
            // FIXME: allow blank ca or require skipping field?
-            ca = parseContentAddressOpt(value);
+            ca = ContentAddress::parseOpt(value);
        }

        pos = eol + 1;
--- a/src/libstore/path-info.cc
+++ b/src/libstore/path-info.cc
@ -49,7 +49,7 @@ std::optional<ContentAddressWithReferences> ValidPathInfo::contentAddressWithRef
                },
            };
        },
-    }, *ca);
+    }, ca->raw);
 }

 bool ValidPathInfo::isContentAddressed(const Store & store) const
@ -116,7 +116,7 @@ ValidPathInfo::ValidPathInfo(
                this->references.insert(path);
            this->ca = std::move((FixedOutputHash &&) foi);
        },
-    }, std::move(ca));
+    }, std::move(ca).raw);
 }


@ -136,7 +136,7 @@ ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned
    if (format >= 16) {
        source >> info.ultimate;
        info.sigs = readStrings<StringSet>(source);
-        info.ca = parseContentAddressOpt(readString(source));
+        info.ca = ContentAddress::parseOpt(readString(source));
    }
    return info;
 }
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@ -44,7 +44,7 @@ void write(const Store & store, Sink & out, const StorePath & storePath)

 ContentAddress read(const Store & store, Source & from, Phantom<ContentAddress> _)
 {
-    return parseContentAddress(readString(from));
+    return ContentAddress::parse(readString(from));
 }

 void write(const Store & store, Sink & out, const ContentAddress & ca)
@ -134,7 +134,7 @@ void write(const Store & store, Sink & out, const std::optional<StorePath> & sto

 std::optional<ContentAddress> read(const Store & store, Source & from, Phantom<std::optional<ContentAddress>> _)
 {
-    return parseContentAddressOpt(readString(from));
+    return ContentAddress::parseOpt(readString(from));
 }

 void write(const Store & store, Sink & out, const std::optional<ContentAddress> & caOpt)
@ -545,7 +545,7 @@ ref<const ValidPathInfo> RemoteStore::addCAToStore(
        conn->to
            << wopAddToStore
            << name
-            << renderContentAddressMethod(caMethod);
+            << caMethod.render();
        worker_proto::write(*this, conn->to, references);
        conn->to << repair;

@ -603,7 +603,7 @@ ref<const ValidPathInfo> RemoteStore::addCAToStore(
                }

            }
-        }, caMethod);
+        }, caMethod.raw);
        auto path = parseStorePath(readString(conn->from));
        // Release our connection to prevent a deadlock in queryPathInfo().
        conn_.reset();
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@ -221,7 +221,7 @@ StorePath Store::makeFixedOutputPathFromCA(std::string_view name, const ContentA
        [&](const FixedOutputInfo & foi) {
            return makeFixedOutputPath(name, foi);
        }
-    }, ca);
+    }, ca.raw);
 }


--- a/src/nix-store/nix-store.cc
+++ b/src/nix-store/nix-store.cc
@ -970,7 +970,7 @@ static void opServe(Strings opFlags, Strings opArgs)
                info.references = worker_proto::read(*store, in, Phantom<StorePathSet> {});
                in >> info.registrationTime >> info.narSize >> info.ultimate;
                info.sigs = readStrings<StringSet>(in);
-                info.ca = parseContentAddressOpt(readString(in));
+                info.ca = ContentAddress::parseOpt(readString(in));

                if (info.narSize == 0)
                    throw Error("narInfo is too old and missing the narSize field");
--- a/src/nix/prefetch.cc
+++ b/src/nix/prefetch.cc
@ -124,7 +124,7 @@ std::tuple<StorePath, Hash> prefetchFile(
        auto info = store->addToStoreSlow(*name, tmpFile, ingestionMethod, hashType, expectedHash);
        storePath = info.path;
        assert(info.ca);
-        hash = getContentAddressHash(*info.ca);
+        hash = info.ca->getHash();
    }

    return {storePath.value(), hash.value()};