From: Joel Rosdahl Date: Sat, 12 Dec 2009 20:27:11 +0000 (+0100) Subject: Don't create empty stderr files in the cache X-Git-Tag: v3.0pre0~140 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=59eb6b9fc3c3835328ff1aa828bd234b8ce72de0;p=thirdparty%2Fccache.git Don't create empty stderr files in the cache --- diff --git a/NEWS b/NEWS index 0b589b065..ed20ae94d 100644 --- a/NEWS +++ b/NEWS @@ -45,6 +45,9 @@ New features and improvements: - Improved the test suite and added tests for most of the new functionality. It's now also possible to specify a subset of tests to run. + - Standard error output from the compiler is now only stored in the cache if + it's non-empty. + Bug fixes: - Fixed build on FreeBSD. diff --git a/ccache.c b/ccache.c index 69026e69e..9558ea2c5 100644 --- a/ccache.c +++ b/ccache.c @@ -410,9 +410,8 @@ static void process_preprocessed_file(struct mdfour *hash, const char *path) /* run the real compiler and put the result in cache */ static void to_cache(ARGS *args) { - char *path_stderr; char *tmp_stdout, *tmp_stderr, *tmp_hashname; - struct stat st1, st2; + struct stat st; int status; int compress; @@ -441,7 +440,7 @@ static void to_cache(ARGS *args) status = execute(args->argv, tmp_stdout, tmp_stderr); args_pop(args, 3); - if (stat(tmp_stdout, &st1) != 0 || st1.st_size != 0) { + if (stat(tmp_stdout, &st) != 0 || st.st_size != 0) { cc_log("Compiler produced stdout for %s\n", output_file); stats_update(STATS_STDOUT); unlink(tmp_stdout); @@ -489,14 +488,28 @@ static void to_cache(ARGS *args) failed(); } - x_asprintf(&path_stderr, "%s.stderr", object_path); compress = !getenv("CCACHE_NOCOMPRESS"); - if (stat(tmp_stderr, &st1) != 0 - || stat(tmp_hashname, &st2) != 0 - || move_file(tmp_hashname, object_path, compress) != 0 - || move_file(tmp_stderr, path_stderr, compress) != 0) { - cc_log("Failed to rename tmp files - %s\n", strerror(errno)); + if (stat(tmp_stderr, &st) != 0) { + cc_log("Failed to stat %s\n", tmp_stderr); + stats_update(STATS_ERROR); + failed(); + } + if (st.st_size > 0) { + char *path_stderr; + x_asprintf(&path_stderr, "%s.stderr", object_path); + if (move_file(tmp_stderr, path_stderr, compress) != 0) { + cc_log("Failed to move tmp stderr to the cache\n"); + stats_update(STATS_ERROR); + failed(); + } + cc_log("Stored stderr from the compiler in the cache\n"); + free(path_stderr); + } else { + unlink(tmp_stderr); + } + if (move_file(tmp_hashname, object_path, compress) != 0) { + cc_log("Failed to move tmp object file into the cache\n"); stats_update(STATS_ERROR); failed(); } @@ -505,19 +518,18 @@ static void to_cache(ARGS *args) * Do an extra stat on the potentially compressed object file for the * size statistics. */ - if (stat(object_path, &st2) != 0) { + if (stat(object_path, &st) != 0) { cc_log("Failed to stat %s\n", strerror(errno)); stats_update(STATS_ERROR); failed(); } cc_log("Placed object file into the cache\n"); - stats_tocache(file_size(&st2)); + stats_tocache(file_size(&st)); free(tmp_hashname); free(tmp_stderr); free(tmp_stdout); - free(path_stderr); } /* @@ -784,6 +796,17 @@ static void from_cache(enum fromcache_call_mode mode, int put_object_in_manifest struct stat st; int produce_dep_file; + /* the user might be disabling cache hits */ + if (mode != FROMCACHE_COMPILED_MODE && getenv("CCACHE_RECACHE")) { + return; + } + + /* Check if the object file is there. */ + if (stat(object_path, &st) != 0) { + cc_log("Did not find object file in cache\n"); + return; + } + /* * (If mode != FROMCACHE_DIRECT_MODE, the dependency file is created by * gcc.) @@ -791,59 +814,15 @@ static void from_cache(enum fromcache_call_mode mode, int put_object_in_manifest produce_dep_file = \ generating_dependencies && mode == FROMCACHE_DIRECT_MODE; - x_asprintf(&stderr_file, "%s.stderr", object_path); + /* If the dependency file should be in the cache, check that it is. */ x_asprintf(&dep_file, "%s.d", object_path); - - fd_stderr = open(stderr_file, O_RDONLY | O_BINARY); - if (fd_stderr == -1) { - /* it isn't in cache ... */ - cc_log("Did not find object file in cache\n"); - free(stderr_file); - return; - } - - /* make sure the output is there too */ - if (stat(object_path, &st) != 0) { - cc_log("Object file missing in cache\n"); - close(fd_stderr); - unlink(stderr_file); - free(stderr_file); - return; - } - - /* ...and the dependency file, if wanted */ if (produce_dep_file && stat(dep_file, &st) != 0) { cc_log("Dependency file missing in cache\n"); - close(fd_stderr); - free(stderr_file); + free(dep_file); return; } - /* the user might be disabling cache hits */ - if (mode != FROMCACHE_COMPILED_MODE && getenv("CCACHE_RECACHE")) { - close(fd_stderr); - unlink(stderr_file); - unlink(object_path); - unlink(dep_file); - free(stderr_file); - return; - } - - /* update timestamps for LRU cleanup - also gives output_file a sensible mtime when hard-linking (for make) */ -#ifdef HAVE_UTIMES - utimes(object_path, NULL); - utimes(stderr_file, NULL); - if (produce_dep_file) { - utimes(dep_file, NULL); - } -#else - utime(object_path, NULL); - utime(stderr_file, NULL); - if (produce_dep_file) { - utime(dep_file, NULL); - } -#endif + x_asprintf(&stderr_file, "%s.stderr", object_path); if (strcmp(output_file, "/dev/null") == 0) { ret = 0; @@ -860,6 +839,7 @@ static void from_cache(enum fromcache_call_mode mode, int put_object_in_manifest if (ret == -1) { if (errno == ENOENT) { + /* Someone removed the file just before we began copying? */ cc_log("Object file missing for %s\n", output_file); stats_update(STATS_MISSING); } else { @@ -868,10 +848,11 @@ static void from_cache(enum fromcache_call_mode mode, int put_object_in_manifest stats_update(STATS_ERROR); failed(); } - close(fd_stderr); + unlink(output_file); unlink(stderr_file); unlink(object_path); unlink(dep_file); + free(dep_file); free(stderr_file); return; } else { @@ -889,6 +870,10 @@ static void from_cache(enum fromcache_call_mode mode, int put_object_in_manifest } if (ret == -1) { if (errno == ENOENT) { + /* + * Someone removed the file just before we + * began copying? + */ cc_log("dependency file missing for %s\n", output_file); stats_update(STATS_MISSING); @@ -899,10 +884,11 @@ static void from_cache(enum fromcache_call_mode mode, int put_object_in_manifest stats_update(STATS_ERROR); failed(); } - close(fd_stderr); + unlink(output_file); unlink(stderr_file); unlink(object_path); unlink(dep_file); + free(dep_file); free(stderr_file); return; } else { @@ -910,6 +896,22 @@ static void from_cache(enum fromcache_call_mode mode, int put_object_in_manifest } } + /* update timestamps for LRU cleanup + also gives output_file a sensible mtime when hard-linking (for make) */ +#ifdef HAVE_UTIMES + utimes(object_path, NULL); + utimes(stderr_file, NULL); + if (produce_dep_file) { + utimes(dep_file, NULL); + } +#else + utime(object_path, NULL); + utime(stderr_file, NULL); + if (produce_dep_file) { + utime(dep_file, NULL); + } +#endif + if (generating_dependencies && mode != FROMCACHE_DIRECT_MODE) { /* Store the dependency file in the cache. */ ret = copy_file(dependency_path, dep_file, 1); @@ -921,6 +923,7 @@ static void from_cache(enum fromcache_call_mode mode, int put_object_in_manifest cc_log("Placed dependency file into the cache\n"); } } + free(dep_file); /* get rid of the intermediate preprocessor file */ if (i_tmpfile) { @@ -938,14 +941,17 @@ static void from_cache(enum fromcache_call_mode mode, int put_object_in_manifest copy_fd(fd_cpp_stderr, 2); close(fd_cpp_stderr); unlink(cpp_stderr); - free(cpp_stderr); - cpp_stderr = NULL; } + free(cpp_stderr); } /* send the stderr */ - copy_fd(fd_stderr, 2); - close(fd_stderr); + fd_stderr = open(stderr_file, O_RDONLY | O_BINARY); + if (fd_stderr != -1) { + copy_fd(fd_stderr, 2); + close(fd_stderr); + } + free(stderr_file); /* Create or update the manifest file. */ if (put_object_in_manifest && included_files) { diff --git a/cleanup.c b/cleanup.c index b82ccfb9f..170b625d6 100644 --- a/cleanup.c +++ b/cleanup.c @@ -129,8 +129,21 @@ static void sort_and_clean(void) } if (is_object_file(files[i]->fname)) { + char *path; + total_object_files -= 1; total_object_size -= files[i]->size; + + /* + * If we have deleted an object file, we should delete + * any .stderr and .d file as well. + */ + x_asprintf(&path, "%s.stderr", files[i]->fname); + unlink(path); + free(path); + x_asprintf(&path, "%s.d", files[i]->fname); + unlink(path); + free(path); } } } diff --git a/test.sh b/test.sh index 22a95e6b8..c18526e59 100755 --- a/test.sh +++ b/test.sh @@ -250,6 +250,23 @@ base_tests() { # checkstat 'cache hit (preprocessed)' 11 # checkstat 'cache miss' 39 + testname="stderr-files" + num=`find $CCACHE_DIR -name '*.stderr' | wc -l` + if [ $num -ne 0 ]; then + test_failed "$num stderr files found, expected 0" + fi + cat <stderr.c +int stderr(void) +{ + /* Trigger warning by having no return statement. */ +} +EOF + $CCACHE_COMPILE -Wall -W -c stderr.c 2>/dev/null + num=`find $CCACHE_DIR -name '*.stderr' | wc -l` + if [ $num -ne 1 ]; then + test_failed "$num stderr files found, expected 1" + fi + testname="zero-stats" $CCACHE -z > /dev/null checkstat 'cache hit (preprocessed)' 0 @@ -705,7 +722,17 @@ mkdir $CCACHE_DIR # --------------------------------------- -all_suites="base link hardlink cpp2 nlevels4 nlevels1 direct basedir" +all_suites=" +base +link +hardlink +cpp2 +nlevels4 +nlevels1 +direct +basedir +" + if [ -z "$suites" ]; then suites="$all_suites" fi