From: Joel Rosdahl Date: Sat, 1 Aug 2020 14:39:11 +0000 (+0200) Subject: Let CCACHE_UMASK apply only to files/directories in the cache directory X-Git-Tag: v4.0~236 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=106a1d22919e65b521721af4124405ba0ed2e54a;p=thirdparty%2Fccache.git Let CCACHE_UMASK apply only to files/directories in the cache directory Ever since the CCACHE_UMASK setting was added in 75dd382407a36b3ebab36bb1027eb43a07498bec it has applied to all files created by ccache, including files such as the object files and executables created when ccache is called for linking. This was presumably the easy thing to do, but it affects the observable output from the compiler. There are also two related bugs: 1. CCACHE_UMASK is not used when creating the initial configuration file. 2. CCACHE_UMASK is used when creating the log file. Fix this by: - saving the original umask in Context, - introducing a UmaskScope class, - using UmaskScope objects to set the original umask when calling the compiler and when copying result files from the cache, and - setting the umask after creating the log file but before creating the initial configuration file. Closes #628. --- diff --git a/doc/MANUAL.adoc b/doc/MANUAL.adoc index b6f973146..b3bb50cc3 100644 --- a/doc/MANUAL.adoc +++ b/doc/MANUAL.adoc @@ -721,10 +721,9 @@ NOTE: In previous versions of ccache, *CCACHE_TEMPDIR* had to be on the same [[config_umask]] *umask* (*CCACHE_UMASK*):: - This setting specifies the umask for ccache and all child processes (such - as the compiler). This is mostly useful when you wish to share your cache - with other users. Note that this also affects the file permissions set on - the object files created from your compilations. + This setting specifies the umask for files and directories in the cache + directory. This is mostly useful when you wish to share your cache with + other users. Cache size management diff --git a/src/Context.hpp b/src/Context.hpp index 2ab6052ce..efd339136 100644 --- a/src/Context.hpp +++ b/src/Context.hpp @@ -130,6 +130,10 @@ public: const std::vector& ignore_options() const; void set_ignore_options(const std::vector& options); + // Original umask before applying the `umask`/`CCACHE_UMASK` configuration, or + // `nullopt` if there is no such configuration. + nonstd::optional original_umask; + #ifdef MTR_ENABLED // Internal tracing. std::unique_ptr mini_trace; diff --git a/src/UmaskScope.hpp b/src/UmaskScope.hpp new file mode 100644 index 000000000..d96f448e2 --- /dev/null +++ b/src/UmaskScope.hpp @@ -0,0 +1,55 @@ +// Copyright (C) 2020 Joel Rosdahl and other contributors +// +// See doc/AUTHORS.adoc for a complete list of contributors. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License as published by the Free +// Software Foundation; either version 3 of the License, or (at your option) +// any later version. +// +// This program is distributed in the hope that it will be useful, but WITHOUT +// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +// more details. +// +// You should have received a copy of the GNU General Public License along with +// this program; if not, write to the Free Software Foundation, Inc., 51 +// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +#pragma once + +#include "system.hpp" + +#include "third_party/nonstd/optional.hpp" + +// This class sets a new (process-global) umask and restores the previous umask +// when destructed. +class UmaskScope +{ +public: + UmaskScope(nonstd::optional new_umask); + ~UmaskScope(); + +private: + nonstd::optional m_saved_umask; +}; + +UmaskScope::UmaskScope(nonstd::optional new_umask) +{ +#ifndef _WIN32 + if (new_umask) { + m_saved_umask = umask(*new_umask); + } +#else + (void)new_umask; +#endif +} + +UmaskScope::~UmaskScope() +{ +#ifndef _WIN32 + if (m_saved_umask) { + umask(*m_saved_umask); + } +#endif +} diff --git a/src/ccache.cpp b/src/ccache.cpp index a4f5cde09..682576b64 100644 --- a/src/ccache.cpp +++ b/src/ccache.cpp @@ -35,6 +35,7 @@ #include "SignalHandler.hpp" #include "StdMakeUnique.hpp" #include "TemporaryFile.hpp" +#include "UmaskScope.hpp" #include "Util.hpp" #include "argprocessing.hpp" #include "cleanup.hpp" @@ -49,6 +50,7 @@ #include "stats.hpp" #include "third_party/fmt/core.h" +#include "third_party/nonstd/optional.hpp" #include "third_party/nonstd/string_view.hpp" #ifdef HAVE_GETOPT_LONG @@ -688,6 +690,8 @@ do_execute(Context& ctx, TemporaryFile&& tmp_stdout, TemporaryFile&& tmp_stderr) { + UmaskScope umask_scope(ctx.original_umask); + if (ctx.diagnostics_color_failed && ctx.guessed_compiler == GuessedCompiler::gcc) { args.erase_with_prefix("-fdiagnostics-color"); @@ -1651,6 +1655,8 @@ calculate_result_name(Context& ctx, static optional from_cache(Context& ctx, enum fromcache_call_mode mode) { + UmaskScope umask_scope(ctx.original_umask); + // The user might be disabling cache hits. if (ctx.config.recache()) { return nullopt; @@ -1763,8 +1769,9 @@ create_initial_config_file(Config& config) } // Read config file(s), populate variables, create configuration file in cache -// directory if missing, etc. -static void +// directory if missing, etc. Returns whether the primary configuration file +// exists. +static bool set_up_config(Config& config) { const char* p = getenv("CCACHE_CONFIGPATH"); @@ -1791,25 +1798,16 @@ set_up_config(Config& config) fmt::format("{}/ccache.conf", config.cache_dir())); } - bool should_create_initial_config = false; MTR_BEGIN("config", "conf_read_primary"); - if (!config.update_from_file(config.primary_config_path()) - && !config.disable()) { - should_create_initial_config = true; - } + bool primary_config_exists = + config.update_from_file(config.primary_config_path()); MTR_END("config", "conf_read_primary"); MTR_BEGIN("config", "conf_update_from_environment"); config.update_from_environment(); MTR_END("config", "conf_update_from_environment"); - if (should_create_initial_config) { - create_initial_config_file(config); - } - - if (config.umask() != std::numeric_limits::max()) { - umask(config.umask()); - } + return primary_config_exists; } static void @@ -1826,10 +1824,24 @@ set_up_context(Context& ctx, int argc, const char* const* argv) static void initialize(Context& ctx, int argc, const char* const* argv) { - set_up_config(ctx.config); + bool primary_config_exists = set_up_config(ctx.config); set_up_context(ctx, argc, argv); init_log(ctx.config); + // Set default umask for all files created by ccache from now on (if + // configured to). This is intentionally done after calling init_log so that + // the log file won't be affected by the umask but before creating the initial + // configuration file. The intention is that all files and directories in the + // cache directory should be affected by the configured umask and that no + // other files and directories should. + if (ctx.config.umask() != std::numeric_limits::max()) { + ctx.original_umask = umask(ctx.config.umask()); + } + + if (!primary_config_exists && !ctx.config.disable()) { + create_initial_config_file(ctx.config); + } + cc_log("=== CCACHE %s STARTED =========================================", CCACHE_VERSION); @@ -1906,6 +1918,10 @@ cache_compilation(int argc, const char* const* argv) } // Else: Fall back to running the real compiler. + if (ctx->original_umask) { + umask(*ctx->original_umask); + } + assert(!ctx->orig_args.empty()); ctx->orig_args.erase_with_prefix("--ccache-"); diff --git a/test/run b/test/run index c24153703..ca8e3097c 100755 --- a/test/run +++ b/test/run @@ -271,6 +271,16 @@ expect_file_newer_than() { fi } +expect_perm() { + local path="$1" + local expected_perm="$2" + local actual_perm=$(ls -ld "$path" | awk '{print $1}') + + if [ "$expected_perm" != "$actual_perm" ]; then + test_failed "Expected permissions for $path to be $expected_perm, actual $actual_perm" + fi +} + run_suite() { local suite_name=$1 @@ -312,9 +322,9 @@ EOF unset GCC_COLORS unset TERM - export CCACHE_CONFIGPATH=$ABS_TESTDIR/ccache.conf export CCACHE_DETECT_SHEBANG=1 export CCACHE_DIR=$ABS_TESTDIR/.ccache + export CCACHE_CONFIGPATH=$CCACHE_DIR/ccache.conf # skip secondary config export CCACHE_LOGFILE=$ABS_TESTDIR/ccache.log export CCACHE_NODIRECT=1 diff --git a/test/suites/base.bash b/test/suites/base.bash index 6edf6e83c..4b4fa2e3e 100644 --- a/test/suites/base.bash +++ b/test/suites/base.bash @@ -909,6 +909,48 @@ EOF CCACHE_COMPILERCHECK="unknown_command" $CCACHE ./compiler.sh -c test1.c 2>/dev/null expect_stat 'compiler check failed' 1 + + # ------------------------------------------------------------------------- + TEST "CCACHE_UMASK" + + saved_umask=$(umask) + umask 022 + export CCACHE_UMASK=002 + + cat <test.c +int main() {} +EOF + + $CCACHE_COMPILE -MMD -c test.c + expect_stat 'cache hit (preprocessed)' 0 + expect_stat 'cache miss' 1 + result_file=$(find $CCACHE_DIR -name '*.result') + level_2_dir=$(dirname $result_file) + level_1_dir=$(dirname $(dirname $result_file)) + expect_perm test.o -rw-r--r-- + expect_perm test.d -rw-r--r-- + expect_perm "$CCACHE_CONFIGPATH" -rw-rw-r-- + expect_perm "$CCACHE_DIR" drwxrwxr-x + expect_perm "$level_1_dir" drwxrwxr-x + expect_perm "$level_1_dir/stats" -rw-rw-r-- + expect_perm "$level_2_dir" drwxrwxr-x + expect_perm "$result_file" -rw-rw-r-- + + rm test.o test.d + $CCACHE_COMPILE -MMD -c test.c + expect_stat 'cache hit (preprocessed)' 1 + expect_stat 'cache miss' 1 + expect_perm test.o -rw-r--r-- + expect_perm test.d -rw-r--r-- + + $CCACHE_COMPILE -o test test.o + expect_stat 'cache hit (preprocessed)' 1 + expect_stat 'cache miss' 1 + expect_stat 'called for link' 1 + expect_perm test -rwxr-xr-x + + umask $saved_umask + # ------------------------------------------------------------------------- TEST "No object file"