refactor: move most of filesytem_writer to internal namespace

CMakeLists.txt

@@ -612,6 +612,7 @@ list(APPEND LIBDWARFS_COMMON_SRC
   src/dwarfs/file_access_generic.cpp
   src/dwarfs/file_stat.cpp
   src/dwarfs/file_type.cpp
+  src/dwarfs/filesystem_writer.cpp
   src/dwarfs/fstypes.cpp
   src/dwarfs/history.cpp
   src/dwarfs/internal/features.cpp
@@ -653,7 +654,7 @@ list(APPEND LIBDWARFS_WRITER_SRC
   src/dwarfs/console_writer.cpp
   src/dwarfs/entry_factory.cpp
   src/dwarfs/filesystem_block_category_resolver.cpp
-  src/dwarfs/filesystem_writer.cpp
+  src/dwarfs/filesystem_writer_factory.cpp
   src/dwarfs/filter_debug.cpp
   src/dwarfs/fragment_category.cpp
   src/dwarfs/fragment_order_parser.cpp

include/dwarfs/filesystem_writer.h

@@ -21,172 +21,34 @@
 
 #pragma once
 
-#include <cstddef>
+#include <dwarfs/block_compressor.h>
-#include <cstdint>
-#include <functional>
-#include <memory>
-#include <ostream>
-#include <span>
-#include <utility>
-#include <vector>
-
-#include <dwarfs/compression_constraints.h>
 #include <dwarfs/fragment_category.h>
-#include <dwarfs/fstypes.h>
-#include <dwarfs/options.h>
 
 namespace dwarfs {
 
-class block_compressor;
-class logger;
-class writer_progress;
-class thread_pool;
-
 namespace internal {
 
-class block_data;
+class filesystem_writer_detail;
-class fs_section;
 
-} // namespace internal
+}
 
 class filesystem_writer {
  public:
-  using physical_block_cb_type = std::function<void(size_t)>;
+  explicit filesystem_writer(
+      std::unique_ptr<internal::filesystem_writer_detail> impl);
 
-  filesystem_writer(
+  ~filesystem_writer();
-      std::ostream& os, logger& lgr, thread_pool& pool, writer_progress& prog,
+  filesystem_writer(filesystem_writer&&);
-      block_compressor const& schema_bc, block_compressor const& metadata_bc,
+  filesystem_writer& operator=(filesystem_writer&&);
-      block_compressor const& history_bc,
-      filesystem_writer_options const& options = filesystem_writer_options(),
-      std::istream* header = nullptr);
-
-  void add_default_compressor(block_compressor bc) {
-    impl_->add_default_compressor(std::move(bc));
-  }
 
+  void add_default_compressor(block_compressor bc);
   void add_category_compressor(fragment_category::value_type cat,
-                               block_compressor bc) {
+                               block_compressor bc);
-    impl_->add_category_compressor(cat, std::move(bc));
-  }
 
-  compression_constraints
+  internal::filesystem_writer_detail& get_internal() { return *impl_; }
-  get_compression_constraints(fragment_category::value_type cat,
-                              std::string const& metadata) const {
-    return impl_->get_compression_constraints(cat, metadata);
-  }
 
-  block_compressor const& get_compressor(
+ protected:
-      section_type type,
+  std::unique_ptr<internal::filesystem_writer_detail> impl_;
-      std::optional<fragment_category::value_type> cat = std::nullopt) const {
-    return impl_->get_compressor(type, cat);
-  }
-
-  void configure(std::vector<fragment_category> const& expected_categories,
-                 size_t max_active_slots) {
-    impl_->configure(expected_categories, max_active_slots);
-  }
-
-  void copy_header(std::span<uint8_t const> header) {
-    impl_->copy_header(header);
-  }
-
-  // TODO: check which write_block() API is actually used
-
-  void write_block(fragment_category cat,
-                   std::shared_ptr<internal::block_data>&& data,
-                   physical_block_cb_type physical_block_cb,
-                   std::optional<std::string> meta = std::nullopt) {
-    impl_->write_block(cat, std::move(data), std::move(physical_block_cb),
-                       std::move(meta));
-  }
-
-  void finish_category(fragment_category cat) { impl_->finish_category(cat); }
-
-  void write_block(fragment_category::value_type cat,
-                   std::shared_ptr<internal::block_data>&& data,
-                   std::optional<std::string> meta = std::nullopt) {
-    impl_->write_block(cat, std::move(data), std::move(meta));
-  }
-
-  void write_metadata_v2_schema(std::shared_ptr<internal::block_data>&& data) {
-    impl_->write_metadata_v2_schema(std::move(data));
-  }
-
-  void write_metadata_v2(std::shared_ptr<internal::block_data>&& data) {
-    impl_->write_metadata_v2(std::move(data));
-  }
-
-  void write_history(std::shared_ptr<internal::block_data>&& data) {
-    impl_->write_history(std::move(data));
-  }
-
-  void check_block_compression(
-      compression_type compression, std::span<uint8_t const> data,
-      std::optional<fragment_category::value_type> cat = std::nullopt) {
-    impl_->check_block_compression(compression, data, cat);
-  }
-
-  void write_section(
-      section_type type, compression_type compression,
-      std::span<uint8_t const> data,
-      std::optional<fragment_category::value_type> cat = std::nullopt) {
-    impl_->write_section(type, compression, data, cat);
-  }
-
-  void write_compressed_section(internal::fs_section const& sec,
-                                std::span<uint8_t const> data) {
-    impl_->write_compressed_section(sec, data);
-  }
-
-  void flush() { impl_->flush(); }
-
-  size_t size() const { return impl_->size(); }
-
-  class impl {
-   public:
-    virtual ~impl() = default;
-
-    virtual void add_default_compressor(block_compressor bc) = 0;
-    virtual void add_category_compressor(fragment_category::value_type cat,
-                                         block_compressor bc) = 0;
-    virtual compression_constraints
-    get_compression_constraints(fragment_category::value_type cat,
-                                std::string const& metadata) const = 0;
-    virtual block_compressor const&
-    get_compressor(section_type type,
-                   std::optional<fragment_category::value_type> cat) const = 0;
-    virtual void
-    configure(std::vector<fragment_category> const& expected_categories,
-              size_t max_active_slots) = 0;
-    virtual void copy_header(std::span<uint8_t const> header) = 0;
-    virtual void write_block(fragment_category cat,
-                             std::shared_ptr<internal::block_data>&& data,
-                             physical_block_cb_type physical_block_cb,
-                             std::optional<std::string> meta) = 0;
-    virtual void finish_category(fragment_category cat) = 0;
-    virtual void write_block(fragment_category::value_type cat,
-                             std::shared_ptr<internal::block_data>&& data,
-                             std::optional<std::string> meta) = 0;
-    virtual void
-    write_metadata_v2_schema(std::shared_ptr<internal::block_data>&& data) = 0;
-    virtual void
-    write_metadata_v2(std::shared_ptr<internal::block_data>&& data) = 0;
-    virtual void
-    write_history(std::shared_ptr<internal::block_data>&& data) = 0;
-    virtual void check_block_compression(
-        compression_type compression, std::span<uint8_t const> data,
-        std::optional<fragment_category::value_type> cat) = 0;
-    virtual void
-    write_section(section_type type, compression_type compression,
-                  std::span<uint8_t const> data,
-                  std::optional<fragment_category::value_type> cat) = 0;
-    virtual void write_compressed_section(internal::fs_section const& sec,
-                                          std::span<uint8_t const> data) = 0;
-    virtual void flush() = 0;
-    virtual size_t size() const = 0;
 };
 
- private:
-  std::unique_ptr<impl> impl_;
-};
 } // namespace dwarfs

