From 5fc97b35110bad129e42760cfdca7bbaa2de4cb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Sj=C3=B6blom?= Date: Wed, 8 Jul 2026 16:32:03 +0200 Subject: [PATCH 1/4] When using --update, the code now checks against the (attributes) file --- docs/commands/add.md | 23 +++-- src/completion/mpqcli.fish | 2 +- src/completion/mpqcli.ps1 | 4 +- src/helpers.cpp | 177 +++++++++++++++++++++++++++++++++++++ src/helpers.h | 8 ++ src/main.cpp | 2 +- src/mpq.cpp | 49 +++++++++- test/test_add.py | 25 +++++- 8 files changed, 274 insertions(+), 16 deletions(-) diff --git a/docs/commands/add.md b/docs/commands/add.md index d07ffb0..99cae71 100644 --- a/docs/commands/add.md +++ b/docs/commands/add.md @@ -58,9 +58,22 @@ $ mpqcli add wow-patch.mpq textures/ --path textures ## Skip unchanged files with --update -When adding a directory, the `--update` flag skips any file whose on-disk size matches the -size already stored in the archive. This is useful for incremental updates where only -changed files need to be re-added. +When adding a directory, the `--update` flag skips files that have not changed since they +were last added to the archive. This is useful for incremental updates where only changed +files need to be re-added. + +The skip decision is made in two steps: + +1. **File size** must match. If the sizes differ the file is always re-added. +2. If the sizes match, the archive's `(attributes)` file is consulted for a stronger + check. The most reliable attribute available wins: + - **MD5** – if the archive stores MD5 checksums, the MD5 of the local file is computed + and compared. A match skips the file; a mismatch re-adds it. + - **CRC32** – used when MD5 is absent. Same logic. + - **Timestamp** – used when neither MD5 nor CRC32 is present. The file's + last-modification time is compared at one-second resolution. + - **No attributes** – if the archive has no `(attributes)` file, the file is always + re-added even when sizes match, because no reliable content check is possible. ```bash $ mpqcli add wow-patch.mpq textures/ --update --overwrite @@ -69,10 +82,6 @@ $ mpqcli add wow-patch.mpq textures/ --update --overwrite [*] For textures: 1 files added, 1 files skipped, 0 files failed. ``` -Note: the skip check is size-based only. Files with the same size but different content -are not detected as changed. If precise change detection matters, pass `--overwrite` -without `--update` to unconditionally replace every file. - ## Control where files are stored For single files, one can specify both directory and filename in one step using `-p` or `--path`: diff --git a/src/completion/mpqcli.fish b/src/completion/mpqcli.fish index cec6666..4cababe 100644 --- a/src/completion/mpqcli.fish +++ b/src/completion/mpqcli.fish @@ -129,7 +129,7 @@ complete -c mpqcli -n '__fish_seen_subcommand_from add' \ complete -c mpqcli -n '__fish_seen_subcommand_from add' \ -s w -l overwrite -d 'Overwrite file if it already exists in the archive' complete -c mpqcli -n '__fish_seen_subcommand_from add' \ - -s u -l update -d 'Skip files whose archived size matches on-disk size' + -s u -l update -d 'Skip unchanged files when adding a directory' complete -c mpqcli -n '__fish_seen_subcommand_from add' \ -l locale -d 'Locale to use for added file' \ -r -a "$__mpqcli_locales" diff --git a/src/completion/mpqcli.ps1 b/src/completion/mpqcli.ps1 index cd651ac..47d77b4 100644 --- a/src/completion/mpqcli.ps1 +++ b/src/completion/mpqcli.ps1 @@ -105,8 +105,8 @@ Register-ArgumentCompleter -Native -CommandName 'mpqcli', 'mpqcli.exe' -ScriptBl '--path' = 'Archive path for a single file, or prefix for a directory' '-w' = 'Overwrite file if it already is in MPQ archive' '--overwrite' = 'Overwrite file if it already is in MPQ archive' - '-u' = 'Skip files whose archived size matches on-disk size' - '--update' = 'Skip files whose archived size matches on-disk size' + '-u' = 'Skip unchanged files when adding a directory' + '--update' = 'Skip unchanged files when adding a directory' '--locale' = 'Locale to use for added file' '-g' = 'Game profile for compression rules' '--game' = 'Game profile for compression rules' diff --git a/src/helpers.cpp b/src/helpers.cpp index cbdb877..68588a4 100644 --- a/src/helpers.cpp +++ b/src/helpers.cpp @@ -1,10 +1,13 @@ #include "helpers.h" #include +#include #include #include #include +#include #include +#include #ifdef _WIN32 #include @@ -128,3 +131,177 @@ void PrintAsBinary(const char *buffer, uint32_t size) { #endif std::cout.write(buffer, size); } + +// ---- CRC32 -------------------------------------------------------------- +// Standard ZIP/gzip CRC32 (polynomial 0xEDB88320, reflected bit order). + +static const std::array kCrc32Table = []() { + std::array t{}; + for (uint32_t i = 0; i < 256; ++i) { + uint32_t c = i; + for (int j = 0; j < 8; ++j) + c = (c >> 1) ^ (c & 1u ? 0xEDB88320u : 0u); + t[i] = c; + } + return t; +}(); + +std::optional ComputeFileCrc32(const fs::path &path) { + std::ifstream f(path, std::ios::binary); + if (!f) { + return std::nullopt; + } + uint32_t crc = 0xFFFFFFFFu; + char buf[65536]; + while (f.read(buf, sizeof(buf)) || f.gcount() > 0) { + for (std::streamsize i = 0; i < f.gcount(); ++i) + crc = (crc >> 8) ^ kCrc32Table[(crc ^ static_cast(buf[i])) & 0xFFu]; + } + return crc ^ 0xFFFFFFFFu; +} + +// ---- MD5 ---------------------------------------------------------------- +// Compact RFC 1321 implementation – no external dependencies. + +namespace { + +constexpr uint32_t kMd5K[64] = { + // Round 1 + 0xd76aa478u, 0xe8c7b756u, 0x242070dbu, 0xc1bdceeeu, + 0xf57c0fafu, 0x4787c62au, 0xa8304613u, 0xfd469501u, + 0x698098d8u, 0x8b44f7afu, 0xffff5bb1u, 0x895cd7beu, + 0x6b901122u, 0xfd987193u, 0xa679438eu, 0x49b40821u, + // Round 2 + 0xf61e2562u, 0xc040b340u, 0x265e5a51u, 0xe9b6c7aau, + 0xd62f105du, 0x02441453u, 0xd8a1e681u, 0xe7d3fbc8u, + 0x21e1cde6u, 0xc33707d6u, 0xf4d50d87u, 0x455a14edu, + 0xa9e3e905u, 0xfcefa3f8u, 0x676f02d9u, 0x8d2a4c8au, + // Round 3 + 0xfffa3942u, 0x8771f681u, 0x6d9d6122u, 0xfde5380cu, + 0xa4beea44u, 0x4bdecfa9u, 0xf6bb4b60u, 0xbebfbc70u, + 0x289b7ec6u, 0xeaa127fau, 0xd4ef3085u, 0x04881d05u, + 0xd9d4d039u, 0xe6db99e5u, 0x1fa27cf8u, 0xc4ac5665u, + // Round 4 + 0xf4292244u, 0x432aff97u, 0xab9423a7u, 0xfc93a039u, + 0x655b59c3u, 0x8f0ccc92u, 0xffeff47du, 0x85845dd1u, + 0x6fa87e4fu, 0xfe2ce6e0u, 0xa3014314u, 0x4e0811a1u, + 0xf7537e82u, 0xbd3af235u, 0x2ad7d2bbu, 0xeb86d391u, +}; + +constexpr uint8_t kMd5S[64] = { + 7, 12, 17, 22, 7, 12, 17, 22, 7, 12, 17, 22, 7, 12, 17, 22, + 5, 9, 14, 20, 5, 9, 14, 20, 5, 9, 14, 20, 5, 9, 14, 20, + 4, 11, 16, 23, 4, 11, 16, 23, 4, 11, 16, 23, 4, 11, 16, 23, + 6, 10, 15, 21, 6, 10, 15, 21, 6, 10, 15, 21, 6, 10, 15, 21, +}; + +struct Md5State { + uint32_t a{0x67452301u}; + uint32_t b{0xEFCDAB89u}; + uint32_t c{0x98BADCFEu}; + uint32_t d{0x10325476u}; + uint64_t count{0}; + uint8_t buf[64]{}; +}; + +static uint32_t RotLeft32(uint32_t x, uint8_t n) { return (x << n) | (x >> (32u - n)); } + +static void Md5Block(Md5State &s, const uint8_t *blk) { + uint32_t w[16]; + for (int i = 0; i < 16; ++i) { + w[i] = static_cast(blk[4 * i]) + | (static_cast(blk[4 * i + 1]) << 8u) + | (static_cast(blk[4 * i + 2]) << 16u) + | (static_cast(blk[4 * i + 3]) << 24u); + } + uint32_t a = s.a, b = s.b, c = s.c, d = s.d; + for (int i = 0; i < 64; ++i) { + uint32_t f = 0; + uint32_t g = 0; + if (i < 16) { + f = (b & c) | (~b & d); + g = static_cast(i); + } else if (i < 32) { + f = (d & b) | (~d & c); + g = static_cast(5 * i + 1) % 16u; + } else if (i < 48) { + f = b ^ c ^ d; + g = static_cast(3 * i + 5) % 16u; + } else { + f = c ^ (b | ~d); + g = static_cast(7 * i) % 16u; + } + f += a + kMd5K[i] + w[g]; + a = d; + d = c; + c = b; + b += RotLeft32(f, kMd5S[i]); + } + s.a += a; + s.b += b; + s.c += c; + s.d += d; +} + +static void Md5Update(Md5State &s, const uint8_t *data, size_t len) { + uint32_t offset = s.count & 63u; + s.count += len; + while (len > 0) { + size_t chunk = std::min(len, static_cast(64u - offset)); + std::memcpy(s.buf + offset, data, chunk); + data += chunk; + len -= chunk; + offset += static_cast(chunk); + if (offset == 64u) { + Md5Block(s, s.buf); + offset = 0; + } + } +} + +static void Md5Final(Md5State &s, uint8_t *digest) { + const uint64_t bits = s.count * 8u; + uint8_t pad = 0x80u; + Md5Update(s, &pad, 1); + pad = 0; + while ((s.count & 63u) != 56u) Md5Update(s, &pad, 1); + uint8_t bits_le[8]; + for (int i = 0; i < 8; ++i) + bits_le[i] = static_cast(bits >> (8u * static_cast(i))); + Md5Update(s, bits_le, 8); + const uint32_t parts[4] = {s.a, s.b, s.c, s.d}; + for (int i = 0; i < 4; ++i) { + digest[4 * i] = static_cast(parts[i]); + digest[4 * i + 1] = static_cast(parts[i] >> 8u); + digest[4 * i + 2] = static_cast(parts[i] >> 16u); + digest[4 * i + 3] = static_cast(parts[i] >> 24u); + } +} + +} // namespace + +bool ComputeFileMd5(const fs::path &path, uint8_t *md5_out) { + std::ifstream f(path, std::ios::binary); + if (!f) { + return false; + } + Md5State state; + uint8_t buf[65536]; + while (f.read(reinterpret_cast(buf), sizeof(buf)) || f.gcount() > 0) + Md5Update(state, buf, static_cast(f.gcount())); + Md5Final(state, md5_out); + return true; +} + +// ---- Timestamp ---------------------------------------------------------- +// Returns the file's last-modification time as a Windows FILETIME value +// (100-nanosecond intervals since 1601-01-01 UTC). Returns 0 on error. + +uint64_t LocalFileTimestamp(const fs::path &path) { + struct stat st {}; + if (stat(path.string().c_str(), &st) != 0) { + return 0; + } + constexpr int64_t kEpochDiff = 11644473600LL; + return static_cast((static_cast(st.st_mtime) + kEpochDiff) * 10000000LL); +} diff --git a/src/helpers.h b/src/helpers.h index 2a3a537..ce9ce5d 100644 --- a/src/helpers.h +++ b/src/helpers.h @@ -1,7 +1,9 @@ #ifndef HELPERS_H #define HELPERS_H +#include #include +#include #include namespace fs = std::filesystem; @@ -14,4 +16,10 @@ uint32_t CalculateMpqMaxFileValue(const std::string &path); uint32_t NextPowerOfTwo(uint32_t n); void PrintAsBinary(const char *buffer, uint32_t size); +// Local file checksum / timestamp helpers used by the --update logic. +// Each returns std::nullopt / false / 0 if the file cannot be read. +std::optional ComputeFileCrc32(const fs::path &path); +bool ComputeFileMd5(const fs::path &path, uint8_t *md5_out); +uint64_t LocalFileTimestamp(const fs::path &path); + #endif diff --git a/src/main.cpp b/src/main.cpp index 796a3d0..a2443bf 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -166,7 +166,7 @@ int main(int argc, char **argv) { add->add_flag("-w,--overwrite", add_overwrite, "Overwrite file if it already is in MPQ archive"); add->add_flag("-u,--update", add_update, - "Skip files whose archived size matches the on-disk size (directory add only)"); + "Skip unchanged files when adding a directory. Compares size, then MD5/CRC32/timestamp"); add->add_option("--locale", base_locale, "Locale to use for added file")->check(locale_valid); add->add_option("-g,--game", base_game_profile, "Game profile for compression rules. Valid options:\n" + diff --git a/src/mpq.cpp b/src/mpq.cpp index 93b9cf2..9b7be94 100644 --- a/src/mpq.cpp +++ b/src/mpq.cpp @@ -1,6 +1,7 @@ #include "mpq.h" #include +#include #include #include #include @@ -230,11 +231,55 @@ int AddFiles(HANDLE archive, const std::string &input_path, const std::string &p int32_t file_locale = GetFileInfo(file, SFileInfoLocale); if (file_locale == locale) { DWORD archived_size = SFileGetFileSize(file, nullptr); - SFileCloseFile(file); uintmax_t disk_size = fs::file_size(entry.path()); + bool skip = false; + std::string skip_reason; if (disk_size == static_cast(archived_size)) { + const DWORD attr_flags = SFileGetAttributes(archive); + if (attr_flags & MPQ_ATTRIBUTE_MD5) { + // Retrieve the MD5 stored in (attributes) via TFileEntry. + // Buffer must accommodate the struct plus the trailing filename. + constexpr DWORD kEntryBufSize = sizeof(TFileEntry) + 1024; + uint8_t fe_buf[kEntryBufSize]{}; + if (SFileGetFileInfo(file, SFileInfoFileEntry, fe_buf, + kEntryBufSize, nullptr)) { + const auto *fe = + reinterpret_cast(fe_buf); + const uint8_t zero_md5[MD5_DIGEST_SIZE]{}; + if (std::memcmp(fe->md5, zero_md5, MD5_DIGEST_SIZE) != 0) { + uint8_t local_md5[MD5_DIGEST_SIZE]{}; + if (ComputeFileMd5(entry.path(), local_md5)) { + skip = (std::memcmp(local_md5, fe->md5, + MD5_DIGEST_SIZE) == 0); + if (skip) skip_reason = "MD5 matches"; + } + } + } + } else if (attr_flags & MPQ_ATTRIBUTE_CRC32) { + const DWORD archived_crc32 = + GetFileInfo(file, SFileInfoCRC32); + if (archived_crc32 != 0) { + if (auto local_crc32 = ComputeFileCrc32(entry.path())) { + skip = (*local_crc32 == archived_crc32); + if (skip) skip_reason = "CRC32 matches"; + } + } + } else if (attr_flags & MPQ_ATTRIBUTE_FILETIME) { + const uint64_t archived_time = + GetFileInfo(file, SFileInfoFileTime); + const uint64_t local_time = LocalFileTimestamp(entry.path()); + // Compare at second resolution: stat() has only second precision. + if (archived_time != 0 && local_time != 0) { + skip = (archived_time / 10000000u == local_time / 10000000u); + if (skip) skip_reason = "Timestamp matches"; + } + } + // If no attributes are present, always add the file. + } + SFileCloseFile(file); + if (skip) { std::cout << "[~] Skipping unchanged file: " << archive_file_path - << std::endl; + << " (" << skip_reason << ")" << std::endl; files_skipped++; continue; } diff --git a/test/test_add.py b/test/test_add.py index efdaa91..58f2e54 100644 --- a/test/test_add.py +++ b/test/test_add.py @@ -714,7 +714,7 @@ def test_add_update_skips_unchanged_files(binary_path, generate_test_files): target_mpq = script_dir / "data" / "files.mpq" update_dir = script_dir / "data" / "update_dir_unchanged" - create_mpq_archive_for_test(binary_path, script_dir) + create_mpq_archive_with_attrs_for_test(binary_path, script_dir) update_dir.mkdir(parents=True, exist_ok=True) (update_dir / "cats.txt").write_text("This is a file about cats.\n") @@ -743,7 +743,7 @@ def test_add_update_adds_changed_files(binary_path, generate_test_files): target_mpq = script_dir / "data" / "files.mpq" update_dir = script_dir / "data" / "update_dir_changed" - create_mpq_archive_for_test(binary_path, script_dir) + create_mpq_archive_with_attrs_for_test(binary_path, script_dir) update_dir.mkdir(parents=True, exist_ok=True) (update_dir / "cats.txt").write_text("This cat content is completely different and longer now.") @@ -777,7 +777,7 @@ def test_add_update_second_run_skips_all(binary_path, generate_test_files): target_mpq = script_dir / "data" / "files.mpq" update_dir = script_dir / "data" / "update_dir_idempotent" - create_mpq_archive_for_test(binary_path, script_dir) + create_mpq_archive_with_attrs_for_test(binary_path, script_dir) update_dir.mkdir(parents=True, exist_ok=True) (update_dir / "cats.txt").write_text("This is a file about cats.\n") @@ -872,6 +872,25 @@ def create_mpq_archive_for_test(binary_path, script_dir): assert target_file.stat().st_size > 0, "MPQ file is empty" +def create_mpq_archive_with_attrs_for_test(binary_path, script_dir): + """Like create_mpq_archive_for_test but uses the wow1 game profile so that + the archive includes a (attributes) file with CRC32, MD5, and FILETIME + checksums. Required by --update tests that rely on checksum comparison.""" + target_dir = script_dir / "data" / "files" + target_file = target_dir.with_suffix(".mpq") + target_file.unlink(missing_ok=True) + result = subprocess.run( + [str(binary_path), "create", "--game", "wow1", str(target_dir)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + assert result.returncode == 0, f"mpqcli failed with error: {result.stderr}" + assert target_file.exists(), "MPQ file was not created" + assert target_file.stat().st_size > 0, "MPQ file is empty" + + def verify_archive_file_content(binary_path, test_file, expected_output): result = subprocess.run( [str(binary_path), "list", str(test_file), "-d", "-p", "locale"], From 375040ebad96f9e72d957f6e6648d3c171b0e783 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Sj=C3=B6blom?= Date: Wed, 8 Jul 2026 16:51:15 +0200 Subject: [PATCH 2/4] Linting --- src/helpers.cpp | 49 +++++++++++++++++++++++++++++-------------------- src/main.cpp | 5 +++-- src/mpq.cpp | 31 ++++++++++++++++--------------- 3 files changed, 48 insertions(+), 37 deletions(-) diff --git a/src/helpers.cpp b/src/helpers.cpp index 68588a4..8468f00 100644 --- a/src/helpers.cpp +++ b/src/helpers.cpp @@ -135,7 +135,7 @@ void PrintAsBinary(const char *buffer, uint32_t size) { // ---- CRC32 -------------------------------------------------------------- // Standard ZIP/gzip CRC32 (polynomial 0xEDB88320, reflected bit order). -static const std::array kCrc32Table = []() { +static const std::array crc32_table = []() { std::array t{}; for (uint32_t i = 0; i < 256; ++i) { uint32_t c = i; @@ -155,7 +155,7 @@ std::optional ComputeFileCrc32(const fs::path &path) { char buf[65536]; while (f.read(buf, sizeof(buf)) || f.gcount() > 0) { for (std::streamsize i = 0; i < f.gcount(); ++i) - crc = (crc >> 8) ^ kCrc32Table[(crc ^ static_cast(buf[i])) & 0xFFu]; + crc = (crc >> 8) ^ crc32_table[(crc ^ static_cast(buf[i])) & 0xFFu]; } return crc ^ 0xFFFFFFFFu; } @@ -165,7 +165,8 @@ std::optional ComputeFileCrc32(const fs::path &path) { namespace { -constexpr uint32_t kMd5K[64] = { +// clang-format off +constexpr uint32_t md5_k[64] = { // Round 1 0xd76aa478u, 0xe8c7b756u, 0x242070dbu, 0xc1bdceeeu, 0xf57c0fafu, 0x4787c62au, 0xa8304613u, 0xfd469501u, @@ -188,12 +189,13 @@ constexpr uint32_t kMd5K[64] = { 0xf7537e82u, 0xbd3af235u, 0x2ad7d2bbu, 0xeb86d391u, }; -constexpr uint8_t kMd5S[64] = { +constexpr uint8_t md5_s[64] = { 7, 12, 17, 22, 7, 12, 17, 22, 7, 12, 17, 22, 7, 12, 17, 22, 5, 9, 14, 20, 5, 9, 14, 20, 5, 9, 14, 20, 5, 9, 14, 20, 4, 11, 16, 23, 4, 11, 16, 23, 4, 11, 16, 23, 4, 11, 16, 23, 6, 10, 15, 21, 6, 10, 15, 21, 6, 10, 15, 21, 6, 10, 15, 21, }; +// clang-format on struct Md5State { uint32_t a{0x67452301u}; @@ -201,19 +203,24 @@ struct Md5State { uint32_t c{0x98BADCFEu}; uint32_t d{0x10325476u}; uint64_t count{0}; - uint8_t buf[64]{}; + uint8_t buf[64]{}; }; -static uint32_t RotLeft32(uint32_t x, uint8_t n) { return (x << n) | (x >> (32u - n)); } +static uint32_t RotLeft32(uint32_t x, uint8_t n) { + return (x << n) | (x >> (32u - n)); +} static void Md5Block(Md5State &s, const uint8_t *blk) { uint32_t w[16]; + // clang-format off for (int i = 0; i < 16; ++i) { - w[i] = static_cast(blk[4 * i]) - | (static_cast(blk[4 * i + 1]) << 8u) - | (static_cast(blk[4 * i + 2]) << 16u) - | (static_cast(blk[4 * i + 3]) << 24u); + const auto off = 4 * static_cast(i); + w[i] = static_cast(blk[off]) + | (static_cast(blk[off + 1]) << 8u) + | (static_cast(blk[off + 2]) << 16u) + | (static_cast(blk[off + 3]) << 24u); } + // clang-format on uint32_t a = s.a, b = s.b, c = s.c, d = s.d; for (int i = 0; i < 64; ++i) { uint32_t f = 0; @@ -231,11 +238,11 @@ static void Md5Block(Md5State &s, const uint8_t *blk) { f = c ^ (b | ~d); g = static_cast(7 * i) % 16u; } - f += a + kMd5K[i] + w[g]; + f += a + md5_k[i] + w[g]; a = d; d = c; c = b; - b += RotLeft32(f, kMd5S[i]); + b += RotLeft32(f, md5_s[i]); } s.a += a; s.b += b; @@ -264,21 +271,23 @@ static void Md5Final(Md5State &s, uint8_t *digest) { uint8_t pad = 0x80u; Md5Update(s, &pad, 1); pad = 0; - while ((s.count & 63u) != 56u) Md5Update(s, &pad, 1); + while ((s.count & 63u) != 56u) + Md5Update(s, &pad, 1); uint8_t bits_le[8]; for (int i = 0; i < 8; ++i) bits_le[i] = static_cast(bits >> (8u * static_cast(i))); Md5Update(s, bits_le, 8); const uint32_t parts[4] = {s.a, s.b, s.c, s.d}; for (int i = 0; i < 4; ++i) { - digest[4 * i] = static_cast(parts[i]); - digest[4 * i + 1] = static_cast(parts[i] >> 8u); - digest[4 * i + 2] = static_cast(parts[i] >> 16u); - digest[4 * i + 3] = static_cast(parts[i] >> 24u); + const auto off = 4 * static_cast(i); + digest[off] = static_cast(parts[i]); + digest[off + 1] = static_cast(parts[i] >> 8u); + digest[off + 2] = static_cast(parts[i] >> 16u); + digest[off + 3] = static_cast(parts[i] >> 24u); } } -} // namespace +} // namespace bool ComputeFileMd5(const fs::path &path, uint8_t *md5_out) { std::ifstream f(path, std::ios::binary); @@ -302,6 +311,6 @@ uint64_t LocalFileTimestamp(const fs::path &path) { if (stat(path.string().c_str(), &st) != 0) { return 0; } - constexpr int64_t kEpochDiff = 11644473600LL; - return static_cast((static_cast(st.st_mtime) + kEpochDiff) * 10000000LL); + constexpr int64_t epoch_diff = 11644473600LL; + return static_cast((static_cast(st.st_mtime) + epoch_diff) * 10000000LL); } diff --git a/src/main.cpp b/src/main.cpp index a2443bf..28523a6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -165,8 +165,9 @@ int main(int argc, char **argv) { "Archive path for a single file, or prefix for a directory"); add->add_flag("-w,--overwrite", add_overwrite, "Overwrite file if it already is in MPQ archive"); - add->add_flag("-u,--update", add_update, - "Skip unchanged files when adding a directory. Compares size, then MD5/CRC32/timestamp"); + add->add_flag( + "-u,--update", add_update, + "Skip unchanged files when adding a directory. Compares size, then MD5/CRC32/timestamp"); add->add_option("--locale", base_locale, "Locale to use for added file")->check(locale_valid); add->add_option("-g,--game", base_game_profile, "Game profile for compression rules. Valid options:\n" + diff --git a/src/mpq.cpp b/src/mpq.cpp index 9b7be94..f54f2a2 100644 --- a/src/mpq.cpp +++ b/src/mpq.cpp @@ -239,29 +239,29 @@ int AddFiles(HANDLE archive, const std::string &input_path, const std::string &p if (attr_flags & MPQ_ATTRIBUTE_MD5) { // Retrieve the MD5 stored in (attributes) via TFileEntry. // Buffer must accommodate the struct plus the trailing filename. - constexpr DWORD kEntryBufSize = sizeof(TFileEntry) + 1024; - uint8_t fe_buf[kEntryBufSize]{}; - if (SFileGetFileInfo(file, SFileInfoFileEntry, fe_buf, - kEntryBufSize, nullptr)) { - const auto *fe = - reinterpret_cast(fe_buf); + constexpr DWORD entry_buf_size = sizeof(TFileEntry) + 1024; + uint8_t fe_buf[entry_buf_size]{}; + if (SFileGetFileInfo(file, SFileInfoFileEntry, fe_buf, entry_buf_size, + nullptr)) { + const auto *fe = reinterpret_cast(fe_buf); const uint8_t zero_md5[MD5_DIGEST_SIZE]{}; if (std::memcmp(fe->md5, zero_md5, MD5_DIGEST_SIZE) != 0) { uint8_t local_md5[MD5_DIGEST_SIZE]{}; if (ComputeFileMd5(entry.path(), local_md5)) { - skip = (std::memcmp(local_md5, fe->md5, - MD5_DIGEST_SIZE) == 0); - if (skip) skip_reason = "MD5 matches"; + skip = + (std::memcmp(local_md5, fe->md5, MD5_DIGEST_SIZE) == 0); + if (skip) + skip_reason = "MD5 matches"; } } } } else if (attr_flags & MPQ_ATTRIBUTE_CRC32) { - const DWORD archived_crc32 = - GetFileInfo(file, SFileInfoCRC32); + const DWORD archived_crc32 = GetFileInfo(file, SFileInfoCRC32); if (archived_crc32 != 0) { if (auto local_crc32 = ComputeFileCrc32(entry.path())) { skip = (*local_crc32 == archived_crc32); - if (skip) skip_reason = "CRC32 matches"; + if (skip) + skip_reason = "CRC32 matches"; } } } else if (attr_flags & MPQ_ATTRIBUTE_FILETIME) { @@ -271,15 +271,16 @@ int AddFiles(HANDLE archive, const std::string &input_path, const std::string &p // Compare at second resolution: stat() has only second precision. if (archived_time != 0 && local_time != 0) { skip = (archived_time / 10000000u == local_time / 10000000u); - if (skip) skip_reason = "Timestamp matches"; + if (skip) + skip_reason = "Timestamp matches"; } } // If no attributes are present, always add the file. } SFileCloseFile(file); if (skip) { - std::cout << "[~] Skipping unchanged file: " << archive_file_path - << " (" << skip_reason << ")" << std::endl; + std::cout << "[~] Skipping unchanged file: " << archive_file_path << " (" + << skip_reason << ")" << std::endl; files_skipped++; continue; } From 9b8db3c1343b9b99439e0d59d6a6ec53f7458a2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Sj=C3=B6blom?= Date: Thu, 9 Jul 2026 20:20:44 +0200 Subject: [PATCH 3/4] Adding tests --- test/test_add.py | 191 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 189 insertions(+), 2 deletions(-) diff --git a/test/test_add.py b/test/test_add.py index 58f2e54..42b2428 100644 --- a/test/test_add.py +++ b/test/test_add.py @@ -1,3 +1,4 @@ +import os import subprocess import shutil from pathlib import Path @@ -729,8 +730,8 @@ def test_add_update_skips_unchanged_files(binary_path, generate_test_files): ) assert result.returncode == 0, f"mpqcli failed with error: {result.stderr}" - assert "[~] Skipping unchanged file: cats.txt" in result.stdout - assert "[~] Skipping unchanged file: dogs.txt" in result.stdout + assert "[~] Skipping unchanged file: cats.txt (MD5 matches)" in result.stdout + assert "[~] Skipping unchanged file: dogs.txt (MD5 matches)" in result.stdout assert "files added" in result.stdout assert "files skipped" in result.stdout finally: @@ -822,6 +823,154 @@ def test_add_update_single_file_emits_warning(binary_path, generate_test_files): assert "--update is only meaningful when adding a directory" in result.stderr +def test_add_update_skips_unchanged_files_via_crc32(binary_path, generate_test_files): + """CRC32 branch: archive has CRC32+FILETIME but no MD5 (wc3 profile). + Unchanged file must be skipped with reason 'CRC32 matches'.""" + _ = generate_test_files + script_dir = Path(__file__).parent + target_mpq = script_dir / "data" / "files.mpq" + update_dir = script_dir / "data" / "update_dir_crc32_unchanged" + + create_mpq_archive_with_crc32_for_test(binary_path, script_dir) + + update_dir.mkdir(parents=True, exist_ok=True) + dst = update_dir / "cats.txt" + shutil.copy2(script_dir / "data" / "files" / "cats.txt", dst) + + try: + result = subprocess.run( + [str(binary_path), "add", str(target_mpq), str(update_dir), "--update", "--overwrite"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + assert result.returncode == 0, f"mpqcli failed with error: {result.stderr}" + assert "[~] Skipping unchanged file: cats.txt (CRC32 matches)" in result.stdout + assert "files skipped" in result.stdout + finally: + shutil.rmtree(update_dir, ignore_errors=True) + + +def test_add_update_adds_changed_files_via_crc32(binary_path, generate_test_files): + """CRC32 branch: archive has CRC32+FILETIME but no MD5 (wc3 profile). + A file with the same size but different content (different CRC32) must be re-added.""" + _ = generate_test_files + script_dir = Path(__file__).parent + target_mpq = script_dir / "data" / "files.mpq" + update_dir = script_dir / "data" / "update_dir_crc32_changed" + + create_mpq_archive_with_crc32_for_test(binary_path, script_dir) + + update_dir.mkdir(parents=True, exist_ok=True) + # Same byte-length as "This is a file about cats.\n" (27 bytes) but different content. + (update_dir / "cats.txt").write_text("This is a file about CATS.\n") + + try: + result = subprocess.run( + [str(binary_path), "add", str(target_mpq), str(update_dir), "--update", "--overwrite"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + assert result.returncode == 0, f"mpqcli failed with error: {result.stderr}" + assert "[+] Adding file: cats.txt" in result.stdout + assert "Skipping unchanged" not in result.stdout + finally: + shutil.rmtree(update_dir, ignore_errors=True) + + +def test_add_update_skips_unchanged_files_via_timestamp(binary_path, generate_test_files): + """Timestamp branch: archive has FILETIME only (no CRC32, no MD5). + Unchanged file (same mtime) must be skipped with reason 'Timestamp matches'.""" + _ = generate_test_files + script_dir = Path(__file__).parent + target_mpq = script_dir / "data" / "files.mpq" + update_dir = script_dir / "data" / "update_dir_ts_unchanged" + + create_mpq_archive_with_filetime_for_test(binary_path, script_dir) + + update_dir.mkdir(parents=True, exist_ok=True) + dst = update_dir / "cats.txt" + # Preserve the original mtime so the timestamp comparison matches. + shutil.copy2(script_dir / "data" / "files" / "cats.txt", dst) + + try: + result = subprocess.run( + [str(binary_path), "add", str(target_mpq), str(update_dir), "--update", "--overwrite"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + assert result.returncode == 0, f"mpqcli failed with error: {result.stderr}" + assert "[~] Skipping unchanged file: cats.txt (Timestamp matches)" in result.stdout + assert "files skipped" in result.stdout + finally: + shutil.rmtree(update_dir, ignore_errors=True) + + +def test_add_update_adds_changed_files_via_timestamp(binary_path, generate_test_files): + """Timestamp branch: archive has FILETIME only (no CRC32, no MD5). + A file with the same size but a different mtime must be re-added.""" + _ = generate_test_files + script_dir = Path(__file__).parent + target_mpq = script_dir / "data" / "files.mpq" + update_dir = script_dir / "data" / "update_dir_ts_changed" + + create_mpq_archive_with_filetime_for_test(binary_path, script_dir) + + update_dir.mkdir(parents=True, exist_ok=True) + dst = update_dir / "cats.txt" + shutil.copy2(script_dir / "data" / "files" / "cats.txt", dst) + # Shift the mtime by one hour so the timestamp no longer matches. + original_mtime = os.path.getmtime(dst) + os.utime(dst, (original_mtime + 3600, original_mtime + 3600)) + + try: + result = subprocess.run( + [str(binary_path), "add", str(target_mpq), str(update_dir), "--update", "--overwrite"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + assert result.returncode == 0, f"mpqcli failed with error: {result.stderr}" + assert "[+] Adding file: cats.txt" in result.stdout + assert "Skipping unchanged" not in result.stdout + finally: + shutil.rmtree(update_dir, ignore_errors=True) + + +def test_add_update_always_adds_without_attributes(binary_path, generate_test_files): + """No-attributes branch: archive has no (attributes) file. + Even an identical file must be re-added because there is nothing to compare.""" + _ = generate_test_files + script_dir = Path(__file__).parent + target_mpq = script_dir / "data" / "files.mpq" + update_dir = script_dir / "data" / "update_dir_no_attrs" + + create_mpq_archive_for_test(binary_path, script_dir) + + update_dir.mkdir(parents=True, exist_ok=True) + (update_dir / "cats.txt").write_text("This is a file about cats.\n") + + try: + result = subprocess.run( + [str(binary_path), "add", str(target_mpq), str(update_dir), "--update", "--overwrite"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + assert result.returncode == 0, f"mpqcli failed with error: {result.stderr}" + assert "[+] Adding file: cats.txt" in result.stdout + assert "Skipping unchanged" not in result.stdout + finally: + shutil.rmtree(update_dir, ignore_errors=True) + + def test_add_files_via_stdin(binary_path, generate_test_files): _ = generate_test_files script_dir = Path(__file__).parent @@ -891,6 +1040,44 @@ def create_mpq_archive_with_attrs_for_test(binary_path, script_dir): assert target_file.stat().st_size > 0, "MPQ file is empty" +def create_mpq_archive_with_crc32_for_test(binary_path, script_dir): + """Creates an archive using the wc3 game profile, which stores CRC32 and + FILETIME attributes but no MD5. Used by --update tests that exercise the + CRC32 comparison branch.""" + target_dir = script_dir / "data" / "files" + target_file = target_dir.with_suffix(".mpq") + target_file.unlink(missing_ok=True) + result = subprocess.run( + [str(binary_path), "create", "--game", "wc3", str(target_dir)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + assert result.returncode == 0, f"mpqcli failed with error: {result.stderr}" + assert target_file.exists(), "MPQ file was not created" + assert target_file.stat().st_size > 0, "MPQ file is empty" + + +def create_mpq_archive_with_filetime_for_test(binary_path, script_dir): + """Creates an archive with FILETIME attributes only (no CRC32, no MD5) by + using the wc3 game profile and overriding attr-flags to 2 (FILETIME only). + Used by --update tests that exercise the timestamp comparison branch.""" + target_dir = script_dir / "data" / "files" + target_file = target_dir.with_suffix(".mpq") + target_file.unlink(missing_ok=True) + result = subprocess.run( + [str(binary_path), "create", "--game", "wc3", "--attr-flags", "2", str(target_dir)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + assert result.returncode == 0, f"mpqcli failed with error: {result.stderr}" + assert target_file.exists(), "MPQ file was not created" + assert target_file.stat().st_size > 0, "MPQ file is empty" + + def verify_archive_file_content(binary_path, test_file, expected_output): result = subprocess.run( [str(binary_path), "list", str(test_file), "-d", "-p", "locale"], From a2ca6b7729d5242980cf57491e455ae6e70a72c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Sj=C3=B6blom?= Date: Thu, 9 Jul 2026 21:09:44 +0200 Subject: [PATCH 4/4] Now checking Timestamps before MD5 and CRC32 --- docs/commands/add.md | 16 ++++++++-------- src/main.cpp | 2 +- src/mpq.cpp | 34 +++++++++++++++++++++------------- test/test_add.py | 7 ++++--- 4 files changed, 34 insertions(+), 25 deletions(-) diff --git a/docs/commands/add.md b/docs/commands/add.md index 99cae71..0d0ca7e 100644 --- a/docs/commands/add.md +++ b/docs/commands/add.md @@ -62,16 +62,16 @@ When adding a directory, the `--update` flag skips files that have not changed s were last added to the archive. This is useful for incremental updates where only changed files need to be re-added. -The skip decision is made in two steps: +The skip decision follows this chain: 1. **File size** must match. If the sizes differ the file is always re-added. -2. If the sizes match, the archive's `(attributes)` file is consulted for a stronger - check. The most reliable attribute available wins: - - **MD5** – if the archive stores MD5 checksums, the MD5 of the local file is computed - and compared. A match skips the file; a mismatch re-adds it. - - **CRC32** – used when MD5 is absent. Same logic. - - **Timestamp** – used when neither MD5 nor CRC32 is present. The file's - last-modification time is compared at one-second resolution. +2. If the sizes match, the archive's `(attributes)` file is consulted: + - **Timestamp** – if the archive stores file timestamps, the local file's + last-modification time is compared at one-second resolution. A match skips the file. + - **MD5** – if the timestamp did not match or is unavailable, and the archive stores MD5 + checksums, the MD5 of the local file is computed and compared. A match skips the file. + - **CRC32** – if neither timestamp nor MD5 produced a match or was available, and the + archive stores CRC32 checksums, those are compared. A match skips the file. - **No attributes** – if the archive has no `(attributes)` file, the file is always re-added even when sizes match, because no reliable content check is possible. diff --git a/src/main.cpp b/src/main.cpp index 28523a6..23f3a6b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -167,7 +167,7 @@ int main(int argc, char **argv) { "Overwrite file if it already is in MPQ archive"); add->add_flag( "-u,--update", add_update, - "Skip unchanged files when adding a directory. Compares size, then MD5/CRC32/timestamp"); + "Skip unchanged files when adding a directory. Compares size, then timestamp/MD5/CRC32"); add->add_option("--locale", base_locale, "Locale to use for added file")->check(locale_valid); add->add_option("-g,--game", base_game_profile, "Game profile for compression rules. Valid options:\n" + diff --git a/src/mpq.cpp b/src/mpq.cpp index f54f2a2..0b68915 100644 --- a/src/mpq.cpp +++ b/src/mpq.cpp @@ -236,7 +236,22 @@ int AddFiles(HANDLE archive, const std::string &input_path, const std::string &p std::string skip_reason; if (disk_size == static_cast(archived_size)) { const DWORD attr_flags = SFileGetAttributes(archive); - if (attr_flags & MPQ_ATTRIBUTE_MD5) { + + // Step 1: Timestamp — cheapest check, no local file I/O. + if (!skip && (attr_flags & MPQ_ATTRIBUTE_FILETIME)) { + const uint64_t archived_time = + GetFileInfo(file, SFileInfoFileTime); + const uint64_t local_time = LocalFileTimestamp(entry.path()); + // Compare at second resolution: stat() has only second precision. + if (archived_time != 0 && local_time != 0 && + archived_time / 10000000u == local_time / 10000000u) { + skip = true; + skip_reason = "Timestamp matches"; + } + } + + // Step 2: MD5 — if timestamp did not match or was unavailable. + if (!skip && (attr_flags & MPQ_ATTRIBUTE_MD5)) { // Retrieve the MD5 stored in (attributes) via TFileEntry. // Buffer must accommodate the struct plus the trailing filename. constexpr DWORD entry_buf_size = sizeof(TFileEntry) + 1024; @@ -255,7 +270,10 @@ int AddFiles(HANDLE archive, const std::string &input_path, const std::string &p } } } - } else if (attr_flags & MPQ_ATTRIBUTE_CRC32) { + } + + // Step 3: CRC32 — if neither timestamp nor MD5 matched or was available. + if (!skip && (attr_flags & MPQ_ATTRIBUTE_CRC32)) { const DWORD archived_crc32 = GetFileInfo(file, SFileInfoCRC32); if (archived_crc32 != 0) { if (auto local_crc32 = ComputeFileCrc32(entry.path())) { @@ -264,18 +282,8 @@ int AddFiles(HANDLE archive, const std::string &input_path, const std::string &p skip_reason = "CRC32 matches"; } } - } else if (attr_flags & MPQ_ATTRIBUTE_FILETIME) { - const uint64_t archived_time = - GetFileInfo(file, SFileInfoFileTime); - const uint64_t local_time = LocalFileTimestamp(entry.path()); - // Compare at second resolution: stat() has only second precision. - if (archived_time != 0 && local_time != 0) { - skip = (archived_time / 10000000u == local_time / 10000000u); - if (skip) - skip_reason = "Timestamp matches"; - } } - // If no attributes are present, always add the file. + // If no attributes are present or none matched, always add the file. } SFileCloseFile(file); if (skip) { diff --git a/test/test_add.py b/test/test_add.py index 42b2428..5b2b35f 100644 --- a/test/test_add.py +++ b/test/test_add.py @@ -825,7 +825,9 @@ def test_add_update_single_file_emits_warning(binary_path, generate_test_files): def test_add_update_skips_unchanged_files_via_crc32(binary_path, generate_test_files): """CRC32 branch: archive has CRC32+FILETIME but no MD5 (wc3 profile). - Unchanged file must be skipped with reason 'CRC32 matches'.""" + Unchanged file must be skipped with reason 'CRC32 matches'. + The file is written fresh (new mtime) so the timestamp check fails first, + forcing the code to fall through to the CRC32 comparison.""" _ = generate_test_files script_dir = Path(__file__).parent target_mpq = script_dir / "data" / "files.mpq" @@ -834,8 +836,7 @@ def test_add_update_skips_unchanged_files_via_crc32(binary_path, generate_test_f create_mpq_archive_with_crc32_for_test(binary_path, script_dir) update_dir.mkdir(parents=True, exist_ok=True) - dst = update_dir / "cats.txt" - shutil.copy2(script_dir / "data" / "files" / "cats.txt", dst) + (update_dir / "cats.txt").write_text("This is a file about cats.\n") try: result = subprocess.run(