diff --git a/CHANGELOG.md b/CHANGELOG.md index 9df09fbad4..d7605c5815 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ # 0.11.0 +- Keep archive member metadata consistent with the first stored entry selected for content reads + when normalized member names repeat. + - Adapt ordered content-output scheduling to heavy batches, apply byte backpressure, and share the matcher/coordinator CPU allowance while preserving whole-file validation and cancellation. - Avoid redundant 64 KiB buffer clearing in legacy archive extraction. diff --git a/TODO.md b/TODO.md index a7b37c58dd..f0924fdefe 100644 --- a/TODO.md +++ b/TODO.md @@ -585,6 +585,10 @@ EP/JC/P work; candidates are not speedup claims until measured on both platforms +27% on Mac and +11% on Linux. Eliminate those costs without arbitrary count/size thresholds, preserving identity, budgets, duplicate semantics, exact-content oracles and the rejected root-offset variant. Native source identity and streamed-ZIP sizes remain separate open work. +- [x] O19b4a: Make archive index metadata follow the first normalized stored header, matching + content selection before retaining payload locators. Preserve synthesized-parent replacement, + first symlink targets, file/directory types, unique listings and direct-child counts with + dedicated regression tests. - [ ] O19b4: Qualify reuse of validated plain-TAR payload locators independently of name preparation. The 10,000-member native search still spends roughly 8-13 seconds replaying headers. Preserve format validation, exact source identity, duplicate selection, nested and diff --git a/docs/archive-safety-recipes.md b/docs/archive-safety-recipes.md index 0a085c4b1d..dbd4a6d799 100644 --- a/docs/archive-safety-recipes.md +++ b/docs/archive-safety-recipes.md @@ -63,6 +63,11 @@ without a suitable profile. A dry run can preview supported actions, but it does blocks. Inspection does not require `--archive-extract`; that flag materializes members for child commands and therefore involves additional writing and execution capabilities. +When multiple stored members normalize to the same virtual path, inspection uses the first +stored member for both metadata and content. Directory listings show that path once. A directory +synthesized from a child can still receive its first explicit stored entry; later duplicates do +not replace it. This reader rule does not change packing collision or archive-rewrite policy. + ## Create a new archive ```sh diff --git a/extra_modules/archive/BUILD.bazel b/extra_modules/archive/BUILD.bazel index 54d13b0fe0..9d9a8fd706 100644 --- a/extra_modules/archive/BUILD.bazel +++ b/extra_modules/archive/BUILD.bazel @@ -697,6 +697,23 @@ cc_test( ], ) +cc_test( + name = "archive_duplicate_metadata_test", + srcs = ["archive_duplicate_metadata_test.cc"], + deps = [ + ":archive_fs_cc", + ":memory_archive_test_cc", + "@abseil-cpp//absl/status", + "@abseil-cpp//absl/status:statusor", + "@googletest//:gtest", + "@googletest//:gtest_main", + "@mboworks_mbo//mbo/status:status_macros_cc", + "@mboworks_mbo//mbo/testing:matchers_cc", + "@mboworks_mbo//mbo/testing:status_cc", + "@xff_extras_api//:vfs_cc", + ], +) + cc_test( name = "archive_member_key_test", srcs = ["archive_member_key_test.cc"], diff --git a/extra_modules/archive/archive_duplicate_metadata_test.cc b/extra_modules/archive/archive_duplicate_metadata_test.cc new file mode 100644 index 0000000000..d10e997b80 --- /dev/null +++ b/extra_modules/archive/archive_duplicate_metadata_test.cc @@ -0,0 +1,159 @@ +// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include +#include + +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "mbo/status/status_macros.h" +#include "mbo/testing/matchers.h" +#include "mbo/testing/status.h" +#include "xff/archive/archive_fs.h" +#include "xff/archive/memory_archive_test.h" +#include "xff/vfs/entry.h" +#include "xff/vfs/filesystem.h" + +namespace xff::archive { +namespace { +using ::mbo::testing::EqualsText; +using ::mbo::testing::IsOk; +using ::mbo::testing::IsOkAndHolds; +using ::mbo::testing::StatusIs; +using ::testing::AllOf; +using ::testing::ElementsAre; +using ::testing::Eq; +using ::testing::Field; +using ::testing::SizeIs; + +struct ArchiveDuplicateMetadataTest : ::testing::Test { + static constexpr std::string_view kContainer = "duplicates.tar"; + static constexpr std::string_view kMember = "duplicates.tar!shared"; + + static absl::StatusOr Open(std::span members) { + MBO_ASSIGN_OR_RETURN(auto bytes, MakeMemoryTar(members)); + return ArchiveFileSystem::OpenBytes(kContainer, std::move(bytes)); + } +}; + +TEST_F(ArchiveDuplicateMetadataTest, FirstRegularSizeDescribesTheContentAndDirectoryObservation) { + const auto members = std::to_array({ + { + .path = "./shared", + .content = "x", + }, + { + .path = "shared", + .content = "replacement", + }, + }); + MBO_ASSERT_OK_AND_ASSIGN(const auto fs, Open(members)); + EXPECT_THAT(fs.Stat(kMember, false), IsOkAndHolds(Field("size", &vfs::Metadata::size, Eq(1U)))); + EXPECT_THAT(fs.ReadContent(kMember), IsOkAndHolds(EqualsText("x"))); + MBO_ASSERT_OK_AND_ASSIGN(const auto observed, fs.ReadDirMetadata(kContainer, {})); + EXPECT_THAT(observed.entries, SizeIs(1)); + EXPECT_THAT( + observed.observations, + ElementsAre(AllOf( + Field("status", &vfs::MetadataObservation::status, IsOk()), + Field("value", &vfs::MetadataObservation::value, Field("size", &vfs::Metadata::size, Eq(1U)))))); +} + +TEST_F(ArchiveDuplicateMetadataTest, FirstRegularFileIsNotReplacedByADirectory) { + const auto members = std::to_array({ + { + .path = "shared", + .content = "first", + }, + { + .path = "./shared/", + .directory = true, + }, + }); + MBO_ASSERT_OK_AND_ASSIGN(const auto fs, Open(members)); + EXPECT_THAT( + fs.Stat(kMember, false), IsOkAndHolds(AllOf( + Field("type", &vfs::Metadata::type, Eq(vfs::FileType::kRegular)), + Field("mode", &vfs::Metadata::mode, Eq(0640U))))); + EXPECT_THAT(fs.ReadContent(kMember), IsOkAndHolds(EqualsText("first"))); +} + +TEST_F(ArchiveDuplicateMetadataTest, FirstDirectoryIsNotReplacedByARegularFile) { + const auto members = std::to_array({ + { + .path = "shared/", + .directory = true, + }, + { + .path = "./shared", + .content = "later", + }, + }); + MBO_ASSERT_OK_AND_ASSIGN(const auto fs, Open(members)); + EXPECT_THAT( + fs.Stat(kMember, false), IsOkAndHolds(AllOf( + Field("type", &vfs::Metadata::type, Eq(vfs::FileType::kDirectory)), + Field("mode", &vfs::Metadata::mode, Eq(0755U))))); + EXPECT_THAT(fs.ReadContent(kMember), StatusIs(absl::StatusCode::kFailedPrecondition)); +} + +TEST_F(ArchiveDuplicateMetadataTest, FirstSymlinkIsNotReplacedByARegularFile) { + const auto members = std::to_array({ + { + .path = "./shared", + .symlink_target = "first-target", + }, + { + .path = "shared", + .content = "later", + }, + }); + MBO_ASSERT_OK_AND_ASSIGN(const auto fs, Open(members)); + EXPECT_THAT(fs.Stat(kMember, false), IsOkAndHolds(Field("type", &vfs::Metadata::type, Eq(vfs::FileType::kSymlink)))); + EXPECT_THAT(fs.ReadLink(kMember), IsOkAndHolds(EqualsText("first-target"))); + EXPECT_THAT(fs.ReadContent(kMember), StatusIs(absl::StatusCode::kFailedPrecondition)); +} + +TEST_F(ArchiveDuplicateMetadataTest, FirstSymlinkTargetIsRetained) { + const auto members = std::to_array({ + { + .path = "shared", + .symlink_target = "first-target", + }, + { + .path = "./shared", + .symlink_target = "later-target", + }, + }); + MBO_ASSERT_OK_AND_ASSIGN(const auto fs, Open(members)); + EXPECT_THAT(fs.ReadLink(kMember), IsOkAndHolds(EqualsText("first-target"))); +} + +TEST_F(ArchiveDuplicateMetadataTest, FirstStoredDirectoryReplacesItsSynthesizedPlaceholder) { + const auto members = std::to_array({ + { + .path = "shared/leaf", + }, + { + .path = "./shared/", + .directory = true, + }, + { + .path = "shared", + .content = "later", + }, + }); + MBO_ASSERT_OK_AND_ASSIGN(const auto fs, Open(members)); + EXPECT_THAT( + fs.Stat(kMember, false), IsOkAndHolds(AllOf( + Field("type", &vfs::Metadata::type, Eq(vfs::FileType::kDirectory)), + Field("mode", &vfs::Metadata::mode, Eq(0755U))))); + EXPECT_THAT(fs.ReadDir(kMember), IsOkAndHolds(SizeIs(1))); +} + +} // namespace +} // namespace xff::archive diff --git a/extra_modules/archive/archive_fs.cc b/extra_modules/archive/archive_fs.cc index 58c9d18f58..56fce619a9 100644 --- a/extra_modules/archive/archive_fs.cc +++ b/extra_modules/archive/archive_fs.cc @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -305,7 +306,7 @@ ArchiveFileSystem ArchiveFileSystem::Index( node.metadata.type = is_dir ? vfs::FileType::kDirectory : member.is_symlink ? vfs::FileType::kSymlink : vfs::FileType::kRegular; - node.metadata.size = static_cast(member.size < 0 ? 0 : member.size); + node.metadata.size = static_cast(std::max(0, member.size)); node.metadata.mode = member.mode != 0 ? member.mode : (is_dir ? 0555U : 0444U); node.metadata.mtime = absl::FromUnixSeconds(member.mtime); node.metadata.dev = device; @@ -317,6 +318,11 @@ ArchiveFileSystem ArchiveFileSystem::Index( node.metadata.source = vfs::Source::kArchiveMember; node.link_target = member.link_target; const auto [entry_it, inserted] = fs.nodes_.try_emplace(key); + // Content readers select the first stored header. A synthesized ancestor may still + // receive its first explicit header, without allowing later duplicates to replace it. + if (!inserted && !entry_it->second.synthesized) { + continue; + } const std::uint32_t direct_children = entry_it->second.direct_child_count; entry_it->second = std::move(node); entry_it->second.direct_child_count = direct_children;