linux: Fix "Failed to start language server" errors when starting Zed from .desktop file

[0xtimsb]

Closes #21406

Context:

A few weeks ago on Linux, we resolved an issue where users could not open more than one file from the file explorer. This was fixed by replacing zed-editor (zed binary in the code) with zed (cli binary in the code) in the .desktop file. The reason for this change was that using the cli to open files is more convenient - it determines weather to spawn a new Zed instance or use an existing one, if we use main binary instead it would throw error Zed is already running.

You can read the complete PR here: linux: Fix file not opening from file explorer.

While this fix resolved the original issue, it introduced a new one.

Problem:

When the cli binary is used, it assumes it is always being invoked from a terminal and relies on std::env::vars() to retrieve the environment variables needed to spawn Zed. These env vars are then passed to the worktree, and eventually, languages use the PATH from this env to find binaries. This leads to the "Failed to start language server" error when the .desktop entry is used on Linux.

Solution:

When the zed-editor binary is used, it uses some clever Unix-specific logic to retrieve the default shell (load_shell_from_passwd) and then fetch the env vars from that shell (load_login_shell_environment). This same logic should be used in the cli binary when it is invoked via a .desktop entry rather than from a terminal.

Approach:

I moved these two functions mentioned above to a utils file and reused them in cli binary to fetch env vars only on Linux when it is not run from a terminal. This provides missing paths, and fix the issue.

It is also possible to handle this in the zed-editor binary by modifying the logic in handle_cli_connection, where CliRequest::Open is processed. There we can discard incoming env, and use our logic. But discarding incoming envs felt weird, and I thought it's better to handle this at source.

Release Notes:

• Fixed Failed to start language server errors when starting from dekstop entry on Linux

[zed-industries-bot]

	Messages
📖	This PR includes links to the following GitHub Issues: #20070 If this PR aims to close an issue, please include a Closes #ISSUE line at the top of the PR body.

Generated by 🚫 dangerJS against 8d2f042

[SomeoneToIgnore]

Much simpler, thank you.

[mrnugget]

Hey! Found this commit after #9786 (comment) and I'm wondering whether we couldn't get away with a shorter solution.

Here's how the CLI/main-binary env handling works, essentially: the CLI passes an environment to the zed executable, if the zed executable doesn't get that, it runs the login-shell to get the environment, per project in the best case.

So what I'm thinking here is that instead of loading the login-shell env in the CLI and then passing it on, why not pass None instead if we detect that we're not in a TTY?

This, basically:

diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs
index 32c5bbb21b..4cb47c1f24 100644
--- a/crates/cli/src/main.rs
+++ b/crates/cli/src/main.rs
@@ -167,16 +167,15 @@ fn main() -> Result<()> {
         None
     };
 
+    let mut env = Some(std::env::vars().collect::<HashMap<_, _>>());
+
     // On Linux, desktop entry uses `cli` to spawn `zed`, so we need to load env vars from the shell
     // since it doesn't inherit env vars from the terminal.
     #[cfg(any(target_os = "linux", target_os = "freebsd"))]
     if !std::io::stdout().is_terminal() {
-        load_shell_from_passwd().log_err();
-        load_login_shell_environment().log_err();
+        env = None;
     }
 
-    let env = Some(std::env::vars().collect::<HashMap<_, _>>());
-
     let exit_status = Arc::new(Mutex::new(None));
     let mut paths = vec![];
     let mut urls = vec![];

Then we don't need to whole env-loading logic shared.

What do you think @SomeoneToIgnore @0xtimsb?

[SomeoneToIgnore]

Sounds great too, I'm somewhat limited in this field and do not see all possible ways to get it working.

[0xtimsb]

Hi @mrnugget,

I think what you're saying makes sense. I somehow missed the code below while working on the fix and assumed the only way to pass the right envs to the LSP was through the CLI. I didn’t realize there was already a fallback setup if env is None. This is a lot cleaner. Let me know if you’d like me to update or test this.

/// Returns the project environment, if possible.
/// If the project was opened from the CLI, then the inherited CLI environment is returned.
/// If it wasn't opened from the CLI, and a worktree is given, then a shell is spawned in
/// the worktree's path, to get environment variables as if the user has `cd`'d into
/// the worktrees path.
pub(crate) fn get_environment(

[mrnugget]

This is a lot cleaner. Let me know if you’d like me to update or test this.

If you want, that'd be neat! I think that comment ~could stay the same, if we add the note that "opened from the CLI" can mean that the Zed CLI binary was used, but not on the CLI.

[0xtimsb]

On it.