Skip to content
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

Add windows support #50

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

piksel
Copy link

@piksel piksel commented Mar 1, 2021

With this change it seems like windows is fully supported.

Usually, building for all platforms on go works out of the box, but since this uses cgo, I think you need to build the windows binaries on windows (or using an appropriate xplat toolchain setup).

@mavogel
Copy link
Contributor

mavogel commented Oct 19, 2021

awesome @piksel! Can you also add tests in GH actions for windows? You might find the windows related commands from here useful: https://github.com/gesellix/awsu/commits/master

@piksel
Copy link
Author

piksel commented Oct 19, 2021

I'm not sure I understand what you mean. The gesellix@ed157b1 commit only deals with a symlink (which does work on recent versions of windows).
The test CI action probably does not work on windows, but that is because it relies on tools that might not be available in the windows image:

  • make
  • xargs
  • grep

Why isn't this used for tests?

go test ./... -cover

(the test PHONY in Makefile is this:)

go list ./... | grep -v exp | xargs go test -cover

@mavogel
Copy link
Contributor

mavogel commented Oct 19, 2021

I'm sorry, I wasn't precise enough. I was more referring to this commit: gesellix@663bc0b
Feel free to adapt the makefile accordingly :) The repo was outdated and we are refurbishing it now.

@piksel
Copy link
Author

piksel commented Oct 20, 2021

I added windows as a matrix target to the test workflow. Gesellix' fork also included macos, and built on ubuntu-latest instead of ubuntu-20.04, but I figured this was what was inside the scope of this PR.
Adding support to goreleaser as well might be tricky, since the dependencies work very differently on windows and linux. Have not tried it though...

@mavogel
Copy link
Contributor

mavogel commented Oct 20, 2021

Awesome :) thanks. Regarding the builds: we will address this in #56

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.

2 participants