Remove implicit dependency on node env for data_dir devcontainer cli

Release Notes:

- (Preview only) Fix devcontainers not starting when certain env
variables were set

Co-authored-by: KyleBarton <kjb@initialcapacity.io>

crates/git_ui/src/commit_view.rs

@@ -8,9 +8,9 @@ use git::{
     parse_git_remote_url,
 };
 use gpui::{
-    AnyElement, App, AppContext as _, AsyncApp, AsyncWindowContext, Context, Element, Entity,
-    EventEmitter, FocusHandle, Focusable, InteractiveElement, IntoElement, ParentElement,
-    PromptLevel, Render, Styled, Task, WeakEntity, Window, actions,
+    AnyElement, App, AppContext as _, AsyncApp, AsyncWindowContext, ClipboardItem, Context,
+    Element, Entity, EventEmitter, FocusHandle, Focusable, InteractiveElement, IntoElement,
+    ParentElement, PromptLevel, Render, Styled, Task, WeakEntity, Window, actions,
 };
 use language::{
     Anchor, Buffer, Capability, DiskState, File, LanguageRegistry, LineEnding, OffsetRangeExt as _,
@@ -24,7 +24,7 @@ use std::{
     sync::Arc,
 };
 use theme::ActiveTheme;
-use ui::{DiffStat, Tooltip, prelude::*};
+use ui::{ButtonLike, DiffStat, Tooltip, prelude::*};
 use util::{ResultExt, paths::PathStyle, rel_path::RelPath, truncate_and_trailoff};
 use workspace::item::TabTooltipContent;
 use workspace::{
@@ -383,6 +383,7 @@ impl CommitView {
     fn render_header(&self, window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
         let commit = &self.commit;
         let author_name = commit.author_name.clone();
+        let commit_sha = commit.sha.clone();
         let commit_date = time::OffsetDateTime::from_unix_timestamp(commit.commit_timestamp)
             .unwrap_or_else(|_| time::OffsetDateTime::now_utc());
         let local_offset = time::UtcOffset::current_local_offset().unwrap_or(time::UtcOffset::UTC);
@@ -429,6 +430,19 @@ impl CommitView {
                 .full_width()
         });
 
+        let clipboard_has_link = cx
+            .read_from_clipboard()
+            .and_then(|entry| entry.text())
+            .map_or(false, |clipboard_text| {
+                clipboard_text.trim() == commit_sha.as_ref()
+            });
+
+        let (copy_icon, copy_icon_color) = if clipboard_has_link {
+            (IconName::Check, Color::Success)
+        } else {
+            (IconName::Copy, Color::Muted)
+        };
+
         h_flex()
             .border_b_1()
             .border_color(cx.theme().colors().border_variant)
@@ -454,13 +468,47 @@ impl CommitView {
                                 h_flex()
                                     .gap_1()
                                     .child(Label::new(author_name).color(Color::Default))
-                                    .child(
-                                        Label::new(format!("Commit:{}", commit.sha))
-                                            .color(Color::Muted)
-                                            .size(LabelSize::Small)
-                                            .truncate()
-                                            .buffer_font(cx),
-                                    ),
+                                    .child({
+                                        ButtonLike::new("sha")
+                                            .child(
+                                                h_flex()
+                                                    .group("sha_btn")
+                                                    .size_full()
+                                                    .max_w_32()
+                                                    .gap_0p5()
+                                                    .child(
+                                                        Label::new(commit_sha.clone())
+                                                            .color(Color::Muted)
+                                                            .size(LabelSize::Small)
+                                                            .truncate()
+                                                            .buffer_font(cx),
+                                                    )
+                                                    .child(
+                                                        div().visible_on_hover("sha_btn").child(
+                                                            Icon::new(copy_icon)
+                                                                .color(copy_icon_color)
+                                                                .size(IconSize::Small),
+                                                        ),
+                                                    ),
+                                            )
+                                            .tooltip({
+                                                let commit_sha = commit_sha.clone();
+                                                move |_, cx| {
+                                                    Tooltip::with_meta(
+                                                        "Copy Commit SHA",
+                                                        None,
+                                                        commit_sha.clone(),
+                                                        cx,
+                                                    )
+                                                }
+                                            })
+                                            .on_click(move |_, _, cx| {
+                                                cx.stop_propagation();
+                                                cx.write_to_clipboard(ClipboardItem::new_string(
+                                                    commit_sha.to_string(),
+                                                ));
+                                            })
+                                    }),
                             )
                             .child(
                                 h_flex()

crates/recent_projects/src/dev_container.rs

@@ -53,7 +53,9 @@ async fn check_for_docker() -> Result<(), DevContainerError> {
     }
 }
 
-async fn ensure_devcontainer_cli(node_runtime: NodeRuntime) -> Result<PathBuf, DevContainerError> {
+async fn ensure_devcontainer_cli(
+    node_runtime: &NodeRuntime,
+) -> Result<(PathBuf, bool), DevContainerError> {
     let mut command = util::command::new_smol_command(&dev_container_cli());
     command.arg("--version");
 
@@ -63,23 +65,42 @@ async fn ensure_devcontainer_cli(node_runtime: NodeRuntime) -> Result<PathBuf, D
             e
         );
 
+        let Ok(node_runtime_path) = node_runtime.binary_path().await else {
+            return Err(DevContainerError::NodeRuntimeNotAvailable);
+        };
+
         let datadir_cli_path = paths::devcontainer_dir()
             .join("node_modules")
-            .join(".bin")
-            .join(&dev_container_cli());
+            .join("@devcontainers")
+            .join("cli")
+            .join(format!("{}.js", &dev_container_cli()));
+
+        log::debug!(
+            "devcontainer not found in path, using local location: ${}",
+            datadir_cli_path.display()
+        );
 
         let mut command =
-            util::command::new_smol_command(&datadir_cli_path.as_os_str().display().to_string());
+            util::command::new_smol_command(node_runtime_path.as_os_str().display().to_string());
+        command.arg(datadir_cli_path.display().to_string());
         command.arg("--version");
 
-        if let Err(e) = command.output().await {
-            log::error!(
+        match command.output().await {
+            Err(e) => log::error!(
                 "Unable to find devcontainer CLI in Data dir. Will try to install. Error: {:?}",
                 e
-            );
-        } else {
-            log::info!("Found devcontainer CLI in Data dir");
-            return Ok(datadir_cli_path.clone());
+            ),
+            Ok(output) => {
+                if output.status.success() {
+                    log::info!("Found devcontainer CLI in Data dir");
+                    return Ok((datadir_cli_path.clone(), false));
+                } else {
+                    log::error!(
+                        "Could not run devcontainer CLI from data_dir. Will try once more to install. Output: {:?}",
+                        output
+                    );
+                }
+            }
         }
 
         if let Err(e) = fs::create_dir_all(paths::devcontainer_dir()).await {
@@ -101,7 +122,9 @@ async fn ensure_devcontainer_cli(node_runtime: NodeRuntime) -> Result<PathBuf, D
             return Err(DevContainerError::DevContainerCliNotAvailable);
         };
 
-        let mut command = util::command::new_smol_command(&datadir_cli_path.display().to_string());
+        let mut command =
+            util::command::new_smol_command(node_runtime_path.as_os_str().display().to_string());
+        command.arg(datadir_cli_path.display().to_string());
         command.arg("--version");
         if let Err(e) = command.output().await {
             log::error!(
@@ -110,22 +133,42 @@ async fn ensure_devcontainer_cli(node_runtime: NodeRuntime) -> Result<PathBuf, D
             );
             Err(DevContainerError::DevContainerCliNotAvailable)
         } else {
-            Ok(datadir_cli_path)
+            Ok((datadir_cli_path, false))
         }
     } else {
         log::info!("Found devcontainer cli on $PATH, using it");
-        Ok(PathBuf::from(&dev_container_cli()))
+        Ok((PathBuf::from(&dev_container_cli()), true))
     }
 }
 
 async fn devcontainer_up(
     path_to_cli: &PathBuf,
+    found_in_path: bool,
+    node_runtime: &NodeRuntime,
     path: Arc<Path>,
 ) -> Result<DevContainerUp, DevContainerError> {
-    let mut command = util::command::new_smol_command(path_to_cli.display().to_string());
-    command.arg("up");
-    command.arg("--workspace-folder");
-    command.arg(path.display().to_string());
+    let Ok(node_runtime_path) = node_runtime.binary_path().await else {
+        log::error!("Unable to find node runtime path");
+        return Err(DevContainerError::NodeRuntimeNotAvailable);
+    };
+
+    let mut command = if found_in_path {
+        let mut command = util::command::new_smol_command(path_to_cli.display().to_string());
+        command.arg("up");
+        command.arg("--workspace-folder");
+        command.arg(path.display().to_string());
+        command
+    } else {
+        let mut command =
+            util::command::new_smol_command(node_runtime_path.as_os_str().display().to_string());
+        command.arg(path_to_cli.display().to_string());
+        command.arg("up");
+        command.arg("--workspace-folder");
+        command.arg(path.display().to_string());
+        command
+    };
+
+    log::debug!("Running full devcontainer up command: {:?}", command);
 
     match command.output().await {
         Ok(output) => {
@@ -235,7 +278,7 @@ pub(crate) async fn start_dev_container(
 ) -> Result<(Connection, String), DevContainerError> {
     check_for_docker().await?;
 
-    let path_to_devcontainer_cli = ensure_devcontainer_cli(node_runtime).await?;
+    let (path_to_devcontainer_cli, found_in_path) = ensure_devcontainer_cli(&node_runtime).await?;
 
     let Some(directory) = project_directory(cx) else {
         return Err(DevContainerError::DevContainerNotFound);
@@ -245,7 +288,13 @@ pub(crate) async fn start_dev_container(
         container_id,
         remote_workspace_folder,
         ..
-    }) = devcontainer_up(&path_to_devcontainer_cli, directory.clone()).await
+    }) = devcontainer_up(
+        &path_to_devcontainer_cli,
+        found_in_path,
+        &node_runtime,
+        directory.clone(),
+    )
+    .await
     {
         let project_name = get_project_name(
             &path_to_devcontainer_cli,
@@ -273,6 +322,7 @@ pub(crate) enum DevContainerError {
     DevContainerUpFailed,
     DevContainerNotFound,
     DevContainerParseFailed,
+    NodeRuntimeNotAvailable,
 }
 
 #[cfg(test)]

crates/remote/src/transport.rs

@@ -158,6 +158,9 @@ fn handle_rpc_messages_over_child_process_stdio(
             }
         };
         let status = remote_proxy_process.status().await?.code().unwrap_or(1);
+        if status != 0 {
+            anyhow::bail!("Remote server exited with status {status}");
+        }
         match result {
             Ok(_) => Ok(status),
             Err(error) => Err(error),

crates/remote/src/transport/docker.rs

@@ -582,19 +582,21 @@ impl RemoteConnection for DockerExecConnection {
             return Task::ready(Err(anyhow!("Remote binary path not set")));
         };
 
-        let mut docker_args = vec![
-            "exec".to_string(),
-            "-w".to_string(),
-            self.remote_dir_for_server.clone(),
-            "-i".to_string(),
-            self.connection_options.container_id.to_string(),
-        ];
+        let mut docker_args = vec!["exec".to_string()];
         for env_var in ["RUST_LOG", "RUST_BACKTRACE", "ZED_GENERATE_MINIDUMPS"] {
             if let Some(value) = std::env::var(env_var).ok() {
                 docker_args.push("-e".to_string());
                 docker_args.push(format!("{}='{}'", env_var, value));
             }
         }
+
+        docker_args.extend([
+            "-w".to_string(),
+            self.remote_dir_for_server.clone(),
+            "-i".to_string(),
+            self.connection_options.container_id.to_string(),
+        ]);
+
         let val = remote_binary_relpath
             .display(self.path_style())
             .into_owned();