✨ Makefile: Create Version-Agnostic Symlinks for Specified Versioned Binaries #4013
Conversation
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
size/L
camilamacedo86
dismissed
their stale review
ignore for now, I need to give a better look
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
camilamacedo86
changed the title
|
The last two pushes should fix upgrades failing due to the binary file already existing (or rather the symlink to it) and it will also ensure the correct binary is used when downgrading by replacing the symlink every time. |
| GOBIN=$(LOCALBIN) go install $${package} ;\ | ||
| mv "$$(echo "$(1)" | sed "s/-$(3)$$//")" $(1) ;\ |
There was a problem hiding this comment.
I like the idea of remove sed since it does not work always at the same way in linux and mac/
sed is usually problematic
There was a problem hiding this comment.
This could also be done without the symlink stuff, in case this doesn't make it in.
There was a problem hiding this comment.
If you have an alternative approach that avoids using symlinks and you believe it would be more suitable, please feel free to suggest it. The use of symlinks might not be very clear for beginner users and could cause some confusion. Otherwise, I think I am ok with
| set -e; \ | ||
| package=$(2)@$(3) ;\ | ||
| echo "Downloading $${package}" ;\ | ||
| rm -f $(1) || true ;\ |
There was a problem hiding this comment.
What are we removing here? The symbolic link or binary?
If is the symbolic link 👍
I do not think that we should remove anything without an explicit request, OK, or demand from a user.
It is not a good practice/ux delete staff without the user knowledge.
There was a problem hiding this comment.
The symbolic link, otherwise the go install will fail.
k8s-ci-robot
added
the
ok-to-test
There was a problem hiding this comment.
Hi @schrej,
Thank you for your time and collab on this one 🥇
Just one question here: https://github.com/kubernetes-sigs/kubebuilder/pull/4013/files#r1676137395
Otherwise, after the explanations I think I am OK with the changes.
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, schrej The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
camilamacedo86
changed the title
camilamacedo86
changed the title
While the new approach (we'll, new since I last used kubebuilder) finally does proper upgrades of tools, which is very nice, it makes it much harder to use the versioned tools in scripts that aren't called by the makefile, e.g. Tilt. (https://tilt.dev/)
This MR introduces symlinks to the latest version of the tools, which are updated whenever a new version is downloaded.
The Makefile also uses those symlinks to access the tools, which makes the
go-install-toolfunction a bit simpler, getting rid of the sed replace.Details
This change does not remove the explicit versioning, and the versioned binaries will still exists in the bin folder. There will just be an additional symlink to the version that was installed last (the symlink is only created during installation, which could be changed to support downgrades to already installed versions).
Use cases:
It is helpful if you write scripts that use the binaries downloaded by the makefile. You can just use e.g.
bin/kustomizeand there is no need to adapt it to the version currently set in the makefile, and also no need to execute the script with the makefile to pass in the binary to use.The only relevant one probably is kustomize, but it should be the same behaviour for all of them.
Another use-case can be to point your IDE to your
bin/golangci-lintbinary to always use the version specified by the project.