[clangd] Store index in '.cache/clangd/index' instead of '.clangd/index'

[sammccall]

Summary

.clangd/index was well-intentioned in 2754942cbaef, but .clangd is the best
filename for the clangd config file (matching .clang-format and .clang-tidy).
And of course we can't have both .clangd/index and .clangd...

There are a few overlapping goals to satisfy:

• it should be clear from the directory name that this is transient data that is safe to delete at the cost of recomputation, i.e. a cache
• it should be easy and self-documenting to blacklist these files in .gitignore
• we should have some consistency between filenames in-tree and corresponding files in user storage (e.g. under XDG's ~/.cache/)
• we should be consistent across platforms (including windows, which doesn't have distinct cache vs config directories)

So the plan is:

$PROJECT/.clangd                    (project config)
$PROJECT/.cache/clangd/index/       (project index)
$PROJECT/.cache/clangd/modules/     (maybe in future)
$XDG_CONFIG_HOME/clangd/config.yaml (user config)
$XDG_CACHE_HOME/clangd/index/       (index of non-project files)
$XDG_CACHE_HOME/clangd/modules/     (maybe in future)

This is sensible if XDG_{CONFIG,CACHE}_HOME coincide, and has a simple
.gitignore rule going forward: .cache/.

The monorepo gitignore is updated to reflect the backwards-compatible practice:

ignore .clangd/ (with trailing slash) matching index files from clangd 9/10
ignore .cache matching index from clangd 11+, and potentially other tools.

The entries from llvm-project/llvm gitignore are removed (obsolete).

[kadircet]

.gitignore

56

the comment above says do not add trailing /, we've got no choice for .clangd/ but maybe drop that for .clangd-index ?

[sammccall]

Slightly different layout after getting input on discourse and elsewhere

[sammccall]

Updating description

[hokein]

looks good from my side.

.gitignore

59

I'm afraid that many projects have to update their .gitignore, but this is a tradeoff...

[kadircet]

.gitignore

58

why do we still need this ? i thought index (and other caches) would reside in .cache ?

clang-tools-extra/clangd/index/Background.h

63

nit: maybe talk about XDG_CACHE instead of home directory as fallback location.

[sammccall]

.gitignore

58

Otherwise we're going to end up with indexes from old versions of clangd checked in :-(

59

Yeah. This is a consequence of naming the config file .clangd, which I think is pretty desirable.

[davidvancleve]

Hey, quick question about this change:

I'm seeing .cache directories appear off of subdirectories too, not just my project root:

Untracked files:

31 #» .cache/
32 #» content/browser/frame_host/.cache/
33 #» content/public/browser/.cache/
34 #» content/test/.cache/
35 #» net/http/.cache/
36 #» services/network/.cache/
37 #» services/network/trust_tokens/.cache/

Is this working as intended?

[sammccall]

In D83099#2154185, @davidvancleve wrote:

Hey, quick question about this change:

I'm seeing .cache directories appear off of subdirectories too, not just my project root:

Untracked files:

31 #» .cache/
32 #» content/browser/frame_host/.cache/
33 #» content/public/browser/.cache/
34 #» content/test/.cache/
35 #» net/http/.cache/
36 #» services/network/.cache/
37 #» services/network/trust_tokens/.cache/

Is this working as intended?

Hmm, maybe not. Do these directories have compile_commands.json files? (That's what we treat as a "project root", it may be a subproject)
Did they previously have .clangd directories created for indexes? (This patch shouldn't have changed which directories have indexes placed in them, just the names of the index directories).

Sorry about the churn, in any case :-(

[davidvancleve]

Thanks for the quick response! Nope, no compile_commands.json files, and they didn't previously contain .clangd files.

[sammccall]

In D83099#2154416, @davidvancleve wrote:

Thanks for the quick response! Nope, no compile_commands.json files, and they didn't previously contain .clangd files.

Well, thanks for reporting this, this seems like a really bad bug, and we'd like to get it fixed on the release branch ASAP.

(Just to check - also no compile_flags.txt or other compilation database markers?)

Our best guess is this is a fairly long-standing bug in OverlayCDB: the storage location would be ".cache/clangd/index" relative to the *working* directory in some cases.
If this theory is correct:

• you'd be using an editor/plugin that sends compile commands over LSP (such as YCM + ycm_extra_conf). What are you using?
• there should be relatively few *.idx files inside the extra directories (the ones not near your compilation database), corresponding to files you've had open rather than the whole project
• we don't know why this wouldn't also have been happenening with the old .clangd directories - possible they were there but masked by .gitignore?

If this doesn't sound likely, it'd be really useful (as we haven't reproduced this) to add a log line if you can, and capture a log.
Immediately below the llvm::sys::path::append that was changed in this patch, if you could add:

log("BackgroundIndexStorage: File={0} SourceRoot={1} StorageDir={2}", File, PI->SourceRoot, StorageDir);

Then delete all the .cache directories, open clangd and open files until they come back, and grab the clangd stderr log (editors/plugins expose this in some way).
This should give us something to go on, else we'll have to keep guessing...

Cheers, Sam

[davidvancleve]

Yes, these two are exactly the case!

• you'd be using an editor/plugin that sends compile commands over LSP (such as YCM + ycm_extra_conf). What are you using?
• there should be relatively few *.idx files inside the extra directories (the ones not near your compilation database), corresponding to files you've had open rather than the whole project

I'm using YCM. We only had /.clangd/ in our gitignore, which AIUI would have only been ignoring a clangd at the level of the project root; I certainly never noticed any .clangd folders in subdirectories.

[sammccall]

In D83099#2155900, @davidvancleve wrote:

Yes, these two are exactly the case!

• you'd be using an editor/plugin that sends compile commands over LSP (such as YCM + ycm_extra_conf). What are you using?
• there should be relatively few *.idx files inside the extra directories (the ones not near your compilation database), corresponding to files you've had open rather than the whole project

Well, that's a relief :-)
46c921003c2ce5f1cdc4de9ef613eb001980780c should fix this then, and we'll get it cherrypicked to the release branch.
Please let us know if you see this again after that commit!

I'm using YCM. We only had /.clangd/ in our gitignore, which AIUI would have only been ignoring a clangd at the level of the project root; I certainly never noticed any .clangd folders in subdirectories.

Yeah, the leading slash shoud do that... I'm stumped then, but I'm also fairly confident this is fixed.

[sammccall]

https://bugs.llvm.org/show_bug.cgi?id=46754 to get the fixed merged

[davidvancleve]

Super! Once we pull in that version (unsure of the latency; this is my first time reporting an issue with clangd), I'll be sure to come back and confirm that the fix is working for me. Thank you again for the quick turnaround!

[davidvancleve]

Seems like it's working as expected now! Thanks again.

.gitignore

 /.idea
 
 #==============================================================================#
 # Directories to ignore (do not add trailing '/'s, they skip symlinks).
 #==============================================================================#
 # VS2017 and VSCode config files.
 .vscode
 .vs
 # clangd index
-.clangd
+.clangd-index/
+.clangd/
 # static analyzer regression testing project files
 /clang/utils/analyzer/projects/*/CachedSource
 /clang/utils/analyzer/projects/*/PatchedSource
 /clang/utils/analyzer/projects/*/ScanBuildResults
-/clang/utils/analyzer/projects/*/RefScanBuildResults
+/clang/utils/analyzer/projects/*/RefScanBuildResults
 

clang-tools-extra/clangd/index/Background.h

public:
   loadShard(llvm::StringRef ShardIdentifier) const = 0;
 
   // The factory provides storage for each File.
   // It keeps ownership of the storage instances, and should manage caching
   // itself. Factory must be threadsafe and never returns nullptr.
   using Factory = llvm::unique_function<BackgroundIndexStorage *(PathRef)>;
 
   // Creates an Index Storage that saves shards into disk. Index storage uses
-  // CDBDirectory + ".clangd/index/" as the folder to save shards. CDBDirectory
+  // CDBDirectory + ".clangd-index/" as the folder to save shards. CDBDirectory
   // is the first directory containing a CDB in parent directories of a file, or
   // user's home directory if none was found, e.g. standard library headers.
   static Factory createDiskBackedStorageFactory(
       std::function<llvm::Optional<ProjectInfo>(PathRef)> GetProjectInfo);
 };
 
 // A priority queue of tasks which can be run on (external) worker threads.
 class BackgroundQueue {
 public:
   /// A work item on the thread pool's queue.
   struct Task {
     explicit Task(std::function<void()> Run) : Run(std::move(Run)) {}

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

 // Uses disk as a storage for index shards.
 class DiskBackedIndexStorage : public BackgroundIndexStorage {
   std::string DiskShardRoot;
 
 public:
   // 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);
+    std::error_code EC = llvm::sys::fs::create_directory(DiskShardRoot);
     if (EC != OK) {
       elog("Failed to create directory {0} for index storage: {1}",
            DiskShardRoot, EC.message());
     }
   }
 
   std::unique_ptr<IndexFileIn>
   loadShard(llvm::StringRef ShardIdentifier) const override {
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.
+// 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> FallbackDir;
     if (llvm::sys::path::cache_directory(FallbackDir))
-      llvm::sys::path::append(FallbackDir, "clangd", "index");
+      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);
     llvm::SmallString<128> StorageDir(FallbackDir);
     if (auto PI = GetProjectInfo(File)) {
       StorageDir = PI->SourceRoot;
-      llvm::sys::path::append(StorageDir, ".clangd", "index");
+      llvm::sys::path::append(StorageDir, ".clangd-index");
     }
     auto &IndexStorage = IndexStorageMap[StorageDir];
     if (!IndexStorage)
       IndexStorage = create(StorageDir);
     return IndexStorage.get();
   }
 
 private:

clang-tools-extra/clangd/test/background-index.test

 
 # We're editing bar.cpp, which includes foo.h.
 # foo() is declared in foo.h and defined in foo.cpp.
 # The background index should allow us to go-to-definition on foo().
 # We should also see indexing progress notifications.
 # RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck %t/definition.jsonrpc --check-prefixes=CHECK,BUILD
 
 # Test that the index is writing files in the expected location.
-# RUN: ls %t/.clangd/index/foo.cpp.*.idx
-# RUN: ls %t/sub_dir/.clangd/index/foo.h.*.idx
+# RUN: ls %t/.clangd-index/foo.cpp.*.idx
+# RUN: ls %t/sub_dir/.clangd-index/foo.h.*.idx
 
 # Test the index is read from disk: delete code and restart clangd.
 # RUN: rm %t/foo.cpp
 # RUN: clangd -background-index -lit-test < %t/definition.jsonrpc | FileCheck %t/definition.jsonrpc --check-prefixes=CHECK,USE

llvm/.gitignore

 tools/polly
 # avrlit, which is tracked independently.
 tools/avrlit
 # Sphinx build tree, if building in-source dir.
 docs/_build
 # VS2017 and VSCode config files.
 .vscode
 .vs
-# clangd index
-.clangd
 
 #==============================================================================#
 # Files created in tree by the Go bindings.
 #==============================================================================#
 bindings/go/llvm/llvm_config.go
 bindings/go/llvm/workdir
 
 #==============================================================================#