archive: refactor bad mutable-state API in the NAR parse listener

Remove the mutable state stuff that assumes that one file is being
written a time. It's true that we don't write multiple files
interleaved, but that mutable state is evil.

Change-Id: Ia1481da48255d901e4b09a9b783e7af44fae8cff

src/libstore/nar-accessor.cc

@@ -45,11 +45,12 @@ struct NarAccessor : public FSAccessor
 
         uint64_t pos = 0;
 
+    public:
         NarIndexer(NarAccessor & acc, Source & source)
             : acc(acc), source(source)
         { }
 
-        void createMember(const Path & path, NarMember member)
+        NarMember & createMember(const Path & path, NarMember member)
         {
             size_t level = std::count(path.begin(), path.end(), '/');
             while (parents.size() > level) parents.pop();
@@ -63,6 +64,8 @@ struct NarAccessor : public FSAccessor
                 auto result = parents.top()->children.emplace(baseNameOf(path), std::move(member));
                 parents.push(&result.first->second);
             }
+
+            return *parents.top();
         }
 
         void createDirectory(const Path & path) override
@@ -70,28 +73,17 @@ struct NarAccessor : public FSAccessor
             createMember(path, {FSAccessor::Type::tDirectory, false, 0, 0});
         }
 
-        void createRegularFile(const Path & path) override
+        std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable) override
         {
-            createMember(path, {FSAccessor::Type::tRegular, false, 0, 0});
+            auto & memb = createMember(path, {FSAccessor::Type::tRegular, false, 0, 0});
-        }
 
-        void closeRegularFile() override
-        { }
-
-        void isExecutable() override
-        {
-            parents.top()->isExecutable = true;
-        }
-
-        void preallocateContents(uint64_t size) override
-        {
             assert(size <= std::numeric_limits<uint64_t>::max());
-            parents.top()->size = (uint64_t) size;
+            memb.size = (uint64_t) size;
-            parents.top()->start = pos;
+            memb.start = pos;
-        }
+            memb.isExecutable = executable;
 
