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

Update Makefile to install govvv properly #6

Closed
t-persson opened this issue Aug 31, 2021 · 1 comment · Fixed by #23
Closed

Update Makefile to install govvv properly #6

t-persson opened this issue Aug 31, 2021 · 1 comment · Fixed by #23
Assignees
Milestone

Comments

@t-persson
Copy link
Collaborator

Description

This issue stems for a comment in PR #1 and since Makefiles are a bit of a mystery for me, I'll copy the comment here

Comment references: https://github.com/eiffel-community/eiffel-goer/blob/main/Makefile#L9

Nothing we have address now, but this'll pull the latest govvv every time we build which potentially could result in different results (or even errors) if an old commit it rebuilt later on (plus it adds latency). I think we could append @v0.3.0 to the package name in the go get command to always get that version. Another option is to track the version via go.mod and use a dummy file named e.g. tools.go that looks like

// +build tools
package tools
 
import (
        _ "github.com/ahmetb/govvv"
)
to make sure it isn't purged from go.mod, then have a makefile rule like

$(GOBIN)/govvv:
        go get github.com/ahmetb/govvv
to install the tool only once per workspace (and add $(GOBIN)/govvv as a prerequisite of the build rule). See also golang/go#25922.

Motivation

Exemplification

Benefits

Possible Drawbacks

@magnusbaeck
Copy link
Member

As of Go 1.16 the recommended way of installing tool is to use go install with a version suffix to the package (see release notes) so that's what I'll do instead. Apart from govvv I'll fix the makefile to install all other dependencies in that manner too.

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 a pull request may close this issue.

2 participants