NixOS · May 15, 2017 · May 15, 2017 · May 15, 2017 · May 15, 2017 · May 24, 2017
Showing with 123 additions and 34 deletions.

+75 −33 src/libstore/nar-accessor.cc

+2 −1 tests/local.mk

+23 −0 tests/nar-index.nix

+23 −0 tests/nar-index.sh
diff --git a/src/libstore/nar-accessor.cc b/src/libstore/nar-accessor.cc
@@ -2,6 +2,8 @@
 #include "archive.hh"
 
 #include <map>
+#include <stack>
+#include <algorithm>
 
 namespace nix {
 
@@ -16,45 +18,62 @@ struct NarMember
     size_t start, size;
 
     std::string target;
+
+    /* If this is a directory, all the children of the directory. */
+    std::map<std::string, NarMember> children;
 };
 
 struct NarIndexer : ParseSink, StringSource
 {
-    // FIXME: should store this as a tree. Now we're vulnerable to
-    // O(nm) memory consumption (e.g. for x_0/.../x_n/{y_0..y_m}).
-    typedef std::map<Path, NarMember> Members;
-    Members members;
+    NarMember root;
+    std::stack<NarMember*> parents;
 
-    Path currentPath;
     std::string currentStart;
     bool isExec = false;
 
     NarIndexer(const std::string & nar) : StringSource(nar)
     {
     }
 
+    void createMember(const Path & path, NarMember member) {
+        size_t level = std::count(path.begin(), path.end(), '/');
+        while(parents.size() > level) {
+            parents.pop();
+        }
+
+        if(parents.empty()) {
+            root = std::move(member);
+            parents.push(&root);
+        } else {
+            if(parents.top()->type != FSAccessor::Type::tDirectory) {
+                throw Error(format("NAR file missing parent directory of path ‘%1%’") % path);
+            }
+            auto result = parents.top()->children.emplace(baseNameOf(path), std::move(member));
+            parents.push(&result.first->second);
+        }
+    }
+
     void createDirectory(const Path & path) override
     {
-        members.emplace(path,
-            NarMember{FSAccessor::Type::tDirectory, false, 0, 0});
+        createMember(path, {FSAccessor::Type::tDirectory, false, 0, 0 });
     }
 
     void createRegularFile(const Path & path) override
     {
-        currentPath = path;
+        createMember(path, {FSAccessor::Type::tRegular, false, 0, 0 });
     }
 
     void isExecutable() override
     {
-        isExec = true;
+        parents.top()->isExecutable = true;
     }
 
     void preallocateContents(unsigned long long size) override
     {
         currentStart = string(s, pos, 16);
         assert(size <= std::numeric_limits<size_t>::max());
-        members.emplace(currentPath,
-            NarMember{FSAccessor::Type::tRegular, isExec, pos, (size_t) size});
+        parents.top()->size = (size_t)size;
+        parents.top()->start = pos;
     }
 
     void receiveContents(unsigned char * data, unsigned int len) override
@@ -68,16 +87,42 @@ struct NarIndexer : ParseSink, StringSource
 
     void createSymlink(const Path & path, const string & target) override
     {
-        members.emplace(path,
+        createMember(path,
             NarMember{FSAccessor::Type::tSymlink, false, 0, 0, target});
     }
 
-    Members::iterator find(const Path & path)
+    NarMember* find(const Path & path)
     {
-        auto i = members.find(path);
-        if (i == members.end())
+        Path canon = path == "" ? "" : canonPath(path);
+        NarMember* current = &root;
+        auto end = path.end();
+        for(auto it = path.begin(); it != end; ) {
+            // because it != end, the remaining component is non-empty so we need
+            // a directory
+            if(current->type != FSAccessor::Type::tDirectory) return nullptr;
+
+            // skip slash (canonPath above ensures that this is always a slash)
+            assert(*it == '/');
+            it += 1;
+
+            // lookup current component
+            auto next = std::find(it, end, '/');
+            auto child = current->children.find(std::string(it, next));
+            if(child == current->children.end()) return nullptr;
+            current = &child->second;
+
+            it = next;
+        }
+
+        return current;
+    }
+
+    NarMember& at(const Path & path) {
+        auto result = find(path);
+        if(result == nullptr) {
             throw Error(format("NAR file does not contain path ‘%1%’") % path);
-        return i;
+        }
+        return *result;
     }
 };
 
@@ -93,44 +138,41 @@ struct NarAccessor : public FSAccessor
 
     Stat stat(const Path & path) override
     {
-        auto i = indexer.members.find(path);
-        if (i == indexer.members.end())
+        auto i = indexer.find(path);
+        if (i == nullptr)
             return {FSAccessor::Type::tMissing, 0, false};
-        return {i->second.type, i->second.size, i->second.isExecutable};
+        return {i->type, i->size, i->isExecutable};
     }
 
     StringSet readDirectory(const Path & path) override
     {
-        auto i = indexer.find(path);
+        auto i = indexer.at(path);
 
-        if (i->second.type != FSAccessor::Type::tDirectory)
+        if (i.type != FSAccessor::Type::tDirectory)
             throw Error(format("path ‘%1%’ inside NAR file is not a directory") % path);
 
-        ++i;
         StringSet res;
-        while (i != indexer.members.end() && isInDir(i->first, path)) {
-            // FIXME: really bad performance.
-            if (i->first.find('/', path.size() + 1) == std::string::npos)
-                res.insert(std::string(i->first, path.size() + 1));
-            ++i;
+        for(auto&& child : i.children) {
+            res.insert(child.first);
+
         }
         return res;
     }
 
     std::string readFile(const Path & path) override
     {
-        auto i = indexer.find(path);
-        if (i->second.type != FSAccessor::Type::tRegular)
+        auto i = indexer.at(path);
+        if (i.type != FSAccessor::Type::tRegular)
             throw Error(format("path ‘%1%’ inside NAR file is not a regular file") % path);
-        return std::string(*nar, i->second.start, i->second.size);
+        return std::string(*nar, i.start, i.size);
     }
 
     std::string readLink(const Path & path) override
     {
-        auto i = indexer.find(path);
-        if (i->second.type != FSAccessor::Type::tSymlink)
+        auto i = indexer.at(path);
+        if (i.type != FSAccessor::Type::tSymlink)
             throw Error(format("path ‘%1%’ inside NAR file is not a symlink") % path);
-        return i->second.target;
+        return i.target;
     }
 };
 

diff --git a/tests/local.mk b/tests/local.mk
@@ -13,7 +13,8 @@ nix_tests = \
   check-reqs.sh pass-as-file.sh tarball.sh restricted.sh \
   placeholders.sh nix-shell.sh \
   linux-sandbox.sh \
-  build-remote.sh
+  build-remote.sh \
+  nar-index.sh
   # parallel.sh
 
 install-tests += $(foreach x, $(nix_tests), tests/$(x))

diff --git a/tests/nar-index.nix b/tests/nar-index.nix
@@ -0,0 +1,23 @@
+with import ./config.nix;
+
+rec {
+    a = mkDerivation {
+        name = "nar-index-a";
+        builder = builtins.toFile "builder.sh"
+      ''
+        mkdir $out
+        mkdir $out/foo
+        touch $out/foo-x
+        touch $out/foo/bar
+        touch $out/foo/baz
+        touch $out/qux
+        mkdir $out/zyx
+
+        cat >$out/foo/data <<EOF
+        lasjdöaxnasd
+asdom 12398
+ä"§Æẞ¢«»”alsd
+EOF
+      '';
+    };
+}
diff --git a/tests/nar-index.sh b/tests/nar-index.sh
@@ -0,0 +1,23 @@
+source common.sh
+
+echo "building test path"
+storePath="$(nix-build nar-index.nix -A a --no-out-link)"
+
+cd "$TEST_ROOT"
+
+echo "dumping path to nar"
+narFile="$TEST_ROOT/path.nar"
+nix-store --dump $storePath > $narFile
+
+echo "check that find and ls-nar match"
+( cd $storePath; find . | sort ) > files.find
+nix ls-nar -R -d $narFile "" | sort > files.ls-nar
+diff -u files.find files.ls-nar
+
+echo "check that file contents of data match"
+nix cat-nar $narFile /foo/data > data.cat-nar
+diff -u data.cat-nar $storePath/foo/data
+
+echo "check that file contents of baz match"
+nix cat-nar $narFile /foo/baz > baz.cat-nar
+diff -u baz.cat-nar $storePath/foo/baz