Better Windows Support (msys + powershell/CMD)

[JanDeDobbeleer]

Expected Behaviour

It works on all Windows flavors and shells

Current Behaviour

It only works for mingw and git bash (which is hardly a native experience)

Are you a GitHub Sponsor (Yes/No?)

Check at https://github.com/sponsors/alexellis

• Yes
• No

Possible Solution

There are two issue

Steps to Reproduce (for bugs)

Supporting msys

1. Get a Windows msys OS
2. Use arcade
3. Error Error: operating system "MSYS_NT-10.0-22000" is not supported. Available prefixes: linux, darwin, ming. pops up.

Proposed fix

commit e4f3ec8a22d8d753805a8311f8bc50ab02e4f4fe
Author: Jan De Dobbeleer <jan.de.dobbeleer@gmail.com>
Date:   Mon Apr 4 13:03:01 2022 +0200

    feat(windows): support msys

diff --git a/pkg/get/get.go b/pkg/get/get.go
index 4edf79b..21c8833 100644
--- a/pkg/get/get.go
+++ b/pkg/get/get.go
@@ -16,7 +16,7 @@ import (
 	"github.com/alexellis/arkade/pkg/env"
 )
 
-var supportedOS = [...]string{"linux", "darwin", "ming"}
+var supportedOS = [...]string{"linux", "darwin", "ming", "msys"}
 var supportedArchitectures = [...]string{"x86_64", "arm", "amd64", "armv6l", "armv7l", "arm64", "aarch64"}
 
 // Tool describes how to download a CLI tool from a binary

Applied fix result

Supporting additional shells

1. Use a shell other than git bash on Windows
2. Use arcade
3. Error Error: env-var HOME, not set pops up. 4.

Proposed fix

