This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by sammccall on Jul 2 2020, 2:56 PM.

Details

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

Diff Detail

Event Timeline

sammccall created this revision.Jul 2 2020, 2:56 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 2 2020, 2:56 PM
kadircet added inline comments.Jul 3 2020, 2:12 AM
.gitignore
55–56

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

sammccall updated this revision to Diff 275643.Jul 6 2020, 3:44 AM

Slightly different layout after getting input on discourse and elsewhere

sammccall marked an inline comment as done.Jul 6 2020, 3:45 AM
sammccall updated this revision to Diff 275645.Jul 6 2020, 3:47 AM
sammccall retitled this revision from Revert "[clangd] Store index in '.clangd/index' instead of '.clangd-index'" to [clangd] Store index in '.cache/clangd/index' instead of '.clangd/index'.
sammccall edited the summary of this revision. (Show Details)

Updating description

hokein accepted this revision.Jul 7 2020, 12:44 AM

looks good from my side.

.gitignore
58

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

This revision is now accepted and ready to land.Jul 7 2020, 12:44 AM
kadircet added inline comments.Jul 7 2020, 4:00 AM
.gitignore
57

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

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

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

sammccall marked 5 inline comments as done.Jul 7 2020, 4:55 AM
sammccall added inline comments.
.gitignore
57

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

58

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

This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.

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?

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

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

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 added a comment.EditedJul 16 2020, 8:21 AM

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.

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.

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!

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