[clang] 4d5c448 - Revert "[clang driver] Move default module cache from system temporary directory"

Nico Weber via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 26 10:26:27 PDT 2020


Author: Nico Weber
Date: 2020-06-26T13:25:45-04:00
New Revision: 4d5c4489435dc1cb3d4989614e96b157c74afdea

URL: https://github.com/llvm/llvm-project/commit/4d5c4489435dc1cb3d4989614e96b157c74afdea
DIFF: https://github.com/llvm/llvm-project/commit/4d5c4489435dc1cb3d4989614e96b157c74afdea.diff

LOG: Revert "[clang driver] Move default module cache from system temporary directory"

This reverts commit bb26838ceffb5feaa18186f55f7525a08084899e.
Breaks Support.CacheDirectoryNoEnv, Support.CacheDirectoryWithEnv
in SupportTests (part of check-llvm) on macOS.

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Driver/Driver.h
    clang/lib/Driver/ToolChains/Clang.cpp
    clang/test/Driver/modules-cache-path.m
    clang/unittests/Driver/ModuleCacheTest.cpp
    llvm/lib/Support/Unix/Path.inc

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c24c289f94b4..15e6d35117b4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -104,10 +104,6 @@ New Compiler Flags
   simplify access to the many single purpose floating point options. The default
   setting is ``precise``.
 
-- The default module cache has moved from /tmp to a per-user cache directory.
-  By default, this is ~/.cache but on some platforms or installations, this
-  might be elsewhere. The -fmodules-cache-path=... flag continues to work.
-
 Deprecated Compiler Flags
 -------------------------
 

diff  --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h
index dc18f1314f81..b024d6a0d3a3 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -621,8 +621,7 @@ class Driver {
   static bool GetReleaseVersion(StringRef Str,
                                 MutableArrayRef<unsigned> Digits);
   /// Compute the default -fmodule-cache-path.
-  /// \return True if the system provides a default cache directory.
-  static bool getDefaultModuleCachePath(SmallVectorImpl<char> &Result);
+  static void getDefaultModuleCachePath(SmallVectorImpl<char> &Result);
 };
 
 /// \return True if the last defined optimization level is -Ofast.

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 1fd638f435b9..8903641a26c6 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -721,6 +721,38 @@ static void addDashXForInput(const ArgList &Args, const InputInfo &Input,
   }
 }
 
+static void appendUserToPath(SmallVectorImpl<char> &Result) {
+#ifdef LLVM_ON_UNIX
+  const char *Username = getenv("LOGNAME");
+#else
+  const char *Username = getenv("USERNAME");
+#endif
+  if (Username) {
+    // Validate that LoginName can be used in a path, and get its length.
+    size_t Len = 0;
+    for (const char *P = Username; *P; ++P, ++Len) {
+      if (!clang::isAlphanumeric(*P) && *P != '_') {
+        Username = nullptr;
+        break;
+      }
+    }
+
+    if (Username && Len > 0) {
+      Result.append(Username, Username + Len);
+      return;
+    }
+  }
+
+// Fallback to user id.
+#ifdef LLVM_ON_UNIX
+  std::string UID = llvm::utostr(getuid());
+#else
+  // FIXME: Windows seems to have an 'SID' that might work.
+  std::string UID = "9999";
+#endif
+  Result.append(UID.begin(), UID.end());
+}
+
 static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
                                    const Driver &D, const InputInfo &Output,
                                    const ArgList &Args,
@@ -3169,13 +3201,11 @@ static void RenderBuiltinOptions(const ToolChain &TC, const llvm::Triple &T,
     CmdArgs.push_back("-fno-math-builtin");
 }
 
