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.

Sign up for GitHub

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

Jump to bottom

✨ Remove deprecated go get from Makefile templates #2486

Merged
merged 1 commit into from
Mar 15, 2022
Merged

✨ Remove deprecated go get from Makefile templates #2486

merged 1 commit into from
Mar 15, 2022

Conversation

ryantking
Copy link
Contributor

@ryantking ryantking commented Jan 18, 2022 •
edited

Signed-off-by: Ryan King ryking@redhat.com

In go 1.18, using go get to install go binaries will be removed from
in favor of using go install. This commit updates the Makefile
templates to use alternative installation methods to install the tools
used at generation.

Since go install adds extra constraints on what go packages can be
installed, not all tools were a simple go get => go install change:

  • kustomize: Incompatible with go install because its go.mod has
    exclude and replace directives. The template now uses the official
    install script.
  • controller-gen: Fully compatible with go install.
  • setup-envtest: Can be installed using go install, but not a
    specific version since the repo does not have the tag scheme since it is
    in a sub-module. It would need a tag in the format
    tools/setup-envtest/vX.Y.Z.

The commit also regenerated all book source code using this new pattern.

Resolves: #2477

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 18, 2022
@k8s-ci-robot
Copy link
Contributor

Hi @ryantking. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 18, 2022
@camilamacedo86
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 20, 2022
@camilamacedo86
Copy link
Member

/ok-to-test

@mikebz
Copy link

mikebz commented Jan 26, 2022

ahh... saw this too late, but it looks like it's more evolved than my initial stab at this problem #2496 - what do we need to do to get this over the line? Happy to help on the kustomize end as well.

@camilamacedo86 camilamacedo86 mentioned this pull request Jan 30, 2022
Closed
@ryantking ryantking marked this pull request as ready for review February 2, 2022 21:04
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 2, 2022
@ryantking
Copy link
Contributor Author

@camilamacedo86 @mikebz sorry for the delay, I think the PR is in a good state to be accepted now.

varshaprasad96
varshaprasad96 reviewed Feb 2, 2022
View reviewed changes
pkg/plugins/golang/v2/scaffolds/internal/templates/makefile.go
.PHONY: controller-gen
controller-gen: $(CONTROLLER_GEN) ## Download controller-gen locally if necessary.
$(CONTROLLER_GEN):
GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_TOOLS_VERSION)
Copy link
Member

Choose a reason for hiding this comment

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

if controller-gen is already installed in the required path, are we running go install again every time ?
Same with kustomize?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes the makefile target dependencies are setup such that it'll only be installed if not present.

@ryantking
Copy link
Contributor Author

/retest

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 3, 2022
@bprashanth bprashanth mentioned this pull request Feb 4, 2022
Closed
@kaovilai kaovilai mentioned this pull request Mar 25, 2022
Closed
@camilamacedo86 camilamacedo86 mentioned this pull request Mar 27, 2022
Closed
@irvinlim irvinlim mentioned this pull request Mar 27, 2022
Merged
@ryantking ryantking mentioned this pull request Mar 28, 2022
Closed
@camilamacedo86 camilamacedo86 mentioned this pull request Apr 1, 2022
Closed
mikenairn added a commit to mikenairn/kcp-glbc that referenced this pull request Apr 5, 2022
@mikenairn
Update Makefile (go 1.18 compatibility)
3803bfa 
Install go dependencies using `go install` instead of `go get`. see
kubernetes-sigs/kubebuilder#2486

Adds a dev-tools job that checks scripts/make targets that need to be
run locally be developers run using different versions of go.
@juanig1 juanig1 mentioned this pull request Apr 7, 2022
Merged
@ryantking ryantking mentioned this pull request Apr 27, 2022
Closed
@jasonliu747 jasonliu747 mentioned this pull request May 1, 2022
Merged
nakamasato added a commit to nakamasato/kubebuilder-quickstart that referenced this pull request May 7, 2022
@nakamasato
Update Makefile to support Go version 1.18
856c942 
Update Makefile based on kubernetes-sigs/kubebuilder#2486
mbana pushed a commit to kubeshop/kusk-gateway that referenced this pull request May 18, 2022
Fix/Cleanup Makefile - Closes kubeshop/kusk-gateway/#418:
36a5da8 
* Replace `go get` with `go install` in `Makefile`.
* Tidy up `Makefile`.
* Introduce `tools` in `Makefile` that downloads all the tools/dependencies required.
* `DOCKER_BUILDKIT`: No point in defining this everyone - exporting and defining should be enough.

For further information, see:

* kubernetes-sigs/kubebuilder#2566
* kubernetes-sigs/kubebuilder#2486
mbana added a commit to kubeshop/kusk-gateway that referenced this pull request May 18, 2022
@mbana
Fix/Cleanup Makefile - Closes #418:
5ef6295 
* Replace `go get` with `go install` in `Makefile`.
* Tidy up `Makefile`.
* Introduce `tools` in `Makefile` that downloads all the tools/dependencies required.
* `DOCKER_BUILDKIT`: No point in defining this everyone - exporting and defining should be enough.

For further information, see:

* kubernetes-sigs/kubebuilder#2566
* kubernetes-sigs/kubebuilder#2486
@mbana mbana mentioned this pull request May 18, 2022
Merged
4 tasks
@camilamacedo86 camilamacedo86 mentioned this pull request May 19, 2022
Closed
mbana added a commit to kubeshop/kusk-gateway that referenced this pull request May 24, 2022
@mbana
Fix/Cleanup Makefile - Closes #418:
2cb13f7 
* Replace `go get` with `go install` in `Makefile`.
* Tidy up `Makefile`.
* Introduce `tools` in `Makefile` that downloads all the tools/dependencies required.
* `DOCKER_BUILDKIT`: No point in defining this everyone - exporting and defining should be enough.

For further information, see:

* kubernetes-sigs/kubebuilder#2566
* kubernetes-sigs/kubebuilder#2486
mbana pushed a commit to kubeshop/kusk-gateway that referenced this pull request May 25, 2022
Fix/Cleanup Makefile - Closes #418 (#419)
42e0b90 
Fix/Cleanup `Makefile` - Closes #418:

* Replace `go get` with `go install` in `Makefile`.
* Tidy up `Makefile`.
* Introduce `tools` in `Makefile` that downloads all the tools/dependencies required.
* `DOCKER_BUILDKIT`: No point in defining this everyone - exporting and defining should be enough.

For further information, see:

* kubernetes-sigs/kubebuilder#2566
* kubernetes-sigs/kubebuilder#2486
@marcoandredinis marcoandredinis mentioned this pull request Jun 1, 2022
Closed
@erikgb erikgb mentioned this pull request Jul 12, 2022
Closed
@rhopp rhopp mentioned this pull request Jul 13, 2022
Merged
@jiayiwang7 jiayiwang7 mentioned this pull request Dec 1, 2022
Merged
@gold-kou gold-kou mentioned this pull request Jul 6, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joelanford joelanford joelanford left review comments

@wmuizelaar wmuizelaar wmuizelaar left review comments

@bprashanth bprashanth bprashanth left review comments

@varshaprasad96 varshaprasad96 varshaprasad96 left review comments

@jmrodri jmrodri jmrodri approved these changes

@rashmigottipati rashmigottipati rashmigottipati approved these changes

@camilamacedo86 camilamacedo86 camilamacedo86 approved these changes

@Adirio Adirio Awaiting requested review from Adirio

Assignees

@jberkhahn jberkhahn

@camilamacedo86 camilamacedo86

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Stop to use go get in favor of go install