Took ref and rev out of FlakeRef

3 files changed, 41 insertions, 87 deletions

diff --git a/src/libexpr/primops/flake.cc b/src/libexpr/primops/flake.cc
index 1ad9ad3f8..cea3854e4 100644
--- a/src/libexpr/primops/flake.cc
+++ b/src/libexpr/primops/flake.cc
@@ -201,10 +201,8 @@ static FlakeRef lookupFlake(EvalState & state, const FlakeRef & flakeRef,
                 auto newRef = FlakeRef(i->second.ref);
                 if (!newRef.isDirect())
                     throw Error("found indirect flake URI '%s' in the flake registry", i->second.ref.to_string());
-                if (refData->ref)
-                    newRef.setRef(*refData->ref);
-                if (refData->rev)
-                    newRef.setRev(*refData->rev);
+                if (flakeRef.ref) newRef.setRef(*flakeRef.ref);
+                if (flakeRef.rev) newRef.setRev(*flakeRef.rev);
                 return newRef;
             }
         }
@@ -220,13 +218,13 @@ struct FlakeSourceInfo
     std::optional<uint64_t> revCount;
 };
 
-static FlakeSourceInfo fetchFlake(EvalState & state, const FlakeRef & flakeRef, bool impureIsAllowed = false)
+static FlakeSourceInfo fetchFlake(EvalState & state, const FlakeRef flakeRef, bool impureIsAllowed = false)
 {
-    FlakeRef directFlakeRef = lookupFlake(state, flakeRef, state.getFlakeRegistries());
+    FlakeRef fRef = lookupFlake(state, flakeRef, state.getFlakeRegistries());
 
-    if (auto refData = std::get_if<FlakeRef::IsGitHub>(&directFlakeRef.data)) {
-        if (evalSettings.pureEval && !impureIsAllowed && !directFlakeRef.isImmutable())
-            throw Error("requested to fetch FlakeRef '%s' purely, which is mutable", directFlakeRef.to_string());
+    if (auto refData = std::get_if<FlakeRef::IsGitHub>(&fRef.data)) {
+        if (evalSettings.pureEval && !impureIsAllowed && !fRef.isImmutable())
+            throw Error("requested to fetch FlakeRef '%s' purely, which is mutable", fRef.to_string());
 
         // FIXME: use regular /archive URLs instead? api.github.com
         // might have stricter rate limits.
@@ -235,14 +233,11 @@ static FlakeSourceInfo fetchFlake(EvalState & state, const FlakeRef & flakeRef,
 
         auto url = fmt("https://api.github.com/repos/%s/%s/tarball/%s",
             refData->owner, refData->repo,
-            refData->rev
-                ? refData->rev->to_string(Base16, false)
-                : refData->ref
-                  ? *refData->ref
-                  : "master");
+            fRef.rev ? fRef.rev->to_string(Base16, false)
+                : fRef.ref ? *fRef.ref : "master");
 
         auto result = getDownloader()->downloadCached(state.store, url, true, "source",
-            Hash(), nullptr, refData->rev ? 1000000000 : settings.tarballTtl);
+            Hash(), nullptr, fRef.rev ? 1000000000 : settings.tarballTtl);
 
         if (!result.etag)
             throw Error("did not receive an ETag header from '%s'", url);
@@ -257,9 +252,9 @@ static FlakeSourceInfo fetchFlake(EvalState & state, const FlakeRef & flakeRef,
         return info;
     }
 
-    else if (auto refData = std::get_if<FlakeRef::IsGit>(&directFlakeRef.data)) {
-        auto gitInfo = exportGit(state.store, refData->uri, refData->ref,
-            refData->rev ? refData->rev->to_string(Base16, false) : "", "source");
+    else if (auto refData = std::get_if<FlakeRef::IsGit>(&fRef.data)) {
+        auto gitInfo = exportGit(state.store, refData->uri, fRef.ref,
+            fRef.rev ? fRef.rev->to_string(Base16, false) : "", "source");
         FlakeSourceInfo info;
         info.storePath = gitInfo.storePath;
         info.rev = Hash(gitInfo.rev, htSHA1);
diff --git a/src/libexpr/primops/flakeref.cc b/src/libexpr/primops/flakeref.cc
index 1df53bfb8..f160b257b 100644
--- a/src/libexpr/primops/flakeref.cc
+++ b/src/libexpr/primops/flakeref.cc
@@ -58,11 +58,11 @@ FlakeRef::FlakeRef(const std::string & uri, bool allowRelative)
         IsFlakeId d;
         d.id = match[1];
         if (match[2].matched)
-            d.rev = Hash(match[2], htSHA1);
+            rev = Hash(match[2], htSHA1);
         else if (match[3].matched) {
-            d.ref = match[3];
+            ref = match[3];
             if (match[4].matched)
-                d.rev = Hash(match[4], htSHA1);
+                rev = Hash(match[4], htSHA1);
         }
         data = d;
     }
@@ -72,9 +72,9 @@ FlakeRef::FlakeRef(const std::string & uri, bool allowRelative)
         d.owner = match[1];
         d.repo = match[2];
         if (match[3].matched)
-            d.rev = Hash(match[3], htSHA1);
+            rev = Hash(match[3], htSHA1);
         else if (match[4].matched) {
-            d.ref = match[4];
+            ref = match[4];
         }
         data = d;
     }
@@ -92,16 +92,16 @@ FlakeRef::FlakeRef(const std::string & uri, bool allowRelative)
             if (name == "rev") {
                 if (!std::regex_match(value, revRegex))
                     throw Error("invalid Git revision '%s'", value);
-                d.rev = Hash(value, htSHA1);
+                rev = Hash(value, htSHA1);
             } else if (name == "ref") {
                 if (!std::regex_match(value, refRegex2))
                     throw Error("invalid Git ref '%s'", value);
-                d.ref = value;
+                ref = value;
             } else
                 // FIXME: should probably pass through unknown parameters
                 throw Error("invalid Git flake reference parameter '%s', in '%s'", name, uri);
         }
-        if (d.rev && !d.ref)
+        if (rev && !ref)
             throw Error("flake URI '%s' lacks a Git ref", uri);
         data = d;
     }
