bug#61637: 30.0.50; Fix Eglot tests that need HOME=~USER

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: João Távora <joaotavora@gmail.com>
>> Cc: "Basil L. Contovounesios" <contovob@tcd.ie>,  61637@debbugs.gnu.org
>> Date: Sat, 04 Mar 2023 01:04:45 +0000
>> 
>> I installed some language servers like clangd, rust-analyzer and pylsp,
>> and they need to a valid HOME to be able to function.
>
> Please tell more about this: what exactly do these servers need from
> the home directory?

Every server is different.  This is the complain for a pylsp installed
with 'pip install pylsp' for the regular user, which you can find in the
output of e.g.

   make -C test lisp/progmodes/eglot-tests SELECTOR='"basic-completions"'

This are the relevant parts of the output.  Here, it looks like the
pylsp trampoline in ~/.local/bin/pylsp can't find the main supporing
library, which probably lives around the corner in
~/.local/lib/python3.10/site-packages.

   [stderr] PWD: /tmp/eglot--fixturelfcOUE/project
   [stderr] XDG_CONFIG_HOME: /dev/null
   [stderr] HOME: /tmp/
   ....
   [stderr] EMACS_TEST_DIRECTORY: /home/capitaomorte/Source/Emacs/emacs/test
   [stderr] Traceback (most recent call last):
   [stderr]   File "/home/capitaomorte/.local/bin/pylsp", line 14, in <module>
   [stderr]     from pylsp.__main__ import main
   [stderr] ModuleNotFoundError: No module named 'pylsp'
   [internal] Sat Mar  4 11:31:48 2023:
   (:message "Connection state changed" :change "exited abnormally with code 
1\n")
    
   ----------b---y---e---b---y---e----------
   [eglot] Killing (something.py), wiping /tmp/eglot--fixturelfcOUE, restoring 
nil
   Test eglot-test-basic-completions backtrace:
     signal(error ("[eglot] -1: Server died"))

I suppose basil found something similar for rust-analyzer, though my
installation of it is system-wide, so I don't get that particular
problem.

>> This is acceptable for Eglot devs like me, but other devs that don't
>> know about this variable will still be confused. WDYT?
>
> As I said, I don't like the idea of using the user's real home
> directory for test purposes.  We could end up clobbering precious
> files there.  We could also have tests fail because some user setting
> in the home directory makes the test results unpredictable.

As I understand it, the concern of cloberring user customizations is
mostly related to Emacs' own packages like ido or auto-save-mode, some
of them do write files in ~/.emacs.d and similar.  That is reasonable.

But this is different IMO.  We're talking about user-installed language
servers, which presumably these users are already using (because they
installed them).  Only for the specific invocations of these servers is
HOME spoofed.  Overall I think the risk is low.  Eglot has had these
types of tests since practically the beginning and I've never had
complains of clobbered files.

>> > Why not do this the other way around: trick Eglot to think the home
>> > directory doesn't exist (as I understand it already knows how to
>> > handle this)?
>> 
>> I don't understand what you mean: this is entirely constrained to how
>> certain language servers work, so eglot.el itself can't handle it.
>
> In the original report, Basil sais:
>
>> The main problem is that:
>> 0. test/Makefile.in sets HOME=/nonexistent
>> 1. lisp/emacs-lisp/ert-x.el sets HOME=/tmp :(
>> 2. eglot--call-with-fixture tries to detect when HOME is nonexistent,
>>    but /tmp exists, so that's left unchanged
>> 3. The Rust tools look under HOME=/tmp for which toolchain to use, but
>>    the answer is under ~USER, so they give up
>
> I interpreted item 2 as meaning that eglot--call-with-fixture can cope
> with the situation where the user's home directory doesn't exist

Ah, I understand.  Indeed it could "cope", but only by doing exactly the
same kind of spoof (or rather, unspoof) that Basil was proposing.  But
at a certain point in Emacs-29 that unspoof stopped working.

In fact, I missed this in Basil's patch, but I think the correct fix is
actually to fix eglot--call-with-fixture directly, with Basil's better
technique of re-constructing $HOME.

diff --git a/test/lisp/progmodes/eglot-tests.el 
b/test/lisp/progmodes/eglot-tests.el
index 5d5de59a19a..5f5adc8e5b3 100644
--- a/test/lisp/progmodes/eglot-tests.el
+++ b/test/lisp/progmodes/eglot-tests.el
@@ -103,22 +103,21 @@ eglot--call-with-fixture
              (set (car spec) (cadr spec)))
             ((stringp (car spec)) (push spec file-specs))))
     (unwind-protect
-        (let* ((home (getenv "HOME"))
-               (process-environment
+        (let* ((process-environment
                 (append
                  `(;; Set XDF_CONFIG_HOME to /dev/null to prevent
-                   ;; user-configuration to have an influence on
+                   ;; user's configuration to have an influence on
                    ;; language servers. (See github#441)
                    "XDG_CONFIG_HOME=/dev/null"
                    ;; ... on the flip-side, a similar technique by
                    ;; Emacs's test makefiles means that HOME is set to
-                   ;; /nonexistent.  This breaks some common
-                   ;; installations for LSP servers like pylsp, making
-                   ;; these tests mostly useless, so we hack around it
-                   ;; here with a great big hack.
-                   ,(format "HOME=%s"
-                            (if (file-exists-p home) home
-                              (format "/home/%s" (getenv "USER")))))
+                   ;; /nonexistent or /tmp/.  This breaks some common
+                   ;; installations for LSP servers like pylsp, and
+                   ;; rust-analyzer, making these tests mostly
+                   ;; useless, so we hack around it here with a great
+                   ;; big hack.
+                   ,(format "HOME=%s" (expand-file-name (concat
+                                                         "~" 
(user-login-name)))))
                  process-environment))
                ;; Prevent "Can't guess python-indent-offset ..." messages.
                (python-indent-guess-indent-offset-verbose . nil)