chore: Remove last traces of legacy mixed encoding hash digests

7d64f3baf1e99ebbaf185aeef5426f04de0638de (feat: Use base16 encoding for
all hash digest keys) transitioned from the mixed base16/base32hex
encoding to just base16, but left backward-compatibility code during a
transition period.

This commit completes the transition by removing backward-compatibility
code for looking up old entries.

diff --git a/src/ccache/ccache.cpp b/src/ccache/ccache.cpp
index 0988e4067d7b1edb30946678ed6a461e892950a2..2ff0cf9d05c418e2593d8661438b9d68418ab74e 100644 (file)
--- a/src/ccache/ccache.cpp
+++ b/src/ccache/ccache.cpp
@@ -464,7 +464,7 @@ remember_include_file(Context& ctx,
     }
     file_digest = *ret;
     cpp_hash.hash_delimiter(using_pch_sum ? "pch_sum_hash" : "pch_hash");
-    cpp_hash.hash(util::format_legacy_digest(file_digest));
+    cpp_hash.hash(util::format_base16(file_digest));
   }
 
   if (ctx.config.direct_mode()) {
@@ -478,7 +478,7 @@ remember_include_file(Context& ctx,
 
     if (depend_mode_hash) {
       depend_mode_hash->hash_delimiter("include");
-      depend_mode_hash->hash(util::format_legacy_digest(file_digest));
+      depend_mode_hash->hash(util::format_base16(file_digest));
     }
   }
 
@@ -941,7 +941,6 @@ update_manifest(Context& ctx,
     LOG("Added result key to manifest {}", util::format_base16(manifest_key));
     core::CacheEntry::Header header(ctx.config, core::CacheEntryType::manifest);
     ctx.storage.put(manifest_key,
-                    core::CacheEntryType::manifest,
                     core::CacheEntry::serialize(header, ctx.manifest));
   } else {
     LOG("Did not add result key to manifest {}",
@@ -1095,7 +1094,7 @@ write_result(Context& ctx,
     }
   }
 
-  ctx.storage.put(result_key, core::CacheEntryType::result, cache_entry_data);
+  ctx.storage.put(result_key, cache_entry_data);
 
   return true;
 }
@@ -2370,7 +2369,7 @@ get_manifest_key(Context& ctx, Hash& hash)
     // Direct mode was disabled by hash_source_code_file.
     return {};
   }
-  hash.hash(util::format_legacy_digest(*input_file_digest));
+  hash.hash(util::format_base16(*input_file_digest));
   return hash.digest();
 }
 
@@ -2496,7 +2495,6 @@ get_result_key_from_manifest(Context& ctx, const Hash::Digest& manifest_key)
         util::format_base16(manifest_key));
     core::CacheEntry::Header header(ctx.config, core::CacheEntryType::manifest);
     ctx.storage.local.put(manifest_key,
-                          core::CacheEntryType::manifest,
                           core::CacheEntry::serialize(header, ctx.manifest),
                           storage::Overwrite::yes);
   }
@@ -3137,7 +3135,7 @@ do_cache_compilation(Context& ctx)
       LOG("Hash from manifest doesn't match preprocessor output");
       LOG("Likely reason: different CCACHE_BASEDIRs used");
       LOG("Removing manifest as a safety measure");
-      ctx.storage.remove(*manifest_key, core::CacheEntryType::manifest);
+      ctx.storage.remove(*manifest_key);
 
       put_result_in_manifest = true;
     }

diff --git a/src/ccache/hashutil.cpp b/src/ccache/hashutil.cpp
index 411e51d918c796f0ac041a1ae56d461c162fa6d9..301caac2784e2a602a847e4814c1f7c25b346d01 100644 (file)
--- a/src/ccache/hashutil.cpp
+++ b/src/ccache/hashutil.cpp
@@ -426,7 +426,7 @@ hash_source_code_file(Context& ctx, const fs::path& path, size_t size_hint)
   // expansions.
 
   Hash hash;
