fix: Windows OS detection assumes MSYSTEM is set v1 APPLIED

Daniel Jay Haskin: 1
 fix: Windows OS detection assumes MSYSTEM is set

 1 files changed, 3 insertions(+), 3 deletions(-)

[PATCH] fix: Windows OS detection assumes MSYSTEM is set Export this patch

When the environment variable `MSYSTEM` is not set, the check for
whether or not the code is running in a "vanilla" Windows environment
fails, and the code assumes that the OS is not Windows. It then tries to
find `sh.exe` everywhere on Windows paths and fails to run any commands.

This is because the boolean check for such environments is incorrect.
The check as written reads something like this:

  "If the target OS is Windows, AND the MSYSTEM variable is
  set AND it is NOT set to `MINGW`, then assume were on Windows and look
  for CMD."

This is, I believe, unintended. It should read something like

Clément DOUIN <clement.douin@posteo.net>

7 months ago

Indeed. The problem is I do not own a Windows and I cannot test the CLI
properly. Maybe something to invest in the future. Did you have the
occasion to test your patch directly on a Windows machine? Anyway sounds
good, I will apply it. Thank you for your help!

  "If the target OS is Windows, and NOT (the MSYSTEM variable is set AND
  it is set to `MINGW`), then assume we are on Windows and look for
  CMD."

This patch moves the logical negation outwards from the `MSYS`
check. This allows the system to assume windows is used either if
the MSYSTEM environment variable is not set, or if it is,
to ensure it isn't MINGW, as (I assume) was originally intended.
---
 src/process.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/process.rs b/src/process.rs
index 56a6659..a294799 100644
--- a/src/process.rs
+++ b/src/process.rs
@@ -49,9 +49,9 @@ pub fn pipe(cmd: &str, input: &[u8]) -> Result<(Vec<u8>, usize)> {
     let mut output = Vec::new();
 
     let windows = cfg!(target_os = "windows")
-        && env::var("MSYSTEM")
-            .map(|env| !env.starts_with("MINGW"))
-            .unwrap_or_default();
+        && !(env::var("MSYSTEM")
+            .map(|env| env.starts_with("MINGW"))
+            .unwrap_or_default());
 
     let pipeline = if windows {
         Command::new("cmd")
-- 
2.37.3.windows.1

Knut Magnus Aasrud <km@aasrud.com>

7 months ago

If the intention here is to override the target os if MSYSTEM=MINGW on 
Windows, and set it to Unix, then I agree with the proposed changes. I'll 
let ~soywod be the final judge of this, though, since he knows the 
context here a lot better than me.