From: Joel Rosdahl Date: Sun, 21 Jan 2018 08:34:51 +0000 (+0100) Subject: Improve file size/number counters for overwritten files X-Git-Tag: v3.4~44 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2ca735dbfbc335e3e7ef97137560929bac1f9cb0;p=thirdparty%2Fccache.git 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 6678f3312..1fc0e8b4b 100644 --- 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 36ed1cca6..cc2167d31 100644 --- 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 d4b5ed2b7..6fdd0d401 100644 --- 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 d324ed9f7..de206fe00 100644 --- 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 05773b0a6..86a538e20 100755 --- 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 # -------------------------------------------------------------------------