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

412

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)

475

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 :-))

513

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

518

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

std::string getShardPathFromFilePath(llvm::StringRef ShardRoot,
                                      llvm::StringRef FilePath) {
   llvm::SmallString<128> ShardRootSS(ShardRoot);
   llvm::sys::path::append(ShardRootSS, llvm::sys::path::filename(FilePath) +
                                            "." + llvm::toHex(digest(FilePath)) +
                                            ".idx");
   return std::string(ShardRootSS.str());
 }
 
-// Uses disk as a storage for index shards. Creates a directory called
-// ".clangd/index/" under the path provided during construction.
+// Uses disk as a storage for index shards.
 class DiskBackedIndexStorage : public BackgroundIndexStorage {
   std::string DiskShardRoot;
 
 public:
-  // Sets DiskShardRoot to (Directory + ".clangd/index/") which is the base
-  // directory for all shard files.
-  DiskBackedIndexStorage(llvm::StringRef Directory) {
-    llvm::SmallString<128> CDBDirectory(Directory);
-    llvm::sys::path::append(CDBDirectory, ".clangd", "index");
-    DiskShardRoot = std::string(CDBDirectory.str());
+  // Creates `DiskShardRoot` and any parents during construction.
+  DiskBackedIndexStorage(llvm::StringRef Directory) : DiskShardRoot(Directory) {
     std::error_code OK;
     std::error_code EC = llvm::sys::fs::create_directories(DiskShardRoot);
     if (EC != OK) {
       elog("Failed to create directory {0} for index storage: {1}",
            DiskShardRoot, EC.message());
     }
   }
 
llvm::Error storeShard(llvm::StringRef ShardIdentifier,
                          IndexFileOut Shard) const override {
     vlog("Couldn't find project for {0}, indexing in-memory only",
          ShardIdentifier);
     return llvm::Error::success();
   }
 };
 
 // Creates and owns IndexStorages for multiple CDBs.
+// When a CDB root is found, shards are stored in $ROOT/.clangd/index.
+// When no root is found, the fallback path is ~/.cache/clangd/index.
 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> FallbackDir;
+    if (llvm::sys::path::cache_directory(FallbackDir))
+      llvm::sys::path::append(FallbackDir, "clangd", "index");
+    this->FallbackDir = FallbackDir.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;
-    if (auto PI = GetProjectInfo(File))
-      CDBDirectory = PI->SourceRoot;
-    auto &IndexStorage = IndexStorageMap[CDBDirectory];
+    llvm::SmallString<128> StorageDir(FallbackDir);
+    if (auto PI = GetProjectInfo(File)) {
+      StorageDir = PI->SourceRoot;
+      llvm::sys::path::append(StorageDir, ".clangd", "index");
+    }
+    auto &IndexStorage = IndexStorageMap[StorageDir];
     if (!IndexStorage)
-      IndexStorage = create(CDBDirectory);
+      IndexStorage = create(StorageDir);
     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 FallbackDir;
 
   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, e.g. $XDG_CACHE_HOME.
+///
+/// @param result Holds the resulting path name.
+/// @result True if the appropriate path was determined, it need not 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

 //===- llvm/unittest/Support/Path.cpp - Path tests ------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/Path.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
TEST(Support, AbsolutePathIteratorEnd) {
   for (auto &Path : RootPaths) {
     SCOPED_TRACE(Path.first);
     StringRef LastComponent = *path::rbegin(Path.first, Path.second);
     EXPECT_EQ(1u, LastComponent.size());
     EXPECT_TRUE(path::is_separator(LastComponent[0], Path.second));
   }
 }
 
-TEST(Support, HomeDirectory) {
-  std::string expected;
 #ifdef _WIN32
-  if (wchar_t const *path = ::_wgetenv(L"USERPROFILE")) {
+std::string getEnvWin(const wchar_t *Var) {
+  std::string expected;
+  if (wchar_t const *path = ::_wgetenv(Var)) {
     auto pathLen = ::wcslen(path);
     ArrayRef<char> ref{reinterpret_cast<char const *>(path),
                        pathLen * sizeof(wchar_t)};
     convertUTF16ToUTF8String(ref, expected);
   }
+  return expected;
+}
+#else
+// RAII helper to set and restore an environment variable.
+class WithEnv {
+  const char *Var;
+  llvm::Optional<std::string> OriginalValue;
+
+public:
+  WithEnv(const char *Var, const char *Value) : Var(Var) {
+    if (const char *V = ::getenv(Var))
+      OriginalValue.emplace(V);
+    if (Value)
+      ::setenv(Var, Value, 1);
+    else
+      ::unsetenv(Var);
+    set(Value);
+  }
+  ~WithEnv() {
+    if (OriginalValue)
+      ::setenv(Var, OriginalValue->c_str(), 1);
+    else
+      ::unsetenv(Var);
+  }
+};
+#endif
+
+TEST(Support, HomeDirectory) {
+  std::string expected;
+#ifdef _WIN32
+  expected = getEnvWin(L"USERPROFILE");
 #else
   if (char const *path = ::getenv("HOME"))
     expected = path;
 #endif
   // Do not try to test it if we don't know what to expect.
   // On Windows we use something better than env vars.
   if (!expected.empty()) {
     SmallString<128> HomeDir;
     auto status = path::home_directory(HomeDir);
     EXPECT_TRUE(status);
     EXPECT_EQ(expected, HomeDir);
   }
 }
 
 #ifdef LLVM_ON_UNIX
 TEST(Support, HomeDirectoryWithNoEnv) {
-  std::string OriginalStorage;
-  char const *OriginalEnv = ::getenv("HOME");
-  if (OriginalEnv) {
-    // We're going to unset it, so make a copy and save a pointer to the copy
-    // so that we can reset it at the end of the test.
-    OriginalStorage = OriginalEnv;
-    OriginalEnv = OriginalStorage.c_str();
-  }
+  WithEnv Env("HOME", nullptr);
 
   // Don't run the test if we have nothing to compare against.
   struct passwd *pw = getpwuid(getuid());
   if (!pw || !pw->pw_dir) return;
-
-  ::unsetenv("HOME");
-  EXPECT_EQ(nullptr, ::getenv("HOME"));
   std::string PwDir = pw->pw_dir;
 
   SmallString<128> HomeDir;
-  auto status = path::home_directory(HomeDir);
-  EXPECT_TRUE(status);
+  EXPECT_TRUE(path::home_directory(HomeDir));
   EXPECT_EQ(PwDir, HomeDir);
+}
+
+TEST(Support, CacheDirectoryWithEnv) {
+  WithEnv Env("XDG_CACHE_HOME", "/xdg/cache");
+
+  SmallString<128> CacheDir;
+  EXPECT_TRUE(path::cache_directory(CacheDir));
+  EXPECT_EQ("/xdg/cache", CacheDir);
+}
+
+TEST(Support, CacheDirectoryNoEnv) {
+  WithEnv Env("XDG_CACHE_HOME", nullptr);
 
-  // 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);
+  SmallString<128> Fallback;
+  ASSERT_TRUE(path::home_directory(Fallback));
+  path::append(Fallback, ".cache");
+
+  SmallString<128> CacheDir;
+  EXPECT_TRUE(path::cache_directory(CacheDir));
+  EXPECT_EQ(Fallback, CacheDir);
+}
+#endif
+
+#ifdef _WIN32
+TEST(Support, CacheDirectory) {
+  std::string Expected = getEnvWin(L"LOCALAPPDATA");
+  // Do not try to test it if we don't know what to expect.
+  if (!Expected.empty()) {
+    SmallString<128> CacheDir;
+    EXPECT_TRUE(path::cache_directory(CacheDir));
+    EXPECT_EQ(Expected, CacheDir);
+  }
 }
 #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());
 }
 
 #ifdef _WIN32
 static std::string path2regex(std::string Path) {
EXPECT_TEMP_DIR(
     _wputenv_s(L"USERPROFILE", L"");
   },
     "C:\\Temp");
 
   // Test evn var / path with 260 chars.
   SmallString<270> Expected{"C:\\Temp\\AB\\123456789"};
   while (Expected.size() < 260)
     Expected.append("\\DirNameWith19Charss");
   ASSERT_EQ(260U, Expected.size());
   EXPECT_TEMP_DIR(_putenv_s("TMP", Expected.c_str()), Expected.c_str());
 }
 #endif
 
 class FileSystemTest : public testing::Test {
 protected:
   /// Unique temporary directory in which all created filesystem entities must
   /// be placed. It is removed at the end of each test (must be empty).
TEST_F(FileSystemTest, Unique) {
   int FileDescriptor;
   SmallString<64> TempPath;
   ASSERT_NO_ERROR(
       fs::createTemporaryFile("prefix", "temp", FileDescriptor, TempPath));
 
   // The same file should return an identical unique id.
   fs::UniqueID F1, F2;
   ASSERT_NO_ERROR(fs::getUniqueID(Twine(TempPath), F1));
   ASSERT_NO_ERROR(fs::getUniqueID(Twine(TempPath), F2));
   ASSERT_EQ(F1, F2);
 
   // Different files should return different unique ids.
   int FileDescriptor2;
   SmallString<64> TempPath2;
   ASSERT_NO_ERROR(
       fs::createTemporaryFile("prefix", "temp", FileDescriptor2, TempPath2));
 
   fs::UniqueID D;
   ASSERT_NO_ERROR(fs::getUniqueID(Twine(TempPath2), D));
   ASSERT_NE(D, F1);
   ::close(FileDescriptor2);