Improve file size/number counters for overwritten files

When ccache overwrites a cached file, the counters are still increased
as if a new file was stored in the cache, for instance when recaching.

This is now fixed by checking if a cached file exists before storing the
cached file and update file size/number counters with the correct
size/count delta.

diff --git a/NEWS.txt b/NEWS.txt
index 6678f3312db8ef4f3bc6454b460586564b93593f..1fc0e8b4b6b35ba86597afdef59e9bddcab4d979 100644 (file)
--- a/NEWS.txt
+++ b/NEWS.txt
@@ -20,6 +20,13 @@ New features and improvements
 - The optional unifier is no longer disabled when the direct mode is enabled.
 
 
+Bug fixes
+~~~~~~~~~
+
+- File size and number counters are now updated correctly when files are
+  overwritten in the cache, e.g. when using `CCACHE_RECACHE`.
+
+
 ccache 3.3.5
 ------------
 Release date: 2018-01-13

diff --git a/ccache.c b/ccache.c
index 36ed1cca690c57563e17796eb6777be07e4cc972..cc2167d31f3ea4558e76500cec5aa2e7a12259c5 100644 (file)
--- a/ccache.c
+++ b/ccache.c
@@ -1004,41 +1004,85 @@ out:
        free(tmp_file);
 }
 
-// Copy or link a file to the cache.
+// Helper method for copy_file_to_cache and move_file_to_cache_same_fs.
 static void
-put_file_in_cache(const char *source, const char *dest)
+do_copy_or_move_file_to_cache(const char *source, const char *dest, bool copy)
 {
        assert(!conf->read_only);
        assert(!conf->read_only_direct);
 
-       bool do_link = conf->hard_link && !conf->compression;
-       if (do_link) {
-               x_unlink(dest);
-               int ret = link(source, dest);
-               if (ret != 0) {
-                       cc_log("Failed to link %s to %s: %s", source, dest, strerror(errno));
-                       cc_log("Falling back to copying");
-                       do_link = false;
-               }
-       }
-       if (!do_link) {
-               int ret = copy_file(
-                 source, dest, conf->compression ? conf->compression_level : 0);
-               if (ret != 0) {
-                       cc_log("Failed to copy %s to %s: %s", source, dest, strerror(errno));
-                       stats_update(STATS_ERROR);
-                       failed();
+       struct stat orig_dest_st;
+       bool orig_dest_existed = stat(dest, &orig_dest_st) == 0;
+       int compression_level = conf->compression ? conf->compression_level : 0;
+       bool do_move = !copy && !conf->compression;
+       bool do_link = copy && conf->hard_link && !conf->compression;
+
+       if (do_move) {
+               move_uncompressed_file(source, dest, compression_level);
+       } else {
+               if (do_link) {
+                       x_unlink(dest);
+                       int ret = link(source, dest);
+                       if (ret == 0) {
+                       } else {
+                               cc_log("Failed to link %s to %s: %s", source, dest, strerror(errno));
+                               cc_log("Falling back to copying");
+                               do_link = false;
+                       }
+               }
+               if (!do_link) {
+                       int ret = copy_file(source, dest, compression_level);
+                       if (ret != 0) {
+                               cc_log("Failed to copy %s to %s: %s", source, dest, strerror(errno));
+                               stats_update(STATS_ERROR);
+                               failed();
+                       }
                }
        }
 
-       cc_log("Stored in cache: %s -> %s", source, dest);
+       if (!copy && conf->compression) {
+               // We fell back to copying since dest should be compressed, so clean up.
+               x_unlink(source);
+       }
+
+       cc_log("Stored in cache: %s -> %s (%s)",
+              source,
+              dest,
+              do_move ? "moved" : (do_link ? "linked" : "copied"));
 
        struct stat st;
        if (x_stat(dest, &st) != 0) {
                stats_update(STATS_ERROR);
                failed();
        }
-       stats_update_size(file_size(&st), 1);
+       stats_update_size(
+               file_size(&st) - (orig_dest_existed ? file_size(&orig_dest_st) : 0),
+               orig_dest_existed ? 0 : 1);
+}
+
+// Copy a file into the cache.
+//
+// dest must be a path in the cache (see get_path_in_cache). source does not
+// have to be on the same file system as dest.
+//
+// An attempt will be made to hard link source to dest if conf->hard_link is
+// true and conf->compression is false, otherwise copy. dest will be compressed
+// if conf->compression is true.
+static void
+copy_file_to_cache(const char *source, const char *dest)
+{
+       do_copy_or_move_file_to_cache(source, dest, true);
+}
+
+// Move a file into the cache.
+//
+// dest must be a path in the cache (see get_path_in_cache). source must be on
+// the same file system as dest. dest will be compressed if conf->compression
+// is true.
+static void
+move_file_to_cache_same_fs(const char *source, const char *dest)
+{
+       do_copy_or_move_file_to_cache(source, dest, false);
 }
 
 // Copy or link a file from the cache.
@@ -1326,20 +1370,7 @@ to_cache(struct args *args)
                failed();
        }
        if (st.st_size > 0) {
-               if (move_uncompressed_file(
-                     tmp_stderr, cached_stderr,
-                     conf->compression ? conf->compression_level : 0) != 0) {
-                       cc_log("Failed to move %s to %s: %s", tmp_stderr, cached_stderr,
-                              strerror(errno));
-                       stats_update(STATS_ERROR);
-                       failed();
-               }
-               cc_log("Stored in cache: %s", cached_stderr);
-               if (!conf->compression
-                   // If the file was compressed, obtain the size again:
-                   || x_stat(cached_stderr, &st) == 0) {
-                       stats_update_size(file_size(&st), 1);
-               }
+               move_file_to_cache_same_fs(tmp_stderr, cached_stderr);
        } else {
                tmp_unlink(tmp_stderr);
                if (conf->recache) {
@@ -1351,17 +1382,17 @@ to_cache(struct args *args)
        if (generating_coverage && tmp_cov) {
                // GCC won't generate notes if there is no code.
                if (stat(tmp_cov, &st) != 0 && errno == ENOENT) {
-                       FILE *f = fopen(cached_cov, "wb");
-                       cc_log("Creating placeholder: %s", cached_cov);
+                       FILE *f = fopen(tmp_cov, "wb");
+                       cc_log("Creating placeholder: %s", tmp_cov);
                        if (!f) {
-                               cc_log("Failed to create %s: %s", cached_cov, strerror(errno));
+                               cc_log("Failed to create %s: %s", tmp_cov, strerror(errno));
                                stats_update(STATS_ERROR);
                                failed();
                        }
                        fclose(f);
-                       stats_update_size(0, 1);
+                       move_file_to_cache_same_fs(tmp_cov, cached_cov);
                } else {
-                       put_file_in_cache(tmp_cov, cached_cov);
+                       copy_file_to_cache(tmp_cov, cached_cov);
                }
        }
 
@@ -1378,7 +1409,7 @@ to_cache(struct args *args)
                        fclose(f);
                        stats_update_size(0, 1);
                } else {
-                       put_file_in_cache(tmp_su, cached_su);
+                       copy_file_to_cache(tmp_su, cached_su);
                }
        }
 
@@ -1388,25 +1419,25 @@ to_cache(struct args *args)
                        failed();
                }
                if (st.st_size > 0) {
-                       put_file_in_cache(output_dia, cached_dia);
+                       copy_file_to_cache(output_dia, cached_dia);
                }
        }
 
