From 6960ee929dcf95c24e0db761fd4bc46c3749abb2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 19 Apr 2019 11:34:23 +0200 Subject: [PATCH] Clean up exportGit argument handling --- src/libexpr/primops/fetchGit.cc | 42 ++++++++++++++++----------------- src/libexpr/primops/fetchGit.hh | 6 ++--- src/libexpr/primops/flake.cc | 10 ++++---- src/libutil/hash.hh | 12 ++++++++++ 4 files changed, 40 insertions(+), 30 deletions(-) diff --git a/src/libexpr/primops/fetchGit.cc b/src/libexpr/primops/fetchGit.cc index 40975d8d8..3a6830cb7 100644 --- a/src/libexpr/primops/fetchGit.cc +++ b/src/libexpr/primops/fetchGit.cc @@ -19,10 +19,13 @@ namespace nix { extern std::regex revRegex; GitInfo exportGit(ref store, const std::string & uri, - std::optional ref, std::string rev, + std::optional ref, + std::optional rev, const std::string & name) { - if (!ref && rev == "" && hasPrefix(uri, "/") && pathExists(uri + "/.git")) { + assert(!rev || rev->type == htSHA1); + + if (!ref && !rev && hasPrefix(uri, "/") && pathExists(uri + "/.git")) { bool clean = true; @@ -40,8 +43,6 @@ GitInfo exportGit(ref store, const std::string & uri, GitInfo gitInfo; gitInfo.ref = "HEAD"; - gitInfo.rev = "0000000000000000000000000000000000000000"; - gitInfo.shortRev = std::string(gitInfo.rev, 0, 7); auto files = tokenizeString>( runProgram("git", true, { "-C", uri, "ls-files", "-z" }), "\0"s); @@ -67,14 +68,11 @@ GitInfo exportGit(ref store, const std::string & uri, } // clean working tree, but no ref or rev specified. Use 'HEAD'. - rev = chomp(runProgram("git", true, { "-C", uri, "rev-parse", "HEAD" })); + rev = Hash(chomp(runProgram("git", true, { "-C", uri, "rev-parse", "HEAD" })), htSHA1); } if (!ref) ref = "HEAD"s; - if (rev != "" && !std::regex_match(rev, revRegex)) - throw Error("invalid Git revision '%s'", rev); - deletePath(getCacheDir() + "/nix/git"); Path cacheDir = getCacheDir() + "/nix/gitv2/" + hashString(htSHA256, uri).to_string(Base32, false); @@ -90,9 +88,9 @@ GitInfo exportGit(ref store, const std::string & uri, time_t now = time(0); /* If a rev was specified, we need to fetch if it's not in the repo. */ - if (rev != "") { + if (rev) { try { - runProgram("git", true, { "-C", cacheDir, "cat-file", "-e", rev }); + runProgram("git", true, { "-C", cacheDir, "cat-file", "-e", rev->gitRev() }); doFetch = false; } catch (ExecError & e) { if (WIFEXITED(e.status)) { @@ -128,19 +126,19 @@ GitInfo exportGit(ref store, const std::string & uri, // FIXME: check whether rev is an ancestor of ref. GitInfo gitInfo; gitInfo.ref = *ref; - gitInfo.rev = rev != "" ? rev : chomp(readFile(localRefFile)); - gitInfo.shortRev = std::string(gitInfo.rev, 0, 7); + gitInfo.rev = rev ? *rev : Hash(chomp(readFile(localRefFile)), htSHA1); printTalkative("using revision %s of repo '%s'", gitInfo.rev, uri); - std::string storeLinkName = hashString(htSHA512, name + std::string("\0"s) + gitInfo.rev).to_string(Base32, false); + std::string storeLinkName = hashString(htSHA512, + name + std::string("\0"s) + gitInfo.rev.gitRev()).to_string(Base32, false); Path storeLink = cacheDir + "/" + storeLinkName + ".link"; PathLocks storeLinkLock({storeLink}, fmt("waiting for lock on '%1%'...", storeLink)); // FIXME: broken try { auto json = nlohmann::json::parse(readFile(storeLink)); - assert(json["name"] == name && json["rev"] == gitInfo.rev); + assert(json["name"] == name && Hash((std::string) json["rev"], htSHA1) == gitInfo.rev); gitInfo.storePath = json["storePath"]; @@ -155,7 +153,7 @@ GitInfo exportGit(ref store, const std::string & uri, // FIXME: should pipe this, or find some better way to extract a // revision. - auto tar = runProgram("git", true, { "-C", cacheDir, "archive", gitInfo.rev }); + auto tar = runProgram("git", true, { "-C", cacheDir, "archive", gitInfo.rev.gitRev() }); Path tmpDir = createTempDir(); AutoDelete delTmpDir(tmpDir, true); @@ -164,13 +162,13 @@ GitInfo exportGit(ref store, const std::string & uri, gitInfo.storePath = store->addToStore(name, tmpDir); - gitInfo.revCount = std::stoull(runProgram("git", true, { "-C", cacheDir, "rev-list", "--count", gitInfo.rev })); + gitInfo.revCount = std::stoull(runProgram("git", true, { "-C", cacheDir, "rev-list", "--count", gitInfo.rev.gitRev() })); nlohmann::json json; json["storePath"] = gitInfo.storePath; json["uri"] = uri; json["name"] = name; - json["rev"] = gitInfo.rev; + json["rev"] = gitInfo.rev.gitRev(); json["revCount"] = *gitInfo.revCount; writeFile(storeLink, json.dump()); @@ -182,7 +180,7 @@ static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Va { std::string url; std::optional ref; - std::string rev; + std::optional rev; std::string name = "source"; PathSet context; @@ -199,7 +197,7 @@ static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Va else if (n == "ref") ref = state.forceStringNoCtx(*attr.value, *attr.pos); else if (n == "rev") - rev = state.forceStringNoCtx(*attr.value, *attr.pos); + rev = Hash(state.forceStringNoCtx(*attr.value, *attr.pos), htSHA1); else if (n == "name") name = state.forceStringNoCtx(*attr.value, *attr.pos); else @@ -216,15 +214,15 @@ static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Va // whitelist. Ah well. state.checkURI(url); - if (evalSettings.pureEval && rev == "") + if (evalSettings.pureEval && !rev) throw Error("in pure evaluation mode, 'fetchGit' requires a Git revision"); auto gitInfo = exportGit(state.store, url, ref, rev, name); state.mkAttrs(v, 8); mkString(*state.allocAttr(v, state.sOutPath), gitInfo.storePath, PathSet({gitInfo.storePath})); - mkString(*state.allocAttr(v, state.symbols.create("rev")), gitInfo.rev); - mkString(*state.allocAttr(v, state.symbols.create("shortRev")), gitInfo.shortRev); + mkString(*state.allocAttr(v, state.symbols.create("rev")), gitInfo.rev.gitRev()); + mkString(*state.allocAttr(v, state.symbols.create("shortRev")), gitInfo.rev.gitShortRev()); mkInt(*state.allocAttr(v, state.symbols.create("revCount")), gitInfo.revCount.value_or(0)); v.attrs->sort(); diff --git a/src/libexpr/primops/fetchGit.hh b/src/libexpr/primops/fetchGit.hh index 5937bdcc0..a867f38f6 100644 --- a/src/libexpr/primops/fetchGit.hh +++ b/src/libexpr/primops/fetchGit.hh @@ -10,13 +10,13 @@ struct GitInfo { Path storePath; std::string ref; - std::string rev; - std::string shortRev; + Hash rev{htSHA1}; std::optional revCount; }; GitInfo exportGit(ref store, const std::string & uri, - std::optional ref, std::string rev, + std::optional ref, + std::optional rev, const std::string & name); } diff --git a/src/libexpr/primops/flake.cc b/src/libexpr/primops/flake.cc index 1149efaac..c5e646412 100644 --- a/src/libexpr/primops/flake.cc +++ b/src/libexpr/primops/flake.cc @@ -231,11 +231,10 @@ static FlakeSourceInfo fetchFlake(EvalState & state, const FlakeRef flakeRef, bo // This downloads the entire git history else if (auto refData = std::get_if(&fRef.data)) { - auto gitInfo = exportGit(state.store, refData->uri, fRef.ref, - fRef.rev ? fRef.rev->to_string(Base16, false) : "", "source"); + auto gitInfo = exportGit(state.store, refData->uri, fRef.ref, fRef.rev, "source"); FlakeSourceInfo info(fRef); info.storePath = gitInfo.storePath; - info.rev = Hash(gitInfo.rev, htSHA1); + info.rev = gitInfo.rev; info.revCount = gitInfo.revCount; info.flakeRef.ref = gitInfo.ref; info.flakeRef.rev = info.rev; @@ -245,11 +244,12 @@ static FlakeSourceInfo fetchFlake(EvalState & state, const FlakeRef flakeRef, bo else if (auto refData = std::get_if(&fRef.data)) { if (!pathExists(refData->path + "/.git")) throw Error("flake '%s' does not reference a Git repository", refData->path); - auto gitInfo = exportGit(state.store, refData->path, {}, "", "source"); + auto gitInfo = exportGit(state.store, refData->path, {}, {}, "source"); FlakeSourceInfo info(fRef); info.storePath = gitInfo.storePath; - info.rev = Hash(gitInfo.rev, htSHA1); + info.rev = gitInfo.rev; info.revCount = gitInfo.revCount; + info.flakeRef.ref = gitInfo.ref; info.flakeRef.rev = info.rev; return info; } diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 2dbc3b630..edede8ace 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -80,6 +80,18 @@ struct Hash or base-64. By default, this is prefixed by the hash type (e.g. "sha256:"). */ std::string to_string(Base base = Base32, bool includeType = true) const; + + std::string gitRev() const + { + assert(type == htSHA1); + return to_string(Base16, false); + } + + std::string gitShortRev() const + { + assert(type == htSHA1); + return std::string(to_string(Base16, false), 0, 7); + } };