Skip to content

Fix Windows 10 support. #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 30, 2019
Merged

Fix Windows 10 support. #14

merged 1 commit into from
Apr 30, 2019

Conversation

AlekSi
Copy link
Contributor

@AlekSi AlekSi commented Dec 10, 2018

Closes #13.

@AlekSi
Copy link
Contributor Author

AlekSi commented Dec 18, 2018

Travis CI failed for an unrelated reason, PTAL.

@syntaqx
Copy link

syntaqx commented Jan 15, 2019

Excited to see this get merged.

@syntaqx
Copy link

syntaqx commented Jan 26, 2019

uname_os() {
  os=$(uname -s | tr '[:upper:]' '[:lower:]')
  case "$os" in
    msys_nt*) os="windows" ;;
    mingw*) os="windows" ;;
  esac
  echo "$os"
}

You'll probably want something like this to be more friendly with git-bash.exe, which is the more common shell installation most people use (not the best, just most common)

Note: I have basically no practical knowledge of shell scripting, so I'm sure there's a more optimized way to write that - Functionally was all I was referring to.

@codyaray
Copy link

codyaray commented Apr 30, 2019

+1 also just ran into this issue for mingw* as well.

Please add this case and merge! :)

@DABH
Copy link
Collaborator

DABH commented Apr 30, 2019

+1 for @syntaqx 's solution cc @client9 -- thanks for taking the time to fix this! (confirmed the fix works on win10 + git bash)

@AlekSi
Copy link
Contributor Author

AlekSi commented Apr 30, 2019

Updated PR with @syntaqx's solution.

@client9 Please find a minute to click merge button – it is a small change, it is tested by us, and fixes a real problem we are having.

@client9 client9 merged commit bddf173 into client9:master Apr 30, 2019
@AlekSi
Copy link
Contributor Author

AlekSi commented Apr 30, 2019

Thank you!

@client9
Copy link
Owner

client9 commented Apr 30, 2019

@AlekSi and friends... I'd be happy to give you commit rights if you want! Windows is something I'm never going to be able to support!

@syntaqx
Copy link

syntaqx commented Apr 30, 2019

@client9 If you need help with Windows support I'm happy to help. I work on Windows full time and fix this kind of stuff constantly :P

@client9
Copy link
Owner

client9 commented Apr 30, 2019

and it was so 💥

thanks @syntaqx

@DABH
Copy link
Collaborator

DABH commented May 1, 2019

Same here, my company is now relying on this package so have a vested interest in keeping it working smoothly on all platforms :) Would be happy to help maintain. Thanks again for the fast reply and for merging! 💯

@client9
Copy link
Owner

client9 commented May 1, 2019

I'd love to know how you all are using it. @syntaqx @DABH !

@DABH
Copy link
Collaborator

DABH commented May 1, 2019

In my case we have a cross-platform installer script for a software package, which users can curl ... | sh to download and install on their machines. So one can imagine how many of these scripts are useful :)

@syntaqx
Copy link

syntaqx commented May 1, 2019

My first use case was when I was using https://goreleaser.com/ to automate binary releases for a Go project. They had the project as a dependency.

Now I use it for pretty much any scripting suite I deliver. I work in operations/devops/infrastructure so scripts and their compatibility is life.

@codyaray
Copy link

codyaray commented May 1, 2019

@syntaqx same here. @DABH and I discovered this via godownloader as well. Pretty nice suite. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

uname_os_check 'MSYS_NT-10.0' got converted to 'msys_nt-10.0' which is not a GOOS value
5 participants