-bool Driver::getDefaultModuleCachePath(SmallVectorImpl<char> &Result) {
-  if (llvm::sys::path::cache_directory(Result)) {
-    llvm::sys::path::append(Result, "clang");
-    llvm::sys::path::append(Result, "ModuleCache");
-    return true;
-  }
-  return false;
+void Driver::getDefaultModuleCachePath(SmallVectorImpl<char> &Result) {
+  llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, Result);
+  llvm::sys::path::append(Result, "org.llvm.clang.");
+  appendUserToPath(Result);
+  llvm::sys::path::append(Result, "ModuleCache");
 }
 
 static void RenderModulesOptions(Compilation &C, const Driver &D,
@@ -3232,7 +3262,6 @@ static void RenderModulesOptions(Compilation &C, const Driver &D,
     if (Arg *A = Args.getLastArg(options::OPT_fmodules_cache_path))
       Path = A->getValue();
 
-    bool HasPath = true;
     if (C.isForDiagnostics()) {
       // When generating crash reports, we want to emit the modules along with
       // the reproduction sources, so we ignore any provided module path.
@@ -3241,16 +3270,12 @@ static void RenderModulesOptions(Compilation &C, const Driver &D,
       llvm::sys::path::append(Path, "modules");
     } else if (Path.empty()) {
       // No module path was provided: use the default.
-      HasPath = Driver::getDefaultModuleCachePath(Path);
+      Driver::getDefaultModuleCachePath(Path);
     }
 
-    // `HasPath` will only be false if getDefaultModuleCachePath() fails.
-    // That being said, that failure is unlikely and not caching is harmless.
-    if (HasPath) {
-      const char Arg[] = "-fmodules-cache-path=";
-      Path.insert(Path.begin(), Arg, Arg + strlen(Arg));
-      CmdArgs.push_back(Args.MakeArgString(Path));
-    }
+    const char Arg[] = "-fmodules-cache-path=";
+    Path.insert(Path.begin(), Arg, Arg + strlen(Arg));
+    CmdArgs.push_back(Args.MakeArgString(Path));
   }
 
   if (HaveModules) {

diff  --git a/clang/test/Driver/modules-cache-path.m b/clang/test/Driver/modules-cache-path.m
index 51df6739a505..419d6d479f6b 100644
--- a/clang/test/Driver/modules-cache-path.m
+++ b/clang/test/Driver/modules-cache-path.m
@@ -1,2 +1,5 @@
+// RUN: env USERNAME=asdf LOGNAME=asdf %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-SET
+// CHECK-SET: -fmodules-cache-path={{.*}}org.llvm.clang.asdf{{[/\\]+}}ModuleCache
+
 // RUN: %clang -fmodules -### %s 2>&1 | FileCheck %s -check-prefix=CHECK-DEFAULT
-// CHECK-DEFAULT: -fmodules-cache-path={{.*}}clang{{[/\\]+}}ModuleCache
+// CHECK-DEFAULT: -fmodules-cache-path={{.*}}org.llvm.clang.{{[A-Za-z0-9_]*[/\\]+}}ModuleCache

diff  --git a/clang/unittests/Driver/ModuleCacheTest.cpp b/clang/unittests/Driver/ModuleCacheTest.cpp
index 6a0f68f26a67..db3395f4abb2 100644
--- a/clang/unittests/Driver/ModuleCacheTest.cpp
+++ b/clang/unittests/Driver/ModuleCacheTest.cpp
@@ -21,7 +21,7 @@ TEST(ModuleCacheTest, GetTargetAndMode) {
   SmallString<128> Buf;
   Driver::getDefaultModuleCachePath(Buf);
   StringRef Path = Buf;
-  EXPECT_TRUE(Path.find("clang") != Path.npos);
+  EXPECT_TRUE(Path.find("org.llvm.clang") != Path.npos);
   EXPECT_TRUE(Path.endswith("ModuleCache"));  
 }
 } // end anonymous namespace.

diff  --git a/llvm/lib/Support/Unix/Path.inc b/llvm/lib/Support/Unix/Path.inc
index 2576da4506a0..bf720b318ded 100644
--- a/llvm/lib/Support/Unix/Path.inc
+++ b/llvm/lib/Support/Unix/Path.inc
@@ -1133,6 +1133,19 @@ bool home_directory(SmallVectorImpl<char> &result) {
   return true;
 }
 
+bool cache_directory(SmallVectorImpl<char> &result) {
+  if (const char *RequestedDir = getenv("XDG_CACHE_HOME")) {
+    result.clear();
+    result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
+    return true;
+  }
+  if (!home_directory(result)) {
+    return false;
+  }
+  append(result, ".cache");
+  return true;
+}
+
 static bool getDarwinConfDir(bool TempDir, SmallVectorImpl<char> &Result) {
   #if defined(_CS_DARWIN_USER_TEMP_DIR) && defined(_CS_DARWIN_USER_CACHE_DIR)
   // On Darwin, use DARWIN_USER_TEMP_DIR or DARWIN_USER_CACHE_DIR.
@@ -1158,27 +1171,6 @@ static bool getDarwinConfDir(bool TempDir, SmallVectorImpl<char> &Result) {
   return false;
 }
 
-bool cache_directory(SmallVectorImpl<char> &result) {
-#ifdef __APPLE__
-  if (getDarwinConfDir(false/*tempDir*/, result)) {
-    return true;
-  }
-#else
-  // XDG_CACHE_HOME as defined in the XDG Base Directory Specification:
-  // http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
-  if (const char *RequestedDir = getenv("XDG_CACHE_HOME")) {
-    result.clear();
-    result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
-    return true;
-  }
-#endif
-  if (!home_directory(result)) {
-    return false;
-  }
-  append(result, ".cache");
-  return true;
-}
-
 static const char *getEnvTempDir() {
   // Check whether the temporary directory is specified by an environment
   // variable.


        


More information about the cfe-commits mailing list