From: Joel Rosdahl <joel@rosdahl.net>
Date: Fri, 7 Jul 2023 14:42:23 +0000 (+0200)
Subject: refactor: Move environment utility functions to util
X-Git-Tag: v4.9~142
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=09c32fb9ed52cbf2a49c116692ecd59d88127d9e;p=thirdparty%2Fccache.git

refactor: Move environment utility functions to util
---

diff --git a/src/Config.cpp b/src/Config.cpp
index b312196f1..971faac8e 100644
--- a/src/Config.cpp
+++ b/src/Config.cpp
@@ -29,6 +29,7 @@
 #include <core/wincompat.hpp>
 #include <fmtmacros.hpp>
 #include <util/Tokenizer.hpp>
+#include <util/environment.hpp>
 #include <util/expected.hpp>
 #include <util/file.hpp>
 #include <util/path.hpp>
@@ -934,7 +935,7 @@ Config::set_item(const std::string& key,
     return;
   }
 
-  std::string value = Util::expand_environment_variables(unexpanded_value);
+  std::string value = util::expand_environment_variables(unexpanded_value);
 
   switch (it->second.item) {
   case ConfigItem::absolute_paths_in_stderr:
diff --git a/src/Util.cpp b/src/Util.cpp
index ec01a5d7a..8eebcc0ca 100644
--- a/src/Util.cpp
+++ b/src/Util.cpp
@@ -429,60 +429,6 @@ dir_name(std::string_view path)
   }
 }
 
-std::string
-expand_environment_variables(const std::string& str)
-{
-  std::string result;
-  const char* left = str.c_str();
-  const char* right = left;
-
-  while (*right) {
-    if (*right == '$') {
-      result.append(left, right - left);
-
-      if (*(right + 1) == '$') {
-        result += '$';
-        right += 2;
-        left = right;
-        continue;
-      }
-
-      left = right + 1;
-      bool curly = *left == '{';
-      if (curly) {
-        ++left;
-      }
-      right = left;
-      while (isalnum(*right) || *right == '_') {
-        ++right;
-      }
-      if (curly && *right != '}') {
-        throw core::Error(FMT("syntax error: missing '}}' after \"{}\"", left));
-      }
-      if (right == left) {
-        // Special case: don't consider a single $ the left of a variable.
-        result += '$';
-        --right;
-      } else {
-        std::string name(left, right - left);
-        const char* value = getenv(name.c_str());
-        if (!value) {
-          throw core::Error(FMT("environment variable \"{}\" not set", name));
-        }
-        result += value;
-        if (!curly) {
-          --right;
-        }
-        left = right + 1;
-      }
-    }
-    ++right;
-  }
-
-  result += left;
-  return result;
-}
-
 int
 fallocate(int fd, long new_size)
 {
@@ -1101,18 +1047,6 @@ set_umask(mode_t mask)
   return umask(mask);
 }
 
-void
-setenv(const std::string& name, const std::string& value)
-{
-#ifdef HAVE_SETENV
-  ::setenv(name.c_str(), value.c_str(), true);
-#else
-  char* string;
-  asprintf(&string, "%s=%s", name.c_str(), value.c_str());
-  putenv(string); // Leak to environment.
-#endif
-}
-
 std::vector<std::string_view>
 split_into_views(std::string_view string,
                  const char* separators,
@@ -1293,18 +1227,6 @@ unlink_tmp(const std::string& path, UnlinkLog unlink_log)
   return success;
 }
 
