fix: Use correct umask when populating primary cache from secondary

Util::get_umask retrieves the process's umask and caches it to avoid
some system calls. This doesn't interact well now when using UmaskScope
to change umask temporarily since Util::get_umask then only sometimes
returns the correct value. This leads to the incorrect umask being used
when writing cache entries to the primary storage for secondary storage
hits.

Fix this by not caching the umask system call.

Closes #1087.

diff --git a/src/Util.cpp b/src/Util.cpp
index b94ee1a81b71bb252db9244be64d6629480981bb..501e2c3d22a4b7f18be22806a50606af602a78a0 100644 (file)
--- a/src/Util.cpp
+++ b/src/Util.cpp
@@ -744,20 +744,13 @@ get_relative_path(std::string_view dir, std::string_view path)
   return result.empty() ? "." : result;
 }
 
-#ifndef _WIN32
 mode_t
 get_umask()
 {
-  static bool mask_retrieved = false;
-  static mode_t mask;
-  if (!mask_retrieved) {
-    mask = umask(0);
-    umask(mask);
-    mask_retrieved = true;
-  }
+  const mode_t mask = umask(0);
+  umask(mask);
   return mask;
 }
-#endif
 
 void
 hard_link(const std::string& oldpath, const std::string& newpath)

diff --git a/src/Util.hpp b/src/Util.hpp
index bf55484409f83ffc573812fcf39da7119f649f9a..6e299f5838042876d9121e7b41fb44dcb7839b44 100644 (file)
--- a/src/Util.hpp
+++ b/src/Util.hpp
@@ -184,10 +184,8 @@ const char* get_hostname();
 // resolve to the same file as `path`.
 std::string get_relative_path(std::string_view dir, std::string_view path);
 
-#ifndef _WIN32
 // Get process umask.
 mode_t get_umask();
-#endif
 
 // Hard-link `oldpath` to `newpath`. Throws `core::Error` on error.
 void hard_link(const std::string& oldpath, const std::string& newpath);

diff --git a/test/suites/secondary_file.bash b/test/suites/secondary_file.bash
index 3c539874f224da7e4ad3d16860363bd9787d5534..1a6886259f51c6d7331f9de63a272a6339f0c3a6 100644 (file)
--- a/test/suites/secondary_file.bash
+++ b/test/suites/secondary_file.bash
@@ -167,18 +167,22 @@ SUITE_secondary_file() {
     # -------------------------------------------------------------------------
     TEST "umask"
 
-    CCACHE_SECONDARY_STORAGE="file://$PWD/secondary|umask=022"
+    export CCACHE_UMASK=042
+    CCACHE_SECONDARY_STORAGE="file://$PWD/secondary|umask=024"
     rm -rf secondary
     $CCACHE_COMPILE -c test.c
-    expect_perm secondary drwxr-xr-x
-    expect_perm secondary/CACHEDIR.TAG -rw-r--r--
+    expect_perm secondary drwxr-x-wx # 777 & 024
+    expect_perm secondary/CACHEDIR.TAG -rw-r---w- # 666 & 024
+    result_file=$(find $CCACHE_DIR -name '*R')
+    expect_perm "$(dirname "${result_file}")" drwx-wxr-x # 777 & 042
+    expect_perm "${result_file}" -rw--w-r-- # 666 & 042
 
-    CCACHE_SECONDARY_STORAGE="file://$PWD/secondary|umask=000"
+    CCACHE_SECONDARY_STORAGE="file://$PWD/secondary|umask=026"
     $CCACHE -C >/dev/null
     rm -rf secondary
     $CCACHE_COMPILE -c test.c
-    expect_perm secondary drwxrwxrwx
-    expect_perm secondary/CACHEDIR.TAG -rw-rw-rw-
+    expect_perm secondary drwxr-x--x # 777 & 026
+    expect_perm secondary/CACHEDIR.TAG -rw-r----- # 666 & 026
 
     # -------------------------------------------------------------------------
     TEST "Sharding"