-        void receiveContents(std::string_view data) override
+            return std::make_unique<FileHandle>();
-        { }
+        }
 
         void createSymlink(const Path & path, const std::string & target) override
         {

src/libstore/store-api.cc

@@ -386,17 +386,28 @@ namespace {
  */
 struct RetrieveRegularNARVisitor : NARParseVisitor
 {
+    struct MyFileHandle : public FileHandle
+    {
+        Sink & sink;
+
+        void receiveContents(std::string_view data) override
+        {
+            sink(data);
+        }
+
+    private:
+        MyFileHandle(Sink & sink) : sink(sink) {}
+
+        friend struct RetrieveRegularNARVisitor;
+    };
+
     Sink & sink;
 
     RetrieveRegularNARVisitor(Sink & sink) : sink(sink) { }
 
-    void createRegularFile(const Path & path) override
+    std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable) override
     {
-    }
+        return std::unique_ptr<MyFileHandle>(new MyFileHandle{sink});
-
-    void receiveContents(std::string_view data) override
-    {
-        sink(data);
     }
 
     void createDirectory(const Path & path) override

src/libutil/archive.cc

@@ -347,16 +347,13 @@ static WireFormatGenerator restore(NARParseVisitor & sink, Generator<nar::Entry>
                 },
                 [&](nar::File f) {
                     return [](auto f, auto & sink) -> WireFormatGenerator {
-                        sink.createRegularFile(f.path);
+                        auto handle = sink.createRegularFile(f.path, f.size, f.executable);
-                        sink.preallocateContents(f.size);
+
-                        if (f.executable) {
-                            sink.isExecutable();
-                        }
                         while (auto block = f.contents.next()) {
-                            sink.receiveContents(std::string_view{block->data(), block->size()});
+                            handle->receiveContents(std::string_view{block->data(), block->size()});
                             co_yield *block;
                         }
-                        sink.closeRegularFile();
+                        handle->close();
                     }(std::move(f), sink);
                 },
                 [&](nar::Symlink sl) {
@@ -422,8 +419,67 @@ void parseDump(NARParseVisitor & sink, Source & source)
 struct NARRestoreVisitor : NARParseVisitor
 {
     Path dstPath;
-    AutoCloseFD fd;
 
+private:
+    class MyFileHandle : public FileHandle
+    {
+        AutoCloseFD fd;
+
+        MyFileHandle(AutoCloseFD && fd, uint64_t size, bool executable) : FileHandle(), fd(std::move(fd))
+        {
+            if (executable) {
+                makeExecutable();
+            }
+
+            maybePreallocateContents(size);
+        }
+
+        void makeExecutable()
+        {
+            struct stat st;
+            if (fstat(fd.get(), &st) == -1)
+                throw SysError("fstat");
+            if (fchmod(fd.get(), st.st_mode | (S_IXUSR | S_IXGRP | S_IXOTH)) == -1)
+                throw SysError("fchmod");
+        }
+
+        void maybePreallocateContents(uint64_t len)
+        {
+            if (!archiveSettings.preallocateContents)
+                return;
+
+#if HAVE_POSIX_FALLOCATE
+            if (len) {
+                errno = posix_fallocate(fd.get(), 0, len);
+                /* Note that EINVAL may indicate that the underlying
+                   filesystem doesn't support preallocation (e.g. on
+                   OpenSolaris).  Since preallocation is just an
+                   optimisation, ignore it. */
+                if (errno && errno != EINVAL && errno != EOPNOTSUPP && errno != ENOSYS)
+                    throw SysError("preallocating file of %1% bytes", len);
+            }
+#endif
+        }
+
+    public:
+
+        ~MyFileHandle() = default;
+
+        virtual void close() override
+        {
+            /* Call close explicitly to make sure the error is checked */
+            fd.close();
+        }
+
+        void receiveContents(std::string_view data) override
+        {
+            writeFull(fd.get(), data);
+        }
+
+        friend struct NARRestoreVisitor;
+    };
+
+public:
     void createDirectory(const Path & path) override
     {
         Path p = dstPath + path;
@@ -431,49 +487,13 @@ struct NARRestoreVisitor : NARParseVisitor
             throw SysError("creating directory '%1%'", p);
     };
 
-    void createRegularFile(const Path & path) override
+    std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable) override
     {
         Path p = dstPath + path;
-        fd = AutoCloseFD{open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666)};
+        AutoCloseFD fd = AutoCloseFD{open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666)};
         if (!fd) throw SysError("creating file '%1%'", p);
-    }
 
-    void closeRegularFile() override
+        return std::unique_ptr<MyFileHandle>(new MyFileHandle(std::move(fd), size, executable));
-    {
-        /* Call close explicitly to make sure the error is checked */
-        fd.close();
-    }
-
-    void isExecutable() override
-    {
-        struct stat st;
-        if (fstat(fd.get(), &st) == -1)
-            throw SysError("fstat");
-        if (fchmod(fd.get(), st.st_mode | (S_IXUSR | S_IXGRP | S_IXOTH)) == -1)
-            throw SysError("fchmod");
-    }
-
-    void preallocateContents(uint64_t len) override
-    {
-        if (!archiveSettings.preallocateContents)
-            return;
-
-#if HAVE_POSIX_FALLOCATE
-        if (len) {
-            errno = posix_fallocate(fd.get(), 0, len);
-            /* Note that EINVAL may indicate that the underlying
-               filesystem doesn't support preallocation (e.g. on
-               OpenSolaris).  Since preallocation is just an
-               optimisation, ignore it. */
-            if (errno && errno != EINVAL && errno != EOPNOTSUPP && errno != ENOSYS)
-                throw SysError("preallocating file of %1% bytes", len);
-        }
-#endif
-    }
-
-    void receiveContents(std::string_view data) override
-    {
-        writeFull(fd.get(), data);
     }
 
     void createSymlink(const Path & path, const std::string & target) override

src/libutil/archive.hh

@@ -83,15 +83,40 @@ WireFormatGenerator dumpString(std::string_view s);
  */
 struct NARParseVisitor
 {
-    virtual void createDirectory(const Path & path) { };
+    /**
+     * A type-erased file handle specific to this particular NARParseVisitor.
+     */
+    struct FileHandle
+    {
+        FileHandle() {}
+        FileHandle(FileHandle const &) = delete;
+        FileHandle & operator=(FileHandle &) = delete;
 
-    virtual void createRegularFile(const Path & path) { };
+        /** Puts one block of data into the file */
-    virtual void closeRegularFile() { };
+        virtual void receiveContents(std::string_view data) { }
-    virtual void isExecutable() { };
-    virtual void preallocateContents(uint64_t size) { };
-    virtual void receiveContents(std::string_view data) { };
 
-    virtual void createSymlink(const Path & path, const std::string & target) { };
+        /**
+         * Explicitly closes the file. Further operations may throw an assert.
+         * This exists so that closing can fail and throw an exception without doing so in a destructor.
+         */
+        virtual void close() { }
+
+        virtual ~FileHandle() = default;
+    };
+
+    virtual void createDirectory(const Path & path) { }
+
+    /**
+     * Creates a regular file in the extraction output with the given size and executable flag.
+     * The size is guaranteed to be the true size of the file.
+     */
+    [[nodiscard]]
+    virtual std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable)
+    {
+        return std::make_unique<FileHandle>();
+    }
+
+    virtual void createSymlink(const Path & path, const std::string & target) { }
 };
 
 namespace nar {