Repository: denoland/deno
Pull request: fix(x): fix executing native binary on windows + some cleanup

Addressing some todos from the original PR. For instance, this allows things like `dx supabase` (which is a native bin) to work on windows

Changed files:
filename: cli/tools/x.rs
patch: @@ -105,21 +105,16 @@ async fn maybe_run_local_npm_bin(
 ) -> Result<Option<i32>, AnyError> {
   let permissions = factory.root_permissions_container()?;
 
-  let bins = resolve_local_bins(node_resolver, npm_resolver)?;
-  let command = if command.starts_with("@") && command.contains("/") {
-    command.split("/").last().unwrap()
-  } else {
-    command
-  };
-  let bin_value = if let Some(bin_value) = bins.get(command) {
+  let mut bins = resolve_local_bins(node_resolver, npm_resolver)?;
+  let bin_value = if let Some(bin_value) = bins.remove(command) {
     bin_value
   } else if let Some(bin_value) = {
     let command = if command.starts_with("@") && command.contains("/") {
       command.split("/").last().unwrap()
     } else {
       command
     };
-    bins.get(command)
+    bins.remove(command)
   } {
     bin_value
   } else {
@@ -134,7 +129,15 @@ async fn maybe_run_local_npm_bin(
         .await
         .map(Some);
     }
-    BinValue::Executable(path_buf) => {
+    BinValue::Executable(mut path_buf) => {
+      if cfg!(windows) && path_buf.extension().is_none() {
+        // prefer cmd shim over sh
+        path_buf.set_extension("cmd");
+        if !path_buf.exists() {
+          //  just fall back to original path
+          path_buf.set_extension("");
+        }
+      }
       permissions.check_run(
         &deno_runtime::deno_permissions::RunQueryDescriptor::Path(
           PathQueryDescriptor::new(

filename: libs/node_resolver/resolution.rs
patch: @@ -2217,16 +2217,32 @@ fn bin_value_from_file<TSys: FsOpen>(
   path: &Path,
   sys: &NodeResolutionSys<TSys>,
 ) -> Option<BinValue> {
-  let mut file = sys.fs_open(path, OpenOptions::new().read()).ok()?;
+  let mut file = match sys.fs_open(path, OpenOptions::new().read()) {
+    Ok(file) => file,
+    Err(err) => {
+      if err.kind() == std::io::ErrorKind::NotFound {
+        return None;
+      }
+      log::debug!(
+        "Failed to open bin file '{}': {:#}; treating as executable",
+        path.display(),
+        err,
+      );
+      return Some(BinValue::Executable(path.to_path_buf()));
+    }
+  };
   let mut buf = [0; 4];
   let (is_binary, buf): (bool, &[u8]) = {
     let result = file.read_exact(&mut buf);
     if let Err(err) = result {
       log::debug!("Failed to read binary file '{}': {:#}", path.display(), err);
       // safer fallback to assume it's a binary
-      (false, &[])
+      (true, &[])
     } else {
-      (is_binary(&buf), &buf[..])
+      (
+        is_binary(&buf) || (!(buf[0] == b'#' && buf[1] == b'!')),
+        &buf[..],
+      )
     }
   };
 
@@ -2255,7 +2271,7 @@ fn resolve_execution_path_from_npx_shim(
   text: &str,
 ) -> Option<PathBuf> {
   static SCRIPT_PATH_RE: Lazy<Regex> =
-    lazy_regex::lazy_regex!(r#""\$basedir\/([^"]+)" "\$@""#);
+    lazy_regex::lazy_regex!(r#"exec\s+node\s+"\$basedir\/([^"]+)" "\$@""#);
 
   let maybe_first_line = {
     let index = text.find("\n")?;

filename: libs/npm_installer/bin_entries.rs
patch: @@ -389,6 +389,8 @@ pub fn set_up_bin_entry<'a>(
   if sys_traits::impls::is_windows() {
     windows_shim::set_up_bin_shim(
       sys,
+      package,
+      extra,
       bin_name,
       bin_script,
       package_path,

filename: libs/npm_installer/bin_entries/windows_shim.rs
patch: @@ -23,6 +23,7 @@ use std::io::BufRead;
 use std::path::Path;
 use std::path::PathBuf;
 
+use sys_traits::FsMetadata;
 use sys_traits::FsOpen;
 use sys_traits::FsWrite;
 
@@ -295,7 +296,9 @@ impl ShimData {
 }
 
 pub fn set_up_bin_shim<'a>(
-  sys: &(impl FsOpen + FsWrite),
+  sys: &(impl FsOpen + FsWrite + FsMetadata),
+  package: &'a deno_npm::NpmResolutionPackage,
+  extra: &'a deno_npm::NpmPackageExtraInfo,
   bin_name: &'a str,
   bin_script: &'a str,
   package_path: &'a Path,
@@ -304,6 +307,22 @@ pub fn set_up_bin_shim<'a>(
   let shim_path = bin_node_modules_dir_path.join(bin_name);
   let target_file = package_path.join(bin_script);
 
+  let target_file = if !sys.fs_exists_no_err(&target_file) {
+    let target_file_exe = target_file.with_extension("exe");
+    if !sys.fs_exists_no_err(&target_file_exe) {
+      return Ok(EntrySetupOutcome::MissingEntrypoint {
+        bin_name,
+        package_path,
+        entrypoint: target_file,
+        package,
+        extra,
+      });
+    }
+    target_file_exe
+  } else {
+    target_file
+  };
+
   let rel_target =
     relative_path(bin_node_modules_dir_path, &target_file).unwrap();
   let shebang = resolve_shebang(sys, &target_file);

filename: tests/specs/x/native_bin_with_postinstall/__test__.jsonc
patch: @@ -1,6 +1,5 @@
 {
   "tempDir": true,
-  "if": "unix",
   "args": "x -y @denotest/deno --version",
   "output": "x.out"
 }