@@ -118,27 +118,18 @@ FlakeRef::FlakeRef(const std::string & uri, bool allowRelative)
 
 std::string FlakeRef::to_string() const
 {
-    if (auto refData = std::get_if<FlakeRef::IsFlakeId>(&data)) {
-        return
-            "flake:" + refData->id +
-            (refData->ref ? "/" + *refData->ref : "") +
-            (refData->rev ? "/" + refData->rev->to_string(Base16, false) : "");
-    }
+    std::string string;
+    if (auto refData = std::get_if<FlakeRef::IsFlakeId>(&data))
+        string = "flake:" + refData->id;
 
     else if (auto refData = std::get_if<FlakeRef::IsGitHub>(&data)) {
-        assert(!refData->ref || !refData->rev);
-        return
-            "github:" + refData->owner + "/" + refData->repo +
-            (refData->ref ? "/" + *refData->ref : "") +
-            (refData->rev ? "/" + refData->rev->to_string(Base16, false) : "");
+        assert(!ref || !rev);
+        string = "github:" + refData->owner + "/" + refData->repo;
     }
 
     else if (auto refData = std::get_if<FlakeRef::IsGit>(&data)) {
-        assert(refData->ref || !refData->rev);
-        return
-            refData->uri +
-            (refData->ref ? "?ref=" + *refData->ref : "") +
-            (refData->rev ? "&rev=" + refData->rev->to_string(Base16, false) : "");
+        assert(ref || !rev);
+        string = refData->uri;
     }
 
     else if (auto refData = std::get_if<FlakeRef::IsPath>(&data)) {
@@ -146,38 +137,22 @@ std::string FlakeRef::to_string() const
     }
 
     else abort();
+
+    string += (ref ? "/" + *ref : "") +
+              (rev ? "/" + rev->to_string(Base16, false) : "");
+    return string;
 }
 
 bool FlakeRef::isImmutable() const
 {
-    if (auto refData = std::get_if<FlakeRef::IsFlakeId>(&data))
-        return (bool) refData->rev;
-
-    else if (auto refData = std::get_if<FlakeRef::IsGitHub>(&data))
-        return (bool) refData->rev;
-
-    else if (auto refData = std::get_if<FlakeRef::IsGit>(&data))
-        return (bool) refData->rev;
-
-    else if (std::get_if<FlakeRef::IsPath>(&data))
-        return false;
-
-    else abort();
+    return (bool) rev;
 }
 
 FlakeRef FlakeRef::baseRef() const // Removes the ref and rev from a FlakeRef.
 {
     FlakeRef result(*this);
-    if (auto refData = std::get_if<FlakeRef::IsGitHub>(&result.data)) {
-        refData->ref = std::nullopt;
-        refData->rev = std::nullopt;
-    } else if (auto refData = std::get_if<FlakeRef::IsGit>(&result.data)) {
-        refData->ref = std::nullopt;
-        refData->rev = std::nullopt;
-    } else if (auto refData = std::get_if<FlakeRef::IsGit>(&result.data)) {
-        refData->ref = std::nullopt;
-        refData->rev = std::nullopt;
-    }
+    result.ref = std::nullopt;
+    result.rev = std::nullopt;
     return result;
 }
 }
diff --git a/src/libexpr/primops/flakeref.hh b/src/libexpr/primops/flakeref.hh
index 94a75fb2b..9276fc737 100644
--- a/src/libexpr/primops/flakeref.hh
+++ b/src/libexpr/primops/flakeref.hh
@@ -101,25 +101,23 @@ typedef std::string FlakeId;
 
 struct FlakeRef
 {
+    std::optional<std::string> ref;
+    std::optional<Hash> rev;
+
     struct IsFlakeId
     {
         FlakeId id;
-        std::optional<std::string> ref;
-        std::optional<Hash> rev;
     };
 
     struct IsGitHub
     {
         std::string owner, repo;
-        std::optional<std::string> ref;
-        std::optional<Hash> rev;
     };
 
+    // Git, Tarball
     struct IsGit
     {
         std::string uri;
-        std::optional<std::string> ref;
-        std::optional<Hash> rev;
     };
 
     struct IsPath
@@ -161,22 +159,8 @@ struct FlakeRef
 
     FlakeRef baseRef() const;
 
-    void setRef(std::optional<std::string> ref) {
-        if (auto refData = std::get_if<IsGit>(&data))
-            refData->ref = ref;
-        else if (auto refData = std::get_if<IsGitHub>(&data))
-            refData->ref = ref;
-        else if (auto refData = std::get_if<IsFlakeId>(&data))
-            refData->ref = ref;
-    }
+    void setRef(std::optional<std::string> ref) { ref = ref; }
 
-    void setRev(std::optional<Hash> rev) {
-        if (auto refData = std::get_if<IsGit>(&data))
-            refData->rev = rev;
-        else if (auto refData = std::get_if<IsGitHub>(&data))
-            refData->rev = rev;
-        else if (auto refData = std::get_if<IsFlakeId>(&data))
-            refData->rev = rev;
-    }
+    void setRev(std::optional<Hash> rev) { rev = rev; }
 };
 }