-void
-unsetenv(const std::string& name)
-{
-#ifdef HAVE_UNSETENV
-  ::unsetenv(name.c_str());
-#elif defined(_WIN32)
-  SetEnvironmentVariable(name.c_str(), NULL);
-#else
-  putenv(strdup(name.c_str())); // Leak to environment.
-#endif
-}
-
 void
 wipe_path(const std::string& path)
 {
diff --git a/src/Util.hpp b/src/Util.hpp
index 2db92fdff..0be214c82 100644
--- a/src/Util.hpp
+++ b/src/Util.hpp
@@ -88,10 +88,6 @@ std::string_view dir_name(std::string_view path);
 // Like create_dir but throws Fatal on error.
 void ensure_dir_exists(std::string_view dir);
 
-// Expand all instances of $VAR or ${VAR}, where VAR is an environment variable,
-// in `str`. Throws `core::Error` if one of the environment variables.
-[[nodiscard]] std::string expand_environment_variables(const std::string& str);
-
 // Extends file size to at least new_size by calling posix_fallocate() if
 // supported, otherwise by writing zeros last to the file.
 //
@@ -241,9 +237,6 @@ void set_cloexec_flag(int fd);
 // Set process umask. Returns the previous mask.
 mode_t set_umask(mode_t mask);
 
-// Set environment variable `name` to `value`.
-void setenv(const std::string& name, const std::string& value);
-
 // Return size change in KiB between `old_stat`  and `new_stat`.
 inline int64_t
 size_change_kibibyte(const Stat& old_stat, const Stat& new_stat)
@@ -298,9 +291,6 @@ bool unlink_safe(const std::string& path,
 bool unlink_tmp(const std::string& path,
                 UnlinkLog unlink_log = UnlinkLog::log_failure);
 
-// Unset environment variable `name`.
-void unsetenv(const std::string& name);
-
 // Remove `path` (and its contents if it's a directory). A nonexistent path is
 // not considered an error.
 //
diff --git a/src/ccache.cpp b/src/ccache.cpp
index be1984748..18338d050 100644
--- a/src/ccache.cpp
+++ b/src/ccache.cpp
@@ -54,6 +54,7 @@
 #include <core/types.hpp>
 #include <core/wincompat.hpp>
 #include <storage/Storage.hpp>
+#include <util/environment.hpp>
 #include <util/expected.hpp>
 #include <util/file.hpp>
 #include <util/path.hpp>
@@ -2215,7 +2216,7 @@ set_up_uncached_err()
     return nonstd::make_unexpected(Statistic::internal_error);
   }
 
-  Util::setenv("UNCACHED_ERR_FD", FMT("{}", uncached_fd));
+  util::setenv("UNCACHED_ERR_FD", FMT("{}", uncached_fd));
   return {};
 }
 
@@ -2398,7 +2399,7 @@ do_cache_compilation(Context& ctx)
   // calling ccache second time. For instance, if the real compiler is a wrapper
   // script that calls "ccache $compiler ..." we want that inner ccache call to
   // be disabled.
-  Util::setenv("CCACHE_DISABLE", "1");
+  util::setenv("CCACHE_DISABLE", "1");
 
   MTR_BEGIN("main", "process_args");
   ProcessArgsResult processed = process_args(ctx);
