[PATCH] D78501: Add a facility to get system cache directory and use it in clangd

2020-04-28 Thread Vojtěch Štěpančík via Phabricator via cfe-commits
VojtechStep added a comment.

Great, thanks for the fixes!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78501/new/

https://reviews.llvm.org/D78501



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D78501: Add a facility to get system cache directory and use it in clangd

2020-04-28 Thread Vojtěch Štěpančík via Phabricator via cfe-commits
VojtechStep added a comment.
Herald added a project: LLVM.

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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78501/new/

https://reviews.llvm.org/D78501



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D78501: Add a facility to get system cache directory and use it in clangd

2020-04-20 Thread Vojtěch Štěpančík via Phabricator via cfe-commits
VojtechStep updated this revision to Diff 258805.
VojtechStep edited the summary of this revision.
VojtechStep added a comment.

@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 
.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78501/new/

https://reviews.llvm.org/D78501

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

Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -358,6 +358,122 @@
 }
 #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 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 ref{reinterpret_cast(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 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 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 ref {reinterpret_cast(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 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);
Index: llvm/lib/Support/Windows/Path.inc
===
--- llvm/lib/Support/Windows/Path.inc
+++ llvm/lib/Support/Windows/Path.inc
@@ -1372,6 +1372,10 @@
   return getKnownFolderPath(FOLDERID_Profile, result);
 }
 
+bool cache_directory(SmallVectorImpl ) {
+  return getKnownFolderPath(FOLDERID_LocalAppData, result);
+}
+
 static bool getTempDirEnvVar(const wchar_t *Var, SmallVectorImpl ) {
   SmallVector Buf;
   size_t Size = 1024;
Index: llvm/lib/Support/Unix/Path.inc

[PATCH] D78501: Add a facility to get system cache directory and use it in clangd

2020-04-20 Thread Vojtěch Štěpančík via Phabricator via cfe-commits
VojtechStep created this revision.
VojtechStep added reviewers: sammccall, chandlerc, Bigcheese.
VojtechStep added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, usaxena95, ormris, kadircet, arphaman, 
dexonsmith, jkorous, MaskRay, ilya-biryukov, hiraditya.
Herald added a project: clang.

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.

Previous discussion is here 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78501

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

Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -358,6 +358,136 @@
 }
 #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
+  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 ref{reinterpret_cast(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.empty()) {
+::_wputenv("XDG_CACHE_HOME", OriginalStorage.c_str());
+  } else {
+::_wputenv("XDG_CACHE_HOME", nullptr);
+  }
+#else
+  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.empty()) {
+::setenv("XDG_CACHE_HOME", OriginalStorage.c_str(), 1);
+  } else {
+::unsetenv("XDG_CACHE_HOME");
+  }
+#endif
+}
+
+TEST(Support, CacheDirectoryNoEnv) {
+  std::string expected;
+#ifdef _WIN32
+  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 ref{reinterpret_cast(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.empty()) {
+::_wputenv("XDG_CACHE_HOME", OriginalStorage.c_str());
+  } else {
+::_wputenv("XDG_CACHE_HOME", nullptr);
+  }
+#else
+  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;
+  if (path::home_directory(HomeDir)) {
+path::append(HomeDir, ".cache");
+expected = std::string(HomeDir.str());
+  }
+
+  if (!expected.empty()) {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(expected, CacheDir);
+  }
+
+  {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(expected, CacheDir);
+  }
+
+  if (!OriginalStorage.empty()) {
+::setenv("XDG_CACHE_HOME", OriginalStorage.c_str(), 1);
+  } else {
+::unsetenv("XDG_CACHE_HOME");
+  }
+#endif
+}
+