include/dwarfs/filesystem_writer_factory.h

@@ -0,0 +1,47 @@
+/* vim:set ts=2 sw=2 sts=2 et: */
+/**
+ * \author     Marcus Holland-Moritz (github@mhxnet.de)
+ * \copyright  Copyright (c) Marcus Holland-Moritz
+ *
+ * This file is part of dwarfs.
+ *
+ * dwarfs is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * dwarfs is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with dwarfs.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include <iosfwd>
+
+#include <dwarfs/filesystem_writer.h>
+#include <dwarfs/options.h>
+
+namespace dwarfs {
+
+class block_compressor;
+class logger;
+class writer_progress;
+class thread_pool;
+
+class filesystem_writer_factory {
+ public:
+  static filesystem_writer
+  create(std::ostream& os, logger& lgr, thread_pool& pool,
+         writer_progress& prog, block_compressor const& schema_bc,
+         block_compressor const& metadata_bc,
+         block_compressor const& history_bc,
+         filesystem_writer_options const& options = filesystem_writer_options(),
+         std::istream* header = nullptr);
+};
+
+} // namespace dwarfs

include/dwarfs/internal/filesystem_writer_detail.h

@@ -0,0 +1,89 @@
+/* vim:set ts=2 sw=2 sts=2 et: */
+/**
+ * \author     Marcus Holland-Moritz (github@mhxnet.de)
+ * \copyright  Copyright (c) Marcus Holland-Moritz
+ *
+ * This file is part of dwarfs.
+ *
+ * dwarfs is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * dwarfs is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with dwarfs.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include <cstddef>
+#include <cstdint>
+#include <functional>
+#include <memory>
+#include <optional>
+#include <span>
+#include <utility>
+#include <vector>
+
+#include <dwarfs/block_compressor.h>
+#include <dwarfs/compression_constraints.h>
+#include <dwarfs/fragment_category.h>
+#include <dwarfs/fstypes.h>
+
+namespace dwarfs::internal {
+
+class block_data;
+class fs_section;
+
+class filesystem_writer_detail {
+ public:
+  virtual ~filesystem_writer_detail() = default;
+
+  using physical_block_cb_type = std::function<void(size_t)>;
+
+  virtual void add_default_compressor(block_compressor bc) = 0;
+  virtual void add_category_compressor(fragment_category::value_type cat,
+                                       block_compressor bc) = 0;
+  virtual compression_constraints
+  get_compression_constraints(fragment_category::value_type cat,
+                              std::string const& metadata) const = 0;
+  virtual block_compressor const&
+  get_compressor(section_type type,
+                 std::optional<fragment_category::value_type> cat =
+                     std::nullopt) const = 0;
+  virtual void
+  configure(std::vector<fragment_category> const& expected_categories,
+            size_t max_active_slots) = 0;
+  virtual void copy_header(std::span<uint8_t const> header) = 0;
+  virtual void write_block(fragment_category cat,
+                           std::shared_ptr<internal::block_data>&& data,
+                           physical_block_cb_type physical_block_cb,
+                           std::optional<std::string> meta = std::nullopt) = 0;
+  virtual void finish_category(fragment_category cat) = 0;
+  virtual void write_block(fragment_category::value_type cat,
+                           std::shared_ptr<internal::block_data>&& data,
+                           std::optional<std::string> meta = std::nullopt) = 0;
+  virtual void
+  write_metadata_v2_schema(std::shared_ptr<internal::block_data>&& data) = 0;
+  virtual void
+  write_metadata_v2(std::shared_ptr<internal::block_data>&& data) = 0;
+  virtual void write_history(std::shared_ptr<internal::block_data>&& data) = 0;
+  virtual void check_block_compression(
+      compression_type compression, std::span<uint8_t const> data,
+      std::optional<fragment_category::value_type> cat = std::nullopt) = 0;
+  virtual void write_section(
+      section_type type, compression_type compression,
+      std::span<uint8_t const> data,
+      std::optional<fragment_category::value_type> cat = std::nullopt) = 0;
+  virtual void write_compressed_section(internal::fs_section const& sec,
+                                        std::span<uint8_t const> data) = 0;
+  virtual void flush() = 0;
+  virtual size_t size() const = 0;
+};
+
+} // namespace dwarfs::internal

src/dwarfs/filesystem_v2.cpp

@@ -50,6 +50,7 @@
 #include <dwarfs/internal/block_cache.h>
 #include <dwarfs/internal/block_data.h>
 #include <dwarfs/internal/filesystem_parser.h>
+#include <dwarfs/internal/filesystem_writer_detail.h>
 #include <dwarfs/internal/fs_section.h>
 #include <dwarfs/internal/inode_reader_v2.h>
 #include <dwarfs/internal/metadata_v2.h>
@@ -287,7 +288,7 @@ class filesystem_ final : public filesystem_v2::impl {
   std::vector<file_stat::gid_type> get_all_gids() const override {
     return meta_.get_all_gids();
   }
-  void rewrite(writer_progress& prog, filesystem_writer& writer,
+  void rewrite(writer_progress& prog, filesystem_writer& fs_writer,
                category_resolver const& cat_resolver,
                rewrite_options const& opts) const override;
 
@@ -506,10 +507,11 @@ filesystem_<LoggerPolicy>::filesystem_(
 
 template <typename LoggerPolicy>
 void filesystem_<LoggerPolicy>::rewrite(writer_progress& prog,
-                                        filesystem_writer& writer,
+                                        filesystem_writer& fs_writer,
                                         category_resolver const& cat_resolver,
                                         rewrite_options const& opts) const {
   filesystem_parser parser(mm_, image_offset_);
+  auto& writer = fs_writer.get_internal();
 
   if (opts.recompress_block) {
     size_t block_no{0};

src/dwarfs/scanner.cpp

@@ -61,6 +61,7 @@
 #include <dwarfs/internal/entry.h>
 #include <dwarfs/internal/features.h>
 #include <dwarfs/internal/file_scanner.h>
+#include <dwarfs/internal/filesystem_writer_detail.h>
 #include <dwarfs/internal/fragment_chunkable.h>
 #include <dwarfs/internal/global_entry_data.h>
 #include <dwarfs/internal/inode.h>
@@ -293,7 +294,7 @@ class scanner_ final : public scanner::impl {
            entry_factory& ef, os_access const& os, std::shared_ptr<script> scr,
            const scanner_options& options);
 
-  void scan(filesystem_writer& fsw, std::filesystem::path const& path,
+  void scan(filesystem_writer& fs_writer, std::filesystem::path const& path,
             writer_progress& wprog,
             std::optional<std::span<std::filesystem::path const>> list,
             std::shared_ptr<file_access const> fa) override;
@@ -607,11 +608,12 @@ scanner_<LoggerPolicy>::scan_list(std::filesystem::path const& path,
 
 template <typename LoggerPolicy>
 void scanner_<LoggerPolicy>::scan(
-    filesystem_writer& fsw, const std::filesystem::path& path,
+    filesystem_writer& fs_writer, const std::filesystem::path& path,
     writer_progress& wprog,
     std::optional<std::span<std::filesystem::path const>> list,
     std::shared_ptr<file_access const> fa) {
   auto& prog = wprog.get_internal();
+  auto& fsw = fs_writer.get_internal();
 
   if (!options_.debug_filter_function) {
     LOG_INFO << "scanning " << path;

src/mkdwarfs_main.cpp

@@ -58,6 +58,7 @@
 #include <dwarfs/builtin_script.h>
 #include <dwarfs/categorizer.h>
 #include <dwarfs/category_parser.h>
+#include <dwarfs/checksum.h>
 #include <dwarfs/chmod_entry_transformer.h>
 #include <dwarfs/console_writer.h>
 #include <dwarfs/conv.h>
@@ -66,7 +67,7 @@
 #include <dwarfs/file_access.h>
 #include <dwarfs/filesystem_block_category_resolver.h>
 #include <dwarfs/filesystem_v2.h>
-#include <dwarfs/filesystem_writer.h>
+#include <dwarfs/filesystem_writer_factory.h>
 #include <dwarfs/filter_debug.h>
 #include <dwarfs/fragment_order_parser.h>
 #include <dwarfs/integral_value_parser.h>
@@ -1275,8 +1276,9 @@ int mkdwarfs_main(int argc, sys_char** argv, iolayer const& iol) {
               },
               [&](std::ostringstream& oss) -> std::ostream& { return oss; }};
 
-    fsw.emplace(fsw_os, lgr, compress_pool, prog, schema_bc, metadata_bc,
+    fsw = filesystem_writer_factory::create(
-                history_bc, fswopts, header_ifs ? &header_ifs->is() : nullptr);
+        fsw_os, lgr, compress_pool, prog, schema_bc, metadata_bc, history_bc,
+        fswopts, header_ifs ? &header_ifs->is() : nullptr);
 
     categorized_option<block_compressor> compression_opt;
     contextual_option_parser cop("--compression", compression_opt, cp,

test/compat_test.cpp

@@ -41,7 +41,7 @@
 #include <dwarfs/filesystem_block_category_resolver.h>
 #include <dwarfs/filesystem_extractor.h>
 #include <dwarfs/filesystem_v2.h>
-#include <dwarfs/filesystem_writer.h>
+#include <dwarfs/filesystem_writer_factory.h>
 #include <dwarfs/logger.h>
 #include <dwarfs/mmap.h>
 #include <dwarfs/options.h>
@@ -1127,7 +1127,8 @@ TEST_P(rewrite, filesystem_rewrite) {
   };
 
   {
-    filesystem_writer fsw(rewritten, lgr, pool, prog, bc, bc, bc);
+    auto fsw = filesystem_writer_factory::create(rewritten, lgr, pool, prog, bc,
+                                                 bc, bc);
     fsw.add_default_compressor(bc);
     auto mm = std::make_shared<mmap>(filename);
     EXPECT_NO_THROW(filesystem_v2::identify(lgr, os, mm, idss));
@@ -1149,8 +1150,8 @@ TEST_P(rewrite, filesystem_rewrite) {
   {
     std::istringstream hdr_iss(format_sh);
     filesystem_writer_options fsw_opts;
-    filesystem_writer fsw(rewritten, lgr, pool, prog, bc, bc, bc, fsw_opts,
+    auto fsw = filesystem_writer_factory::create(rewritten, lgr, pool, prog, bc,
-                          &hdr_iss);
+                                                 bc, bc, fsw_opts, &hdr_iss);
     fsw.add_default_compressor(bc);
     rewrite_fs(fsw, std::make_shared<mmap>(filename));
   }
@@ -1175,8 +1176,8 @@ TEST_P(rewrite, filesystem_rewrite) {
   {
     std::istringstream hdr_iss("D");
     filesystem_writer_options fsw_opts;
-    filesystem_writer fsw(rewritten2, lgr, pool, prog, bc, bc, bc, fsw_opts,
+    auto fsw = filesystem_writer_factory::create(
-                          &hdr_iss);
+        rewritten2, lgr, pool, prog, bc, bc, bc, fsw_opts, &hdr_iss);
     fsw.add_default_compressor(bc);
     rewrite_fs(fsw, std::make_shared<test::mmap_mock>(rewritten.str()));
   }
@@ -1193,7 +1194,8 @@ TEST_P(rewrite, filesystem_rewrite) {
   std::ostringstream rewritten3;
 
   {
-    filesystem_writer fsw(rewritten3, lgr, pool, prog, bc, bc, bc);
+    auto fsw = filesystem_writer_factory::create(rewritten3, lgr, pool, prog,
+                                                 bc, bc, bc);
     fsw.add_default_compressor(bc);
     rewrite_fs(fsw, std::make_shared<test::mmap_mock>(rewritten2.str()));
   }
@@ -1212,7 +1214,8 @@ TEST_P(rewrite, filesystem_rewrite) {
   {
     filesystem_writer_options fsw_opts;
     fsw_opts.remove_header = true;
-    filesystem_writer fsw(rewritten4, lgr, pool, prog, bc, bc, bc, fsw_opts);
+    auto fsw = filesystem_writer_factory::create(rewritten4, lgr, pool, prog,
+                                                 bc, bc, bc, fsw_opts);
     fsw.add_default_compressor(bc);
     rewrite_fs(fsw, std::make_shared<test::mmap_mock>(rewritten3.str()));
   }
@@ -1231,7 +1234,8 @@ TEST_P(rewrite, filesystem_rewrite) {
   {
     filesystem_writer_options fsw_opts;
     fsw_opts.no_section_index = true;
-    filesystem_writer fsw(rewritten5, lgr, pool, prog, bc, bc, bc, fsw_opts);
+    auto fsw = filesystem_writer_factory::create(rewritten5, lgr, pool, prog,
+                                                 bc, bc, bc, fsw_opts);
     fsw.add_default_compressor(bc);
     rewrite_fs(fsw, std::make_shared<test::mmap_mock>(rewritten4.str()));
   }

test/dwarfs_benchmark.cpp

@@ -29,7 +29,7 @@
 #include <dwarfs/entry_factory.h>
 #include <dwarfs/file_stat.h>
 #include <dwarfs/filesystem_v2.h>
-#include <dwarfs/filesystem_writer.h>
+#include <dwarfs/filesystem_writer_factory.h>
 #include <dwarfs/iovec_read_buf.h>
 #include <dwarfs/logger.h>
 #include <dwarfs/options.h>
@@ -132,7 +132,8 @@ std::string make_filesystem(::benchmark::State const& state) {
   std::ostringstream oss;
 
   block_compressor bc("null");
-  filesystem_writer fsw(oss, lgr, pool, prog, bc, bc, bc);
+  auto fsw =
+      filesystem_writer_factory::create(oss, lgr, pool, prog, bc, bc, bc);
   fsw.add_default_compressor(bc);
 
   s.scan(fsw, "", prog);

test/dwarfs_test.cpp

@@ -41,7 +41,7 @@
 #include <dwarfs/file_stat.h>
 #include <dwarfs/file_type.h>
 #include <dwarfs/filesystem_v2.h>
-#include <dwarfs/filesystem_writer.h>
+#include <dwarfs/filesystem_writer_factory.h>
 #include <dwarfs/filter_debug.h>
 #include <dwarfs/iovec_read_buf.h>
 #include <dwarfs/logger.h>
@@ -104,7 +104,8 @@ build_dwarfs(logger& lgr, std::shared_ptr<test::os_access_mock> input,
   std::ostringstream oss;
 
   block_compressor bc(compression);
-  filesystem_writer fsw(oss, lgr, pool, *prog, bc, bc, bc);
+  auto fsw =
+      filesystem_writer_factory::create(oss, lgr, pool, *prog, bc, bc, bc);
   fsw.add_default_compressor(bc);
 
   s.scan(fsw, std::filesystem::path("/"), *prog, input_list);
@@ -966,7 +967,8 @@ class filter_test
 
     block_compressor bc("null");
     std::ostringstream null;
-    filesystem_writer fsw(null, lgr, pool, prog, bc, bc, bc);
+    auto fsw =
+        filesystem_writer_factory::create(null, lgr, pool, prog, bc, bc, bc);
     s.scan(fsw, std::filesystem::path("/"), prog);
 
     return oss.str();