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

[David Van Cleve via Phabricator via cfe-commits]

davidvancleve added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

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

[David Van Cleve via Phabricator via cfe-commits]

davidvancleve added a comment.

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!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

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

[David Van Cleve via Phabricator via cfe-commits]

davidvancleve added a comment.

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




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

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

[David Van Cleve via Phabricator via cfe-commits]

davidvancleve added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

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

[David Van Cleve via Phabricator via cfe-commits]

davidvancleve added a comment.

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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

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

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
Closed by commit rG9b55bc4d1197: [clangd] Store index in 
.cache/clangd/index instead of .clangd/index (authored 
by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D83099?vs=275645=276027#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099

Files:
  .gitignore
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
  clang-tools-extra/clangd/test/background-index.test
  llvm/.gitignore


Index: llvm/.gitignore
===
--- llvm/.gitignore
+++ llvm/.gitignore
@@ -59,8 +59,6 @@
 # VS2017 and VSCode config files.
 .vscode
 .vs
-# clangd index
-.clangd
 
 
#==#
 # Files created in tree by the Go bindings.
Index: clang-tools-extra/clangd/test/background-index.test
===
--- clang-tools-extra/clangd/test/background-index.test
+++ clang-tools-extra/clangd/test/background-index.test
@@ -15,8 +15,8 @@
 # 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/.cache/clangd/index/foo.cpp.*.idx
+# RUN: ls %t/sub_dir/.cache/clangd/index/foo.h.*.idx
 
 # Test the index is read from disk: delete code and restart clangd.
 # RUN: rm %t/foo.cpp
Index: clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
===
--- clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
+++ clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
@@ -95,8 +95,8 @@
 };
 
 // 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/.cache/clangd/index/.
+// When no root is found, the fallback path is ~/.cache/clangd/index/.
 class DiskBackedIndexStorageManager {
 public:
   DiskBackedIndexStorageManager(
@@ -115,7 +115,7 @@
 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, ".cache", "clangd", "index");
 }
 auto  = IndexStorageMap[StorageDir];
 if (!IndexStorage)
Index: clang-tools-extra/clangd/index/Background.h
===
--- clang-tools-extra/clangd/index/Background.h
+++ clang-tools-extra/clangd/index/Background.h
@@ -56,9 +56,9 @@
   using Factory = llvm::unique_function;
 
   // Creates an Index Storage that saves shards into disk. Index storage uses
-  // 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.
+  // CDBDirectory + ".cache/clangd/index/" as the folder to save shards.
+  // CDBDirectory is the first directory containing a CDB in parent directories
+  // of a file, or user cache directory if none was found, e.g. stdlib headers.
   static Factory createDiskBackedStorageFactory(
   std::function(PathRef)> GetProjectInfo);
 };
Index: .gitignore
===
--- .gitignore
+++ .gitignore
@@ -53,10 +53,11 @@
 # VS2017 and VSCode config files.
 .vscode
 .vs
-# clangd index
-.clangd
+# clangd index. (".clangd" is a config file now, thus trailing slash)
+.clangd/
+.cache
 # 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
\ No newline at end of file
+/clang/utils/analyzer/projects/*/RefScanBuildResults


Index: llvm/.gitignore
===
--- llvm/.gitignore
+++ llvm/.gitignore
@@ -59,8 +59,6 @@
 # VS2017 and VSCode config files.
 .vscode
 .vs
-# clangd index
-.clangd
 
 #==#
 # Files created in tree by the Go bindings.
Index: clang-tools-extra/clangd/test/background-index.test
===
--- clang-tools-extra/clangd/test/background-index.test
+++ clang-tools-extra/clangd/test/background-index.test
@@ 

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

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 5 inline comments as done.
sammccall added inline comments.



Comment at: .gitignore:57
+# clangd index. (".clangd" is a config file now, thus trailing slash)
+.clangd/
+.cache

kadircet wrote:
> why do we still need this ? i thought index (and other caches) would reside 
> in `.cache` ?
Otherwise we're going to end up with indexes from old versions of clangd 
checked in :-(



Comment at: .gitignore:58
+.clangd/
+.cache
 # static analyzer regression testing project files

hokein wrote:
> I'm afraid that many projects have to update their `.gitignore`, but this is 
> a tradeoff...
Yeah. This is a consequence of naming the config file `.clangd`, which I think 
is pretty desirable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: .gitignore:57
+# clangd index. (".clangd" is a config file now, thus trailing slash)
+.clangd/
+.cache

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



Comment at: clang-tools-extra/clangd/index/Background.h:61
+  // 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. stdlib 
headers.
   static Factory createDiskBackedStorageFactory(

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

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

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

looks good from my side.




Comment at: .gitignore:58
+.clangd/
+.cache
 # static analyzer regression testing project files

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099



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

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

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 275645.
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.
sammccall added a comment.

Updating description


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83099

Files:
  .gitignore
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
  clang-tools-extra/clangd/test/background-index.test
  llvm/.gitignore


Index: llvm/.gitignore
===
--- llvm/.gitignore
+++ llvm/.gitignore
@@ -59,8 +59,6 @@
 # VS2017 and VSCode config files.
 .vscode
 .vs
-# clangd index
-.clangd
 
 
#==#
 # Files created in tree by the Go bindings.
Index: clang-tools-extra/clangd/test/background-index.test
===
--- clang-tools-extra/clangd/test/background-index.test
+++ clang-tools-extra/clangd/test/background-index.test
@@ -15,8 +15,8 @@
 # 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/.cache/clangd/index/foo.cpp.*.idx
+# RUN: ls %t/sub_dir/.cache/clangd/index/foo.h.*.idx
 
 # Test the index is read from disk: delete code and restart clangd.
 # RUN: rm %t/foo.cpp
Index: clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
===
--- clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
+++ clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
@@ -95,8 +95,8 @@
 };
 
 // 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/.cache/clangd/index/.
+// When no root is found, the fallback path is ~/.cache/clangd/index/.
 class DiskBackedIndexStorageManager {
 public:
   DiskBackedIndexStorageManager(
@@ -115,7 +115,7 @@
 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, ".cache", "clangd", "index");
 }
 auto  = IndexStorageMap[StorageDir];
 if (!IndexStorage)
Index: clang-tools-extra/clangd/index/Background.h
===
--- clang-tools-extra/clangd/index/Background.h
+++ clang-tools-extra/clangd/index/Background.h
@@ -56,9 +56,9 @@
   using Factory = llvm::unique_function;
 
   // Creates an Index Storage that saves shards into disk. Index storage uses
-  // 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.
+  // CDBDirectory + ".cache/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. stdlib 
headers.
   static Factory createDiskBackedStorageFactory(
   std::function(PathRef)> GetProjectInfo);
 };
Index: .gitignore
===
--- .gitignore
+++ .gitignore
@@ -53,10 +53,11 @@
 # VS2017 and VSCode config files.
 .vscode
 .vs
-# clangd index
-.clangd
+# clangd index. (".clangd" is a config file now, thus trailing slash)
+.clangd/
+.cache
 # 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
\ No newline at end of file
+/clang/utils/analyzer/projects/*/RefScanBuildResults


Index: llvm/.gitignore
===
--- llvm/.gitignore
+++ llvm/.gitignore
@@ -59,8 +59,6 @@
 # VS2017 and VSCode config files.
 .vscode
 .vs
-# clangd index
-.clangd
 
 #==#
 # Files created in tree by the Go bindings.
Index: clang-tools-extra/clangd/test/background-index.test
===
--- clang-tools-extra/clangd/test/background-index.test
+++