<div dir="ltr">This was temporarily reverted here:<div><br></div><div>commit faf2dce1dd6ae25aa75d2685ac7bb27ec31e2ced (HEAD -> master, origin/master, origin/HEAD)<br>Author: Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>><br>Date: Tue Apr 28 15:48:56 2020 -0700<br><br> Temporarily revert "Add a facility to get system cache directory and use it in clangd"<br><br> This reverts commit ad38f4b371bdca214e3a3cda9a76ec2213215c68.<br><br> As it broke building the unittests:<br><br> .../sources/llvm-project/llvm/unittests/Support/Path.cpp:334:5: error: use of undeclared identifier 'set'<br> set(Value);<br> ^<br> 1 error generated.<br></div><div><br></div><div>I followed up on the review thread.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Apr 28, 2020 at 2:18 PM Sam McCall via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Vojtěch Štěpančík<br>
Date: 2020-04-28T23:18:31+02:00<br>
New Revision: ad38f4b371bdca214e3a3cda9a76ec2213215c68<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/ad38f4b371bdca214e3a3cda9a76ec2213215c68" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/ad38f4b371bdca214e3a3cda9a76ec2213215c68</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/ad38f4b371bdca214e3a3cda9a76ec2213215c68.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/ad38f4b371bdca214e3a3cda9a76ec2213215c68.diff</a><br>
<br>
LOG: Add a facility to get system cache directory and use it in clangd<br>
<br>
Summary:<br>
This patch adds a function that is similar to `llvm::sys::path::home_directory`, but provides access to the system cache directory.<br>
<br>
For Windows, that is %LOCALAPPDATA%, and applications should put their files under %LOCALAPPDATA%\Organization\Product\.<br>
<br>
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/.<br>
<br>
Subsequently, the Clangd Index storage leverages this new API to put index files somewhere else than the users home directory.<br>
<br>
Fixes <a href="https://github.com/clangd/clangd/issues/341" rel="noreferrer" target="_blank">https://github.com/clangd/clangd/issues/341</a><br>
<br>
Reviewers: sammccall, chandlerc, Bigcheese<br>
<br>
Reviewed By: sammccall<br>
<br>
Subscribers: hiraditya, ilya-biryukov, MaskRay, jkorous, dexonsmith, arphaman, kadircet, ormris, usaxena95, cfe-commits, llvm-commits<br>
<br>
Tags: #clang-tools-extra, #clang, #llvm<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D78501" rel="noreferrer" target="_blank">https://reviews.llvm.org/D78501</a><br>
<br>
Added: <br>
<br>
<br>
Modified: <br>
clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp<br>
llvm/include/llvm/Support/Path.h<br>
llvm/lib/Support/Unix/Path.inc<br>
llvm/lib/Support/Windows/Path.inc<br>
llvm/unittests/Support/Path.cpp<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp b/clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp<br>
index b07728ee6a2c..765b710ba149 100644<br>
--- a/clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp<br>
+++ b/clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp<br>
@@ -36,18 +36,13 @@ std::string getShardPathFromFilePath(llvm::StringRef ShardRoot,<br>
return std::string(ShardRootSS.str());<br>
}<br>
<br>
-// Uses disk as a storage for index shards. Creates a directory called<br>
-// ".clangd/index/" under the path provided during construction.<br>
+// Uses disk as a storage for index shards.<br>
class DiskBackedIndexStorage : public BackgroundIndexStorage {<br>
std::string DiskShardRoot;<br>
<br>
public:<br>
- // Sets DiskShardRoot to (Directory + ".clangd/index/") which is the base<br>
- // directory for all shard files.<br>
- DiskBackedIndexStorage(llvm::StringRef Directory) {<br>
- llvm::SmallString<128> CDBDirectory(Directory);<br>
- llvm::sys::path::append(CDBDirectory, ".clangd", "index");<br>
- DiskShardRoot = std::string(CDBDirectory.str());<br>
+ // Creates `DiskShardRoot` and any parents during construction.<br>
+ DiskBackedIndexStorage(llvm::StringRef Directory) : DiskShardRoot(Directory) {<br>
std::error_code OK;<br>
std::error_code EC = llvm::sys::fs::create_directories(DiskShardRoot);<br>
if (EC != OK) {<br>
@@ -100,26 +95,31 @@ class NullStorage : public BackgroundIndexStorage {<br>
};<br>
<br>
// Creates and owns IndexStorages for multiple CDBs.<br>
+// When a CDB root is found, shards are stored in $ROOT/.clangd/index.<br>
+// When no root is found, the fallback path is ~/.cache/clangd/index.<br>
class DiskBackedIndexStorageManager {<br>
public:<br>
DiskBackedIndexStorageManager(<br>
std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo)<br>
: IndexStorageMapMu(std::make_unique<std::mutex>()),<br>
GetProjectInfo(std::move(GetProjectInfo)) {<br>
- llvm::SmallString<128> HomeDir;<br>
- llvm::sys::path::home_directory(HomeDir);<br>
- this->HomeDir = HomeDir.str().str();<br>
+ llvm::SmallString<128> FallbackDir;<br>
+ if (llvm::sys::path::cache_directory(FallbackDir))<br>
+ llvm::sys::path::append(FallbackDir, "clangd", "index");<br>
+ this->FallbackDir = FallbackDir.str().str();<br>
}<br>
<br>
// Creates or fetches to storage from cache for the specified project.<br>
BackgroundIndexStorage *operator()(PathRef File) {<br>
std::lock_guard<std::mutex> Lock(*IndexStorageMapMu);<br>
- Path CDBDirectory = HomeDir;<br>
- if (auto PI = GetProjectInfo(File))<br>
- CDBDirectory = PI->SourceRoot;<br>
- auto &IndexStorage = IndexStorageMap[CDBDirectory];<br>
+ llvm::SmallString<128> StorageDir(FallbackDir);<br>
+ if (auto PI = GetProjectInfo(File)) {<br>
+ StorageDir = PI->SourceRoot;<br>
+ llvm::sys::path::append(StorageDir, ".clangd", "index");<br>
+ }<br>
+ auto &IndexStorage = IndexStorageMap[StorageDir];<br>
if (!IndexStorage)<br>
- IndexStorage = create(CDBDirectory);<br>
+ IndexStorage = create(StorageDir);<br>
return IndexStorage.get();<br>
}<br>
<br>
@@ -132,7 +132,7 @@ class DiskBackedIndexStorageManager {<br>
return std::make_unique<DiskBackedIndexStorage>(CDBDirectory);<br>
}<br>
<br>
- Path HomeDir;<br>
+ Path FallbackDir;<br>
<br>
llvm::StringMap<std::unique_ptr<BackgroundIndexStorage>> IndexStorageMap;<br>
std::unique_ptr<std::mutex> IndexStorageMapMu;<br>
<br>
diff --git a/llvm/include/llvm/Support/Path.h b/llvm/include/llvm/Support/Path.h<br>
index 728b63c54c05..9edd9c4183ee 100644<br>
--- a/llvm/include/llvm/Support/Path.h<br>
+++ b/llvm/include/llvm/Support/Path.h<br>
@@ -368,6 +368,13 @@ void system_temp_directory(bool erasedOnReboot, SmallVectorImpl<char> &result);<br>
/// @result True if a home directory is set, false otherwise.<br>
bool home_directory(SmallVectorImpl<char> &result);<br>
<br>
+/// Get the directory where installed packages should put their<br>
+/// machine-local cache, e.g. $XDG_CACHE_HOME.<br>
+///<br>
+/// @param result Holds the resulting path name.<br>
+/// @result True if the appropriate path was determined, it need not exist.<br>
+bool cache_directory(SmallVectorImpl<char> &result);<br>
+<br>
/// Has root name?<br>
///<br>
/// root_name != ""<br>
<br>
diff --git a/llvm/lib/Support/Unix/Path.inc b/llvm/lib/Support/Unix/Path.inc<br>
index 001ab81b23af..783a7ace1005 100644<br>
--- a/llvm/lib/Support/Unix/Path.inc<br>
+++ b/llvm/lib/Support/Unix/Path.inc<br>
@@ -1138,6 +1138,19 @@ bool home_directory(SmallVectorImpl<char> &result) {<br>
return true;<br>
}<br>
<br>
+bool cache_directory(SmallVectorImpl<char> &result) {<br>
+ if (const char *RequestedDir = getenv("XDG_CACHE_HOME")) {<br>
+ result.clear();<br>
+ result.append(RequestedDir, RequestedDir + strlen(RequestedDir));<br>
+ return true;<br>
+ }<br>
+ if (!home_directory(result)) {<br>
+ return false;<br>
+ }<br>
+ append(result, ".cache");<br>
+ return true;<br>
+}<br>
+<br>
static bool getDarwinConfDir(bool TempDir, SmallVectorImpl<char> &Result) {<br>
#if defined(_CS_DARWIN_USER_TEMP_DIR) && defined(_CS_DARWIN_USER_CACHE_DIR)<br>
// On Darwin, use DARWIN_USER_TEMP_DIR or DARWIN_USER_CACHE_DIR.<br>
<br>
diff --git a/llvm/lib/Support/Windows/Path.inc b/llvm/lib/Support/Windows/Path.inc<br>
index 0eadefb689fd..2e1488479095 100644<br>
--- a/llvm/lib/Support/Windows/Path.inc<br>
+++ b/llvm/lib/Support/Windows/Path.inc<br>
@@ -1372,6 +1372,10 @@ bool home_directory(SmallVectorImpl<char> &result) {<br>
return getKnownFolderPath(FOLDERID_Profile, result);<br>
}<br>
<br>
+bool cache_directory(SmallVectorImpl<char> &result) {<br>
+ return getKnownFolderPath(FOLDERID_LocalAppData, result);<br>
+}<br>
+<br>
static bool getTempDirEnvVar(const wchar_t *Var, SmallVectorImpl<char> &Res) {<br>
SmallVector<wchar_t, 1024> Buf;<br>
size_t Size = 1024;<br>
<br>
diff --git a/llvm/unittests/Support/Path.cpp b/llvm/unittests/Support/Path.cpp<br>
index 1a2ac1818eb0..7667702377e8 100644<br>
--- a/llvm/unittests/Support/Path.cpp<br>
+++ b/llvm/unittests/Support/Path.cpp<br>
@@ -13,6 +13,7 @@<br>
#include "llvm/ADT/Triple.h"<br>
#include "llvm/BinaryFormat/Magic.h"<br>
#include "llvm/Config/llvm-config.h"<br>
+#include "llvm/Support/Compiler.h"<br>
#include "llvm/Support/ConvertUTF.h"<br>
#include "llvm/Support/Errc.h"<br>
#include "llvm/Support/ErrorHandling.h"<br>
@@ -305,15 +306,46 @@ TEST(Support, AbsolutePathIteratorEnd) {<br>
}<br>
}<br>
<br>
-TEST(Support, HomeDirectory) {<br>
- std::string expected;<br>
#ifdef _WIN32<br>
- if (wchar_t const *path = ::_wgetenv(L"USERPROFILE")) {<br>
+std::string getEnvWin(const wchar_t *Var) {<br>
+ std::string expected;<br>
+ if (wchar_t const *path = ::_wgetenv(Var)) {<br>
auto pathLen = ::wcslen(path);<br>
ArrayRef<char> ref{reinterpret_cast<char const *>(path),<br>
pathLen * sizeof(wchar_t)};<br>
convertUTF16ToUTF8String(ref, expected);<br>
}<br>
+ return expected;<br>
+}<br>
+#else<br>
+// RAII helper to set and restore an environment variable.<br>
+class WithEnv {<br>
+ const char *Var;<br>
+ llvm::Optional<std::string> OriginalValue;<br>
+<br>
+public:<br>
+ WithEnv(const char *Var, const char *Value) : Var(Var) {<br>
+ if (const char *V = ::getenv(Var))<br>
+ OriginalValue.emplace(V);<br>
+ if (Value)<br>
+ ::setenv(Var, Value, 1);<br>
+ else<br>
+ ::unsetenv(Var);<br>
+ set(Value);<br>
+ }<br>
+ ~WithEnv() {<br>
+ if (OriginalValue)<br>
+ ::setenv(Var, OriginalValue->c_str(), 1);<br>
+ else<br>
+ ::unsetenv(Var);<br>
+ }<br>
+};<br>
+#endif<br>
+<br>
+TEST(Support, HomeDirectory) {<br>
+ std::string expected;<br>
+#ifdef _WIN32<br>
+ expected = getEnvWin(L"USERPROFILE");<br>
#else<br>
if (char const *path = ::getenv("HOME"))<br>
expected = path;<br>
@@ -330,31 +362,48 @@ TEST(Support, HomeDirectory) {<br>
<br>
#ifdef LLVM_ON_UNIX<br>
TEST(Support, HomeDirectoryWithNoEnv) {<br>
- std::string OriginalStorage;<br>
- char const *OriginalEnv = ::getenv("HOME");<br>
- if (OriginalEnv) {<br>
- // We're going to unset it, so make a copy and save a pointer to the copy<br>
- // so that we can reset it at the end of the test.<br>
- OriginalStorage = OriginalEnv;<br>
- OriginalEnv = OriginalStorage.c_str();<br>
- }<br>
+ WithEnv Env("HOME", nullptr);<br>
<br>
// Don't run the test if we have nothing to compare against.<br>
struct passwd *pw = getpwuid(getuid());<br>
if (!pw || !pw->pw_dir) return;<br>
-<br>
- ::unsetenv("HOME");<br>
- EXPECT_EQ(nullptr, ::getenv("HOME"));<br>
std::string PwDir = pw->pw_dir;<br>
<br>
SmallString<128> HomeDir;<br>
- auto status = path::home_directory(HomeDir);<br>
- EXPECT_TRUE(status);<br>
+ EXPECT_TRUE(path::home_directory(HomeDir));<br>
EXPECT_EQ(PwDir, HomeDir);<br>
+}<br>
+<br>
+TEST(Support, CacheDirectoryWithEnv) {<br>
+ WithEnv Env("XDG_CACHE_HOME", "/xdg/cache");<br>
+<br>
+ SmallString<128> CacheDir;<br>
+ EXPECT_TRUE(path::cache_directory(CacheDir));<br>
+ EXPECT_EQ("/xdg/cache", CacheDir);<br>
+}<br>
+<br>
+TEST(Support, CacheDirectoryNoEnv) {<br>
+ WithEnv Env("XDG_CACHE_HOME", nullptr);<br>
<br>
- // Now put the environment back to its original state (meaning that if it was<br>
- // unset before, we don't reset it).<br>
- if (OriginalEnv) ::setenv("HOME", OriginalEnv, 1);<br>
+ SmallString<128> Fallback;<br>
+ ASSERT_TRUE(path::home_directory(Fallback));<br>
+ path::append(Fallback, ".cache");<br>
+<br>
+ SmallString<128> CacheDir;<br>
+ EXPECT_TRUE(path::cache_directory(CacheDir));<br>
+ EXPECT_EQ(Fallback, CacheDir);<br>
+}<br>
+#endif<br>
+<br>
+#ifdef _WIN32<br>
+TEST(Support, CacheDirectory) {<br>
+ std::string Expected = getEnvWin(L"LOCALAPPDATA");<br>
+ // Do not try to test it if we don't know what to expect.<br>
+ if (!Expected.empty()) {<br>
+ SmallString<128> CacheDir;<br>
+ EXPECT_TRUE(path::cache_directory(CacheDir));<br>
+ EXPECT_EQ(Expected, CacheDir);<br>
+ }<br>
}<br>
#endif<br>
<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>