Remove implicit dependency on Node env for data_dir devcontainer CLI (#45589)

KyleBarton created

Closes #45058

Release Notes:

- Fixed implicit dependency on a system `node` for running
devcontainers.

Change summary

crates/recent_projects/src/dev_container.rs | 175 ++++++++++++++++------
1 file changed, 129 insertions(+), 46 deletions(-)

Detailed changes

crates/recent_projects/src/dev_container.rs 🔗

@@ -1,3 +1,4 @@
+use std::fmt::Display;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
@@ -53,7 +54,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 +66,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 +123,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 +134,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) => {
@@ -139,17 +183,20 @@ async fn devcontainer_up(
                     DevContainerError::DevContainerParseFailed
                 })
             } else {
-                log::error!(
+                let message = format!(
                     "Non-success status running devcontainer up for workspace: out: {:?}, err: {:?}",
                     String::from_utf8_lossy(&output.stdout),
                     String::from_utf8_lossy(&output.stderr)
                 );
-                Err(DevContainerError::DevContainerUpFailed)
+
+                log::error!("{}", &message);
+                Err(DevContainerError::DevContainerUpFailed(message))
             }
         }
         Err(e) => {
-            log::error!("Error running devcontainer up: {:?}", e);
-            Err(DevContainerError::DevContainerUpFailed)
+            let message = format!("Error running devcontainer up: {:?}", e);
+            log::error!("{}", &message);
+            Err(DevContainerError::DevContainerUpFailed(message))
         }
     }
 }
@@ -174,17 +221,19 @@ async fn devcontainer_read_configuration(
                     DevContainerError::DevContainerParseFailed
                 })
             } else {
-                log::error!(
+                let message = format!(
                     "Non-success status running devcontainer read-configuration for workspace: out: {:?}, err: {:?}",
                     String::from_utf8_lossy(&output.stdout),
                     String::from_utf8_lossy(&output.stderr)
                 );
-                Err(DevContainerError::DevContainerUpFailed)
+                log::error!("{}", &message);
+                Err(DevContainerError::DevContainerUpFailed(message))
             }
         }
         Err(e) => {
-            log::error!("Error running devcontainer read-configuration: {:?}", e);
-            Err(DevContainerError::DevContainerUpFailed)
+            let message = format!("Error running devcontainer read-configuration: {:?}", e);
+            log::error!("{}", &message);
+            Err(DevContainerError::DevContainerUpFailed(message))
         }
     }
 }
@@ -235,34 +284,44 @@ 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);
     };
 
-    if let Ok(DevContainerUp {
-        container_id,
-        remote_workspace_folder,
-        ..
-    }) = devcontainer_up(&path_to_devcontainer_cli, directory.clone()).await
+    match devcontainer_up(
+        &path_to_devcontainer_cli,
+        found_in_path,
+        &node_runtime,
+        directory.clone(),
+    )
+    .await
     {
-        let project_name = get_project_name(
-            &path_to_devcontainer_cli,
-            directory,
-            remote_workspace_folder.clone(),
-            container_id.clone(),
-        )
-        .await?;
+        Ok(DevContainerUp {
+            container_id,
+            remote_workspace_folder,
+            ..
+        }) => {
+            let project_name = get_project_name(
+                &path_to_devcontainer_cli,
+                directory,
+                remote_workspace_folder.clone(),
+                container_id.clone(),
+            )
+            .await?;
 
-        let connection = Connection::DevContainer(DevContainerConnection {
-            name: project_name.into(),
-            container_id: container_id.into(),
-        });
+            let connection = Connection::DevContainer(DevContainerConnection {
+                name: project_name.into(),
+                container_id: container_id.into(),
+            });
 
-        Ok((connection, remote_workspace_folder))
-    } else {
-        Err(DevContainerError::DevContainerUpFailed)
+            Ok((connection, remote_workspace_folder))
+        }
+        Err(err) => {
+            let message = format!("Failed with nested error: {}", err);
+            Err(DevContainerError::DevContainerUpFailed(message))
+        }
     }
 }
 
@@ -270,9 +329,33 @@ pub(crate) async fn start_dev_container(
 pub(crate) enum DevContainerError {
     DockerNotAvailable,
     DevContainerCliNotAvailable,
-    DevContainerUpFailed,
+    DevContainerUpFailed(String),
     DevContainerNotFound,
     DevContainerParseFailed,
+    NodeRuntimeNotAvailable,
+}
+
+impl Display for DevContainerError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            "{}",
+            match self {
+                DevContainerError::DockerNotAvailable =>
+                    "Docker CLI not found on $PATH".to_string(),
+                DevContainerError::DevContainerCliNotAvailable =>
+                    "Docker not found on path".to_string(),
+                DevContainerError::DevContainerUpFailed(message) => {
+                    format!("DevContainer creation failed with error: {}", message)
+                }
+                DevContainerError::DevContainerNotFound => "TODO what".to_string(),
+                DevContainerError::DevContainerParseFailed =>
+                    "Failed to parse file .devcontainer/devcontainer.json".to_string(),
+                DevContainerError::NodeRuntimeNotAvailable =>
+                    "Cannot find a valid node runtime".to_string(),
+            }
+        )
+    }
 }
 
 #[cfg(test)]