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

Closes #21406

Context:

A few weeks ago on Linux, we resolved an
[issue](https://github.com/zed-industries/zed/issues/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](https://github.com/zed-industries/zed/pull/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
This commit is contained in:
tims 2024-12-22 15:35:52 +05:30 committed by GitHub
parent a2022d7da3
commit 204af9cac5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 126 additions and 107 deletions

2
Cargo.lock generated
View file

@ -13984,6 +13984,7 @@ dependencies = [
"git2",
"globset",
"itertools 0.13.0",
"libc",
"log",
"rand 0.8.5",
"regex",
@ -16045,7 +16046,6 @@ dependencies = [
"language_selector",
"language_tools",
"languages",
"libc",
"log",
"markdown",
"markdown_preview",

View file

@ -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![];

View file

@ -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"

View file

@ -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;

View file

@ -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

View file

@ -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()
}