]> git.ipfire.org Git - thirdparty/ccache.git/commitdiff
Improve functions related to CWD
authorJoel Rosdahl <joel@rosdahl.net>
Sun, 16 Feb 2020 12:06:24 +0000 (13:06 +0100)
committerJoel Rosdahl <joel@rosdahl.net>
Sun, 16 Feb 2020 21:02:21 +0000 (22:02 +0100)
The different functions related to current working directory (CWD) have
become messy during the years:

- gnu_getcwd is a simple wrapper around getcwd(3), thus returning the
  actual canonical path.
- get_cwd returns $PWD, falling back to getcwd(3) if $PWD is not sane.
- get_current_working_dir (local function in ccache.cpp) memoizes
  x_realpath(get_cwd()) (i.e., getcwd(3) in essence...) in the global
  current_working_dir variable. Unit tests may manipulate
  current_working_dir.

Improve this by:

- Replacing gnu_getcwd with Util::get_actual_cwd.
- Replacing get_cwd with Util::get_apparent_cwd.
- Removing get_current_working_dir and placing both actual and apparent
  CWD in the context object.

12 files changed:
src/Context.cpp
src/Context.hpp
src/Util.cpp
src/Util.hpp
src/ccache.cpp
src/legacy_globals.cpp
src/legacy_globals.hpp
src/legacy_util.cpp
src/legacy_util.hpp
unittest/framework.cpp
unittest/main.cpp
unittest/test_argument_processing.cpp

index c60df719a444cb48f6e3b1fba9f4de4ab5a37da5..cf670878f9ea31d8c684ee42afa42c0ca9b0cb92 100644 (file)
 
 #include "Context.hpp"
 
+#include "Util.hpp"
 #include "args.hpp"
 
