linux: Fix "Failed to start language server" errors when starting Zed…

… from .desktop file (zed-industries#22335)

Closes zed-industries#21406

Context:

A few weeks ago on Linux, we resolved an
[issue](zed-industries#20070) 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](zed-industries#21137).

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

crates/cli/src/main.rs

@@ -18,6 +18,12 @@ use std::{
 use tempfile::NamedTempFile;
 use util::paths::PathWithPosition;
 
+#[cfg(any(target_os = "linux", target_os = "freebsd"))]
+use {
+    std::io::IsTerminal,
+    util::{load_login_shell_environment, load_shell_from_passwd, ResultExt},
+};
+
 struct Detect;
 
 trait InstalledApp {
@@ -161,7 +167,16 @@ fn main() -> Result<()> {
         None
     };
 
+    // 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();
+    }
+
     let env = Some(std::env::vars().collect::<HashMap<_, _>>());
+
     let exit_status = Arc::new(Mutex::new(None));
     let mut paths = vec![];
     let mut urls = vec![];

crates/util/Cargo.toml

@@ -36,6 +36,9 @@ take-until = "0.2.0"
 tempfile = { workspace = true, optional = true }
 unicase.workspace = true
 
+[target.'cfg(unix)'.dependencies]
+libc.workspace = true
+
 [target.'cfg(windows)'.dependencies]
 tendril = "0.4.3"
 dunce = "1.0"

crates/util/src/util.rs

@@ -6,6 +6,7 @@ pub mod serde;
 #[cfg(any(test, feature = "test-support"))]
 pub mod test;
 
+use anyhow::{anyhow, Context as _, Result};
 use futures::Future;
 
 use itertools::Either;
@@ -110,6 +111,106 @@ where
     }
 }
 
+#[cfg(unix)]
+pub fn load_shell_from_passwd() -> Result<()> {
+    let buflen = match unsafe { libc::sysconf(libc::_SC_GETPW_R_SIZE_MAX) } {
+        n if n < 0 => 1024,
+        n => n as usize,
+    };
+    let mut buffer = Vec::with_capacity(buflen);
+
+    let mut pwd: std::mem::MaybeUninit<libc::passwd> = std::mem::MaybeUninit::uninit();
+    let mut result: *mut libc::passwd = std::ptr::null_mut();
+
+    let uid = unsafe { libc::getuid() };
+    let status = unsafe {
+        libc::getpwuid_r(
+            uid,
+            pwd.as_mut_ptr(),
+            buffer.as_mut_ptr() as *mut libc::c_char,
+            buflen,
+            &mut result,
+        )
+    };
+    let entry = unsafe { pwd.assume_init() };
+
+    anyhow::ensure!(
+        status == 0,
+        "call to getpwuid_r failed. uid: {}, status: {}",
+        uid,
+        status
+    );
+    anyhow::ensure!(!result.is_null(), "passwd entry for uid {} not found", uid);
+    anyhow::ensure!(
+        entry.pw_uid == uid,
+        "passwd entry has different uid ({}) than getuid ({}) returned",
+        entry.pw_uid,
+        uid,
+    );
+
+    let shell = unsafe { std::ffi::CStr::from_ptr(entry.pw_shell).to_str().unwrap() };
+    if env::var("SHELL").map_or(true, |shell_env| shell_env != shell) {
+        log::info!(
+            "updating SHELL environment variable to value from passwd entry: {:?}",
+            shell,
+        );
+        env::set_var("SHELL", shell);
+    }
+
+    Ok(())
+}
+
+pub fn load_login_shell_environment() -> Result<()> {
+    let marker = "ZED_LOGIN_SHELL_START";
+    let shell = env::var("SHELL").context(
+        "SHELL environment variable is not assigned so we can't source login environment variables",
+    )?;
+
+    // If possible, we want to `cd` in the user's `$HOME` to trigger programs
+    // such as direnv, asdf, mise, ... to adjust the PATH. These tools often hook
+    // into shell's `cd` command (and hooks) to manipulate env.
+    // We do this so that we get the env a user would have when spawning a shell
+    // in home directory.
+    let shell_cmd_prefix = std::env::var_os("HOME")
+        .and_then(|home| home.into_string().ok())
+        .map(|home| format!("cd '{home}';"));
+
+    // The `exit 0` is the result of hours of debugging, trying to find out
+    // why running this command here, without `exit 0`, would mess
+    // up signal process for our process so that `ctrl-c` doesn't work
+    // anymore.
+    // We still don't know why `$SHELL -l -i -c '/usr/bin/env -0'`  would
+    // do that, but it does, and `exit 0` helps.
+    let shell_cmd = format!(
+        "{}printf '%s' {marker}; /usr/bin/env; exit 0;",
+        shell_cmd_prefix.as_deref().unwrap_or("")
+    );
+
+    let output = std::process::Command::new(&shell)
+        .args(["-l", "-i", "-c", &shell_cmd])
+        .output()
+        .context("failed to spawn login shell to source login environment variables")?;
+    if !output.status.success() {
+        Err(anyhow!("login shell exited with error"))?;
+    }
+
+    let stdout = String::from_utf8_lossy(&output.stdout);
+
+    if let Some(env_output_start) = stdout.find(marker) {
+        let env_output = &stdout[env_output_start + marker.len()..];
+
+        parse_env_output(env_output, |key, value| env::set_var(key, value));
+
+        log::info!(
+            "set environment variables from shell:{}, path:{}",
+            shell,
+            env::var("PATH").unwrap_or_default(),
+        );
+    }
+
+    Ok(())
+}
+
 /// Parse the result of calling `usr/bin/env` with no arguments
 pub fn parse_env_output(env: &str, mut f: impl FnMut(String, String)) {
     let mut current_key: Option<String> = None;

crates/zed/Cargo.toml

@@ -68,7 +68,6 @@ language_models.workspace = true
 language_selector.workspace = true
 language_tools.workspace = true
 languages = { workspace = true, features = ["load-grammars"] }
-libc.workspace = true
 log.workspace = true
 markdown.workspace = true
 markdown_preview.workspace = true

crates/zed/src/main.rs

@@ -37,7 +37,6 @@ use settings::{
     handle_settings_file_changes, watch_config_file, InvalidSettingsError, Settings, SettingsStore,
 };
 use simplelog::ConfigBuilder;
-use smol::process::Command;
 use std::{
     env,
     fs::OpenOptions,
@@ -48,7 +47,7 @@ use std::{
 };
 use theme::{ActiveTheme, SystemAppearance, ThemeRegistry, ThemeSettings};
 use time::UtcOffset;
-use util::{maybe, parse_env_output, ResultExt, TryFutureExt};
+use util::{load_login_shell_environment, maybe, ResultExt, TryFutureExt};
 use uuid::Uuid;
 use welcome::{show_welcome_view, BaseKeymap, FIRST_OPEN};
 use workspace::{
@@ -63,6 +62,9 @@ use zed::{
 
 use crate::zed::inline_completion_registry;
 
+#[cfg(unix)]
+use util::load_shell_from_passwd;
+
 #[cfg(feature = "mimalloc")]
 #[global_allocator]
 static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc;
@@ -219,9 +221,9 @@ fn main() {
             .spawn(async {
                 #[cfg(unix)]
                 {
-                    load_shell_from_passwd().await.log_err();
+                    load_shell_from_passwd().log_err();
                 }
-                load_login_shell_environment().await.log_err();
+                load_login_shell_environment().log_err();
             })
             .detach()
     };
@@ -998,107 +1000,6 @@ fn init_stdout_logger() {
         .init();
 }
 
-#[cfg(unix)]
-async fn load_shell_from_passwd() -> Result<()> {
-    let buflen = match unsafe { libc::sysconf(libc::_SC_GETPW_R_SIZE_MAX) } {
-        n if n < 0 => 1024,
-        n => n as usize,
-    };
-    let mut buffer = Vec::with_capacity(buflen);
-
-    let mut pwd: std::mem::MaybeUninit<libc::passwd> = std::mem::MaybeUninit::uninit();
-    let mut result: *mut libc::passwd = std::ptr::null_mut();
-
-    let uid = unsafe { libc::getuid() };
-    let status = unsafe {
-        libc::getpwuid_r(
-            uid,
-            pwd.as_mut_ptr(),
-            buffer.as_mut_ptr() as *mut libc::c_char,
-            buflen,
-            &mut result,
-        )
-    };
-    let entry = unsafe { pwd.assume_init() };
-
-    anyhow::ensure!(
-        status == 0,
-        "call to getpwuid_r failed. uid: {}, status: {}",
-        uid,
-        status
-    );
-    anyhow::ensure!(!result.is_null(), "passwd entry for uid {} not found", uid);
-    anyhow::ensure!(
-        entry.pw_uid == uid,
-        "passwd entry has different uid ({}) than getuid ({}) returned",
-        entry.pw_uid,
-        uid,
-    );
-
-    let shell = unsafe { std::ffi::CStr::from_ptr(entry.pw_shell).to_str().unwrap() };
-    if env::var("SHELL").map_or(true, |shell_env| shell_env != shell) {
-        log::info!(
-            "updating SHELL environment variable to value from passwd entry: {:?}",
-            shell,
-        );
-        env::set_var("SHELL", shell);
-    }
-
-    Ok(())
-}
-
-async fn load_login_shell_environment() -> Result<()> {
-    let marker = "ZED_LOGIN_SHELL_START";
-    let shell = env::var("SHELL").context(
-        "SHELL environment variable is not assigned so we can't source login environment variables",
-    )?;
-
-    // If possible, we want to `cd` in the user's `$HOME` to trigger programs
-    // such as direnv, asdf, mise, ... to adjust the PATH. These tools often hook
-    // into shell's `cd` command (and hooks) to manipulate env.
-    // We do this so that we get the env a user would have when spawning a shell
-    // in home directory.
-    let shell_cmd_prefix = std::env::var_os("HOME")
-        .and_then(|home| home.into_string().ok())
-        .map(|home| format!("cd '{home}';"));
-
-    // The `exit 0` is the result of hours of debugging, trying to find out
-    // why running this command here, without `exit 0`, would mess
-    // up signal process for our process so that `ctrl-c` doesn't work
-    // anymore.
-    // We still don't know why `$SHELL -l -i -c '/usr/bin/env -0'`  would
-    // do that, but it does, and `exit 0` helps.
-    let shell_cmd = format!(
-        "{}printf '%s' {marker}; /usr/bin/env; exit 0;",
-        shell_cmd_prefix.as_deref().unwrap_or("")
-    );
-
-    let output = Command::new(&shell)
-        .args(["-l", "-i", "-c", &shell_cmd])
-        .output()
-        .await
-        .context("failed to spawn login shell to source login environment variables")?;
-    if !output.status.success() {
-        Err(anyhow!("login shell exited with error"))?;
-    }
-
-    let stdout = String::from_utf8_lossy(&output.stdout);
-
-    if let Some(env_output_start) = stdout.find(marker) {
-        let env_output = &stdout[env_output_start + marker.len()..];
-
-        parse_env_output(env_output, |key, value| env::set_var(key, value));
-
-        log::info!(
-            "set environment variables from shell:{}, path:{}",
-            shell,
-            env::var("PATH").unwrap_or_default(),
-        );
-    }
-
-    Ok(())
-}
-
 fn stdout_is_a_pty() -> bool {
     std::env::var(FORCE_CLI_MODE_ENV_VAR_NAME).ok().is_none() && std::io::stdout().is_terminal()
 }