diff --git a/.gitmodules b/.gitmodules index bb2ee67..286ba9b 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,3 +4,6 @@ [submodule "extern/StormLib"] path = extern/StormLib url = https://github.com/ladislav-zezula/StormLib.git +[submodule "extern/hash-library"] + path = extern/hash-library + url = https://github.com/stbrumme/hash-library.git diff --git a/CMakeLists.txt b/CMakeLists.txt index 71dd805..faae1fa 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -64,6 +64,24 @@ if(BUILD_MPQCLI) add_subdirectory(extern/CLI11) + # Handle hash-library dependency (CRC32 + MD5 for the add --update checks) + if (NOT EXISTS "${CMAKE_SOURCE_DIR}/extern/hash-library/crc32.cpp") + message(FATAL_ERROR + "Missing dependency: hash-library + mpqcli requires the hash-library library. + It is provided as a submodule of this repository. + Did you forget to execute the following commands? + git submodule init + git submodule update") + endif() + + # hash-library ships no CMakeLists; compile only the two algorithms we use + add_library(hash-library STATIC + extern/hash-library/crc32.cpp + extern/hash-library/md5.cpp + ) + target_include_directories(hash-library SYSTEM INTERFACE "${CMAKE_SOURCE_DIR}/extern") + # Add the main application add_subdirectory(src) endif() diff --git a/docs/commands/add.md b/docs/commands/add.md index d07ffb0..aa92343 100644 --- a/docs/commands/add.md +++ b/docs/commands/add.md @@ -58,9 +58,28 @@ $ 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 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: + - **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. + +Note: a timestamp match alone skips the file, without comparing checksums. A file whose +content changed but whose size and modification time were both preserved (for example by +`cp -p` or tools that restore timestamps) will therefore not be detected as changed. This +is the same trade-off tools like `rsync` make by default. If exact change detection +matters, pass `--overwrite` without `--update` to unconditionally replace every file. ```bash $ mpqcli add wow-patch.mpq textures/ --update --overwrite @@ -69,10 +88,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/extern/hash-library b/extern/hash-library new file mode 160000 index 0000000..d389d18 --- /dev/null +++ b/extern/hash-library @@ -0,0 +1 @@ +Subproject commit d389d18112bcf7e4786ec5e8723f3658a7f433d7 diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 0b7a52c..c6c5495 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -48,4 +48,4 @@ target_include_directories(mpqcli PRIVATE ) # Link libraries -target_link_libraries(mpqcli PRIVATE storm CLI11::CLI11) +target_link_libraries(mpqcli PRIVATE storm CLI11::CLI11 hash-library) 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..b56dcfa 100644 --- a/src/helpers.cpp +++ b/src/helpers.cpp @@ -4,7 +4,11 @@ #include #include #include +#include +#include +#include #include +#include #ifdef _WIN32 #include @@ -128,3 +132,56 @@ void PrintAsBinary(const char *buffer, uint32_t size) { #endif std::cout.write(buffer, size); } + +// CRC32 (ZIP/gzip polynomial) and MD5 are provided by the hash-library +// submodule, matching the values StormLib stores in (attributes). +std::optional ComputeFileCrc32(const fs::path &path) { + std::ifstream f(path, std::ios::binary); + if (!f) { + return std::nullopt; + } + CRC32 crc32; + char buf[65536]; + while (f.read(buf, sizeof(buf)) || f.gcount() > 0) { + crc32.add(buf, static_cast(f.gcount())); + } + // getHash yields the checksum as big-endian bytes; reassemble the value. + unsigned char digest[CRC32::HashBytes]; + crc32.getHash(digest); + return (static_cast(digest[0]) << 24) | (static_cast(digest[1]) << 16) | + (static_cast(digest[2]) << 8) | static_cast(digest[3]); +} + +bool ComputeFileMd5(const fs::path &path, uint8_t *md5_out) { + std::ifstream f(path, std::ios::binary); + if (!f) { + return false; + } + MD5 md5; + char buf[65536]; + while (f.read(buf, sizeof(buf)) || f.gcount() > 0) { + md5.add(buf, static_cast(f.gcount())); + } + md5.getHash(md5_out); + return true; +} + +// 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) { +#ifdef _WIN32 + // _wstat64 handles paths with non-ASCII characters, which the narrow + // stat() would mangle on Windows. + struct _stat64 st {}; + if (_wstat64(path.wstring().c_str(), &st) != 0) { + return 0; + } +#else + struct stat st {}; + if (stat(path.string().c_str(), &st) != 0) { + return 0; + } +#endif + constexpr int64_t epoch_diff = 11644473600LL; + return static_cast((static_cast(st.st_mtime) + epoch_diff) * 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..23f3a6b 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 files whose archived size matches the on-disk size (directory add only)"); + add->add_flag( + "-u,--update", add_update, + "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 93b9cf2..bdaa6c6 100644 --- a/src/mpq.cpp +++ b/src/mpq.cpp @@ -1,6 +1,7 @@ #include "mpq.h" #include +#include #include #include #include @@ -230,11 +231,71 @@ 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)) { - std::cout << "[~] Skipping unchanged file: " << archive_file_path - << std::endl; + const DWORD attr_flags = SFileGetAttributes(archive); + + // 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)) { + // StormLib has no SFileInfoMD5 class, so SFileInfoFileEntry + // (TFileEntry, declared in StormLib.h) is the only public way + // to read the MD5 stored in (attributes). + // Buffer must accommodate the struct plus the trailing filename. + 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); + // An all-zero digest means "no MD5 stored". A file whose + // real MD5 is all zeroes is astronomically unlikely; the + // worst case is a redundant re-add. + 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"; + } + } + } + } + + // 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); + // Zero means "no CRC32 stored"; a file whose real CRC32 is + // zero just gets a redundant re-add. + if (archived_crc32 != 0) { + if (auto local_crc32 = ComputeFileCrc32(entry.path())) { + skip = (*local_crc32 == archived_crc32); + if (skip) + skip_reason = "CRC32 matches"; + } + } + } + // If no attributes are present or none matched, always add the file. + } + SFileCloseFile(file); + if (skip) { + std::cout << "[~] Skipping unchanged file: " << archive_file_path << " (" + << skip_reason << ")" << std::endl; files_skipped++; continue; } diff --git a/test/test_add.py b/test/test_add.py index efdaa91..5b2b35f 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 @@ -714,7 +715,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") @@ -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: @@ -743,7 +744,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 +778,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") @@ -822,6 +823,155 @@ 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'. + 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" + 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) + (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 "[~] 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 @@ -872,6 +1022,63 @@ 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 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"],