diff --git a/pkg/config/config.go b/pkg/config/config.go
index 5024e44..4697c80 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -8,16 +8,18 @@ import (
 	"os"
 	"path"
 	"strings"
+
+	"github.com/alexellis/arkade/pkg/env"
 )
 
 func GetUserDir() string {
-	home := os.Getenv("HOME")
+	home := env.GetUserHome()
 	root := fmt.Sprintf("%s/.arkade/", home)
 	return root
 }
 
 func InitUserDir() (string, error) {
-	home := os.Getenv("HOME")
+	home := env.GetUserHome()
 	root := fmt.Sprintf("%s/.arkade/", home)
 
 	if len(home) == 0 {
@@ -40,7 +42,7 @@ func InitUserDir() (string, error) {
 }
 
 func GetDefaultKubeconfig() string {
-	kubeConfigPath := path.Join(os.Getenv("HOME"), ".kube/config")
+	kubeConfigPath := path.Join(env.GetUserHome(), ".kube/config")
 
 	if val, ok := os.LookupEnv("KUBECONFIG"); ok {
 		kubeConfigPath = val
diff --git a/pkg/env/env.go b/pkg/env/env.go
index 7316b5c..006cc7c 100644
--- a/pkg/env/env.go
+++ b/pkg/env/env.go
@@ -7,6 +7,7 @@ import (
 	"log"
 	"os"
 	"path"
+	"runtime"
 	"strings"
 
 	execute "github.com/alexellis/go-execute/pkg/v1"
@@ -33,8 +34,19 @@ func GetClientArch() (arch string, os string) {
 	return archResult, osResult
 }
 
+func GetUserHome() string {
+	if runtime.GOOS != "windows" {
+		return os.Getenv("HOME")
+	}
+	home := os.Getenv("HOMEDRIVE") + os.Getenv("HOMEPATH")
+	if home == "" {
+		home = os.Getenv("USERPROFILE")
+	}
+	return home
+}
+
 func LocalBinary(name, subdir string) string {
-	home := os.Getenv("HOME")
+	home := GetUserHome()
 	val := path.Join(home, ".arkade/bin/")
 	if len(subdir) > 0 {
 		val = path.Join(val, subdir)

To apply the result, I also had to ensure oh-my-posh can resolve on msys:

diff --git a/pkg/get/tools.go b/pkg/get/tools.go
index 159fcc6..347a6e1 100644
--- a/pkg/get/tools.go
+++ b/pkg/get/tools.go
@@ -2120,7 +2120,7 @@ https://github.com/{{.Owner}}/{{.Repo}}/releases/download/{{.Version}}/{{.Name}}
 			Description: "A prompt theme engine for any shell that can display kubernetes information.",
 			BinaryTemplate: `{{ $ext := "" }}
 			{{ $osStr := "linux" }}
-			{{ if HasPrefix .OS "ming" -}}
+			{{ if or (HasPrefix .OS "ming") (HasPrefix .OS "msys") -}}
 			{{ $osStr = "windows" }}
 			{{ $ext = ".exe" }}
 			{{- else if eq .OS "darwin" -}}

Applied fix result

This fix has also been validate on macOS to ensure no breaking change occurs:

I would additionally propose to create a helper function to correctly resolve Windows in the templates instead of relying on the ming string in all tools. That I haven't done yet due to time constraints.

Context

I want to allow people to use arkade anywhere, without friction

• Operating System and version (e.g. Linux, Windows, MacOS):

MSYS_NT-10.0-22000 JANB7C0 3.1.7-340.x86_64 2020-10-23 13:08 UTC x86_64 Msys

• What arkade version is this?

Version: 0.8.19
Git Commit: 8fc22630a8b38d52391d276406aa068cada6e253

[alexellis]

Hi @JanDeDobbeleer thanks for logging this.

When I put together arkade originally, I left this note in the README file:

Windows users: arkade requires bash to be available, therefore Windows users should install and use Git Bash

What I am wondering, is whether arkade would support CMD.exe/PS with the patches above, or whether we'd run into issues elsewhere?

The following may also not work as expected, since Git Bash may report Windows as an OS to Go:

+	if runtime.GOOS != "windows" {
+		return os.Getenv("HOME")
+	}

Do you see Windows users not adopting WSL, or Git Bash and needing to run arkade from cmd.exe?

So I have an interest in supporting Windows users better, but wonder what other hidden gotchas may exist.

All the templates we maintain use "MING" as their string, so I wonder how you'd suggest working around that when the value is MSYS instead?

We fork the uname -a command to get this data in the first place, so how does your cmd.exe system have that available, or is it coming from somewhere else in your patch?

Alex

[JanDeDobbeleer]

What I am wondering, is whether arkade would support CMD.exe/PS with the patches above, or whether we'd run into issues elsewhere?

With this change, every shell should be supported as it only needs to understand where to store the binaries. The binaries are compatible. I was only focussing on installing tools, there might be something I didn't see yet for other functionality. I would rewrite the logic to identify the user's HOME folder the same way we do for oh-my-posh as that has been proven to work cross platform for quite a while already (mainly removing theruntime.GOOS != "windows" check above):

home := os.Getenv("HOME")
if len(home) > 0 {
	return home
}
// fallback to older implementations on Windows
home = os.Getenv("HOMEDRIVE") + os.Getenv("HOMEPATH")
if home == "" {
	home = os.Getenv("USERPROFILE")
}
return home

Do you see Windows users not adopting WSL, or Git Bash and needing to run arkade from cmd.exe?

More from PowerShell, but yes.

All the templates we maintain use "MING" as their string, so I wonder how you'd suggest working around that when the value is MSYS instead?

In my opinion, there's no need to use uname -a on Windows as with the change above you can simply populate the OS variable with windows when runtime.GOOS == "windows" and everything will work, regardless of the shell being used In that case it's a simple find and replace if HasPrefix .OS "ming" -> if eq .OS "windows" . That's the cleanest fix, but I tried to stay compliant with the current functionality.

The fact that I have uname in my path is because I added the git's bin folder to my path. That one contains the executable (so this is the things that needs to be highlighted in the README for Windows.

[JanDeDobbeleer]

@alexellis I did a "small" rework based on the comments above as indeed, you'd want the Windows executable to work in all shells and that implies we can't force the user to do any work other than download and run arkade. I did some digging and we can get the architecture from Windows itself, this will work anyways, regardless of where you run it as we can use a win32 call. I have a branch with the changes and adjustments to the tests/tools and all validations pass. I also validated this to work on Windows and macOS.

There are two commits to look at:

• remove dependency on uname for windows (JanDeDobbeleer@0c48c24)
• fetch user home correctly regardless of the shell used (JanDeDobbeleer@dc4973a)

[alexellis]

In my opinion, there's no need to use uname -a on Windows as with the change above you can simply populate the OS variable with windows when runtime.GOOS == "windows" and everything will work, regardless of the shell being used In that case it's a simple find and replace if HasPrefix .OS "ming" -> if eq .OS "windows" . That's the cleanest fix, but I tried to stay compliant with the current functionality.

What about 64-bit ARM Windows?

[alexellis]

Couple of comments - > JanDeDobbeleer@0c48c24

[JanDeDobbeleer]

What about 64-bit ARM Windows?

@alexellis that should return the same, oh-my-posh uses the same logic.

[Shikachuu]

Hey @JanDeDobbeleer,
I have several questions about this implementation:

• Is it possible to implement this in a way, that it maps msys to ming, and ming stays compatible with mingw aswell?
  See, if we were to implement this 1:1 we would have to rewirte every single template that currently uses ming as the OS flag in the template conditions, then we need to change the tests as well, then we need to change the end2end tests too cause they only run on linux amd64...I think you can see the never ending to-do-list here, it is a really huge leap tho'.
  Isn't it easier to map the "windows terminal emulators" to mingw?

• Is it possible to avoid calling dll files?
  To keep arkade as a selfcontained, statically linked binary is a design choice that I don't think we need to change and also really good thing. (but correct me if I'm wrong here, @alexellis)

[JanDeDobbeleer]

@Shikachuu

1. sure, that could be done I guess.
2. no, and there's no harm in calling win32 on Windows, golang does the same itself btw. They just abstract it away for you 😄

[Shikachuu]

@Shikachuu

1. sure, that could be done I guess.

2. no, and there's no harm in calling win32 on Windows, golang does the same itself btw. They just abstract it away for you 😄

I'm not much of a windows user myself, but reading about this, it seems similar to calling gnu binaries on a unix system, so it should be fine as you said so.

If you willing to create a PR with the mingw version as discussed above, I don't see any reasons why we shouldn't add this.

[alexellis]

It only works for mingw and git bash (which is hardly a native experience)

As a side-note, I'd consider working with cmd.exe a non-goal at this time.

It may change in the future, at which point we now know what's required.

[alexellis]

Error Error: operating system "MSYS_NT-10.0-22000" is not supported. Available prefixes: linux, darwin, ming. pops up.

@JanDeDobbeleer when do Windows users need to use Msys2 over Git bash?

[JanDeDobbeleer]

@alexellis this was me running this inside PowerShell if I'm not mistaken. But it's been a while, I'll have a proper look in the course of next week to address the comments too.

[JanDeDobbeleer]

@alexellis just had a look and made sure to make the most minimal changes to support this correctly.

I'd consider working with cmd.exe a non-goal at this time

This is not about cmd.exe but any shell outside of git bash (and truth be told, we need to get rid of that monster). You can find the slimmed down version here.

[alexellis]

Thanks for continuing the discussion @JanDeDobbeleer 👍

What I like:

It's got a windows-only build tag: //go:build windows

What I'm unsure about:

import (
	"fmt"
	"unsafe"

	"golang.org/x/sys/windows"
)

var (
	dllKernel               = windows.NewLazyDLL("kernel32.dll")
	procGetNativeSystemInfo = dllKernel.NewProc("GetNativeSystemInfo")
)

	_, _, _ = procGetNativeSystemInfo.Call(uintptr(unsafe.Pointer(&systemInfo)))

Does this require CGO?

Am I over-simplifying this?

If all we're doing is finding out whether we're on Windows or not, can't we get that by the .exe extension of the binary or the GOOS variable?

[alexellis]

What do you get with this?

GOOS=windows go build -o jan.exe
./jan.exe

package main

import (
	"fmt"
	"runtime"
)

func main() {
	fmt.Printf("The current operating system is: %s\n", runtime.GOOS)
}

[JanDeDobbeleer]

@alexellis it doesn't require cgo, all it does is call the native windows API (which, I understand, looks very alien of you've never been there before). Additionally, you can run amd64 on Windows ARM and 386 on amd64 so you'll want to know the actual architecture to offer the best compatible experience.

runtime.GOOS is a hardcoded "windows" string, it never tells you the architecture.

[alexellis]

Do we need an architecture? Only x86_64 is supported by arkade for Windows downloads.

I can't see a need to ever support 386 (32-bit Windows). Perhaps one day ARM64 will be popular because Windows devs are flocking to the Apple M1/M2?

Then can't that also be derived from the binary too?

GOARCH
GOARM

[JanDeDobbeleer]

@alexellis those two are whatever you built for. It doesn't necessarily reflect the actual OS's architecture (see my previous remark). For oh-my-posh this is required as we do have that support, and this way it's also available to anyone going forward. Windows ARM will become the norm, just like Apple also moved to it for obvious reasons.

[alexellis]

That’s OK with me for where we are today. There is no support in any of our app definitions for Windows ARM. So the only platform is AMD64 - I.e. if you’re using a .exe we know the platform exactly

If we added ARM64, the built flag would be different and so input the correct architecture.

Or are you telling me that someone could install the arm binary on amd64, then execute it?

[JanDeDobbeleer]

Or are you telling me that someone could install the arm binary on amd64, then execute it?

The other way around, you can run a amd64 binary on ARM and nothing would work. So you need this.

[alexellis]

Spell out the scenario for me please?

[JanDeDobbeleer]

@alexellis can we take a step back before I do that as it's important to understand why I did this. All it does is mimic the same functionality that's in GetClientArch for unix and mac. That one uses uname -m to identify the architecture and calling GetNativeSystemInfo is the equivalent on Windows. Yes, we could resort to GOOS and GOARCH but the same argument goes for unix and mac as well. There's no difference. The only "downside" is that you always have to install the correct arkade binary.

As for your question, I can install and run the Windows amd64 arkade binary on a Windows arm64 system which will then also install amd64 binaries and not arm64 binaries when we only look at GOOS and GOARCH.