Don't create empty stderr files in the cache

diff --git a/NEWS b/NEWS
index 0b589b0657e6f2162ede5aa4401c391e5dcca2e6..ed20ae94d964941d27828fb9accdfe729ba80c00 100644 (file)
--- 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 69026e69e2ccf100ba2545e216052cf4bd92074f..9558ea2c50db9eb6ebc2a9942228f81b24404b6e 100644 (file)
--- 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 b82ccfb9fd56dd64c119a8744b55076de20e8ca8..170b625d6e7c20ea617d3503fba3e704d9931303 100644 (file)
--- 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 22a95e6b8aede183e2fc51192fc25bb185909caa..c18526e590d1c023c7671578a6eecd36921fb7b4 100755 (executable)
--- 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 <<EOF >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