+Context::Context()
+  : actual_cwd(Util::get_actual_cwd()),
+    apparent_cwd(Util::get_apparent_cwd(actual_cwd))
+{
+}
+
 Context::~Context()
 {
   args_free(orig_args);
index 26862207196be71b09cb298dc54652153ba247b9..15c86944084c11a4a0b4b2190c0d8f10240e9c91 100644 (file)
@@ -28,12 +28,18 @@ struct args;
 
 struct Context : NonCopyable
 {
-  Context() = default;
+  Context();
   ~Context();
 
   ArgsInfo args_info;
   Config config;
 
+  // Current working directory as returned by getcwd(3).
+  std::string actual_cwd;
+
+  // Current working directory according to $PWD (falling back to getcwd(3)).
+  std::string apparent_cwd;
+
   // Full path to the statistics file in the subdirectory where the cached
   // result belongs (<cache_dir>/<x>/stats).
   std::string stats_file;
index 216559d769c8bf35bfd564bc29d0bf247d6db563..058a1cc7264e879bcff9bf3d0aa72740f19dd0be 100644 (file)
@@ -188,6 +188,41 @@ for_each_level_1_subdir(const std::string& cache_dir,
   progress_receiver(1.0);
 }
 
+std::string
+get_actual_cwd()
+{
+  char buffer[PATH_MAX];
+  if (getcwd(buffer, sizeof(buffer))) {
+    return buffer;
+  } else {
+    return {};
+  }
+}
+
+std::string
+get_apparent_cwd(const std::string& actual_cwd)
+{
+#ifdef _WIN32
+  return actual_cwd;
+#else
+  auto pwd = getenv("PWD");
+  if (!pwd) {
+    return actual_cwd;
+  }
+
+  auto st_pwd = Stat::stat(pwd);
+  auto st_cwd = Stat::stat(actual_cwd);
+  if (!st_pwd || !st_cwd) {
+    return actual_cwd;
+  }
+  if (st_pwd.device() == st_cwd.device() && st_pwd.inode() == st_cwd.inode()) {
+    return pwd;
+  } else {
+    return actual_cwd;
+  }
+#endif
+}
+
 string_view
 get_extension(string_view path)
 {
index f4576bc4194123bbb0d617d991439565fac8a217..c438171cd0aab7c6dbe27347d2d7324c7545d08b 100644 (file)
@@ -109,6 +109,16 @@ void for_each_level_1_subdir(const std::string& cache_dir,
                              const SubdirVisitor& visitor,
                              const ProgressReceiver& progress_receiver);
 
+// Return current working directory (CWD) as returned from getcwd(3) (i.e.,
+// canonical path without symlink parts). Returns the empty string on error.
+std::string get_actual_cwd();
+
+// Return current working directory (CWD) by reading the environment variable
+// PWD (thus keeping any symlink parts in the path and potentially ".." or "//"
+// parts). If PWD does not resolve to the same i-node as `actual_cwd` then
+// `actual_cwd` is returned instead.
+std::string get_apparent_cwd(const std::string& actual_cwd);
+
 // Return the file extension (including the dot) as a view into `path`. If
 // `path` has no file extension, an empty string_view is returned.
 nonstd::string_view get_extension(nonstd::string_view path);
index b5be7935571d259ccdc0a3ea5a5a79670004e4c0..6655cb0eb8fc7dbba78c9b9e2c7dc53aac96fd17 100644 (file)
@@ -416,25 +416,6 @@ guess_compiler(const char* path)
   return result;
 }
 
-static char*
-get_current_working_dir(void)
-{
-  if (!current_working_dir) {
-    char* cwd = get_cwd();
-    if (cwd) {
-      current_working_dir = x_strdup(Util::real_path(cwd).c_str());
-      free(cwd);
-    }
-    if (!current_working_dir) {
-      cc_log("Unable to determine current working directory: %s",
-             strerror(errno));
-      stats_update(STATS_ERROR);
-      failed();
-    }
-  }
-  return current_working_dir;
-}
-
 static bool
 do_remember_include_file(const Context& ctx,
                          std::string path,
@@ -623,8 +604,8 @@ print_included_files(FILE* fp)
   }
 }
 
-// Make a relative path from current working directory to path if path is under
-// the base directory.
+// Make a relative path from current working directory `cwd` to `path` if `path`
+// is under the base directory.
 static std::string
 make_relative_path(const Context& ctx, const char* path)
 {
@@ -648,7 +629,6 @@ make_relative_path(const Context& ctx, const char* path)
 
   char* dir = nullptr;
   char* path_suffix = nullptr;
-  char* relpath = nullptr;
 
   // Util::real_path only works for existing paths, so if path doesn't exist,
   // try x_dirname(path) and assemble the path afterwards. We only bother to try
@@ -678,12 +658,14 @@ make_relative_path(const Context& ctx, const char* path)
 
   std::string canon_path = Util::real_path(path, true);
   if (!canon_path.empty()) {
-    relpath = get_relative_path(get_current_working_dir(), canon_path.c_str());
+    char* relpath =
+      get_relative_path(ctx.actual_cwd.c_str(), canon_path.c_str());
     if (path_suffix) {
       result = fmt::format("{}/{}", relpath, path_suffix);
     } else {
       result = relpath;
     }
+    free(relpath);
   } else {
     // path doesn't exist, so leave it as it is.
     result = path;
@@ -694,7 +676,6 @@ make_relative_path(const Context& ctx, const char* path)
 #endif
   free(dir);
   free(path_suffix);
-  free(relpath);
 
   return result;
 }
@@ -733,8 +714,6 @@ process_preprocessed_file(Context& ctx,
     free(p);
   }
 
-  char* cwd = get_cwd();
-
   // Bytes between p and q are pending to be hashed.
   char* p = data;
   char* q = data;
@@ -808,7 +787,6 @@ process_preprocessed_file(Context& ctx,
       if (q >= end) {
         cc_log("Failed to parse included file path");
         free(data);
-        free(cwd);
         return false;
       }
       // q points to the beginning of an include file path
@@ -837,7 +815,8 @@ process_preprocessed_file(Context& ctx,
 
       bool should_hash_inc_path = true;
       if (!ctx.config.hash_dir()) {
-        if (str_startswith(inc_path, cwd) && str_endswith(inc_path, "//")) {
+        if (Util::starts_with(inc_path, ctx.apparent_cwd)
+            && str_endswith(inc_path, "//")) {
           // When compiling with -g or similar, GCC adds the absolute path to
           // CWD like this:
           //
@@ -886,7 +865,6 @@ process_preprocessed_file(Context& ctx,
 
   hash_string_buffer(hash, p, (end - p));
   free(data);
-  free(cwd);
 
   // Explicitly check the .gch/.pch/.pth file as Clang does not include any
   // mention of it in the preprocessed output.
@@ -1643,30 +1621,27 @@ hash_common_info(const Context& ctx,
 
   // Possibly hash the current working directory.
   if (args_info.generating_debuginfo && ctx.config.hash_dir()) {
-    char* cwd = get_cwd();
+    std::string dir_to_hash = ctx.apparent_cwd;
     for (size_t i = 0; i < args_info.debug_prefix_maps_len; i++) {
       char* map = args_info.debug_prefix_maps[i];
       char* sep = strchr(map, '=');
       if (sep) {
         char* old_path = x_strndup(map, sep - map);
         char* new_path = static_cast<char*>(x_strdup(sep + 1));
-        cc_log(
-          "Relocating debuginfo CWD %s from %s to %s", cwd, old_path, new_path);
-        if (str_startswith(cwd, old_path)) {
-          char* dir = format("%s%s", new_path, cwd + strlen(old_path));
-          free(cwd);
-          cwd = dir;
+        cc_log("Relocating debuginfo from %s to %s (CWD: %s)",
+               old_path,
+               new_path,
+               ctx.apparent_cwd.c_str());
+        if (Util::starts_with(ctx.apparent_cwd, old_path)) {
+          dir_to_hash = new_path + ctx.apparent_cwd.substr(strlen(old_path));
         }
         free(old_path);
         free(new_path);
       }
     }
-    if (cwd) {
-      cc_log("Hashing CWD %s", cwd);
-      hash_delimiter(hash, "cwd");
-      hash_string(hash, cwd);
-      free(cwd);
-    }
+    cc_log("Hashing CWD %s", dir_to_hash.c_str());
+    hash_delimiter(hash, "cwd");
+    hash_string(hash, dir_to_hash);
   }
 
   if (ctx.args_info.generating_dependencies || ctx.args_info.seen_split_dwarf) {
@@ -1929,7 +1904,7 @@ calculate_result_name(Context& ctx,
   // -fprofile-generate=, -fprofile-use= or -fprofile-dir=.
   if (ctx.args_info.profile_generate) {
     if (ctx.args_info.profile_dir.empty()) {
-      ctx.args_info.profile_dir = from_cstr(get_cwd());
+      ctx.args_info.profile_dir = ctx.apparent_cwd;
     }
     cc_log("Adding profile directory %s to our hash",
            ctx.args_info.profile_dir.c_str());
@@ -1940,7 +1915,7 @@ calculate_result_name(Context& ctx,
   if (ctx.args_info.profile_use) {
     // Calculate gcda name.
     if (ctx.args_info.profile_dir.empty()) {
-      ctx.args_info.profile_dir = from_cstr(get_cwd());
+      ctx.args_info.profile_dir = ctx.apparent_cwd;
     }
     string_view base_name = Util::remove_extension(ctx.args_info.output_obj);
     std::string gcda_name =
@@ -3551,7 +3526,6 @@ free_and_nullify(T*& ptr)
 void
 cc_reset()
 {
-  free_and_nullify(current_working_dir);
   free_and_nullify(included_pch_file);
   free_and_nullify(cached_result_name);
   free_and_nullify(cached_result_path);
@@ -3653,6 +3627,12 @@ cache_compilation(int argc, char* argv[])
 static void
 do_cache_compilation(Context& ctx, char* argv[])
 {
+  if (ctx.actual_cwd.empty()) {
+    cc_log("Unable to determine current working directory: %s",
+           strerror(errno));
+    throw Failure(STATS_ERROR);
+  }
+
   MTR_BEGIN("main", "clean_up_internal_tempdir");
   if (ctx.config.temporary_dir().empty()) {
     clean_up_internal_tempdir(ctx);
@@ -3675,7 +3655,10 @@ do_cache_compilation(Context& ctx, char* argv[])
 
   cc_log_argv("Command line: ", argv);
   cc_log("Hostname: %s", get_hostname());
-  cc_log("Working directory: %s", get_current_working_dir());
+  cc_log("Working directory: %s", ctx.actual_cwd.c_str());
+  if (ctx.apparent_cwd != ctx.actual_cwd) {
+    cc_log("Apparent working directory: %s", ctx.apparent_cwd.c_str());
+  }
 
   ctx.config.set_limit_multiple(
     std::min(std::max(ctx.config.limit_multiple(), 0.0), 1.0));
index d174b98246371b99adcf7dd88228534f59aa23f1..d856dbd97d88200f592b92f50175b407f23ba4fd 100644 (file)
@@ -18,9 +18,6 @@
 
 #include "legacy_globals.hpp"
 
-// Current working directory taken from $PWD, or getcwd() if $PWD is bad.
-char* current_working_dir = nullptr;
-
 // The original argument list.
 extern struct args* orig_args;
 struct args* orig_args = nullptr;
index 6144171949ad2141f59aa6397e7d9a3bf44b7695..d3fa54452346bd53f2ebd8bd5f8ef74356fdb25c 100644 (file)
@@ -28,8 +28,6 @@
 
 // variable descriptions are in the .cpp file
 
-extern char* current_working_dir;
-
 extern unsigned lock_staleness_limit;
 
 #define MAX_ARCH_ARGS 10
index 48840a0734640770a18239a9677db2133c0d98c2..40ada66bc0f5abc227645fb1e8de06e74a43ed4b 100644 (file)
@@ -582,26 +582,6 @@ parse_size_with_suffix(const char* str, uint64_t* size)
   return true;
 }
 
-// A getcwd that will returns an allocated buffer.
-char*
-gnu_getcwd(void)
-{
-  unsigned size = 128;
-
-  while (true) {
-    char* buffer = (char*)x_malloc(size);
-    if (getcwd(buffer, size) == buffer) {
-      return buffer;
-    }
-    free(buffer);
-    if (errno != ERANGE) {
-      cc_log("getcwd error: %d (%s)", errno, strerror(errno));
-      return NULL;
-    }
-    size *= 2;
-  }
-}
-
 #if !defined(_WIN32) && !defined(HAVE_LOCALTIME_R)
 // localtime_r replacement. (Mingw-w64 has an inline localtime_r which is not
 // detected by AC_CHECK_FUNCS.)
@@ -711,34 +691,6 @@ get_home_directory(void)
   return NULL;
 }
 
-// Get the current directory by reading $PWD. If $PWD isn't sane, gnu_getcwd()
-// is used. Caller frees.
-char*
-get_cwd(void)
-{
-  char* cwd = gnu_getcwd();
-  if (!cwd) {
-    return NULL;
-  }
-
-  char* pwd = getenv("PWD");
-  if (!pwd) {
-    return cwd;
-  }
-
-  auto st_pwd = Stat::stat(pwd);
-  auto st_cwd = Stat::stat(cwd);
-  if (!st_pwd || !st_cwd) {
-    return cwd;
-  }
-  if (st_pwd.device() == st_cwd.device() && st_pwd.inode() == st_cwd.inode()) {
-    free(cwd);
-    return x_strdup(pwd);
-  } else {
-    return cwd;
-  }
-}
-
 // Check whether s1 and s2 have the same executable name.
 bool
 same_executable_name(const char* s1, const char* s2)
index 7a995390169959f9f2f5ca58d88a38ffbebf624a..4c549a277f75bfdef6e2ad3bd7548b295ae02850 100644 (file)
@@ -44,7 +44,6 @@ const char* get_extension(const char* path);
 char* format_human_readable_size(uint64_t size);
 char* format_parsable_size_with_suffix(uint64_t size);
 bool parse_size_with_suffix(const char* str, uint64_t* size);
-char* gnu_getcwd();
 #ifndef HAVE_LOCALTIME_R
 struct tm* localtime_r(const time_t* timep, struct tm* result);
 #endif
@@ -54,7 +53,6 @@ char* strtok_r(char* str, const char* delim, char** saveptr);
 int create_tmp_fd(char** fname);
 FILE* create_tmp_file(char** fname, const char* mode);
 const char* get_home_directory();
-char* get_cwd();
 bool same_executable_name(const char* s1, const char* s2);
 size_t common_dir_prefix_length(const char* s1, const char* s2);
 char* get_relative_path(const char* from, const char* to);
index 298ec3cc239fdc641f2727276998e65485f8ed63..32bb3a05f2cd502f63acaee1bed54783419a4817 100644 (file)
@@ -37,8 +37,8 @@ static unsigned total_suites;
 static unsigned failed_tests;
 static const char* current_suite;
 static const char* current_test;
-static char* dir_before_suite;
-static char* dir_before_test;
+static std::string dir_before_suite;
+static std::string dir_before_test;
 static int verbose;
 
 static const char COLOR_END[] = "\x1b[m";
@@ -112,7 +112,7 @@ cct_suite_begin(const char* name)
   if (verbose) {
     printf("=== SUITE: %s ===\n", name);
   }
-  dir_before_suite = gnu_getcwd();
+  dir_before_suite = Util::get_actual_cwd();
   Util::create_dir(name);
   cct_chdir(name);
   current_suite = name;
@@ -121,9 +121,8 @@ cct_suite_begin(const char* name)
 void
 cct_suite_end()
 {
-  cct_chdir(dir_before_suite);
-  free(dir_before_suite);
-  dir_before_suite = NULL;
+  cct_chdir(dir_before_suite.c_str());
+  dir_before_suite.clear();
 }
 
 void
@@ -133,7 +132,7 @@ cct_test_begin(const char* name)
   if (verbose) {
     printf("--- TEST: %s ---\n", name);
   }
-  dir_before_test = gnu_getcwd();
+  dir_before_test = Util::get_actual_cwd();
   Util::create_dir(name);
   cct_chdir(name);
   current_test = name;
@@ -144,10 +143,9 @@ cct_test_begin(const char* name)
 void
 cct_test_end()
 {
-  if (dir_before_test) {
-    cct_chdir(dir_before_test);
-    free(dir_before_test);
-    dir_before_test = NULL;
+  if (!dir_before_test.empty()) {
+    cct_chdir(dir_before_test.c_str());
+    dir_before_test.clear();
   }
 }
 
index 567b25f42e0fad52d2d3625f24c99ee5edc2f9ce..3d07a6fb55d1bb1b9cbf1a793ab69adc7488bf50 100644 (file)
@@ -16,6 +16,7 @@
 // this program; if not, write to the Free Software Foundation, Inc., 51
 // Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
+#include "../src/Util.hpp"
 #include "../src/legacy_util.hpp"
 #include "catch2_tests.hpp"
 #include "framework.hpp"
@@ -51,9 +52,9 @@ main(int argc, char** argv)
   x_setenv("CCACHE_DETECT_SHEBANG", "1");
 #endif
 
+  std::string dir_before = Util::get_actual_cwd();
   char* testdir = format("testdir.%d", (int)getpid());
   cct_create_fresh_dir(testdir);
-  char* dir_before = gnu_getcwd();
   cct_chdir(testdir);
 
   // Run Catch2 tests.
@@ -66,10 +67,9 @@ main(int argc, char** argv)
   }
 
   if (result == 0) {
-    cct_chdir(dir_before);
+    cct_chdir(dir_before.c_str());
     cct_wipe(testdir);
   }
   free(testdir);
-  free(dir_before);
   return result;
 }
index a98da1ace0956a7477b4d83e7ceb33f59a89b67f..caadfb069d17b988a6c91a2ce0bfb9c49056dc86 100644 (file)
@@ -35,13 +35,13 @@ get_root()
   return "/";
 #else
   char volume[4]; // "C:\"
-  GetVolumePathName(get_cwd(), volume, sizeof(volume));
+  GetVolumePathName(Util::get_actual_cwd().c_str(), volume, sizeof(volume));
   return volume;
 #endif
 }
 
 static char*
-get_posix_path(char* path)
+get_posix_path(const char* path)
 {
 #ifndef _WIN32
   return x_strdup(path);
@@ -353,7 +353,6 @@ TEST(sysroot_should_be_rewritten_if_basedir_is_used)
 {
   Context ctx;
 
-  extern char* current_working_dir;
   char* arg_string;
   struct args* orig;
   struct args* act_cpp = NULL;
@@ -362,8 +361,8 @@ TEST(sysroot_should_be_rewritten_if_basedir_is_used)
 
   create_file("foo.c", "");
   ctx.config.set_base_dir(get_root());
-  current_working_dir = get_cwd();
-  arg_string = format("cc --sysroot=%s/foo/bar -c foo.c", current_working_dir);
+  arg_string =
+    format("cc --sysroot=%s/foo/bar -c foo.c", ctx.actual_cwd.c_str());
   orig = args_init_from_string(arg_string);
   free(arg_string);
 
@@ -379,7 +378,6 @@ TEST(sysroot_with_separate_argument_should_be_rewritten_if_basedir_is_used)
 {
   Context ctx;
 
-  extern char* current_working_dir;
   char* arg_string;
   struct args* orig;
   struct args* act_cpp = NULL;
@@ -388,8 +386,7 @@ TEST(sysroot_with_separate_argument_should_be_rewritten_if_basedir_is_used)
 
   create_file("foo.c", "");
   ctx.config.set_base_dir(get_root());
-  current_working_dir = get_cwd();
-  arg_string = format("cc --sysroot %s/foo -c foo.c", current_working_dir);
+  arg_string = format("cc --sysroot %s/foo -c foo.c", ctx.actual_cwd.c_str());
   orig = args_init_from_string(arg_string);
   free(arg_string);
 
@@ -633,7 +630,6 @@ TEST(isystem_flag_with_separate_arg_should_be_rewritten_if_basedir_is_used)
 {
   Context ctx;
 
-  extern char* current_working_dir;
   char* arg_string;
   struct args* orig;
   struct args* act_cpp = NULL;
@@ -642,8 +638,7 @@ TEST(isystem_flag_with_separate_arg_should_be_rewritten_if_basedir_is_used)
 
   create_file("foo.c", "");
   ctx.config.set_base_dir(get_root());
-  current_working_dir = get_cwd();
-  arg_string = format("cc -isystem %s/foo -c foo.c", current_working_dir);
+  arg_string = format("cc -isystem %s/foo -c foo.c", ctx.actual_cwd.c_str());
   orig = args_init_from_string(arg_string);
   free(arg_string);
 
@@ -659,7 +654,6 @@ TEST(isystem_flag_with_concat_arg_should_be_rewritten_if_basedir_is_used)
 {
   Context ctx;
 
-  extern char* current_working_dir;
   char* cwd;
   char* arg_string;
   struct args* orig;
@@ -669,9 +663,8 @@ TEST(isystem_flag_with_concat_arg_should_be_rewritten_if_basedir_is_used)
 
   create_file("foo.c", "");
   ctx.config.set_base_dir("/"); // posix
-  current_working_dir = get_cwd();
   // Windows path doesn't work concatenated.
-  cwd = get_posix_path(current_working_dir);
+  cwd = get_posix_path(ctx.actual_cwd.c_str());
   arg_string = format("cc -isystem%s/foo -c foo.c", cwd);
   orig = args_init_from_string(arg_string);
   free(arg_string);
@@ -689,7 +682,6 @@ TEST(I_flag_with_concat_arg_should_be_rewritten_if_basedir_is_used)
 {
   Context ctx;
 
-  extern char* current_working_dir;
   char* cwd;
   char* arg_string;
   struct args* orig;
@@ -699,9 +691,8 @@ TEST(I_flag_with_concat_arg_should_be_rewritten_if_basedir_is_used)
 
   create_file("foo.c", "");
   ctx.config.set_base_dir(x_strdup("/")); // posix
-  current_working_dir = get_cwd();
   // Windows path doesn't work concatenated.
-  cwd = get_posix_path(current_working_dir);
+  cwd = get_posix_path(ctx.actual_cwd.c_str());
   arg_string = format("cc -I%s/foo -c foo.c", cwd);
   orig = args_init_from_string(arg_string);
   free(arg_string);