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 configuration for goreleaser #257

Merged
merged 5 commits into from May 10, 2020
Merged

Conversation

rogchap
Copy link
Contributor

@rogchap rogchap commented May 8, 2020

This adds basic configuration for goreleaser.
On a tagged release it will build for linux, osx and windows, and add to the GitHub release page, including the artifacts, changelog and checksum file.
To test locally you can run goreleaser --snapshot --skip-publish --rm-dist

I've also change the SemVer to be auto generated at build time via ldflags; This may cause slight confusion for people installing via go get as it will display 0.0.0-dev, but allows easier version management via CI releases (with goreleaser); In the future we should favour CI builds over using go get.

This is a prerequisite to having other distributions such as docker (#180) and homebrew (recommended)

I've amended CI (travis) to run the tests and do a deploy when there is a valid release tagged.

@rogchap
Copy link
Contributor Author

rogchap commented May 8, 2020

Looks like there is a issue with the golden files and line endings on Windows \n vs \r\n 😞
Not sure the best way to fix? Maybe needs a build flag to select a different golden file based on platform?

Rather than fix this now, I'm going to remove windows from the test script. We can still build for Windows though.

@rogchap rogchap mentioned this pull request May 8, 2020
@LandonTClipp
Copy link
Contributor

LandonTClipp commented May 8, 2020

Hi @rogchap , thanks a lot for the PR, this looks great. Unfortunately I think we really need to have the versioning saved into the go source code so that you can go get a specific version, as well as mockery being able to correctly identify its version when doing go get. The workflow I'd really like to have is this:

  1. A human changes the SemVer variable in source, and commits to master.
  2. Travis CI initiates build on master. Runs tests.
  3. If tests are successful, Travis gets the SemVer using something like mockery -version
  4. If the tagged version doesn't previously exist, tags the commit to master using the version from step 3, creates GH release and uploads necessary files.

The pattern of baking the version info on build time is a common one but one I'm not very fond of for Golang projects.

I don't think that should be too difficult to implement. What do you think?

@rogchap
Copy link
Contributor Author

rogchap commented May 9, 2020

Hey @LandonTClipp,

I strongly recommend moving away from go get to install binaries. The Go Team have previously stated that this is a feature that they want to remove, but is only there for backward compatibility reasons.

go get results in inconsistent behavior depending on "gopath mode". If you use go get outside a Go modules repo this will install using "gopath mode" and would get, and install, the code in the master branch, which could have unreleased code. In "modules mode" this would install the latest git tagged version (unless the user does go get ...@v1.0.0). The version number installed is based off the git tag, not SemVer, so there is a possibility of version miss-matches.
Using a package manager for exec binaries are far easier to manage and maintain (homebrew etc); from both a user perspective and maintainer perspective.

goreleaser requires a git tag before it will build; so you always need a new valid git tag to do a deployment. The flow you describe is actually quite complex, because you would have to build the binary before building the binary (!!??) to get the version, check that there are no tags with that version and then (and only then) do a deployment based of that version. Not impossible, but quite messy, and unconventional from managing Go releases.

You could grep (or something like that) to get the version; but this would be brittle. You also need to remember to always bump the version number in a Go file and then update the test (I guess we could change the test to read SemVer?). This will get tedious if every patch you merge you manually have to update master with a new version, rather than just doing a git tag and it being automated.

If you are not keen on master containing version 0.0.0-dev maybe we could change this to master instead!?

If you really really really still want to maintain go get as a way to install, and really really really care that it does not output master (or 0.0.0-dev) when doing --version; Then we can change the default SemVer to be 1.0.0 (or whatever is latest) but still have the binary build process "override" the value from the git tag (we should change the test so that the golden generated file matches correctly).
This would result in two manual steps: update SemVer then git tag with same version. If you ever forgot to bump the SemVer at least the built binaries would have the correct version embedded from the git tag.


go:
- 1.11.x
- 1.12.x
Copy link
Contributor

Choose a reason for hiding this comment

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

stretchr/testify still supports 1.11/1.12. Yes I know they're ancient but let's keep them in here for now.

https://github.com/stretchr/testify#supported-go-versions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Surly this only effects the build of the mockery binary, rather than the supported Go output by the tool? We would only build mockery binary with the latest (1.14) so we could actually remove all but 1 Go version to run the tests. 💁‍♂️

That said, I've added both 1.11 and 1.12 back in as requested.

@LandonTClipp
Copy link
Contributor

LandonTClipp commented May 10, 2020

@rogchap thank you for all your input. After thinking it over I think what you are saying makes sense and we can go ahead and move forward. I really don't think that it's that complicated of a workflow but if that's how most of the Go community releases binaries I think we should follow that. Let's keep the idea of v0.0.0-dev but clearly mark how releases will be pushed in the README. Currently README says to go get stuff, an asterisk will need to be added to specify this is to get a development version. I can add this myself.

I will look this over once more on Monday and get this merged. I'll put in a few minor comments. Thanks!!

@LandonTClipp LandonTClipp added approved feature Feature request approved for development Tentative Approval PR has been approved by maintainers, pending merge and removed approved feature Feature request approved for development labels May 10, 2020
@LandonTClipp LandonTClipp merged commit 232ff3e into vektra:master May 10, 2020
@mnaboka
Copy link

mnaboka commented May 10, 2020

oops, this change has broken our CI, we are installing latest mockery and check for version to be 1.0.0 :(
this is the command we use to install mockery
GO111MODULE=off go get -u github.com/vektra/mockery/.../
any recommendation how to pin to a 1.0.0 release?
thanks 🙏

@LandonTClipp
Copy link
Contributor

LandonTClipp commented May 10, 2020 via email

LandonTClipp added a commit that referenced this pull request May 10, 2020
People relying on version of latest commit being 1.0.0. Adding this back
in for backwards-compatibility.
@rogchap
Copy link
Contributor Author

rogchap commented May 10, 2020

If you can run in modules mode, then go get github.com/vektra/mockery/.../@1.0.0 should work?
Otherwise I would change your CI to not check for 1.0.0, but rather 1.x

@LandonTClipp
Copy link
Contributor

LandonTClipp commented May 10, 2020

@mnaboka can you confirm your CI is fixed from the new commit? I think to prevent other CI's from breaking we need to add this back in. We should add a deprecation note for a few months to inform people the default SemVer is going to be v0.0.0-dev

@mnaboka
Copy link

mnaboka commented May 10, 2020

@LandonTClipp sure, i've fixed it on my end, another issue is that all generated mocks also include a string with version, so all those mocks will also be changed.
Thank you

@LandonTClipp
Copy link
Contributor

Yes the commit reverts back to the old behavior where SemVer is 1.0.0. Thank you for fixing it on your end. I've added a note in the README about this, we'll keep that deprecation for a while and then we can switch it back over to 0.0.0-dev after a few months. After that people will just need to fix their CI.

@rogchap rogchap deleted the goreleaser branch May 10, 2020 23:08
gitlab-runner-bot pushed a commit to gitlabhq/gitlab-runner that referenced this pull request May 11, 2020
In vektra/mockery#257 mockery changed how the
version is specified
https://github.com/vektra/mockery/pull/257/files#diff-3a895417c300af3c34fab6ebb556e984R9.
When we run `go get github.com/vektra/mockery/cmd/mockery` the version
is `0-0-dev` when in reality it's a different version.

Update the installation of mockery to download the file from GitHub
instead of `go get` so that the version is set correctly and prevents
changes in the generated mocks.

This causes pipelines to fail like https://gitlab.com/gitlab-org/gitlab-runner/-/jobs/546233640

closes https://gitlab.com/gitlab-org/gitlab-runner/-/issues/25645
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Tentative Approval PR has been approved by maintainers, pending merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants