Do not enable venv in terminal for bash-like oneshot task invocations (#8444)

Release Notes: - Work around #8334 by only activating venv in the terminal not in tasks (see #8440 for a proper solution) - To use venv modify your tasks in the following way: ```json { "label": "Python main.py", "command": "sh", "args": ["-c", "source .venv/bin/activate && python3 main.py"] } ``` --------- Co-authored-by: Kirill Bulatov <kirill@zed.dev>
2025-01-26 20:22:30 +00:00 · 2024-03-21 19:40:33 +02:00 · 2024-03-21 19:40:33 +02:00 · 85fdcef564
commit 85fdcef564
parent cd61297740
4 changed files with 109 additions and 49 deletions
--- a/Cargo.lock
+++ b/Cargo.lock
@ -87,10 +87,11 @@ dependencies = [

 [[package]]
 name = "alacritty_terminal"
-version = "0.22.1-dev"
-source = "git+https://github.com/alacritty/alacritty?rev=992011a4cd9a35f197acc0a0bd430d89a0d01013#992011a4cd9a35f197acc0a0bd430d89a0d01013"
+version = "0.23.0-rc1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "bc2c16faa5425a10be102dda76f73d76049b44746e18ddeefc44d78bbe76cbce"
 dependencies = [
- "base64 0.21.4",
+ "base64 0.22.0",
 "bitflags 2.4.2",
 "home",
 "libc",
@ -1292,6 +1293,12 @@ version = "0.21.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "9ba43ea6f343b788c8764558649e08df62f86c6ef251fdaeb1ffd010a9ae50a2"

+[[package]]
+name = "base64"
+version = "0.22.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9475866fec1451be56a3c2400fd081ff546538961565ccb5b7142cbd22bc7a51"
+
 [[package]]
 name = "base64-simd"
 version = "0.8.0"
--- a/crates/project/src/terminals.rs
+++ b/crates/project/src/terminals.rs
@ -1,4 +1,5 @@
 use crate::Project;
+use collections::HashMap;
 use gpui::{AnyWindowHandle, Context, Entity, Model, ModelContext, WeakModel};
 use settings::Settings;
 use smol::channel::bounded;
@ -7,6 +8,7 @@ use terminal::{
    terminal_settings::{self, Shell, TerminalSettings, VenvSettingsContent},
    SpawnTask, TaskState, Terminal, TerminalBuilder,
 };
+use util::ResultExt;

 // #[cfg(target_os = "macos")]
 // use std::os::unix::ffi::OsStrExt;
@ -28,12 +30,27 @@ impl Project {
            "creating terminals as a guest is not supported yet"
        );

+        let is_terminal = spawn_task.is_none();
        let settings = TerminalSettings::get_global(cx);
        let python_settings = settings.detect_venv.clone();
        let (completion_tx, completion_rx) = bounded(1);
+
        let mut env = settings.env.clone();
+        // Alacritty uses parent project's working directory when no working directory is provided
+        // https://github.com/alacritty/alacritty/blob/fd1a3cc79192d1d03839f0fd8c72e1f8d0fce42e/extra/man/alacritty.5.scd?plain=1#L47-L52
+
+        let current_directory = std::env::current_dir().ok();
+        let venv_base_directory = working_directory
+            .as_deref()
+            .or(current_directory.as_deref())
+            .unwrap_or_else(|| Path::new("")); // if everything fails, use relative path
+
        let (spawn_task, shell) = if let Some(spawn_task) = spawn_task {
            env.extend(spawn_task.env);
+            // Activate minimal Python virtual environment
+            if let Some(python_settings) = &python_settings.as_option() {
+                self.set_python_venv_path_for_tasks(python_settings, venv_base_directory, &mut env);
+            }
            (
                Some(TaskState {
                    id: spawn_task.id,
@ -82,16 +99,20 @@ impl Project {
            })
            .detach();

-            if let Some(python_settings) = &python_settings.as_option() {
-                let activate_command = Project::get_activate_command(python_settings);
-                let activate_script_path =
-                    self.find_activate_script_path(python_settings, working_directory);
-                self.activate_python_virtual_environment(
-                    activate_command,
-                    activate_script_path,
-                    &terminal_handle,
-                    cx,
-                );
+            // if the terminal is not a task, activate full Python virtual environment
+            if is_terminal {
+                if let Some(python_settings) = &python_settings.as_option() {
+                    if let Some(activate_script_path) =
+                        self.find_activate_script_path(python_settings, venv_base_directory)
+                    {
+                        self.activate_python_virtual_environment(
+                            Project::get_activate_command(python_settings),
+                            activate_script_path,
+                            &terminal_handle,
+                            cx,
+                        );
+                    }
+                }
            }
            terminal_handle
        });
@ -102,12 +123,8 @@ impl Project {
    pub fn find_activate_script_path(
        &mut self,
        settings: &VenvSettingsContent,
-        working_directory: Option<PathBuf>,
+        venv_base_directory: &Path,
    ) -> Option<PathBuf> {
-        // When we are unable to resolve the working directory, the terminal builder
-        // defaults to '/'. We should probably encode this directly somewhere, but for
-        // now, let's just hard code it here.
-        let working_directory = working_directory.unwrap_or_else(|| Path::new("/").to_path_buf());
        let activate_script_name = match settings.activate_script {
            terminal_settings::ActivateScript::Default => "activate",
            terminal_settings::ActivateScript::Csh => "activate.csh",
@ -115,17 +132,53 @@ impl Project {
            terminal_settings::ActivateScript::Nushell => "activate.nu",
        };

-        for virtual_environment_name in settings.directories {
-            let mut path = working_directory.join(virtual_environment_name);
-            path.push("bin/");
-            path.push(activate_script_name);
+        settings
+            .directories
+            .into_iter()
+            .find_map(|virtual_environment_name| {
+                let path = venv_base_directory
+                    .join(virtual_environment_name)
+                    .join("bin")
+                    .join(activate_script_name);
+                path.exists().then_some(path)
+            })
+    }

-            if path.exists() {
-                return Some(path);
+    pub fn set_python_venv_path_for_tasks(
+        &mut self,
+        settings: &VenvSettingsContent,
+        venv_base_directory: &Path,
+        env: &mut HashMap<String, String>,
+    ) {
+        let activate_path = settings
+            .directories
+            .into_iter()
+            .find_map(|virtual_environment_name| {
+                let path = venv_base_directory.join(virtual_environment_name);
+                path.exists().then_some(path)
+            });
+
+        if let Some(path) = activate_path {
+            // Some tools use VIRTUAL_ENV to detect the virtual environment
+            env.insert(
+                "VIRTUAL_ENV".to_string(),
+                path.to_string_lossy().to_string(),
+            );
+
+            let path_bin = path.join("bin");
+            // We need to set the PATH to include the virtual environment's bin directory
+            if let Some(paths) = std::env::var_os("PATH") {
+                let paths = std::iter::once(path_bin).chain(std::env::split_paths(&paths));
+                if let Some(new_path) = std::env::join_paths(paths).log_err() {
+                    env.insert("PATH".to_string(), new_path.to_string_lossy().to_string());
+                }
+            } else {
+                env.insert(
+                    "PATH".to_string(),
+                    path.join("bin").to_string_lossy().to_string(),
+                );
            }
        }
-
-        None
    }

    fn get_activate_command(settings: &VenvSettingsContent) -> &'static str {
@ -138,22 +191,20 @@ impl Project {
    fn activate_python_virtual_environment(
        &mut self,
        activate_command: &'static str,
-        activate_script: Option<PathBuf>,
+        activate_script: PathBuf,
        terminal_handle: &Model<Terminal>,
        cx: &mut ModelContext<Project>,
    ) {
-        if let Some(activate_script) = activate_script {
-            // Paths are not strings so we need to jump through some hoops to format the command without `format!`
-            let mut command = Vec::from(activate_command.as_bytes());
-            command.push(b' ');
-            // Wrapping path in double quotes to catch spaces in folder name
-            command.extend_from_slice(b"\"");
-            command.extend_from_slice(activate_script.as_os_str().as_encoded_bytes());
-            command.extend_from_slice(b"\"");
-            command.push(b'\n');
+        // Paths are not strings so we need to jump through some hoops to format the command without `format!`
+        let mut command = Vec::from(activate_command.as_bytes());
+        command.push(b' ');
+        // Wrapping path in double quotes to catch spaces in folder name
+        command.extend_from_slice(b"\"");
+        command.extend_from_slice(activate_script.as_os_str().as_encoded_bytes());
+        command.extend_from_slice(b"\"");
+        command.push(b'\n');

-            terminal_handle.update(cx, |this, _| this.input_bytes(command));
-        }
+        terminal_handle.update(cx, |this, _| this.input_bytes(command));
    }

    pub fn local_terminal_handles(&self) -> &Vec<WeakModel<terminal::Terminal>> {
--- a/crates/terminal/Cargo.toml
+++ b/crates/terminal/Cargo.toml
@ -15,7 +15,7 @@ doctest = false

 [dependencies]
 # TODO: when new version of this crate is released, change it
-alacritty_terminal = { git = "https://github.com/alacritty/alacritty", rev = "992011a4cd9a35f197acc0a0bd430d89a0d01013" }
+alacritty_terminal = "0.23.0-rc1"
 anyhow.workspace = true
 collections.workspace = true
 dirs = "4.0.0"
--- a/crates/terminal/src/terminal.rs
+++ b/crates/terminal/src/terminal.rs
@ -310,13 +310,19 @@ impl TerminalBuilder {
        working_directory: Option<PathBuf>,
        task: Option<TaskState>,
        shell: Shell,
-        env: HashMap<String, String>,
+        mut env: HashMap<String, String>,
        blink_settings: Option<TerminalBlink>,
        alternate_scroll: AlternateScroll,
        max_scroll_history_lines: Option<usize>,
        window: AnyWindowHandle,
        completion_tx: Sender<()>,
    ) -> Result<TerminalBuilder> {
+        // TODO: Properly set the current locale,
+        env.entry("LC_ALL".to_string())
+            .or_insert_with(|| "en_US.UTF-8".to_string());
+
+        env.insert("ZED_TERM".to_string(), "true".to_string());
+
        let pty_options = {
            let alac_shell = match shell.clone() {
                Shell::System => None,
@ -332,20 +338,13 @@ impl TerminalBuilder {
                shell: alac_shell,
                working_directory: working_directory.clone(),
                hold: false,
+                env: env.into_iter().collect(),
            }
        };

-        // First, setup Alacritty's env
+        // Setup Alacritty's env
        setup_env();

-        // Then setup configured environment variables
-        for (key, value) in env {
-            std::env::set_var(key, value);
-        }
-        //TODO: Properly set the current locale,
-        std::env::set_var("LC_ALL", "en_US.UTF-8");
-        std::env::set_var("ZED_TERM", "true");
-
        let scrolling_history = if task.is_some() {
            // Tasks like `cargo build --all` may produce a lot of output, ergo allow maximum scrolling.
            // After the task finishes, we do not allow appending to that terminal, so small tasks output should not
@ -650,6 +649,9 @@ impl Terminal {
                self.events
                    .push_back(InternalEvent::ColorRequest(*idx, fun_ptr.clone()));
            }
+            AlacTermEvent::ChildExit(_) => {
+                // TODO: Handle child exit
+            }
        }
    }