Add a facility to get system cache directory and use it in clangd

[VojtechStep]

Summary

This patch adds a function that is similar to llvm::sys::path::home_directory, but provides access to the system cache directory.

For Windows, that is %LOCALAPPDATA%, and applications should put their files under %LOCALAPPDATA%\Organization\Product\.

For *nixes, it adheres to the XDG Base Directory Specification, so it first looks at the XDG_CACHE_HOME environment variable and falls back to ~/.cache/.

Subsequently, the Clangd Index storage leverages this new API to put index files somewhere else than the users home directory.

Fixes https://github.com/clangd/clangd/issues/341

[sammccall]

Thanks! Looks good with a few nits.

Once you're done, let me know if I should land this for you (after a few patches landed this way you can apply for commit access: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)

You can add "Fixes https://github.com/clangd/clangd/issues/341" to the description to close the associated bug.

Note to self: maybe we should add this to the release notes so people can delete ~/.clangd.

llvm/include/llvm/Support/Path.h

372

I'd add "e.g. $XDG_CACHE_HOME" to this comment. (No need to spell out the fallback or windows behavior, but this gives a bit more of a hint)

375

This kind of implies you check that it exists (which it may not, particularly on the ~/.cache fallback). Avoiding IO seems like the right thing, but I'd mention in the function description that the returned path may not exist yet.

llvm/lib/Support/Unix/Path.inc

1142

nit: suggest const char* to signal we're not doing anything bad with this terrifying API :-)

1143

nit: llvm style is to inline this if (char * RequestedDir = ...)

llvm/unittests/Support/Path.cpp

363

nit: Expected
(uppercase for vars in llvm style. The style in Path.h/.inc is very old/mixed up and you've done a great job being consistent, but at least for local vars in the cpp file we should follow the guidelines)

426

nit: use an llvm::Optional<wstring> so we correctly distinguish unset vs empty?, and elsewhere

(I guess we should extract a RAII environment-restorer for this file, but not going to ask you to boil that ocean :-))

464

you can just ASSERT_TRUE, this should always succeed in our test setups I think.

469

Why are we testing this twice, once conditionally?

[VojtechStep]

@sammccall I think I fixed all raised issues. Let me know if there is anything else I can do, otherwise feel free to commit it.

Also Note to self: Once this is pushed to Arch packages we should add Clangd to the Supported list of XDG Base Dir Spec support on Arch Wiki.

[VojtechStep]

Ping @sammccall, could you commit the changes? Thanks 👍

[sammccall]

Fix(?) test compile for windows, split out windows/unix tests for simplicity.

[sammccall]

Use $cache/clangd/index, not $cache/.clangd/index

[sammccall]

Sorry about the delay, ran into some problems with the tests and decided to simplify them a little by extracting the helper.

Also I think the subdirectory inside $XDG_CACHE_HOME isn't supposed to be hidden, so .clangd->clangd which meant a little refactoring.

Thanks for the patch! Landed as https://github.com/llvm/llvm-project/commit/ad38f4b371bdca214e3a3cda9a76ec2213215c68

[VojtechStep]

Great, thanks for the fixes!

clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp

 
 // Creates and owns IndexStorages for multiple CDBs.
 class DiskBackedIndexStorageManager {
 public:
   DiskBackedIndexStorageManager(
       std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo)
       : IndexStorageMapMu(std::make_unique<std::mutex>()),
         GetProjectInfo(std::move(GetProjectInfo)) {
-    llvm::SmallString<128> HomeDir;
-    llvm::sys::path::home_directory(HomeDir);
-    this->HomeDir = HomeDir.str().str();
+    llvm::SmallString<128> CacheDir;
+    llvm::sys::path::cache_directory(CacheDir);
+    this->CacheDir = CacheDir.str().str();
   }
 
   // Creates or fetches to storage from cache for the specified project.
   BackgroundIndexStorage *operator()(PathRef File) {
     std::lock_guard<std::mutex> Lock(*IndexStorageMapMu);
-    Path CDBDirectory = HomeDir;
+    Path CDBDirectory = CacheDir;
     if (auto PI = GetProjectInfo(File))
       CDBDirectory = PI->SourceRoot;
     auto &IndexStorage = IndexStorageMap[CDBDirectory];
     if (!IndexStorage)
       IndexStorage = create(CDBDirectory);
     return IndexStorage.get();
   }
 
 private:
   std::unique_ptr<BackgroundIndexStorage> create(PathRef CDBDirectory) {
     if (CDBDirectory.empty()) {
       elog("Tried to create storage for empty directory!");
       return std::make_unique<NullStorage>();
     }
     return std::make_unique<DiskBackedIndexStorage>(CDBDirectory);
   }
 
-  Path HomeDir;
+  Path CacheDir;
 
   llvm::StringMap<std::unique_ptr<BackgroundIndexStorage>> IndexStorageMap;
   std::unique_ptr<std::mutex> IndexStorageMapMu;
 
   std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo;
 };
 
 } // namespace