-       put_file_in_cache(output_obj, cached_obj);
+       copy_file_to_cache(output_obj, cached_obj);
 
        if (using_split_dwarf) {
                assert(tmp_dwo);
                assert(cached_dwo);
-               put_file_in_cache(tmp_dwo, cached_dwo);
+               copy_file_to_cache(tmp_dwo, cached_dwo);
        }
 
        if (generating_dependencies) {
                use_relative_paths_in_depfile(output_dep);
-               put_file_in_cache(output_dep, cached_dep);
+               copy_file_to_cache(output_dep, cached_dep);
        }
 
        if (output_su) {
-               put_file_in_cache(output_su, cached_su);
+               copy_file_to_cache(output_su, cached_su);
        }
 
        stats_update(STATS_TOCACHE);

diff --git a/ccache.h b/ccache.h
index d4b5ed2b7c2b1e8bf36d9d1ea207ffa3f16ca84a..6fdd0d4014b4a227968b769c353ea693085e8211 100644 (file)
--- a/ccache.h
+++ b/ccache.h
@@ -196,7 +196,7 @@ void stats_flush(void);
 unsigned stats_get_pending(enum stats stat);
 void stats_zero(void);
 void stats_summary(struct conf *conf);
-void stats_update_size(uint64_t size, unsigned files);
+void stats_update_size(int64_t size, int files);
 void stats_get_obsolete_limits(const char *dir, unsigned *maxfiles,
                                uint64_t *maxsize);
 void stats_set_sizes(const char *dir, unsigned num_files, uint64_t total_size);

diff --git a/stats.c b/stats.c
index d324ed9f73a31cadd43f2e4a7b0f11e6b9c938c4..de206fe00fa8e0379570daeaf693d3ab3683ee1d 100644 (file)
--- a/stats.c
+++ b/stats.c
@@ -313,7 +313,7 @@ init_counter_updates(void)
 // Record that a number of bytes and files have been added to the cache. Size
 // is in bytes.
 void
-stats_update_size(uint64_t size, unsigned files)
+stats_update_size(int64_t size, int files)
 {
        init_counter_updates();
        counter_updates->data[STATS_NUMFILES] += files;

diff --git a/test.sh b/test.sh
index 05773b0a628082bbc41a96e24614f00085dabbdc..86a538e20cd6ab1544d7c5ce007c6c2e76ae0fca 100755 (executable)
--- a/test.sh
+++ b/test.sh
@@ -383,10 +383,6 @@ base_tests() {
     $UNCACHED_COMPILE -c -o reference_test1.o test1.c
     expect_equal_object_files reference_test1.o test1.o
 
-    # CCACHE_RECACHE replaces the object file, so the statistics counter will
-    # be off-by-one until next cleanup.
-    expect_stat 'files in cache' 2
-    $CCACHE -c >/dev/null
     expect_stat 'files in cache' 1
 
     # -------------------------------------------------------------------------