Re: master 08c80c45dde: Don't use file-truepath in Eglot (bug#70036)

[João Távora]

On Wed, Apr 17, 2024 at 7:41 PM Theodor Thornhill <theo@thornhill.no> wrote:
have been looking for an Eglot maintainer but everyone I contacted

> If you want I can do this - so that you don't have to. I've been
> considering it for some time after I responded to your call for
> maintainers.

OK, I've added you to the GitHub repo as a collaborator.  Let me know
off-list what your intentions are (to phase it out, keep it and take
it over, or whatever) and let's see how it goes.

> > The change seems well structured, well coded, and well described in the
> > commit message, so I could understand it easily.  Do keep that up.
> >
> > But of course going from file-buffer-visiting to something else
> > whose underlying implementation is faster but doesn't chase
> > symlinks is probably going to have some kind of functional implication
> > right?  I wonder if (or rather "I hope that") you guys considered it.
>
> I did - and afaict it has no such implications. It

I've identified at least one implication.  A rather obvious one, given
that you've effectively removed symlink-awareness from Eglot.

When working in a project with symlinks and visiting such symlinks,
the LSP server  is now informed (via LSP 'didOpen')  of the symlinked
file as if it was its own file, whereas before your changes Eglot
sent over the true name of the file.   You can see this easily
from M-x eglot-events-buffer.

What does this mean in practice?  Many things potentially, but
at least this one: I've run this experiment:  In a new dir, create
a lib.h file with this silly content:

  int foo();

Then create a main.cpp like this:

  #include "lib.h"
  int main() { return foo(); }

Then create a mainlink.cpp symbolic link to main.cpp

Then M-x eglot (launches clangd server to manage the new directory
as a project).

Let's say that during the Eglot session I visit both main.cpp
and mainlink.cpp in different buffers (either because I don't visit
them at the same time or because find-file-existing-other-name is
nil).  Then I press M-? on lib.h's foo() to tell me who references it.

Before you change, Eglot will -- correctly -- tell me there  is a single
user of lib.h's foo() function in my project.

After your change, it tells me there are two users.  This is wrong,
there is only one.

It could be that some servers with direct access to the file system
can deduplicate the information and add back the symlink smarts.

But clangd doesn't do this, and in general servers _can't_ do
this because LSP models a virtual file system.

And for symlinks to large enough files, I'd be surprised if this
doesn't slow down the performance of the server because it has to
analyse what it is told is a completely new file.

So this seems like a pretty big flaw to me after just minimal
surface scratching.  Please reinstate the previous code.

We can resume discussion of your performance problems in the
bug report.  I've seen the profile reports and file-truename is
not thaat big of a problem.  But nevertheless I think I can
help you think of other places to cache things.  Caching
eglot--TextDocumentIdentifier per-buffer seems like a much
easier way to solve your problems.

João