diff --git a/crates/activity_indicator/src/activity_indicator.rs b/crates/activity_indicator/src/activity_indicator.rs index 58e737dfcf..84d1291dad 100644 --- a/crates/activity_indicator/src/activity_indicator.rs +++ b/crates/activity_indicator/src/activity_indicator.rs @@ -11,8 +11,7 @@ use language::{ LanguageServerStatusUpdate, ServerHealth, }; use project::{ - EnvironmentErrorMessage, LanguageServerProgress, LspStoreEvent, Project, - ProjectEnvironmentEvent, + LanguageServerProgress, LspStoreEvent, Project, ProjectEnvironmentEvent, git_store::{GitStoreEvent, Repository}, }; use smallvec::SmallVec; @@ -327,20 +326,20 @@ impl ActivityIndicator { .flatten() } - fn pending_environment_error<'a>(&'a self, cx: &'a App) -> Option<&'a EnvironmentErrorMessage> { + fn pending_environment_error<'a>(&'a self, cx: &'a App) -> Option<&'a String> { self.project.read(cx).peek_environment_error(cx) } fn content_to_render(&mut self, cx: &mut Context) -> Option { // Show if any direnv calls failed - if let Some(error) = self.pending_environment_error(cx) { + if let Some(message) = self.pending_environment_error(cx) { return Some(Content { icon: Some( Icon::new(IconName::Warning) .size(IconSize::Small) .into_any_element(), ), - message: error.0.clone(), + message: message.clone(), on_click: Some(Arc::new(move |this, window, cx| { this.project.update(cx, |project, cx| { project.pop_environment_error(cx); diff --git a/crates/project/src/direnv.rs b/crates/project/src/direnv.rs deleted file mode 100644 index 75c381dda9..0000000000 --- a/crates/project/src/direnv.rs +++ /dev/null @@ -1,82 +0,0 @@ -use crate::environment::EnvironmentErrorMessage; -use std::process::ExitStatus; - -use {collections::HashMap, std::path::Path, util::ResultExt}; - -#[derive(Clone)] -pub enum DirenvError { - NotFound, - FailedRun, - NonZeroExit(ExitStatus, Vec), - InvalidJson, -} - -impl From for Option { - fn from(value: DirenvError) -> Self { - match value { - DirenvError::NotFound => None, - DirenvError::FailedRun | DirenvError::NonZeroExit(_, _) => { - Some(EnvironmentErrorMessage(String::from( - "Failed to run direnv. See logs for more info", - ))) - } - DirenvError::InvalidJson => Some(EnvironmentErrorMessage(String::from( - "Direnv returned invalid json. See logs for more info", - ))), - } - } -} - -pub async fn load_direnv_environment( - env: &HashMap, - dir: &Path, -) -> Result>, DirenvError> { - let Ok(direnv_path) = which::which("direnv") else { - return Err(DirenvError::NotFound); - }; - - let args = &["export", "json"]; - let Some(direnv_output) = smol::process::Command::new(&direnv_path) - .args(args) - .envs(env) - .env("TERM", "dumb") - .current_dir(dir) - .output() - .await - .log_err() - else { - return Err(DirenvError::FailedRun); - }; - - if !direnv_output.status.success() { - log::error!( - "Loading direnv environment failed ({}), stderr: {}", - direnv_output.status, - String::from_utf8_lossy(&direnv_output.stderr) - ); - return Err(DirenvError::NonZeroExit( - direnv_output.status, - direnv_output.stderr, - )); - } - - let output = String::from_utf8_lossy(&direnv_output.stdout); - if output.is_empty() { - // direnv outputs nothing when it has no changes to apply to environment variables - return Ok(HashMap::default()); - } - - match serde_json::from_str(&output) { - Ok(env) => Ok(env), - Err(err) => { - log::error!( - "json parse error {}, while parsing output of `{} {}`:\n{}", - err, - direnv_path.display(), - args.join(" "), - output - ); - Err(DirenvError::InvalidJson) - } - } -} diff --git a/crates/project/src/environment.rs b/crates/project/src/environment.rs index c888cdd11f..519000f6a6 100644 --- a/crates/project/src/environment.rs +++ b/crates/project/src/environment.rs @@ -1,4 +1,5 @@ -use futures::{FutureExt, future::Shared}; +use anyhow::{Context as _, bail}; +use futures::{FutureExt, StreamExt as _, channel::mpsc, future::Shared}; use language::Buffer; use remote::RemoteClient; use rpc::proto::{self, REMOTE_SERVER_PROJECT_ID}; @@ -20,7 +21,9 @@ pub struct ProjectEnvironment { cli_environment: Option>, local_environments: HashMap<(Shell, Arc), Shared>>>>, remote_environments: HashMap<(Shell, Arc), Shared>>>>, - environment_error_messages: VecDeque, + environment_error_messages: VecDeque, + environment_error_messages_tx: mpsc::UnboundedSender, + _tasks: Vec>, } pub enum ProjectEnvironmentEvent { @@ -30,12 +33,24 @@ pub enum ProjectEnvironmentEvent { impl EventEmitter for ProjectEnvironment {} impl ProjectEnvironment { - pub fn new(cli_environment: Option>) -> Self { + pub fn new(cli_environment: Option>, cx: &mut Context) -> Self { + let (tx, mut rx) = mpsc::unbounded(); + let task = cx.spawn(async move |this, cx| { + while let Some(message) = rx.next().await { + this.update(cx, |this, cx| { + this.environment_error_messages.push_back(message); + cx.emit(ProjectEnvironmentEvent::ErrorsUpdated); + }) + .ok(); + } + }); Self { cli_environment, local_environments: Default::default(), remote_environments: Default::default(), environment_error_messages: Default::default(), + environment_error_messages_tx: tx, + _tasks: vec![task], } } @@ -128,7 +143,37 @@ impl ProjectEnvironment { self.local_environments .entry((shell.clone(), abs_path.clone())) .or_insert_with(|| { - get_local_directory_environment_impl(shell, abs_path.clone(), cx).shared() + let load_direnv = ProjectSettings::get_global(cx).load_direnv.clone(); + let shell = shell.clone(); + let tx = self.environment_error_messages_tx.clone(); + cx.spawn(async move |_, cx| { + let mut shell_env = cx + .background_spawn(load_directory_shell_environment( + shell, + abs_path.clone(), + load_direnv, + tx, + )) + .await + .log_err(); + + if let Some(shell_env) = shell_env.as_mut() { + let path = shell_env + .get("PATH") + .map(|path| path.as_str()) + .unwrap_or_default(); + log::info!( + "using project environment variables shell launched in {:?}. PATH={:?}", + abs_path, + path + ); + + set_origin_marker(shell_env, EnvironmentOrigin::WorktreeShell); + } + + shell_env + }) + .shared() }) .clone() } @@ -165,11 +210,11 @@ impl ProjectEnvironment { .clone() } - pub fn peek_environment_error(&self) -> Option<&EnvironmentErrorMessage> { + pub fn peek_environment_error(&self) -> Option<&String> { self.environment_error_messages.front() } - pub fn pop_environment_error(&mut self) -> Option { + pub fn pop_environment_error(&mut self) -> Option { self.environment_error_messages.pop_front() } } @@ -194,125 +239,72 @@ impl From for String { } } -#[derive(Debug)] -pub struct EnvironmentErrorMessage(pub String); - -impl std::fmt::Display for EnvironmentErrorMessage { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.0) - } -} - -impl EnvironmentErrorMessage { - #[allow(dead_code)] - fn from_str(s: &str) -> Self { - Self(String::from(s)) - } -} - async fn load_directory_shell_environment( - shell: &Shell, - abs_path: &Path, - load_direnv: &DirenvSettings, -) -> ( - Option>, - Option, -) { - match smol::fs::metadata(abs_path).await { - Ok(meta) => { - let dir = if meta.is_dir() { - abs_path - } else if let Some(parent) = abs_path.parent() { - parent - } else { - return ( - None, - Some(EnvironmentErrorMessage(format!( - "Failed to load shell environment in {}: not a directory", - abs_path.display() - ))), - ); - }; + shell: Shell, + abs_path: Arc, + load_direnv: DirenvSettings, + tx: mpsc::UnboundedSender, +) -> anyhow::Result> { + let meta = smol::fs::metadata(&abs_path).await.with_context(|| { + tx.unbounded_send(format!("Failed to open {}", abs_path.display())) + .ok(); + format!("stat {abs_path:?}") + })?; - load_shell_environment(shell, dir, load_direnv).await - } - Err(err) => ( - None, - Some(EnvironmentErrorMessage(format!( - "Failed to load shell environment in {}: {}", - abs_path.display(), - err - ))), - ), - } -} + let dir = if meta.is_dir() { + abs_path.clone() + } else { + abs_path + .parent() + .with_context(|| { + tx.unbounded_send(format!("Failed to open {}", abs_path.display())) + .ok(); + format!("getting parent of {abs_path:?}") + })? + .into() + }; -async fn load_shell_environment( - shell: &Shell, - dir: &Path, - load_direnv: &DirenvSettings, -) -> ( - Option>, - Option, -) { - use crate::direnv::load_direnv_environment; - use util::shell_env; - - if cfg!(any(test, feature = "test-support")) { - let fake_env = [("ZED_FAKE_TEST_ENV".into(), "true".into())] - .into_iter() - .collect(); - (Some(fake_env), None) - } else if cfg!(target_os = "windows") { + if cfg!(target_os = "windows") { + // Note: direnv is not available on Windows, so we skip direnv processing + // and just return the shell environment let (shell, args) = shell.program_and_args(); - let mut envs = match shell_env::capture(shell, args, dir).await { - Ok(envs) => envs, - Err(err) => { - util::log_err(&err); - return ( - None, - Some(EnvironmentErrorMessage(format!( - "Failed to load environment variables: {}", - err - ))), - ); - } - }; + let mut envs = util::shell_env::capture(shell.clone(), args, abs_path) + .await + .with_context(|| { + tx.unbounded_send("Failed to load environment variables".into()) + .ok(); + format!("capturing shell environment with {shell:?}") + })?; if let Some(path) = envs.remove("Path") { // windows env vars are case-insensitive, so normalize the path var // so we can just assume `PATH` in other places envs.insert("PATH".into(), path); } - - // Note: direnv is not available on Windows, so we skip direnv processing - // and just return the shell environment - (Some(envs), None) + Ok(envs) } else { - let dir_ = dir.to_owned(); let (shell, args) = shell.program_and_args(); - let mut envs = match shell_env::capture(shell, args, &dir_).await { - Ok(envs) => envs, - Err(err) => { - util::log_err(&err); - return ( - None, - Some(EnvironmentErrorMessage::from_str( - "Failed to load environment variables. See log for details", - )), - ); - } - }; + let mut envs = util::shell_env::capture(shell.clone(), args, abs_path) + .await + .with_context(|| { + tx.unbounded_send("Failed to load environment variables".into()) + .ok(); + format!("capturing shell environment with {shell:?}") + })?; // If the user selects `Direct` for direnv, it would set an environment // variable that later uses to know that it should not run the hook. // We would include in `.envs` call so it is okay to run the hook // even if direnv direct mode is enabled. - let (direnv_environment, direnv_error) = match load_direnv { - DirenvSettings::ShellHook => (None, None), - DirenvSettings::Direct => match load_direnv_environment(&envs, dir).await { - Ok(env) => (Some(env), None), - Err(err) => (None, err.into()), - }, + let direnv_environment = match load_direnv { + DirenvSettings::ShellHook => None, + DirenvSettings::Direct => load_direnv_environment(&envs, &dir) + .await + .with_context(|| { + tx.unbounded_send("Failed to load direnv environment".into()) + .ok(); + "load direnv environment" + }) + .log_err(), }; if let Some(direnv_environment) = direnv_environment { for (key, value) in direnv_environment { @@ -324,51 +316,39 @@ async fn load_shell_environment( } } - (Some(envs), direnv_error) + Ok(envs) } } -fn get_local_directory_environment_impl( - shell: &Shell, - abs_path: Arc, - cx: &Context, -) -> Task>> { - let load_direnv = ProjectSettings::get_global(cx).load_direnv.clone(); +async fn load_direnv_environment( + env: &HashMap, + dir: &Path, +) -> anyhow::Result>> { + let direnv_path = which::which("direnv").context("finding direnv binary")?; - let shell = shell.clone(); - cx.spawn(async move |this, cx| { - let (mut shell_env, error_message) = cx - .background_spawn({ - let abs_path = abs_path.clone(); - async move { - load_directory_shell_environment(&shell, &abs_path, &load_direnv).await - } - }) - .await; + let args = &["export", "json"]; + let direnv_output = smol::process::Command::new(&direnv_path) + .args(args) + .envs(env) + .env("TERM", "dumb") + .current_dir(dir) + .output() + .await + .context("running direnv")?; - if let Some(shell_env) = shell_env.as_mut() { - let path = shell_env - .get("PATH") - .map(|path| path.as_str()) - .unwrap_or_default(); - log::info!( - "using project environment variables shell launched in {:?}. PATH={:?}", - abs_path, - path - ); + if !direnv_output.status.success() { + bail!( + "Loading direnv environment failed ({}), stderr: {}", + direnv_output.status, + String::from_utf8_lossy(&direnv_output.stderr) + ); + } - set_origin_marker(shell_env, EnvironmentOrigin::WorktreeShell); - } + let output = String::from_utf8_lossy(&direnv_output.stdout); + if output.is_empty() { + // direnv outputs nothing when it has no changes to apply to environment variables + return Ok(HashMap::default()); + } - if let Some(error) = error_message { - this.update(cx, |this, cx| { - log::error!("{error}"); - this.environment_error_messages.push_back(error); - cx.emit(ProjectEnvironmentEvent::ErrorsUpdated) - }) - .log_err(); - } - - shell_env - }) + serde_json::from_str(&output).context("parsing direnv json") } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index f9a3f20fa7..3a141da70a 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -23,11 +23,10 @@ pub mod worktree_store; #[cfg(test)] mod project_tests; -mod direnv; mod environment; use buffer_diff::BufferDiff; use context_server_store::ContextServerStore; -pub use environment::{EnvironmentErrorMessage, ProjectEnvironmentEvent}; +pub use environment::ProjectEnvironmentEvent; use git::repository::get_git_committer; use git_store::{Repository, RepositoryId}; pub mod search_history; @@ -1071,7 +1070,7 @@ impl Project { let context_server_store = cx.new(|cx| ContextServerStore::new(worktree_store.clone(), weak_self, cx)); - let environment = cx.new(|_| ProjectEnvironment::new(env)); + let environment = cx.new(|cx| ProjectEnvironment::new(env, cx)); let manifest_tree = ManifestTree::new(worktree_store.clone(), cx); let toolchain_store = cx.new(|cx| { ToolchainStore::local( @@ -1306,7 +1305,7 @@ impl Project { cx.subscribe(&settings_observer, Self::on_settings_observer_event) .detach(); - let environment = cx.new(|_| ProjectEnvironment::new(None)); + let environment = cx.new(|cx| ProjectEnvironment::new(None, cx)); let lsp_store = cx.new(|cx| { LspStore::new_remote( @@ -1519,7 +1518,7 @@ impl Project { ImageStore::remote(worktree_store.clone(), client.clone().into(), remote_id, cx) })?; - let environment = cx.new(|_| ProjectEnvironment::new(None))?; + let environment = cx.new(|cx| ProjectEnvironment::new(None, cx))?; let breakpoint_store = cx.new(|_| BreakpointStore::remote(remote_id, client.clone().into()))?; @@ -1951,10 +1950,7 @@ impl Project { } #[inline] - pub fn peek_environment_error<'a>( - &'a self, - cx: &'a App, - ) -> Option<&'a EnvironmentErrorMessage> { + pub fn peek_environment_error<'a>(&'a self, cx: &'a App) -> Option<&'a String> { self.environment.read(cx).peek_environment_error() } diff --git a/crates/remote_server/src/headless_project.rs b/crates/remote_server/src/headless_project.rs index 534eae6f44..83000c8bac 100644 --- a/crates/remote_server/src/headless_project.rs +++ b/crates/remote_server/src/headless_project.rs @@ -94,7 +94,7 @@ impl HeadlessProject { store }); - let environment = cx.new(|_| ProjectEnvironment::new(None)); + let environment = cx.new(|cx| ProjectEnvironment::new(None, cx)); let manifest_tree = ManifestTree::new(worktree_store.clone(), cx); let toolchain_store = cx.new(|cx| { ToolchainStore::local( diff --git a/crates/util/src/util.rs b/crates/util/src/util.rs index 3f1205f965..f725167724 100644 --- a/crates/util/src/util.rs +++ b/crates/util/src/util.rs @@ -353,7 +353,10 @@ pub async fn load_login_shell_environment() -> Result<()> { // 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. - for (name, value) in shell_env::capture(get_system_shell(), &[], paths::home_dir()).await? { + for (name, value) in shell_env::capture(get_system_shell(), &[], paths::home_dir()) + .await + .with_context(|| format!("capturing environment with {:?}", get_system_shell()))? + { unsafe { env::set_var(&name, &value) }; } @@ -627,7 +630,7 @@ where } pub fn log_err(error: &E) { - log_error_with_caller(*Location::caller(), error, log::Level::Warn); + log_error_with_caller(*Location::caller(), error, log::Level::Error); } pub trait TryFutureExt {