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

Enable creation of release assets #124

Merged
merged 6 commits into from
Jun 4, 2024
Merged

Enable creation of release assets #124

merged 6 commits into from
Jun 4, 2024

Conversation

philrz
Copy link
Contributor

@philrz philrz commented May 30, 2024

We recently had a request from a community user to have Windows binaries of zync available so they could run it without having to install Go and compile the code on their workstation. To provide this, here I've enabled the same approach we use in the Zed repo for creating tagged release assets, which in this case would allow the user to download and unpack the ZIP on their Windows system and copy the zync.exe to a folder that's in their PATH.

I've tested out the approach in this branch in a personal fork https://github.com/philrz/zync and you can see the successfully-created artifacts in https://github.com/philrz/zync/releases. As a smoke test I did unzip the Windows bundle and was able to successfully invoke the zync.exe. I also went ahead and made a fork of our Homebrew repo at https://github.com/philrz/homebrew-tap and you can see the zync formula populated ok there as well.

One thing I'll point out is that my artifact from the personal fork does not return a proper version string.

C:\Users\Phil\Downloads\zync-v1.0.0.windows-amd64>.\zync.exe version
(devel)

I suspect this problem may not show up when we create artifacts in the true zync repo, since we have a very similar known problem with Zed in personal forks (brimdata/super#4921). However, this whole topic of how the version strings get assembled has always been a little mysterious to me so I figured I'd point it out in case it concerns anyone enough that they want to dig deeper before merging this.

@philrz philrz requested a review from nwt May 30, 2024 19:29
@philrz philrz self-assigned this May 30, 2024
.goreleaser.yaml Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
philrz and others added 2 commits June 3, 2024 18:07
Co-authored-by: Noah Treuhaft <noah.treuhaft@gmail.com>
Co-authored-by: Noah Treuhaft <noah.treuhaft@gmail.com>
@philrz
Copy link
Contributor Author

philrz commented Jun 4, 2024

@nwt: I accepted your suggested changes, mirrored them into my personal fork at https://github.com/philrz/zync , made a test release v0.10.0, and successfully installed it via brew. So, I think this PR is ready for another look and possible Approve.

@philrz philrz requested a review from nwt June 4, 2024 01:09
.goreleaser.yaml Outdated Show resolved Hide resolved
.goreleaser.yaml Outdated Show resolved Hide resolved
.goreleaser.yaml Outdated Show resolved Hide resolved
.goreleaser.yaml Outdated Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
philrz and others added 3 commits June 4, 2024 09:13
Co-authored-by: Noah Treuhaft <noah.treuhaft@gmail.com>
goreleaser trims any "v" prefix from .Version but we want to keep it.
.Summary is equivalent to "git describe --always --dirty --tags" so it
includes the "v" prefix.
@philrz
Copy link
Contributor Author

philrz commented Jun 4, 2024

As a final test after that last commit from @nwt, I repeated the creation of a tagged release on my fork repo. The version string now appears as expected:

C:\Users\Phil\Downloads\zync-v0.10.0.windows-amd64>.\zync.exe version
v0.10.0

The same was also true on Linux and macOS (both the unpacked artifact from the GitHub Releases and the brew-installed binary).

@philrz philrz merged commit 6ef4f3e into main Jun 4, 2024
1 check passed
@philrz philrz deleted the release-assets branch June 4, 2024 20:55
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