-  hash.hash(util::format_legacy_digest(digest));
+  hash.hash(util::format_base16(digest));
 
   if (result.contains(SourceCodeScan::found_date)) {
     hash.hash_delimiter("date");
@@ -482,7 +482,7 @@ hash_binary_file(const Context& ctx, Hash& hash, const fs::path& path)
 {
   const auto result = hash_binary_file(ctx, path);
   if (result) {
-    hash.hash(util::format_legacy_digest(*result));
+    hash.hash(util::format_base16(*result));
   }
   return result.has_value();
 }

diff --git a/src/ccache/storage/local/localstorage.cpp b/src/ccache/storage/local/localstorage.cpp
index e4f7bf4b678c76bf0195c723c469fe1f844ce058..5993d41cb93d0b45b4cb3f213d86ee937933d78c 100644 (file)
--- a/src/ccache/storage/local/localstorage.cpp
+++ b/src/ccache/storage/local/localstorage.cpp
@@ -486,7 +486,7 @@ LocalStorage::get(const Hash::Digest& key, const core::CacheEntryType type)
 {
   std::optional<util::Bytes> return_value;
 
-  const auto cache_file = look_up_cache_file(key, type);
+  const auto cache_file = look_up_cache_file(key);
   if (cache_file.dir_entry.is_regular_file()) {
     const auto value = util::read_file<util::Bytes>(cache_file.path);
     if (value) {
@@ -516,11 +516,10 @@ LocalStorage::get(const Hash::Digest& key, const core::CacheEntryType type)
 
 void
 LocalStorage::put(const Hash::Digest& key,
-                  const core::CacheEntryType type,
                   std::span<const uint8_t> value,
                   Overwrite overwrite)
 {
-  const auto cache_file = look_up_cache_file(key, type);
+  const auto cache_file = look_up_cache_file(key);
   if (overwrite == Overwrite::no && cache_file.dir_entry.exists()) {
     LOG("Not storing {} in local storage since it already exists",
         cache_file.path);
@@ -581,9 +580,9 @@ LocalStorage::put(const Hash::Digest& key,
 }
 
 void
-LocalStorage::remove(const Hash::Digest& key, const core::CacheEntryType type)
+LocalStorage::remove(const Hash::Digest& key)
 {
-  const auto cache_file = look_up_cache_file(key, type);
+  const auto cache_file = look_up_cache_file(key);
   if (!cache_file.dir_entry) {
     LOG("No {} to remove from local storage", util::format_base16(key));
     return;
@@ -617,8 +616,7 @@ fs::path
 LocalStorage::get_raw_file_path(const Hash::Digest& result_key,
                                 uint8_t file_number) const
 {
-  const auto cache_file =
-    look_up_cache_file(result_key, core::CacheEntryType::result);
+  const auto cache_file = look_up_cache_file(result_key);
   return get_raw_file_path(cache_file.path, file_number);
 }
 
@@ -633,7 +631,7 @@ LocalStorage::put_raw_files(
       FMT("Too many raw files: {} > {}", raw_files.size(), k_max_raw_files));
   }
 
-  const auto cache_file = look_up_cache_file(key, core::CacheEntryType::result);
+  const auto cache_file = look_up_cache_file(key);
   core::ensure_dir_exists(cache_file.path.parent_path());
 
   int64_t files_change = 0;
@@ -1061,10 +1059,8 @@ LocalStorage::get_subdir(uint8_t l1_index, uint8_t l2_index) const
 }
 
 LocalStorage::LookUpCacheFileResult
-LocalStorage::look_up_cache_file(const Hash::Digest& key,
-                                 const core::CacheEntryType type) const
+LocalStorage::look_up_cache_file(const Hash::Digest& key) const
 {
-  // Try new format first: base16 without suffix
   const auto key_string = util::format_base16(key);
 
   for (uint8_t level = k_min_cache_levels; level <= k_max_cache_levels;
@@ -1076,27 +1072,6 @@ LocalStorage::look_up_cache_file(const Hash::Digest& key,
     }
   }
 
-  // Try old format with R/M suffix
-  const auto old_key_string = util::format_legacy_digest(key);
-  const std::string old_suffix =
-    type == core::CacheEntryType::manifest ? "M" : "R";
-  const auto old_key_string_with_suffix = old_key_string + old_suffix;
-  for (uint8_t level = k_min_cache_levels; level <= k_max_cache_levels;
-       ++level) {
-    const auto path = get_path_in_cache(level, old_key_string_with_suffix);
-    DirEntry dir_entry(path);
-    if (dir_entry.is_regular_file()) {
-      const auto new_path = get_path_in_cache(level, key_string);
-      LOG("Migrating {} to {}", path, new_path);
-      if (fs::rename(path, new_path)) {
-        return {new_path, DirEntry(new_path), level};
-      } else {
-        LOG("Failed to rename {} to {}", path, new_path);
-        return {path, dir_entry, level};
-      }
-    }
-  }
-
   const auto shallowest_path =
     get_path_in_cache(k_min_cache_levels, key_string);
   return {shallowest_path, DirEntry(), k_min_cache_levels};

diff --git a/src/ccache/storage/local/localstorage.hpp b/src/ccache/storage/local/localstorage.hpp
index 76b5f2e4b12ee13d5e36449cbd85b380942b0302..fa137a7ad5d4398465896775b987192d66d11c03 100644 (file)
--- a/src/ccache/storage/local/localstorage.hpp
+++ b/src/ccache/storage/local/localstorage.hpp
@@ -72,11 +72,10 @@ public:
                                  core::CacheEntryType type);
 
   void put(const Hash::Digest& key,
-           core::CacheEntryType type,
            std::span<const uint8_t> value,
            Overwrite overwrite);
 
-  void remove(const Hash::Digest& key, core::CacheEntryType type);
+  void remove(const Hash::Digest& key);
 
   static std::filesystem::path
   get_raw_file_path(const std::filesystem::path& result_path,
@@ -152,8 +151,7 @@ private:
     uint8_t level;
   };
 
-  LookUpCacheFileResult look_up_cache_file(const Hash::Digest& key,
-                                           core::CacheEntryType type) const;
+  LookUpCacheFileResult look_up_cache_file(const Hash::Digest& key) const;
 
   std::filesystem::path get_subdir(uint8_t l1_index) const;
   std::filesystem::path get_subdir(uint8_t l1_index, uint8_t l2_index) const;

diff --git a/src/ccache/storage/storage.cpp b/src/ccache/storage/storage.cpp
index e5ecf6cc2cf93f5d33d2399402939976c2c9ae9e..a4c5574bde9f7f5e9bbbd10333af8a53651c0e94 100644 (file)
--- a/src/ccache/storage/storage.cpp
+++ b/src/ccache/storage/storage.cpp
@@ -451,28 +451,26 @@ Storage::get(const Hash::Hash::Digest& key,
 
   get_from_remote_storage(key, type, [&](util::Bytes&& data) {
     if (!m_config.remote_only()) {
-      local.put(key, type, data, Overwrite::no);
+      local.put(key, data, Overwrite::no);
     }
     return entry_receiver(std::move(data));
   });
 }
 
 void
-Storage::put(const Hash::Digest& key,
-             const core::CacheEntryType type,
-             std::span<const uint8_t> value)
+Storage::put(const Hash::Digest& key, std::span<const uint8_t> value)
 {
   if (!m_config.remote_only()) {
-    local.put(key, type, value, Overwrite::yes);
+    local.put(key, value, Overwrite::yes);
   }
   put_in_remote_storage(key, value, Overwrite::yes);
 }
 
 void
-Storage::remove(const Hash::Digest& key, const core::CacheEntryType type)
+Storage::remove(const Hash::Digest& key)
 {
   if (!m_config.remote_only()) {
-    local.remove(key, type);
+    local.remove(key);
   }
   remove_from_remote_storage(key);
 }

diff --git a/src/ccache/storage/storage.hpp b/src/ccache/storage/storage.hpp
index a160db24a680ba329c949ca618d1a7608a4f1eb7..11c9cb800f41bad8133dca6bbaa9259d2264158c 100644 (file)
--- a/src/ccache/storage/storage.hpp
+++ b/src/ccache/storage/storage.hpp
@@ -63,11 +63,9 @@ public:
            core::CacheEntryType type,
            const EntryReceiver& entry_receiver);
 
-  void put(const Hash::Digest& key,
-           core::CacheEntryType type,
-           std::span<const uint8_t> value);
+  void put(const Hash::Digest& key, std::span<const uint8_t> value);
 
-  void remove(const Hash::Digest& key, core::CacheEntryType type);
+  void remove(const Hash::Digest& key);
 
   void stop_remote_storage_helpers();
 

diff --git a/src/ccache/util/string.cpp b/src/ccache/util/string.cpp
index 1fdb42e152601dd78fbcc5bfa5fba33cddfc6e0e..fffc16817e5dbba81dc1eff0d7eb3929b78178a0 100644 (file)
--- a/src/ccache/util/string.cpp
+++ b/src/ccache/util/string.cpp
@@ -140,40 +140,6 @@ format_base16(std::span<const uint8_t> data)
   return result;
 }
 
-std::string
-format_base32hex(std::span<const uint8_t> data)
-{
-  static const char digits[] = "0123456789abcdefghijklmnopqrstuv";
-  std::string result;
-  result.reserve(data.size() * 8 / 5 + 1);
-  uint8_t i = 0;
-  uint16_t bits = 0;
-  for (uint8_t b : data) {
-    bits <<= 8;
-    bits |= b;
-    i += 8;
-    while (i >= 5) {
-      result += digits[(bits >> (i - 5)) & 0x1f];
-      i -= 5;
-    }
-  }
-  if (i > 0) {
-    DEBUG_ASSERT(i < 5);
-    result += digits[(bits << (5 - i)) & 0x1f];
-  }
-  return result;
-}
-
-std::string
-format_legacy_digest(std::span<const uint8_t> data)
-{
-  const size_t base16_bytes = 2;
-  ASSERT(data.size() >= base16_bytes);
-  return format_base16({data.data(), base16_bytes})
-         + format_base32hex(
-           {data.data() + base16_bytes, data.size() - base16_bytes});
-}
-
 std::string
 format_duration(std::chrono::milliseconds ms)
 {

diff --git a/src/ccache/util/string.hpp b/src/ccache/util/string.hpp
index b78194635d609252ad9d46cf50a697a4fab10a0c..89ad8259d89e5ea3a2d0f3f8ef087c19a1a42a6d 100644 (file)
--- a/src/ccache/util/string.hpp
+++ b/src/ccache/util/string.hpp
@@ -63,10 +63,6 @@ std::string format_argv_for_logging(const char* const* argv);
 // `2 * data.size()` long.
 std::string format_base16(std::span<const uint8_t> data);
 
-// Format a lowercase base32hex string representing `data`. No padding
-// characters will be added.
-std::string format_base32hex(std::span<const uint8_t> data);
-
 // Format `ms` as a duration string.
 std::string format_duration(std::chrono::milliseconds ms);
 
@@ -74,14 +70,6 @@ std::string format_duration(std::chrono::milliseconds ms);
 std::string format_human_readable_diff(int64_t diff,
                                        SizeUnitPrefixType prefix_type);
 
-// Format a hash digest representing `data`.
-//
-// The first two bytes are encoded as four lowercase base16 digits to maintain
-// compatibility with the cleanup algorithm in older ccache versions and to
-// allow for up to four uniform cache levels. The rest are encoded as lowercase
-// base32hex digits without padding characters.
-std::string format_legacy_digest(std::span<const uint8_t> data);
-
 // Format `size` as a human-readable string.
 std::string format_human_readable_size(uint64_t size,
                                        SizeUnitPrefixType prefix_type);

diff --git a/unittest/test_util_string.cpp b/unittest/test_util_string.cpp
index 08af49107cd56ad78df7dc37ad3b15164c2b3e37..5f70d02c2ac95f081693f82ce5f5249308786b4a 100644 (file)
--- a/unittest/test_util_string.cpp
+++ b/unittest/test_util_string.cpp
@@ -242,19 +242,6 @@ TEST_CASE("util::parse_base16")
   }
 }
 
-TEST_CASE("util::format_base32hex")
-{
-  // Test vectors (without padding) from RFC 4648.
-  const uint8_t input[] = {'f', 'o', 'o', 'b', 'a', 'r'};
-  CHECK(util::format_base32hex({input, 0}) == "");
-  CHECK(util::format_base32hex({input, 1}) == "co");
-  CHECK(util::format_base32hex({input, 2}) == "cpng");
-  CHECK(util::format_base32hex({input, 3}) == "cpnmu");
-  CHECK(util::format_base32hex({input, 4}) == "cpnmuog");
-  CHECK(util::format_base32hex({input, 5}) == "cpnmuoj1");
-  CHECK(util::format_base32hex({input, 6}) == "cpnmuoj1e8");
-}
-
 TEST_CASE("util::format_human_readable_diff")
 {
   using SUPT = util::SizeUnitPrefixType;