feat: Windows release

[ghost]

Closes #108

[stepanstipl]

Nice 👍 , seems the cross-compile works, and as it already works for mac/linux fine, I would kind of hope that it will work for Win too, would be nice to test though - did you manage to?

And I think it will need corresponding - name: Upload Release Asset - Windows step added to .github/workflows/main.yaml to actually upload the windows binary.

[ghost]

@stepanstipl yeah will add just wanted to see if cross compile worked

[ghost]

@stepanstipl added and removed WIP

[stepanstipl]

Cool 👍 I think this looks good, I would just like to do a test on a real windows machine to confirm it actually works before merging this (seen various issues with CGO and dns and stuff historically around K8s go-client - saying that I believe it should not be a problem anymore).

[ghost]

@stepanstipl agreed I don't have a windows machine

[gerardgorrion]

Hi all, there're some update of this feature? shows "Merging is blocked". Thanks!

[ghost]

@gerardgorrion apologies, we have been busy. Have you had a chance to try the binary from this build?

[stepanstipl]

I did a quick test on Win and found 2 issues:

1. I think the binary has to have .exe extension, otherwise I was not able to convince the system to recognize it as executable (funnily enough once it has the .exe, you can invoke it both by kubent-windows-amd64 and kubent-windows-amd64.exe names... both will execute the .exe binary)
   -> we should name the download artefact that way.

2. The coloured output is messed up - probably we should disable it for windows, I assume it won't recognize the same terminal control characters. Happy to handle this in a separate issue, as it's not affecting core functionality, just looks ugly ↓

Otherwise hooray 🎉 - seems to work and collect resources!

[ghost]

Ok, this is now a .exe

[ghost]

@stepanstipl this one is all done now just awaiting your review

[stepanstipl]

I'm afraid this didn't quite work:

$ GOOS=windows GOARCH=amd64 gmake all
...
$ cd release-artifacts
$ tar -xvzf kubent-dev-windows-amd64.tar.gz
x kubent
$ ls
kubent                          kubent-dev-windows-amd64.tar.gz

Seems like the binary in the archive is still called just kubent.

[ghost]

@stepanstipl my bad it looks like it just packing the wrong one, will fix.

ls bin
kubent-linux-amd64  kubent-windows-amd64.exe  packed

[ghost]

@stepanstipl this has been fixed:

$ GOOS=windows GOARCH=amd64 make all
$ tar -xvzf release-artifacts/kubent-dev-windows-amd64.tar.gz
kubent.exe

[ghost]

I had to add another nasty if statement tho

[stepanstipl]

Good stuff, some suggestions:

I would suggest introducing a new variable to avoid 3x repeated if condition (and use make logic):

ifeq (windows,$(GOOS))
  BIN_RELEASE_SUFFIX ?= .exe
endif
BIN_RELEASE_SUFFIX ?=

I think it's a bit cleaner. Then we can just change the name in the transform arg:

$(TAR) -cvz --transform 's,$(PACKED_DIR)/$(*)-$(BIN_ARCH),$(*)$(BIN_RELEASE_SUFFIX),gi' -f "$@" "$<"

This has the benefit/disadvantage - depending how you look at it, as compared to the current implementation, of the binary name being consistent, and only appending the .exe when we build the archive. WDYT?

[stepanstipl]

Some minor bits to clean up, but I think it looks good and it's pretty much there! 🎉

Super nice that you used the matrix functionality also for uploading the artefacts! 👍 👍

[stepanstipl]

Looks GOOD!