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 goreleaser #49

Merged
merged 16 commits into from
Sep 25, 2018
Merged

add goreleaser #49

merged 16 commits into from
Sep 25, 2018

Conversation

leighmcculloch
Copy link
Contributor

Add a goreleaser configuration file that builds binaries for Linux,
MacOS and Windows, for 32bit (x86/386) and 64bit (x64/amd64). The
binaries will be archived into a tar.gz/zip file along with the LICENSE
file.

The dist/ directory will be written to by goreleaser with the binaries
during the build process, so it's also been added to .gitignore.

Goreleaser can be installed with:
go get github.com/goreleaser/goreleaser

Or other methods for install can be found at:
https://goreleaser.com

To build all the binaries and release them to GitHub:

  1. Tag the release. e.g. git tag -a v1.0.0 -m 'First release'
  2. Generate a GitHub Personal Access Token. See https://github.com/settings/tokens.
  3. Push the release to GitHub. e.g. git push origin v1.0.0
  4. Run goreleaser. e.g. GITHUB_TOKEN=xxxxxxx... goreleaser

Goreleaser will build the binaries, and upload the binaries to the
GitHub project's release page for the release that was tagged.

Goreleaser will also set the main.version variable with the version
being released, so I added a -version flag that will print:
grpcurl v<version>

Closes #47

Add a goreleaser configuration file that builds binaries for Linux,
MacOS and Windows, for 32bit (x86/386) and 64bit (x64/amd64). The
binaries will be archived into a tar.gz/zip file along with the LICENSE
file.

The `dist/` directory will be written to by goreleaser with the binaries
during the build process, so it's also been added to .gitignore.

Goreleaser can be installed with:
go get github.com/goreleaser/goreleaser

Or other methods for install can be found at:
https://goreleaser.com

To build all the binaries and release them to GitHub:
1. Tag the release. e.g. `git tag -a v1.0.0 -m 'First release'`
2. Generate a GitHub Personal Access Token. See https://github.com/settings/tokens.
3. Push the release to GitHub. e.g. `git push origin v1.0.0`
4. Run goreleaser. e.g.  `GITHUB_TOKEN=xxxxxxx... goreleaser`

Go releaser will build the binaries, and upload the binaries to the
GitHub project's release page for the release that was tagged.
@leighmcculloch leighmcculloch mentioned this pull request Sep 15, 2018
386: x86
darwin: macos
files:
- LICENSE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tried to run this locally, this failed because the LICENSE file is not in this directory. However, when I tried editing this to point to ../../LICENSE, it generated a gnarly entry in the resulting TGZ that had ../LICENSE as the path (e.g. it created a directory entry in the archive for .., which seems bad).

I have not yet tried to read through goreleaser docs to see how to fix this situation, but I think we need this configuration to run and produce a good archive before we merge this PR.

I also noticed that the TGZ has a directory inside named grpcurl_<version>_<platform>_<arch>, which seems quite verbose. Many binary distributions I've seen (IIRC) just have a folder named grpcurl (without the version, platform, and architecture). Is there a "best practice" that I don't know about for putting all of this information into the resulting directory after unzipping a TGZ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes. I originally had the .goreleaser.yml in the root, then moved it to the subdirectory. I can change the config so there's no nested directory. Some binaries I've seen don't both with the LICENSE, and they just have the binary without the directory. The easiest way to install these then is:

curl https://... | tar xz -C /usr/local/bin

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the license can be nice if someone redistributes the binary, but practically for all of the users just downloading it to install immediately, it's an annoyance and it makes the install instructions slightly longer to extract only the binary from the tar.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, I think we should keep the license in the distribution. I think it's simple enough to extract the binary into /usr/local/bin, for people that want to install it there.

In the future, I may look into adding a homebrew recipe, which will simplify it further for OS X users, and make it even easier to auto-install into that location.

- goos: windows
format: zip
replacements:
amd64: x64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just looking at the protoc distribution, and they use x86_64 as the architecture here and x86_32 for 386. Maybe do the same, for consistency with a closely related binary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 👍

replacements:
amd64: x64
386: x86
darwin: macos
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The protoc distribution uses osx. So same remark as above: maybe change for consistency with that project?

@@ -30,13 +30,17 @@ import (
"github.com/fullstorydev/grpcurl"
)

var version = "<not set>"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something a little more informative, like dev build <no version set>? And it would also be nice to update the Makefile so it uses -ldflags to set main.version to something like dev build <git sha> (though that's fine to omit from this PR and maybe just add a TODO; if you want to tackle in another branch, great, and if not I can address it).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Lets tackle it in this branch. I'll add it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jhump I took a stab at doing the git sha. When doing make install it will build a binary where the output is like:

$ grpcurl -version
grpcurl dev build e198d3e

If the working directory is dirty, it will look like:

$ grpcurl -version
grpcurl dev build e198d3e-dirty

If it's for a released version, it will look like:

$ grpcurl -version
grpcurl 1.0.1

I dropped the v because it couldn't be uniformly applied to a version number of the dev build... string, but if you particularly like that format I can add it back in by customizing the configuring what goreleaser sets with the ldflags so that it is prefixed with a v.

@leighmcculloch
Copy link
Contributor Author

I've pushed some changes to this branch, but I want to add the dev build <git sha> and will circle back to do that. Still would appreciate thoughts on #49 (comment). Thanks for already reviewing it so quickly!

@leighmcculloch
Copy link
Contributor Author

@jhump Ready for another 👀 from you 😄 .

@jhump
Copy link
Contributor

jhump commented Sep 20, 2018

@leighmcculloch, promise I haven't forgotten. I will try to make time for further review later today.

Copy link
Contributor

@jhump jhump left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is really close. I just left a couple of more comments. Sorry it took me so long to get to looking at this.

.goreleaser.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@leighmcculloch
Copy link
Contributor Author

@jhump Addressed your last round of feedback. I also changed the format of the version text that's injected so it has a v on the beginning of it for the goreleaser builds. That's in 0620c43.

- amd64
- 386
ldflags:
- -s -w -X main.version=v{{.Version}}
Copy link
Contributor Author

@leighmcculloch leighmcculloch Sep 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied this line from the documentation, and the -s -w are part of the default (see below). The main difference I made was to drop the fields we don't use and to add a v at the beginning of the version. I did the prefix here and not in the code because the code would print out v in front of the dev build... string which wouldn't look great.

The default value is:

-s -w -X main.version={{.Version}} -X main.commit={{.Commit}} -X main.date={{.Date}}

Ref: https://goreleaser.com/customization/#Name%20Templates


.PHONY: release
release:
@GO111MODULE=off go get github.com/goreleaser/goreleaser
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also added GO111MODULE=off here to make sure the go get command doesn't mess with the go.mod file, otherwise go get will add goreleaser and it's dependencies to go.mod. There's a chance the behavior of go get might change in go1.12 so that we don't have to turn it off because of very active conversation on golang/go#27643.

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