diff --git a/src/libfetchers-tests/git-utils.cc b/src/libfetchers-tests/git-utils.cc index cdd920e54..9a01207af 100644 --- a/src/libfetchers-tests/git-utils.cc +++ b/src/libfetchers-tests/git-utils.cc @@ -70,12 +70,6 @@ TEST_F(GitUtilsTest, sink_basic) auto repo = openRepo(); auto sink = repo->getFileSystemObjectSink(); - // TODO/Question: It seems a little odd that we use the tarball-like convention of requiring a top-level directory - // here - // The sync method does not document this behavior, should probably renamed because it's not very - // general, and I can't imagine that "non-conventional" archives or any other source to be handled by - // this sink. - sink->createDirectory(CanonPath("foo-1.1")); sink->createRegularFile(CanonPath("foo-1.1/hello"), [](CreateRegularFileSink & fileSink) { @@ -91,7 +85,7 @@ TEST_F(GitUtilsTest, sink_basic) // sink->createHardlink("foo-1.1/links/foo-2", CanonPath("foo-1.1/hello")); - auto result = repo->dereferenceSingletonDirectory(sink->flush()); + auto result = repo->dereferenceSingletonDirectory(sink->flush().hash); auto accessor = repo->getAccessor(result, {}, getRepoName()); auto entries = accessor->readDirectory(CanonPath::root); ASSERT_EQ(entries.size(), 5u); @@ -123,6 +117,51 @@ TEST_F(GitUtilsTest, sink_hardlink) } }; +TEST_F(GitUtilsTest, sink_root_file) +{ + auto repo = openRepo(); + auto sink = repo->getFileSystemObjectSink(); + + sink->createRegularFile( + CanonPath::root, [](CreateRegularFileSink & fileSink) { writeString(fileSink, "hello world", false); }); + + auto result = sink->flush(); + ASSERT_EQ(result.mode, nix::git::Mode::Regular); + auto accessor = repo->getAccessor(result.hash, false, getRepoName()); + ASSERT_EQ(accessor->readFile(CanonPath::root), "hello world"); +}; + +TEST_F(GitUtilsTest, sink_root_symlink) +{ + auto repo = openRepo(); + auto sink = repo->getFileSystemObjectSink(); + + sink->createSymlink(CanonPath::root, "target"); + + auto result = sink->flush(); + ASSERT_EQ(result.mode, nix::git::Mode::Symlink); + auto accessor = repo->getAccessor(result.hash, false, getRepoName()); + ASSERT_EQ(accessor->readLink(CanonPath::root), "target"); +}; + +TEST_F(GitUtilsTest, sink_hardlink_to_root) +{ + auto repo = openRepo(); + auto sink = repo->getFileSystemObjectSink(); + + sink->createRegularFile( + CanonPath("hello"), [](CreateRegularFileSink & fileSink) { writeString(fileSink, "hello world", false); }); + + sink->createDirectory(CanonPath("subdir")); + sink->createHardlink(CanonPath("subdir/link"), CanonPath("hello")); + + auto result = sink->flush(); + ASSERT_EQ(result.mode, nix::git::Mode::Directory); + auto accessor = repo->getAccessor(result.hash, false, getRepoName()); + ASSERT_EQ(accessor->readFile(CanonPath("hello")), "hello world"); + ASSERT_EQ(accessor->readFile(CanonPath("subdir/link")), "hello world"); +}; + TEST_F(GitUtilsTest, peel_reference) { // Create a commit in the repo diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 7bd22e5e4..91ac51450 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -962,15 +962,15 @@ struct GitSourceAccessor : SourceAccessor Blob getBlob(State & state, const CanonPath & path, bool expectSymlink) { - if (!expectSymlink && git_object_type(state.root.get()) == GIT_OBJECT_BLOB) - return dupObject((git_blob *) &*state.root); - auto notExpected = [&]() { throw Error(expectSymlink ? "'%s' is not a symlink" : "'%s' is not a regular file", showPath(path)); }; - if (path.isRoot()) + if (path.isRoot()) { + if (git_object_type(state.root.get()) == GIT_OBJECT_BLOB) + return dupObject((git_blob *) &*state.root); notExpected(); + } auto entry = need(state, path); @@ -1054,16 +1054,23 @@ struct GitExportIgnoreSourceAccessor : CachingFilteringSourceAccessor } }; -struct GitFileSystemObjectSinkImpl : GitFileSystemObjectSink -{ - ref repo; +namespace { +struct DirEntry +{ + git_oid oid; + git_filemode_t mode; +}; + +struct DirectoryState +{ struct PendingDir { std::string name; TreeBuilder builder; }; + GitRepoImpl & repo; TreeBuilder rootBuilder; std::vector pendingDirs; @@ -1079,46 +1086,7 @@ struct GitFileSystemObjectSinkImpl : GitFileSystemObjectSink */ decltype(::git_odb_backend::refresh) packfileOdbRefresh = nullptr; - TreeBuilder makeBuilder(git_tree * prevTree = nullptr) - { - git_treebuilder * b; - if (git_treebuilder_new(&b, *repo, prevTree)) - throw Error("creating a tree builder: %s", git_error_last()->message); - return TreeBuilder(b); - } - - git_oid finishBuilder(TreeBuilder & builder) - { - git_oid oid; - if (git_treebuilder_write(&oid, builder.get())) - throw Error("creating a tree object: %s", git_error_last()->message); - return oid; - } - - void pushBuilder(std::string name) - { - auto & parentBuilder = pendingDirs.empty() ? rootBuilder : pendingDirs.back().builder; - const git_tree_entry * entry; - Tree prevTree = nullptr; - - if ((entry = git_treebuilder_get(parentBuilder.get(), name.c_str()))) { - /* Clone a tree that we've already finished. This happens - if a tarball has directory entries that are not - contiguous. */ - if (git_tree_entry_type(entry) != GIT_OBJECT_TREE) - throw Error("parent of '%s' is not a directory", name); - - if (git_tree_entry_to_object((git_object **) (git_tree **) Setter(prevTree), *repo, entry)) - throw Error("looking up parent of '%s': %s", name, git_error_last()->message); - } - - pendingDirs.push_back({ - .name = std::move(name), - .builder = makeBuilder(prevTree.get()), - }); - }; - - GitFileSystemObjectSinkImpl(ref repo) + DirectoryState(GitRepoImpl & repo) : repo(repo) , rootBuilder(makeBuilder()) { @@ -1147,62 +1115,143 @@ struct GitFileSystemObjectSinkImpl : GitFileSystemObjectSink } } - std::pair popBuilder() - { - assert(!pendingDirs.empty()); - auto pending = std::move(pendingDirs.back()); - pendingDirs.pop_back(); - return {finishBuilder(pending.builder), pending.name}; - }; + TreeBuilder makeBuilder(git_tree * prevTree = nullptr); - void addToTree(const std::string & name, const git_oid & oid, git_filemode_t mode) - { - auto & builder = pendingDirs.empty() ? rootBuilder : pendingDirs.back().builder; - if (git_treebuilder_insert(nullptr, builder.get(), name.c_str(), &oid, mode)) - throw Error("adding a file to a tree builder: %s", git_error_last()->message); - }; + std::vector prepareDirs(const CanonPath & path); + void updateBuilders(std::span names); + std::pair popBuilder(); + void pushBuilder(std::string name); + void addToTree(const std::string & name, const DirEntry & entry); +}; - void updateBuilders(std::span names) - { - // Find the common prefix of pendingDirs and names. - size_t prefixLen = 0; - for (; prefixLen < names.size() && prefixLen < pendingDirs.size(); ++prefixLen) - if (names[prefixLen] != pendingDirs[prefixLen].name) - break; +TreeBuilder DirectoryState::makeBuilder(git_tree * prevTree) +{ + TreeBuilder builder; + if (git_treebuilder_new(Setter(builder), repo, prevTree)) + throw Error("creating a tree builder: %s", git_error_last()->message); + return builder; +} - // Finish the builders that are not part of the common prefix. - for (auto n = pendingDirs.size(); n > prefixLen; --n) { - auto [oid, name] = popBuilder(); - addToTree(name, oid, GIT_FILEMODE_TREE); - } +git_oid finishBuilder(TreeBuilder & builder) +{ + git_oid oid; + if (git_treebuilder_write(&oid, builder.get())) + throw Error("creating a tree object: %s", git_error_last()->message); + return oid; +} - // Create builders for the new directories. - for (auto n = prefixLen; n < names.size(); ++n) - pushBuilder(names[n]); - }; +void DirectoryState::pushBuilder(std::string name) +{ + auto & parentBuilder = pendingDirs.empty() ? rootBuilder : pendingDirs.back().builder; + const git_tree_entry * entry; + Tree prevTree = nullptr; - void prepareDirs(const std::vector & pathComponents, bool isDir) - { - std::span pathComponents2{pathComponents}; + if ((entry = git_treebuilder_get(parentBuilder.get(), name.c_str()))) { + /* Clone a tree that we've already finished. This happens + if a tarball has directory entries that are not + contiguous. */ + if (git_tree_entry_type(entry) != GIT_OBJECT_TREE) + throw Error("parent of '%s' is not a directory", name); - updateBuilders(isDir ? pathComponents2 : pathComponents2.first(pathComponents2.size() - 1)); + if (git_tree_entry_to_object((git_object **) (git_tree **) Setter(prevTree), repo, entry)) + throw Error("looking up parent of '%s': %s", name, git_error_last()->message); } - std::vector prepareDirs(const CanonPath & path, bool isDir) + pendingDirs.push_back({ + .name = std::move(name), + .builder = makeBuilder(prevTree.get()), + }); +} + +std::pair DirectoryState::popBuilder() +{ + assert(!pendingDirs.empty()); + auto pending = std::move(pendingDirs.back()); + pendingDirs.pop_back(); + return {finishBuilder(pending.builder), pending.name}; +} + +void DirectoryState::addToTree(const std::string & name, const DirEntry & entry) +{ + auto & builder = pendingDirs.empty() ? rootBuilder : pendingDirs.back().builder; + if (git_treebuilder_insert(nullptr, builder.get(), name.c_str(), &entry.oid, entry.mode)) + throw Error("adding a file to a tree builder: %s", git_error_last()->message); +} + +void DirectoryState::updateBuilders(std::span names) +{ + // Find the common prefix of pendingDirs and names. + size_t prefixLen = 0; + for (; prefixLen < names.size() && prefixLen < pendingDirs.size(); ++prefixLen) + if (names[prefixLen] != pendingDirs[prefixLen].name) + break; + + // Finish the builders that are not part of the common prefix. + for (auto n = pendingDirs.size(); n > prefixLen; --n) { + auto [oid, name] = popBuilder(); + addToTree(name, DirEntry{.oid = oid, .mode = GIT_FILEMODE_TREE}); + } + + // Create builders for the new directories. + for (auto n = prefixLen; n < names.size(); ++n) + pushBuilder(names[n]); +} + +std::vector DirectoryState::prepareDirs(const CanonPath & path) +{ + std::vector pathComponents; + for (auto & c : path) + pathComponents.emplace_back(c); + + updateBuilders(pathComponents); + + return pathComponents; +} + +struct GitFileSystemObjectSinkImpl : GitFileSystemObjectSink +{ + ref repo; + + /** + * Root state: + * - nullopt: nothing created yet + * - DirEntry: root is a single non-directory object (file/symlink) + * - DirectoryState: root is a directory with builders + */ + std::optional> root; + + DirectoryState & ensureDirectoryState() { - std::vector pathComponents; - for (auto & c : path) - pathComponents.emplace_back(c); + if (!root) { + root.emplace(*repo); + } + if (auto * dirState = std::get_if(&*root)) { + return *dirState; + } + throw Error("cannot create directory entry: root is not a directory"); + } - prepareDirs(pathComponents, isDir); + GitFileSystemObjectSinkImpl(ref repo) + : repo(repo) + { + } - return pathComponents; + void addEntry(const CanonPath & path, const DirEntry & entry) + { + if (auto res = path.parentAndChild()) { + auto & [parent, child] = *res; + auto & dirState = ensureDirectoryState(); + dirState.prepareDirs(parent); + dirState.addToTree(std::string(child), entry); + } else { + if (root) + throw Error("root already exists"); + root = entry; + } } void createRegularFile(const CanonPath & path, std::function func) override { - auto pathComponents = prepareDirs(path, false); - using WriteStream = std::unique_ptr<::git_writestream, decltype([](::git_writestream * stream) { if (stream) stream->free(stream); @@ -1281,44 +1330,59 @@ struct GitFileSystemObjectSinkImpl : GitFileSystemObjectSink "creating a blob object for '%s' from in-memory buffer: %s", path, git_error_last()->message); } - addToTree(*pathComponents.rbegin(), oid, crf.executable ? GIT_FILEMODE_BLOB_EXECUTABLE : GIT_FILEMODE_BLOB); + addEntry( + path, + DirEntry{ + .oid = oid, + .mode = crf.executable ? GIT_FILEMODE_BLOB_EXECUTABLE : GIT_FILEMODE_BLOB, + }); } void createDirectory(const CanonPath & path, std::optional callback = {}) override { - auto pathComponents = prepareDirs(path, true); + ensureDirectoryState().prepareDirs(path); if (callback) (*callback)(*this, path); } void createSymlink(const CanonPath & path, const std::string & target) override { - auto pathComponents = prepareDirs(path, false); - git_oid oid; if (git_blob_create_from_buffer(&oid, *repo, target.c_str(), target.size())) throw Error("creating a blob object for tarball symlink member '%s': %s", path, git_error_last()->message); - addToTree(*pathComponents.rbegin(), oid, GIT_FILEMODE_LINK); + addEntry( + path, + DirEntry{ + .oid = oid, + .mode = GIT_FILEMODE_LINK, + }); } void createHardlink(const CanonPath & path, const CanonPath & target) override { - auto pathComponents = prepareDirs(path, false); + auto & dirState = ensureDirectoryState(); + + auto res = path.parentAndChild(); + if (!res) + throw Error("hard link cannot be root file system object"); + auto & [parent, name] = *res; + + dirState.prepareDirs(parent); // We can't just look up the path from the start of the root, since // some parent directories may not have finished yet, so we compute // a relative path that helps us find the right git_tree_builder or object. - auto relTarget = CanonPath(path).parent()->makeRelative(target); + auto relTarget = parent.makeRelative(target); - auto dir = pendingDirs.rbegin(); + auto dir = dirState.pendingDirs.rbegin(); // For each ../ component at the start, go up one directory. // CanonPath::makeRelative() always puts all .. elements at the start, // so they're all handled by this loop: std::string_view relTargetLeft(relTarget); while (hasPrefix(relTargetLeft, "../")) { - if (dir == pendingDirs.rend()) + if (dir == dirState.pendingDirs.rend()) throw Error("invalid hard link target '%s' for path '%s'", target, path); ++dir; relTargetLeft = relTargetLeft.substr(3); @@ -1327,7 +1391,7 @@ struct GitFileSystemObjectSinkImpl : GitFileSystemObjectSink // Look up the remainder of the target, starting at the // top-most `git_treebuilder`. std::variant curDir{ - dir == pendingDirs.rend() ? rootBuilder.get() : dir->builder.get()}; + dir == dirState.pendingDirs.rend() ? dirState.rootBuilder.get() : dir->builder.get()}; Object tree; // needed to keep `entry` alive const git_tree_entry * entry = nullptr; @@ -1347,26 +1411,41 @@ struct GitFileSystemObjectSinkImpl : GitFileSystemObjectSink assert(entry); - addToTree(*pathComponents.rbegin(), *git_tree_entry_id(entry), git_tree_entry_filemode(entry)); + dirState.addToTree( + std::string(name), + DirEntry{.oid = *git_tree_entry_id(entry), .mode = git_tree_entry_filemode(entry)}); } - Hash flush() override + git::TreeEntry flush() override { - updateBuilders({}); + if (!root) + throw Error("nothing to flush"); - auto oid = finishBuilder(rootBuilder); + auto [oid, mode] = std::visit( + overloaded{ + [](DirEntry & entry) -> std::pair { + return {entry.oid, entry.mode}; + }, + [](DirectoryState & dirState) -> std::pair { + dirState.updateBuilders({}); + return {finishBuilder(dirState.rootBuilder), GIT_FILEMODE_TREE}; + }, + }, + *root); if (auto * backend = repo->packBackend) { /* We are done writing blobs, can restore refresh functionality. */ - backend->refresh = packfileOdbRefresh; + backend->refresh = dirState.packfileOdbRefresh; } repo->flush(); - return toHash(oid); + return git::TreeEntry{.mode = static_cast(mode), .hash = toHash(oid)}; } }; +} + ref GitRepoImpl::getRawAccessor(const Hash & rev, const GitAccessorOptions & options) { auto self = ref(shared_from_this()); diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index b86fa926a..f0743d3fe 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -321,12 +321,12 @@ struct GitArchiveInputScheme : InputScheme auto tarballCache = settings.getTarballCache(); auto parseSink = tarballCache->getFileSystemObjectSink(); auto lastModified = unpackTarfileToSink(archive, *parseSink); - auto tree = parseSink->flush(); + auto treeEntry = parseSink->flush(); act.reset(); TarballInfo tarballInfo{ - .treeHash = tarballCache->dereferenceSingletonDirectory(tree), .lastModified = lastModified}; + .treeHash = tarballCache->dereferenceSingletonDirectory(treeEntry.hash), .lastModified = lastModified}; cache->upsert(treeHashKey, Attrs{{"treeHash", tarballInfo.treeHash.gitRev()}}); cache->upsert(lastModifiedKey, Attrs{{"lastModified", (uint64_t) tarballInfo.lastModified}}); diff --git a/src/libfetchers/include/nix/fetchers/git-utils.hh b/src/libfetchers/include/nix/fetchers/git-utils.hh index 24a7b8008..7f7f808ba 100644 --- a/src/libfetchers/include/nix/fetchers/git-utils.hh +++ b/src/libfetchers/include/nix/fetchers/git-utils.hh @@ -2,6 +2,7 @@ #include "nix/fetchers/filtering-source-accessor.hh" #include "nix/util/fs-sink.hh" +#include "nix/util/git.hh" namespace nix { @@ -17,9 +18,9 @@ struct Settings; struct GitFileSystemObjectSink : ExtendedFileSystemObjectSink { /** - * Flush builder and return a final Git hash. + * Flush builder and return the final Git tree entry (hash and mode). */ - virtual Hash flush() = 0; + virtual git::TreeEntry flush() = 0; }; struct GitAccessorOptions diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index b1ebd749d..6f8def578 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -182,7 +182,7 @@ static DownloadTarballResult downloadTarball_( auto tarballCache = settings.getTarballCache(); auto parseSink = tarballCache->getFileSystemObjectSink(); auto lastModified = unpackTarfileToSink(archive, *parseSink); - auto tree = parseSink->flush(); + auto treeEntry = parseSink->flush(); act.reset(); @@ -196,7 +196,7 @@ static DownloadTarballResult downloadTarball_( infoAttrs = cached->value; } else { infoAttrs.insert_or_assign("etag", res->etag); - infoAttrs.insert_or_assign("treeHash", tarballCache->dereferenceSingletonDirectory(tree).gitRev()); + infoAttrs.insert_or_assign("treeHash", tarballCache->dereferenceSingletonDirectory(treeEntry.hash).gitRev()); infoAttrs.insert_or_assign("lastModified", uint64_t(lastModified)); if (res->immutableUrl) infoAttrs.insert_or_assign("immutableUrl", *res->immutableUrl); diff --git a/src/libutil/canon-path.cc b/src/libutil/canon-path.cc index 22ca3e066..cbb99142e 100644 --- a/src/libutil/canon-path.cc +++ b/src/libutil/canon-path.cc @@ -54,6 +54,17 @@ std::optional CanonPath::parent() const return CanonPath(unchecked_t(), path.substr(0, std::max((size_t) 1, path.rfind('/')))); } +std::optional> CanonPath::parentAndChild() const +{ + if (isRoot()) + return std::nullopt; + auto slash = path.rfind('/'); + return {{ + CanonPath(unchecked_t(), path.substr(0, std::max((size_t) 1, slash))), + std::string_view{path}.substr(slash + 1) + }}; +} + void CanonPath::pop() { assert(!isRoot()); diff --git a/src/libutil/include/nix/util/canon-path.hh b/src/libutil/include/nix/util/canon-path.hh index 2156b02fc..f45e2d971 100644 --- a/src/libutil/include/nix/util/canon-path.hh +++ b/src/libutil/include/nix/util/canon-path.hh @@ -238,6 +238,13 @@ public: return ((std::string_view) path).substr(path.rfind('/') + 1); } + /** + * Combination of `parent` and `baseName`. + * + * @note that because of the `std::string_view`, we are borrowing for the base name. + */ + std::optional> parentAndChild() const; + bool operator==(const CanonPath & x) const { return path == x.path;