fix(filesystem_extractor): handle sparse files/hardlinks progress

Author: mhx

src/utility/filesystem_extractor.cpp

@@ -102,6 +102,32 @@ sparse_file_mode get_sparse_file_mode_for_format(int format) {
   return sparse_file_mode::no_sparse;
 }
 
+bool format_supports_hardlinks(int const format) {
+  switch (format) {
+  case ARCHIVE_FORMAT_CPIO_SVR4_NOCRC:
+  case ARCHIVE_FORMAT_CPIO_SVR4_CRC:
+    return true;
+
+  default:
+    break;
+  }
+
+  int const fmtbase = format & ARCHIVE_FORMAT_BASE_MASK;
+
+  switch (fmtbase) {
+  case ARCHIVE_FORMAT_ISO9660:
+  case ARCHIVE_FORMAT_SHAR:
+  case ARCHIVE_FORMAT_TAR:
+  case ARCHIVE_FORMAT_XAR:
+    return true;
+
+  default:
+    break;
+  }
+
+  return false;
+}
+
 la_ssize_t write_range_data(sparse_file_mode mode, struct archive* a,
                             void const* data, size_t size, la_int64_t offset) {
   if (mode == sparse_file_mode::sparse_disk) {
@@ -399,17 +425,22 @@ bool filesystem_extractor_<LoggerPolicy>::extract(
   DWARFS_CHECK(a_, "filesystem not opened");
 
   auto sparse_mode = sparse_mode_;
+  bool supports_hardlinks{true};
 
   auto lr = ::archive_entry_linkresolver_new();
 
   scope_exit free_resolver{[&] { ::archive_entry_linkresolver_free(lr); }};
 
   if (auto fmt = ::archive_format(a_.get())) {
+    LOG_DEBUG << "setting link resolver strategy for format " << fmt;
+
     ::archive_entry_linkresolver_set_strategy(lr, fmt);
 
     if (sparse_mode == sparse_file_mode::auto_detect) {
       sparse_mode = get_sparse_file_mode_for_format(fmt);
     }
+
+    supports_hardlinks = format_supports_hardlinks(fmt);
   }
 
   ::archive_entry* spare = nullptr;
@@ -589,11 +620,14 @@ bool filesystem_extractor_<LoggerPolicy>::extract(
 
           if (opts.enable_progress) {
             auto stat = fs.getattr(inode);
-            if (stat.nlink() == 1 ||
+            if (!supports_hardlinks || stat.nlink() == 1 ||
                 seen_hardlinks.insert(inode.inode_num()).second) {
-              data_size += stat.allocated_size();
+              data_size += sparse_mode == sparse_file_mode::sparse_disk
+                               ? stat.allocated_size()
+                               : stat.size();
             }
           }
+
           while (auto parent = entry.parent()) {
             if (!matched_dirs.insert(parent->unix_path()).second) {
               break;
@@ -618,8 +652,19 @@ bool filesystem_extractor_<LoggerPolicy>::extract(
   } else if (opts.enable_progress) {
     vfs_stat vfs;
     fs.statvfs(&vfs);
+
+    uint64_t data_size;
+
+    if (sparse_mode == sparse_file_mode::sparse_disk) {
+      data_size = vfs.total_allocated_fs_size;
+    } else if (supports_hardlinks) {
+      data_size = vfs.total_fs_size;
+    } else {
+      data_size = vfs.total_fs_size + vfs.total_hardlink_size;
+    }
+
     std::lock_guard lock(bytes_total_mx_);
-    bytes_total_.emplace(vfs.blocks);
+    bytes_total_.emplace(data_size);
   }
 
   if (opts.enable_progress) {
@@ -766,11 +811,6 @@ bool filesystem_extractor_<LoggerPolicy>::extract(
   archiver->wait();
   reg_archiver->wait();
 
-  if (opts.enable_progress) {
-    LOG_DEBUG << "progress: " << bytes_written_.load() << "/"
-              << bytes_total_.value() << " bytes written";
-  }
-
   if (hard_error) {
     DWARFS_THROW(runtime_error, "extraction aborted");
   }
@@ -799,6 +839,15 @@ bool filesystem_extractor_<LoggerPolicy>::extract(
     }
   }
 
+  if (opts.enable_progress) {
+    DWARFS_CHECK(bytes_written_.load() == bytes_total_.value(),
+                 fmt::format("progress mismatch: {} (written) != {} (total)",
+                             bytes_written_.load(), bytes_total_.value()));
+
+    LOG_DEBUG << "progress: " << bytes_written_.load() << "/"
+              << bytes_total_.value() << " bytes written";
+  }
+
   if (soft_error > 0) {
     LOG_ERROR << "extraction finished with " << soft_error << " error(s)";
     return false;

test/compat_test.cpp

@@ -42,6 +42,8 @@
 #include <dwarfs/checksum.h>
 #include <dwarfs/config.h>
 #include <dwarfs/file_stat.h>
+#include <dwarfs/file_util.h>
+#include <dwarfs/glob_matcher.h>
 #include <dwarfs/logger.h>
 #include <dwarfs/reader/filesystem_options.h>
 #include <dwarfs/reader/filesystem_v2.h>
@@ -1161,7 +1163,14 @@ void check_compat(logger& lgr [[maybe_unused]], reader::filesystem_v2 const& fs,
   std::ostringstream oss;
 
   EXPECT_NO_THROW(ext.open_stream(oss, {.name = "mtree"}));
-  EXPECT_NO_THROW(ext.extract(fs));
+  EXPECT_NO_THROW(ext.extract(fs, {.enable_progress = true}));
+
+  {
+    auto const pi = ext.get_progress();
+    ASSERT_TRUE(pi.total_bytes.has_value());
+    EXPECT_EQ(pi.total_bytes.value(), pi.extracted_bytes);
+  }
+
   EXPECT_NO_THROW(ext.close());
 
   ref_entries.erase("");
@@ -1270,6 +1279,72 @@ void check_compat(logger& lgr [[maybe_unused]], reader::filesystem_v2 const& fs,
   }
 }
 
+void check_extract_format(logger& lgr, test::os_access_mock& os,
+                          reader::filesystem_v2 const& fs,
+                          std::optional<std::string_view> format = std::nullopt,
+                          glob_matcher const* matcher = nullptr) {
+  utility::filesystem_extractor fsx(lgr, os);
+
+  auto do_extract = [&] {
+    auto const context = fmt::format("{}{}", format.value_or("disk"),
+                                     matcher ? " with matcher" : "");
+
+    EXPECT_NO_THROW(fsx.extract(fs, matcher,
+                                {
+                                    .enable_progress = true,
+                                    .skip_devices = true,
+                                    .skip_specials = true,
+                                }))
+        << context;
+
+    auto const pi = fsx.get_progress();
+    ASSERT_TRUE(pi.total_bytes.has_value()) << context;
+    EXPECT_EQ(pi.total_bytes.value(), pi.extracted_bytes) << context;
+
+    EXPECT_NO_THROW(fsx.close()) << context;
+  };
+
+  if (format.has_value()) {
+#ifndef DWARFS_FILESYSTEM_EXTRACTOR_NO_OPEN_FORMAT
+    std::ostringstream oss;
+    EXPECT_NO_THROW(fsx.open_stream(oss, {.name = std::string{*format}}));
+    do_extract();
+#endif
+  } else {
+    temporary_directory tempdir("dwarfs");
+    auto const td = tempdir.path();
+    EXPECT_NO_THROW(fsx.open_disk(td));
+    do_extract();
+  }
+}
+
+void check_extract_matcher(logger& lgr, test::os_access_mock& os,
+                           reader::filesystem_v2 const& fs,
+                           glob_matcher const* matcher = nullptr) {
+  // check extract-to-disk
+  check_extract_format(lgr, os, fs, std::nullopt, matcher);
+
+#ifndef DWARFS_FILESYSTEM_EXTRACTOR_NO_OPEN_FORMAT
+  // check extract to various formats
+  for (auto const& fmt : test::supported_libarchive_formats()) {
+    if (utility::filesystem_extractor::supports_format(
+            {.name = std::string{fmt.name}})) {
+      check_extract_format(lgr, os, fs, fmt.name, matcher);
+    }
+  }
+#endif
+}
+
+void check_extract(logger& lgr, test::os_access_mock& os,
+                   reader::filesystem_v2 const& fs) {
+  // no matcher - extract all
+  check_extract_matcher(lgr, os, fs);
+
+  // extract only .sh files
+  glob_matcher matcher{"**/*.sh"};
+  check_extract_matcher(lgr, os, fs, &matcher);
+}
+
 auto get_image_path(std::string const& version) {
   return test_dir / "compat" / fmt::format("compat-v{}.dwarfs", version);
 }
@@ -1409,6 +1484,7 @@ TEST_P(compat_filesystem, backwards_compat) {
     reader::filesystem_v2 fs(lgr, os, test::make_real_file_view(filename),
                              opts);
     check_compat(lgr, fs, version);
+    check_extract(lgr, os, fs);
   }
 
   opts.image_offset = reader::filesystem_options::IMAGE_OFFSET_AUTO;
@@ -1481,6 +1557,7 @@ TEST_P(rewrite, filesystem_rewrite) {
     reader::filesystem_v2 fs(lgr, os, mm);
     check_dynamic(version, fs, origmm, rebuild_metadata.has_value());
     check_checksums(fs);
+    check_extract(lgr, os, fs);
     EXPECT_TRUE(fs.has_valid_section_index());
   }
 
@@ -1566,6 +1643,7 @@ TEST_P(rewrite, filesystem_rewrite) {
     reader::filesystem_v2 fs(lgr, os, mm);
     check_dynamic(version, fs, origmm, rebuild_metadata.has_value());
     check_checksums(fs);
+    check_extract(lgr, os, fs);
   }
 
   std::ostringstream rewritten5;
@@ -1587,6 +1665,7 @@ TEST_P(rewrite, filesystem_rewrite) {
     reader::filesystem_v2 fs(lgr, os, mm);
     check_dynamic(version, fs, origmm, rebuild_metadata.has_value());
     check_checksums(fs);
+    check_extract(lgr, os, fs);
   }
 }
 

test/tool_dwarfsextract_main_basic_test.cpp

@@ -32,6 +32,7 @@
 
 #include <dwarfs/binary_literals.h>
 #include <dwarfs/file_util.h>
+#include <dwarfs/reader/filesystem_v2.h>
 #include <dwarfs/utility/filesystem_extractor.h>
 #include <dwarfs/utility/filesystem_extractor_archive_format.h>
 #include <dwarfs/vfs_stat.h>
@@ -259,16 +260,22 @@ std::vector<libarchive_format_def> const libarchive_formats{
 } // namespace
 
 class dwarfsextract_format_test
-    : public testing::TestWithParam<libarchive_format_def> {};
+    : public testing::TestWithParam<std::tuple<libarchive_format_def, bool>> {};
 
 TEST_P(dwarfsextract_format_test, basic) {
-  auto fmt = GetParam();
+  auto const [fmt, use_matcher] = GetParam();
   bool const is_ar = fmt.name.starts_with("ar");
   bool const is_shar = fmt.name.starts_with("shar");
   auto t = dwarfsextract_tester::create_with_image();
   int const expected_exit = fmt.expected_error ? 1 : 0;
-  int exit_code =
-      t.run({"-i", "image.dwarfs", "-f", fmt.name, "--log-level=debug"});
+  std::vector<std::string> args{"-i", "image.dwarfs", "-f", fmt.name,
+                                "--log-level=debug"};
+  if (use_matcher) {
+    args.push_back("**/b*.pl");
+    args.push_back("**/ipsum.*");
+    args.push_back("**/*link");
+  }
+  int exit_code = t.run(args);
   if (!fmt.expected_error && exit_code != 0) {
     if (t.err().find("not supported on this platform") != std::string::npos) {
       GTEST_SKIP();
@@ -280,14 +287,21 @@ TEST_P(dwarfsextract_format_test, basic) {
     EXPECT_THAT(t.err(), ::testing::HasSubstr("extraction aborted"));
   } else if (!is_shar and !is_ar) {
     auto out = t.out();
-    EXPECT_GE(out.size(), fmt.min_size);
+
+    if (!use_matcher) {
+      EXPECT_GE(out.size(), fmt.min_size);
+    }
 
     std::set<std::string> paths;
     std::set<std::string> expected_paths{
         "bar.pl",           "baz.pl",   "empty",       "foo.pl",
         "ipsum.txt",        "somedir",  "somedir/bad", "somedir/empty",
         "somedir/ipsum.py", "somelink", "test.pl",
     };
+    std::set<std::string> expected_paths_with_matcher{
+        "bar.pl",  "baz.pl",           "ipsum.txt",
+        "somedir", "somedir/ipsum.py", "somelink",
+    };
 
     auto ar = ::archive_read_new();
     ASSERT_EQ(ARCHIVE_OK, ::archive_read_support_format_all(ar))
@@ -319,12 +333,18 @@ TEST_P(dwarfsextract_format_test, basic) {
     EXPECT_EQ(ARCHIVE_OK, ::archive_read_free(ar))
         << ::archive_error_string(ar);
 
-    EXPECT_EQ(expected_paths, paths);
+    if (use_matcher) {
+      EXPECT_EQ(expected_paths_with_matcher, paths);
+    } else {
+      EXPECT_EQ(expected_paths, paths);
+    }
   }
 }
 
-INSTANTIATE_TEST_SUITE_P(dwarfs, dwarfsextract_format_test,
-                         ::testing::ValuesIn(libarchive_formats));
+INSTANTIATE_TEST_SUITE_P(
+    dwarfs, dwarfsextract_format_test,
+    ::testing::Combine(::testing::ValuesIn(libarchive_formats),
+                       ::testing::Bool()));
 
 #endif