[llvm] ad38f4b - Add a facility to get system cache directory and use it in clangd
Eric Christopher via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 28 15:53:10 PDT 2020
This was temporarily reverted here:
commit faf2dce1dd6ae25aa75d2685ac7bb27ec31e2ced (HEAD -> master,
origin/master, origin/HEAD)
Author: Eric Christopher <echristo at gmail.com>
Date: Tue Apr 28 15:48:56 2020 -0700
Temporarily revert "Add a facility to get system cache directory and
use it in clangd"
This reverts commit ad38f4b371bdca214e3a3cda9a76ec2213215c68.
As it broke building the unittests:
.../sources/llvm-project/llvm/unittests/Support/Path.cpp:334:5: error:
use of undeclared identifier 'set'
set(Value);
^
1 error generated.
I followed up on the review thread.
On Tue, Apr 28, 2020 at 2:18 PM Sam McCall via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
>
> Author: Vojtěch Štěpančík
> Date: 2020-04-28T23:18:31+02:00
> New Revision: ad38f4b371bdca214e3a3cda9a76ec2213215c68
>
> URL:
> https://github.com/llvm/llvm-project/commit/ad38f4b371bdca214e3a3cda9a76ec2213215c68
> DIFF:
> https://github.com/llvm/llvm-project/commit/ad38f4b371bdca214e3a3cda9a76ec2213215c68.diff
>
> LOG: Add a facility to get system cache directory and use it in clangd
>
> 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
>
> Reviewers: sammccall, chandlerc, Bigcheese
>
> Reviewed By: sammccall
>
> Subscribers: hiraditya, ilya-biryukov, MaskRay, jkorous, dexonsmith,
> arphaman, kadircet, ormris, usaxena95, cfe-commits, llvm-commits
>
> Tags: #clang-tools-extra, #clang, #llvm
>
> Differential Revision: https://reviews.llvm.org/D78501
>
> Added:
>
>
> Modified:
> clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
> llvm/include/llvm/Support/Path.h
> llvm/lib/Support/Unix/Path.inc
> llvm/lib/Support/Windows/Path.inc
> llvm/unittests/Support/Path.cpp
>
> Removed:
>
>
>
>
> ################################################################################
> diff --git a/clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
> b/clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
> index b07728ee6a2c..765b710ba149 100644
> --- a/clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
> +++ b/clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
> @@ -36,18 +36,13 @@ std::string getShardPathFromFilePath(llvm::StringRef
> ShardRoot,
> 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) {
> @@ -100,26 +95,31 @@ class NullStorage : public BackgroundIndexStorage {
> };
>
> // 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();
> }
>
> @@ -132,7 +132,7 @@ class DiskBackedIndexStorageManager {
> return std::make_unique<DiskBackedIndexStorage>(CDBDirectory);
> }
>
> - Path HomeDir;
> + Path FallbackDir;
>
> llvm::StringMap<std::unique_ptr<BackgroundIndexStorage>>
> IndexStorageMap;
> std::unique_ptr<std::mutex> IndexStorageMapMu;
>
> diff --git a/llvm/include/llvm/Support/Path.h
> b/llvm/include/llvm/Support/Path.h
> index 728b63c54c05..9edd9c4183ee 100644
> --- a/llvm/include/llvm/Support/Path.h
> +++ b/llvm/include/llvm/Support/Path.h
> @@ -368,6 +368,13 @@ void system_temp_directory(bool erasedOnReboot,
> SmallVectorImpl<char> &result);
> /// @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 != ""
>
> diff --git a/llvm/lib/Support/Unix/Path.inc
> b/llvm/lib/Support/Unix/Path.inc
> index 001ab81b23af..783a7ace1005 100644
> --- a/llvm/lib/Support/Unix/Path.inc
> +++ b/llvm/lib/Support/Unix/Path.inc
> @@ -1138,6 +1138,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.
>
> diff --git a/llvm/lib/Support/Windows/Path.inc
> b/llvm/lib/Support/Windows/Path.inc
> index 0eadefb689fd..2e1488479095 100644
> --- a/llvm/lib/Support/Windows/Path.inc
> +++ b/llvm/lib/Support/Windows/Path.inc
> @@ -1372,6 +1372,10 @@ 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;
>
> diff --git a/llvm/unittests/Support/Path.cpp
> b/llvm/unittests/Support/Path.cpp
> index 1a2ac1818eb0..7667702377e8 100644
> --- a/llvm/unittests/Support/Path.cpp
> +++ b/llvm/unittests/Support/Path.cpp
> @@ -13,6 +13,7 @@
> #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"
> @@ -305,15 +306,46 @@ TEST(Support, AbsolutePathIteratorEnd) {
> }
> }
>
> -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;
> @@ -330,31 +362,48 @@ TEST(Support, HomeDirectory) {
>
> #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
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200428/49a0cc8e/attachment.html>
More information about the llvm-commits
mailing list