Refactor distant-ssh2 is_windows with fix for exec command

Cargo.lock

@@ -834,7 +834,6 @@ dependencies = [
  "shell-words",
  "smol",
  "tokio",
- "typed-path",
  "wezterm-ssh",
  "which",
  "whoami",
@@ -3044,12 +3043,6 @@ dependencies = [
  "once_cell",
 ]
 
-[[package]]
-name = "typed-path"
-version = "0.1.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0303bfe6ef379273be7ce99d8a6a2e54261fd110a01cf70986ec0fe855ff075e"
-
 [[package]]
 name = "typenum"
 version = "1.15.0"

distant-ssh2/Cargo.toml

@@ -29,7 +29,6 @@ rpassword = "7.0.0"
 shell-words = "1.1.0"
 smol = "1.2.5"
 tokio = { version = "1.20.1", features = ["full"] }
-typed-path = "0.1.0"
 wezterm-ssh = { version = "0.4.0", default-features = false }
 winsplit = "0.1.0"
 

distant-ssh2/src/lib.rs

@@ -507,7 +507,7 @@ impl Ssh {
 
         INSTANCE
             .get_or_try_init(async move {
-                let is_windows = utils::is_windows(&self.session.sftp()).await?;
+                let is_windows = utils::is_windows(&self.session).await?;
 
                 Ok(if is_windows {
                     SshFamily::Windows

distant-ssh2/src/utils.rs

@@ -4,7 +4,6 @@ use std::{
     path::{Path, PathBuf},
     time::Duration,
 };
-use typed_path::{windows::WindowsComponent, WindowsEncoding, WindowsPathBuf};
 use wezterm_ssh::{ExecResult, Session, Sftp};
 
 #[allow(dead_code)]
@@ -37,12 +36,16 @@ impl fmt::Debug for ExecOutput {
 }
 
 #[allow(dead_code)]
-pub async fn execute_output(session: &Session, cmd: &str) -> io::Result<ExecOutput> {
+pub async fn execute_output(
+    session: &Session,
+    cmd: &str,
+    timeout: Option<Duration>,
+) -> io::Result<ExecOutput> {
     let ExecResult {
         mut child,
         mut stdout,
         mut stderr,
-        ..
+        stdin: _stdin,
     } = session
         .exec(cmd, None)
         .compat()
@@ -74,13 +77,24 @@ pub async fn execute_output(session: &Session, cmd: &str) -> io::Result<ExecOutp
     let stdout_handle = spawn_reader!(stdout);
     let stderr_handle = spawn_reader!(stderr);
 
-    // Wait for our handles to conclude
-    let stdout = stdout_handle.await.map_err(to_other_error)??;
-    let stderr = stderr_handle.await.map_err(to_other_error)??;
-
     // Wait for process to conclude
     let status = child.async_wait().compat().await.map_err(to_other_error)?;
 
+    // Wait for our handles to conclude (max of timeout if provided)
+    let (stdout, stderr) = match timeout {
+        Some(duration) => {
+            let (res1, res2) = tokio::try_join!(
+                tokio::time::timeout(duration, stdout_handle),
+                tokio::time::timeout(duration, stderr_handle)
+            )?;
+            (res1??, res2??)
+        }
+        None => {
+            let (res1, res2) = tokio::try_join!(stdout_handle, stderr_handle)?;
+            (res1?, res2?)
+        }
+    };
+
     Ok(ExecOutput {
         success: status.success(),
         stdout,
@@ -96,31 +110,30 @@ where
 }
 
 /// Determines if using windows by checking the canonicalized path of '.'
-pub async fn is_windows(sftp: &Sftp) -> io::Result<bool> {
-    // Look up the current directory
-    let current_dir = canonicalize(sftp, ".").await?;
-
-    // TODO: Ideally, we would determine the family using something like the following:
-    //
-    //      cmd.exe /C echo %OS%
-    //
-    //      Determine OS by printing OS variable (works with Windows 2000+)
-    //      If it matches Windows_NT, then we are on windows
-    //
-    // However, the above is not working for whatever reason (always has success == false); so,
-    // we're purely using a check if we have a drive letter on the canonicalized path to
-    // determine if on windows for now. Some sort of failure with SIGPIPE
-    let windows_path = WindowsPathBuf::from(current_dir.to_string_lossy().to_string());
-    let mut components = windows_path.components();
-    if let Some(WindowsComponent::Prefix(_)) = components.next() {
-        Ok(true)
-    } else if let Some(WindowsComponent::Prefix(_)) =
-        components.as_path::<WindowsEncoding>().components().next()
-    {
-        Ok(true)
-    } else {
-        Ok(false)
+pub async fn is_windows(session: &Session) -> io::Result<bool> {
+    let output = execute_output(
+        session,
+        "cmd.exe /C echo %OS%",
+        Some(Duration::from_secs(1)),
+    )
+    .await?;
+
+    fn contains_subslice(slice: &[u8], subslice: &[u8]) -> bool {
+        for i in 0..slice.len() {
+            if i + subslice.len() > slice.len() {
+                break;
             }
+
+            if slice[i..].starts_with(subslice) {
+                return true;
+            }
+        }
+
+        false
+    }
+
+    Ok(contains_subslice(&output.stdout, b"Windows_NT")
+        || contains_subslice(&output.stderr, b"Windows_NT"))
 }
 
 /// Performs canonicalization of the given path using SFTP

distant-ssh2/tests/ssh2/client.rs

@@ -1,4 +1,4 @@
-use crate::{sshd::*, utils};
+use crate::sshd::*;
 use assert_fs::{prelude::*, TempDir};
 use distant_core::{
     data::{ChangeKindSet, Environment, FileType, Metadata},
@@ -1441,12 +1441,7 @@ async fn system_info_should_return_system_info_based_on_binary(
 
     let system_info = client.system_info().await.unwrap();
 
-    // NOTE: This is failing on our Github CI for Windows as I think it's running via wsl
-    //       and being read as Unix, but could be wrong. Either way, ignoring for now
-    if !*utils::IS_CI {
     assert_eq!(system_info.family, std::env::consts::FAMILY.to_string());
-    }
-
     assert_eq!(system_info.os, "");
     assert_eq!(system_info.arch, "");
     assert_eq!(system_info.main_separator, std::path::MAIN_SEPARATOR);

distant-ssh2/tests/ssh2/launched.rs

@@ -1,4 +1,4 @@
-use crate::{sshd::*, utils};
+use crate::sshd::*;
 use assert_fs::{prelude::*, TempDir};
 use distant_core::{
     data::{ChangeKindSet, Environment, FileType, Metadata},
@@ -1463,12 +1463,7 @@ async fn system_info_should_return_system_info_based_on_binary(
 
     let system_info = client.system_info().await.unwrap();
 
-    // NOTE: This is failing on our Github CI for Windows as I think it's running via wsl
-    //       and being read as Unix, but could be wrong. Either way, ignoring for now
-    if !*utils::IS_CI {
     assert_eq!(system_info.family, std::env::consts::FAMILY.to_string());
-    }
-
     assert_eq!(system_info.os, std::env::consts::OS.to_string());
     assert_eq!(system_info.arch, std::env::consts::ARCH.to_string());
     assert_eq!(system_info.main_separator, std::path::MAIN_SEPARATOR);