From 9db070d7a21fd4e362a314c2c35a0f369c410b16 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 10 Feb 2025 15:46:46 +0100 Subject: [PATCH] Misc code cleanups --- src/libfetchers/git-lfs-fetch.hh | 96 ++++++++++++-------------------- src/libfetchers/git-utils.cc | 7 +-- 2 files changed, 39 insertions(+), 64 deletions(-) diff --git a/src/libfetchers/git-lfs-fetch.hh b/src/libfetchers/git-lfs-fetch.hh index 71d086a5a..1af838e93 100644 --- a/src/libfetchers/git-lfs-fetch.hh +++ b/src/libfetchers/git-lfs-fetch.hh @@ -1,18 +1,17 @@ -#include -#include -#include -#include - #include "filetransfer.hh" #include "processes.hh" -#include "sync.hh" #include "url.hh" #include "users.hh" -namespace fs = std::filesystem; +#include +#include +#include +#include +#include -namespace nix { -namespace lfs { +#include + +namespace nix::lfs { /** * git-lfs pointer @@ -37,10 +36,10 @@ struct Fetch nix::ParsedURL url; Fetch(git_repository * repo, git_oid rev); - bool shouldFetch(const std::string & path) const; + bool shouldFetch(const CanonPath & path) const; void fetch( - const std::string content, - const std::string & pointerFilePath, + const std::string & content, + const CanonPath & pointerFilePath, StringSink & sink, std::function sizeCallback) const; std::vector fetchUrls(const std::vector & pointers) const; @@ -51,7 +50,6 @@ void downloadToSink( const std::string & url, const std::string & authHeader, StringSink & sink, - std::string path, std::string sha256Expected, size_t sizeExpected) { @@ -61,13 +59,12 @@ void downloadToSink( headers.push_back({"Authorization", authHeader}); request.headers = headers; getFileTransfer()->download(std::move(request), sink); - std::string data = sink.s; - const auto sizeActual = data.length(); + auto sizeActual = sink.s.length(); if (sizeExpected != sizeActual) throw Error("size mismatch while fetching %s: expected %d but got %d", url, sizeExpected, sizeActual); - const auto sha256Actual = hashString(HashAlgorithm::SHA256, data).to_string(HashFormat::Base16, false); + auto sha256Actual = hashString(HashAlgorithm::SHA256, sink.s).to_string(HashFormat::Base16, false); if (sha256Actual != sha256Expected) throw Error( "hash mismatch while fetching %s: expected sha256:%s but got sha256:%s", url, sha256Expected, sha256Actual); @@ -82,18 +79,16 @@ std::string getLfsApiToken(const ParsedURL & url) if (output.empty()) throw Error( - "git-lfs-authenticate: no output (cmd: ssh " + *url.authority + " git-lfs-authenticate " + url.path - + " download)"); + "git-lfs-authenticate: no output (cmd: ssh %s git-lfs-authenticate %s download)", + url.authority.value_or(""), url.path); - nlohmann::json query_resp = nlohmann::json::parse(output); - if (!query_resp.contains("header")) + auto queryResp = nlohmann::json::parse(output); + if (!queryResp.contains("header")) throw Error("no header in git-lfs-authenticate response"); - if (!query_resp["header"].contains("Authorization")) + if (!queryResp["header"].contains("Authorization")) throw Error("no Authorization in git-lfs-authenticate response"); - std::string res = query_resp["header"]["Authorization"].get(); - - return res; + return queryResp["header"]["Authorization"].get(); } std::string getLfsEndpointUrl(git_repository * repo) @@ -123,7 +118,7 @@ std::string getLfsEndpointUrl(git_repository * repo) return std::string(url_c_str); } -std::optional parseLfsPointer(const std::string & content, const std::string & filename) +std::optional parseLfsPointer(std::string_view content, std::string_view filename) { // https://github.com/git-lfs/git-lfs/blob/2ef4108/docs/spec.md // @@ -144,13 +139,10 @@ std::optional parseLfsPointer(const std::string & content, const std::s return std::nullopt; } - std::istringstream iss(content); - std::string line; - std::string oid; std::string size; - while (getline(iss, line)) { + for (auto & line : tokenizeString(content, "\n")) { if (line.starts_with("version ")) { continue; } @@ -189,15 +181,15 @@ Fetch::Fetch(git_repository * repo, git_oid rev) this->url = nix::parseURL(nix::fixGitURL(remoteUrl)).canonicalise(); } -bool Fetch::shouldFetch(const std::string & path) const +bool Fetch::shouldFetch(const CanonPath & path) const { const char * attr = nullptr; git_attr_options opts = GIT_ATTR_OPTIONS_INIT; opts.attr_commit_id = this->rev; opts.flags = GIT_ATTR_CHECK_INCLUDE_COMMIT | GIT_ATTR_CHECK_NO_SYSTEM; - if (git_attr_get_ext(&attr, (git_repository *) (this->repo), &opts, path.c_str(), "filter")) + if (git_attr_get_ext(&attr, (git_repository *) (this->repo), &opts, path.rel_c_str(), "filter")) throw Error("cannot get git-lfs attribute: %s", git_error_last()->message); - debug("Git filter for %s is %s", path, attr ? attr : "null"); + debug("Git filter for '%s' is '%s'", path, attr ? attr : "null"); return attr != nullptr && !std::string(attr).compare("lfs"); } @@ -249,25 +241,23 @@ std::vector Fetch::fetchUrls(const std::vector & pointe } void Fetch::fetch( - const std::string content, - const std::string & pointerFilePath, + const std::string & content, + const CanonPath & pointerFilePath, StringSink & sink, std::function sizeCallback) const { - debug("Trying to fetch %s using git-lfs", pointerFilePath); + debug("trying to fetch '%s' using git-lfs", pointerFilePath); if (content.length() >= 1024) { - debug("Skip git-lfs, pointer file too large"); - warn("Encountered a file that should have been a pointer, but wasn't: %s", pointerFilePath); + warn("encountered file '%s' that should have been a git-lfs pointer, but is too large", pointerFilePath); sizeCallback(content.length()); sink(content); return; } - const auto pointer = parseLfsPointer(std::string(content), std::string(pointerFilePath)); + const auto pointer = parseLfsPointer(content, pointerFilePath.rel()); if (pointer == std::nullopt) { - debug("Skip git-lfs, invalid pointer file"); - warn("Encountered a file that should have been a pointer, but wasn't: %s", pointerFilePath); + warn("encountered file '%s' that should have been a git-lfs pointer, but is invalid", pointerFilePath); sizeCallback(content.length()); sink(content); return; @@ -275,19 +265,11 @@ void Fetch::fetch( Path cacheDir = getCacheDir() + "/git-lfs"; std::string key = - hashString(HashAlgorithm::SHA256, pointerFilePath).to_string(HashFormat::Base16, false) + "/" + pointer->oid; + hashString(HashAlgorithm::SHA256, pointerFilePath.rel()).to_string(HashFormat::Base16, false) + "/" + pointer->oid; Path cachePath = cacheDir + "/" + key; if (pathExists(cachePath)) { debug("using cache entry %s -> %s", key, cachePath); - std::ifstream stream(cachePath); - const auto chunkSize = 128 * 1024; // 128 KiB - char buffer[chunkSize]; - do { - if (!stream.read(buffer, chunkSize)) - if (!stream.eof()) - throw Error("I/O error while reading cached file"); - sink(std::string(buffer, stream.gcount())); - } while (stream.gcount() > 0); + sink(readFile(cachePath)); return; } debug("did not find cache entry for %s", key); @@ -307,23 +289,17 @@ void Fetch::fetch( } const uint64_t size = obj.at("size"); sizeCallback(size); - downloadToSink(ourl, authHeader, sink, pointerFilePath, sha256, size); + downloadToSink(ourl, authHeader, sink, sha256, size); debug("creating cache entry %s -> %s", key, cachePath); if (!pathExists(dirOf(cachePath))) createDirs(dirOf(cachePath)); - std::ofstream stream(cachePath); - if (!stream.write(sink.s.c_str(), size)) - throw Error("I/O error while writing cache file"); + writeFile(cachePath, sink.s); debug("%s fetched with git-lfs", pointerFilePath); } catch (const nlohmann::json::out_of_range & e) { - std::stringstream ss; - ss << "bad json from /info/lfs/objects/batch: " << obj << " " << e.what(); - throw Error(ss.str()); + throw Error("bad json from /info/lfs/objects/batch: %s %s", obj, e.what()); } } -} // namespace lfs - -} // namespace nix +} // namespace nix::lfs diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index b58a83195..0f56b3c79 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -692,13 +692,12 @@ struct GitSourceAccessor : SourceAccessor const auto blob = getBlob(path, symlink); if (lfsFetch) { - auto pathStr = std::string(path.rel()); - if (lfsFetch->shouldFetch(pathStr)) { + if (lfsFetch->shouldFetch(path)) { StringSink s; try { auto contents = std::string((const char *) git_blob_rawcontent(blob.get()), git_blob_rawsize(blob.get())); - lfsFetch->fetch(contents, pathStr, s, [&s](uint64_t size){ s.s.reserve(size); }); - } catch (Error &e) { + lfsFetch->fetch(contents, path, s, [&s](uint64_t size){ s.s.reserve(size); }); + } catch (Error & e) { e.addTrace({}, "while smudging git-lfs file '%s'", path); throw; }