@@ -2413,7 +2414,7 @@ do_cache_compilation(Context& ctx)
   // VS_UNICODE_OUTPUT prevents capturing stdout/stderr, as the output is sent
   // directly to Visual Studio.
   if (ctx.config.compiler_type() == CompilerType::msvc) {
-    Util::unsetenv("VS_UNICODE_OUTPUT");
+    util::unsetenv("VS_UNICODE_OUTPUT");
   }
 
   for (const auto& name : {"DEPENDENCIES_OUTPUT", "SUNPRO_DEPENDENCIES"}) {
diff --git a/src/core/mainoptions.cpp b/src/core/mainoptions.cpp
index b32afc5bb..685b34f47 100644
--- a/src/core/mainoptions.cpp
+++ b/src/core/mainoptions.cpp
@@ -44,6 +44,7 @@
 #include <storage/local/LocalStorage.hpp>
 #include <util/TextTable.hpp>
 #include <util/XXH3_128.hpp>
+#include <util/environment.hpp>
 #include <util/expected.hpp>
 #include <util/file.hpp>
 #include <util/string.hpp>
@@ -504,11 +505,11 @@ process_main_options(int argc, const char* const* argv)
 
     switch (c) {
     case 'd': // --dir
-      Util::setenv("CCACHE_DIR", arg);
+      util::setenv("CCACHE_DIR", arg);
       break;
 
     case CONFIG_PATH:
-      Util::setenv("CCACHE_CONFIGPATH", arg);
+      util::setenv("CCACHE_CONFIGPATH", arg);
       break;
 
     case RECOMPRESS_THREADS:
diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt
index d08916bbc..234318bea 100644
--- a/src/util/CMakeLists.txt
+++ b/src/util/CMakeLists.txt
@@ -6,6 +6,7 @@ set(
   TextTable.cpp
   TimePoint.cpp
   Tokenizer.cpp
+  environment.cpp
   file.cpp
   path.cpp
   string.cpp
diff --git a/src/util/environment.cpp b/src/util/environment.cpp
new file mode 100644
index 000000000..1e4e70f86
--- /dev/null
+++ b/src/util/environment.cpp
@@ -0,0 +1,106 @@
+// Copyright (C) 2023 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
+
+#include "environment.hpp"
+
+#include <Win32Util.hpp> // for asprintf
+#include <core/exceptions.hpp>
+#include <core/wincompat.hpp>
+#include <fmtmacros.hpp>
+
+namespace util {
+
+std::string
+expand_environment_variables(const std::string& str)
+{
+  std::string result;
+  const char* left = str.c_str();
+  const char* right = left;
+
+  while (*right) {
+    if (*right == '$') {
+      result.append(left, right - left);
+
+      if (*(right + 1) == '$') {
+        result += '$';
+        right += 2;
+        left = right;
+        continue;
+      }
+
+      left = right + 1;
+      bool curly = *left == '{';
+      if (curly) {
+        ++left;
+      }
+      right = left;
+      while (isalnum(*right) || *right == '_') {
+        ++right;
+      }
+      if (curly && *right != '}') {
+        throw core::Error(FMT("syntax error: missing '}}' after \"{}\"", left));
+      }
+      if (right == left) {
+        // Special case: don't consider a single $ the left of a variable.
+        result += '$';
+        --right;
+      } else {
+        std::string name(left, right - left);
+        const char* value = getenv(name.c_str());
+        if (!value) {
+          throw core::Error(FMT("environment variable \"{}\" not set", name));
+        }
+        result += value;
+        if (!curly) {
+          --right;
+        }
+        left = right + 1;
+      }
+    }
+    ++right;
+  }
+
+  result += left;
+  return result;
+}
+
+void
+setenv(const std::string& name, const std::string& value)
+{
+#ifdef HAVE_SETENV
+  ::setenv(name.c_str(), value.c_str(), true);
+#else
+  char* string;
+  asprintf(&string, "%s=%s", name.c_str(), value.c_str());
+  putenv(string); // Leak to environment.
+#endif
+}
+
+void
+unsetenv(const std::string& name)
+{
+#ifdef HAVE_UNSETENV
+  ::unsetenv(name.c_str());
+#elif defined(_WIN32)
+  SetEnvironmentVariable(name.c_str(), NULL);
+#else
+  putenv(strdup(name.c_str())); // Leak to environment.
+#endif
+}
+
+} // namespace util
diff --git a/src/util/environment.hpp b/src/util/environment.hpp
new file mode 100644
index 000000000..29a99de59
--- /dev/null
+++ b/src/util/environment.hpp
@@ -0,0 +1,35 @@
+// Copyright (C) 2023 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 <string>
+
+namespace util {
+
+// Expand all instances of $VAR or ${VAR}, where VAR is an environment variable,
+// in `str`. Throws `core::Error` if one of the environment variables.
+[[nodiscard]] std::string expand_environment_variables(const std::string& str);
+
+// Set environment variable `name` to `value`.
+void setenv(const std::string& name, const std::string& value);
+
+// Unset environment variable `name`.
+void unsetenv(const std::string& name);
+
+} // namespace util
diff --git a/unittest/CMakeLists.txt b/unittest/CMakeLists.txt
index 208dd8bbb..ec743312f 100644
--- a/unittest/CMakeLists.txt
+++ b/unittest/CMakeLists.txt
@@ -30,6 +30,7 @@ set(
   test_util_XXH3_128.cpp
   test_util_XXH3_64.cpp
   test_util_conversion.cpp
+  test_util_environment.cpp
   test_util_expected.cpp
   test_util_file.cpp
   test_util_path.cpp
diff --git a/unittest/main.cpp b/unittest/main.cpp
index df9bf599b..d67126f71 100644
--- a/unittest/main.cpp
+++ b/unittest/main.cpp
@@ -1,4 +1,4 @@
-// Copyright (C) 2010-2020 Joel Rosdahl and other contributors
+// Copyright (C) 2010-2023 Joel Rosdahl and other contributors
 //
 // See doc/AUTHORS.adoc for a complete list of contributors.
 //
@@ -20,6 +20,8 @@
 #include "../src/fmtmacros.hpp"
 #include "TestUtil.hpp"
 
+#include <util/environment.hpp>
+
 #include "third_party/fmt/core.h"
 
 #define DOCTEST_THREAD_LOCAL // Avoid MinGW thread_local bug
@@ -30,9 +32,9 @@ int
 main(int argc, char** argv)
 {
 #ifdef _WIN32
-  Util::setenv("CCACHE_DETECT_SHEBANG", "1");
+  util::setenv("CCACHE_DETECT_SHEBANG", "1");
 #endif
-  Util::unsetenv("GCC_COLORS"); // Don't confuse argument processing tests.
+  util::unsetenv("GCC_COLORS"); // Don't confuse argument processing tests.
 
   std::string dir_before = Util::get_actual_cwd();
   std::string testdir = FMT("testdir/{}", getpid());
diff --git a/unittest/test_Config.cpp b/unittest/test_Config.cpp
index 6b97e4e7e..c2f46e9d9 100644
--- a/unittest/test_Config.cpp
+++ b/unittest/test_Config.cpp
@@ -22,6 +22,7 @@
 #include "TestUtil.hpp"
 
 #include <core/exceptions.hpp>
+#include <util/environment.hpp>
 #include <util/file.hpp>
 
 #include "third_party/doctest.h"
@@ -87,7 +88,7 @@ TEST_CASE("Config::update_from_file")
   TestContext test_context;
 
   const char user[] = "rabbit";
-  Util::setenv("USER", user);
+  util::setenv("USER", user);
 
 #ifndef _WIN32
   std::string base_dir = FMT("/{0}/foo/{0}", user);
@@ -289,13 +290,13 @@ TEST_CASE("Config::update_from_environment")
 {
   Config config;
 
-  Util::setenv("CCACHE_COMPRESS", "1");
+  util::setenv("CCACHE_COMPRESS", "1");
   config.update_from_environment();
   CHECK(config.compression());
 
-  Util::unsetenv("CCACHE_COMPRESS");
+  util::unsetenv("CCACHE_COMPRESS");
 
-  Util::setenv("CCACHE_NOCOMPRESS", "1");
+  util::setenv("CCACHE_NOCOMPRESS", "1");
   config.update_from_environment();
   CHECK(!config.compression());
 }
diff --git a/unittest/test_Stat.cpp b/unittest/test_Stat.cpp
index ffdb68095..fdba8c2ba 100644
--- a/unittest/test_Stat.cpp
+++ b/unittest/test_Stat.cpp
@@ -23,6 +23,7 @@
 
 #include <core/exceptions.hpp>
 #include <core/wincompat.hpp>
+#include <util/environment.hpp>
 #include <util/file.hpp>
 
 #include "third_party/doctest.h"
@@ -663,14 +664,14 @@ TEST_CASE("Win32 No Sharing")
 // Instead, test a well-known junction that has existed in all Windows versions
 // since Vista. (Not present on Wine.)
 TEST_CASE("Win32 Directory Junction"
-          * doctest::skip(!win32_is_junction(Util::expand_environment_variables(
+          * doctest::skip(!win32_is_junction(util::expand_environment_variables(
             "${ALLUSERSPROFILE}\\Application Data"))))
 {
   TestContext test_context;
 
   SUBCASE("junction stat")
   {
-    auto stat = Stat::stat(Util::expand_environment_variables(
+    auto stat = Stat::stat(util::expand_environment_variables(
       "${ALLUSERSPROFILE}\\Application Data"));
     CHECK(stat);
     CHECK(stat.error_number() == 0);
@@ -686,7 +687,7 @@ TEST_CASE("Win32 Directory Junction"
 
   SUBCASE("junction lstat")
   {
-    auto stat = Stat::lstat(Util::expand_environment_variables(
+    auto stat = Stat::lstat(util::expand_environment_variables(
       "${ALLUSERSPROFILE}\\Application Data"));
     CHECK(stat);
     CHECK(stat.error_number() == 0);
diff --git a/unittest/test_Util.cpp b/unittest/test_Util.cpp
index 062bad5da..5b5832541 100644
--- a/unittest/test_Util.cpp
+++ b/unittest/test_Util.cpp
@@ -24,6 +24,7 @@
 
 #include <core/exceptions.hpp>
 #include <core/wincompat.hpp>
+#include <util/environment.hpp>
 #include <util/file.hpp>
 
 #include "third_party/doctest.h"
@@ -146,29 +147,6 @@ TEST_CASE("Util::ensure_dir_exists")
     "Failed to create directory create/dir/file: Not a directory");
 }
 
-TEST_CASE("Util::expand_environment_variables")
-{
-  Util::setenv("FOO", "bar");
-
-  CHECK(Util::expand_environment_variables("") == "");
-  CHECK(Util::expand_environment_variables("$FOO") == "bar");
-  CHECK(Util::expand_environment_variables("$$FOO") == "$FOO");
-  CHECK(Util::expand_environment_variables("$$$FOO") == "$bar");
-  CHECK(Util::expand_environment_variables("$ $$ $") == "$ $ $");
-  CHECK(Util::expand_environment_variables("$FOO $FOO:$FOO") == "bar bar:bar");
-  CHECK(Util::expand_environment_variables("x$FOO") == "xbar");
-  CHECK(Util::expand_environment_variables("${FOO}x") == "barx");
-
-  DOCTEST_GCC_SUPPRESS_WARNING_PUSH
-  DOCTEST_GCC_SUPPRESS_WARNING("-Wunused-result")
-  CHECK_THROWS_WITH(
-    (void)Util::expand_environment_variables("$surelydoesntexist"),
-    "environment variable \"surelydoesntexist\" not set");
-  CHECK_THROWS_WITH((void)Util::expand_environment_variables("${FOO"),
-                    "syntax error: missing '}' after \"FOO\"");
-  DOCTEST_GCC_SUPPRESS_WARNING_POP
-}
-
 TEST_CASE("Util::fallocate")
 {
   TestContext test_context;
@@ -337,7 +315,7 @@ TEST_CASE("Util::make_relative_path")
   REQUIRE(symlink("d", "s") == 0);
 #endif
   REQUIRE(chdir("d") == 0);
-  Util::setenv("PWD", apparent_cwd);
+  util::setenv("PWD", apparent_cwd);
 
   SUBCASE("No base directory")
   {
diff --git a/unittest/test_util_environment.cpp b/unittest/test_util_environment.cpp
new file mode 100644
index 000000000..06654672a
--- /dev/null
+++ b/unittest/test_util_environment.cpp
@@ -0,0 +1,48 @@
+// Copyright (C) 2023 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
+
+#include <util/environment.hpp>
+
+#include <third_party/doctest.h>
+
+TEST_SUITE_BEGIN("util");
+
+TEST_CASE("util::expand_environment_variables")
+{
+  util::setenv("FOO", "bar");
+
+  CHECK(util::expand_environment_variables("") == "");
+  CHECK(util::expand_environment_variables("$FOO") == "bar");
+  CHECK(util::expand_environment_variables("$$FOO") == "$FOO");
+  CHECK(util::expand_environment_variables("$$$FOO") == "$bar");
+  CHECK(util::expand_environment_variables("$ $$ $") == "$ $ $");
+  CHECK(util::expand_environment_variables("$FOO $FOO:$FOO") == "bar bar:bar");
+  CHECK(util::expand_environment_variables("x$FOO") == "xbar");
+  CHECK(util::expand_environment_variables("${FOO}x") == "barx");
+
+  DOCTEST_GCC_SUPPRESS_WARNING_PUSH
+  DOCTEST_GCC_SUPPRESS_WARNING("-Wunused-result")
+  CHECK_THROWS_WITH(
+    (void)util::expand_environment_variables("$surelydoesntexist"),
+    "environment variable \"surelydoesntexist\" not set");
+  CHECK_THROWS_WITH((void)util::expand_environment_variables("${FOO"),
+                    "syntax error: missing '}' after \"FOO\"");
+  DOCTEST_GCC_SUPPRESS_WARNING_POP
+}
+
+TEST_SUITE_END();