llvm/include/llvm/Support/Path.h

 void system_temp_directory(bool erasedOnReboot, SmallVectorImpl<char> &result);
 
 /// Get the user's home directory.
 ///
 /// @param result Holds the resulting path name.
 /// @result True if a home directory is set, false otherwise.
 bool home_directory(SmallVectorImpl<char> &result);
 
+/// Get the directory where installed packages should put their
+/// machine-local cache. Respects $XDG_CACHE_HOME.
+///
+/// @param result Holds the resulting path name.
+/// @result True if the appropriate path was put together, doesn't need to exist.
+bool cache_directory(SmallVectorImpl<char> &result);
+
 /// Has root name?
 ///
 /// root_name != ""
 ///
 /// @param path Input path.
 /// @result True if the path has a root name, false otherwise.
 bool has_root_name(const Twine &path, Style style = Style::native);
 

llvm/lib/Support/Unix/Path.inc

bool home_directory(SmallVectorImpl<char> &result) {
   if (!RequestedDir)
     return false;
 
   result.clear();
   result.append(RequestedDir, RequestedDir + strlen(RequestedDir));
   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.
   // macros defined in <unistd.h> on darwin >= 9
   int ConfName = TempDir ? _CS_DARWIN_USER_TEMP_DIR
                          : _CS_DARWIN_USER_CACHE_DIR;
   size_t ConfLen = confstr(ConfName, nullptr, 0);
   if (ConfLen > 0) {

llvm/lib/Support/Windows/Path.inc

static bool getKnownFolderPath(KNOWNFOLDERID folderId,
   ::CoTaskMemFree(path);
   return ok;
 }
 
 bool home_directory(SmallVectorImpl<char> &result) {
   return getKnownFolderPath(FOLDERID_Profile, result);
 }
 
+bool cache_directory(SmallVectorImpl<char> &result) {
+  return getKnownFolderPath(FOLDERID_LocalAppData, result);
+}
+
 static bool getTempDirEnvVar(const wchar_t *Var, SmallVectorImpl<char> &Res) {
   SmallVector<wchar_t, 1024> Buf;
   size_t Size = 1024;
   do {
     Buf.reserve(Size);
     Size = GetEnvironmentVariableW(Var, Buf.data(), Buf.capacity());
     if (Size == 0)
       return false;

llvm/unittests/Support/Path.cpp

TEST(Support, HomeDirectoryWithNoEnv) {
   EXPECT_EQ(PwDir, HomeDir);
 
   // Now put the environment back to its original state (meaning that if it was
   // unset before, we don't reset it).
   if (OriginalEnv) ::setenv("HOME", OriginalEnv, 1);
 }
 #endif
 
+TEST(Support, CacheDirectoryWithEnv) {
+  std::string Expected;
+  // The value of the environment variable is set to a non-standard
+  // location for the test, so we have to store the original value
+  // even if it's not set.
+  // Use an std::string to copy the data
+#ifdef _WIN32
+  Optional<std::wstring> OriginalStorage;
+  if (wchar_t const *path = ::_wgetenv(L"XDG_CACHE_HOME")) {
+    OriginalStorage = path;
+  }
+
+  ::_wputenv("XDG_CACHE_HOME", L"C:\\xdg\\cache");
+
+  if (wchar_t const *localAppData = ::_wgetenv(L"LOCALAPPDATA")) {
+    auto pathLen = ::wcslen(path);
+    ArrayRef<char> ref{reinterpret_cast<char const *>(path),
+                       pathLen * sizeof(wchar_t)};
+    convertUTF16ToUTF8String(ref, Expected);
+  }
+
+  if (!Expected.empty()) {
+    SmallString<128> CacheDir;
+    auto status = path::cache_directory(CacheDir);
+    EXPECT_TRUE(status);
+    EXPECT_EQ(Expected, CacheDir);
+  }
+
+  if (OriginalStorage.hasValue()) {
+    ::_wputenv("XDG_CACHE_HOME", OriginalStorage->c_str());
+  } else {
+    ::_wputenv("XDG_CACHE_HOME", nullptr);
+  }
+#else
+  Optional<std::string> OriginalStorage;
+  if (char const *path = ::getenv("XDG_CACHE_HOME")) {
+    OriginalStorage = path;
+  }
+
+  Expected = "/xdg/cache";
+  ::setenv("XDG_CACHE_HOME", Expected.c_str(), 1);
+  EXPECT_EQ(Expected, std::string(::getenv("XDG_CACHE_HOME")));
+
+  SmallString<128> CacheDir;
+  auto status = path::cache_directory(CacheDir);
+  EXPECT_TRUE(status);
+  EXPECT_EQ(Expected, CacheDir);
+
+  if (OriginalStorage.hasValue()) {
+    ::setenv("XDG_CACHE_HOME", OriginalStorage->c_str(), 1);
+  } else {
+    ::unsetenv("XDG_CACHE_HOME");
+  }
+#endif
+}
+
+TEST(Support, CacheDirectoryNoEnv) {
+  std::string Expected;
+#ifdef _WIN32
+  Optional<std::wstring> OriginalStorage;
+  if (wchar_t const *path = ::_wgetenv(L"XDG_CACHE_HOME")) {
+    OriginalStorage = path;
+  }
+
+  ::_wputenv("XDG_CACHE_HOME", nullptr);
+  EXPECT_EQ(nullptr, ::_wgetenv("XDG_CACHE_HOME"));
+
+  if (wchar_t const *localAppData = ::_wgetenv(L"LOCALAPPDATA")) {
+    auto pathLen = ::wcslen(path);
+    ArrayRef<char> ref {reinterpret_cast<char const *>(path),
+                       pathLen * sizeof(wchar_t)};
+    convertUTF16ToUTF8String(ref, Expected);
+  }
+
+  // LocalAppData should always be set, but better safe than sorry
+  if (!Expected.empty()) {
+    SmallString<128> CacheDir;
+    auto status = path::cache_directory(CacheDir);
+    EXPECT_TRUE(status);
+    EXPECT_EQ(Expected, CacheDir);
+  }
+
+  if (OriginalStorage.hasValue()) {
+    ::_wputenv("XDG_CACHE_HOME", OriginalStorage->c_str());
+  } else {
+    ::_wputenv("XDG_CACHE_HOME", nullptr);
+  }
+#else
+  Optional<std::string> OriginalStorage;
+  if (char const *path = ::getenv("XDG_CACHE_HOME")) {
+    OriginalStorage = path;
+  }
+
+  ::unsetenv("XDG_CACHE_HOME");
+  EXPECT_EQ(nullptr, ::getenv("XDG_CACHE_HOME"));
+
+  SmallString<128> HomeDir;
+
+  auto home_status = path::home_directory(HomeDir);
+  EXPECT_TRUE(home_status);
+  path::append(HomeDir, ".cache");
+  Expected = HomeDir.str().str();
+
+  SmallString<128> CacheDir;
+  auto status = path::cache_directory(CacheDir);
+  EXPECT_TRUE(status);
+  EXPECT_EQ(Expected, CacheDir);
+
+  if (OriginalStorage.hasValue()) {
+    ::setenv("XDG_CACHE_HOME", OriginalStorage->c_str(), 1);
+  } else {
+    ::unsetenv("XDG_CACHE_HOME");
+  }
+#endif
+}
+
 TEST(Support, TempDirectory) {
   SmallString<32> TempDir;
   path::system_temp_directory(false, TempDir);
   EXPECT_TRUE(!TempDir.empty());
   TempDir.clear();
   path::system_temp_directory(true, TempDir);
   EXPECT_TRUE(!TempDir.empty());
 }