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
Use module mode when building/installing #109464
Conversation
0f04fa4
to
0ddef73
Compare
0ddef73
to
3ab57d5
Compare
cc @thockin for visibility to chipping away at getting builds into the module world |
echo "${target}" | ||
elif [[ "${target}" =~ ^vendor/ ]]; then | ||
# Strip vendor/ prefix, since we're building in gomodule mode. | ||
echo "${target#"vendor/"}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is because we have external callers invoking our build scripts passing in WHAT=vendor/...
I'll track cleaning that up once all supported release branches have moved to module builds, but until then, we should continue accepting those WHAT targets and fix them up internally to keep producing the right binaries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for the most part (or rather all of?) those should be ginkgo, and the move to v2 will give us a clean point to switch to not using vendor seeing as v2 isn't merged yet. #109111
3ab57d5
to
3ec30b5
Compare
/triage accepted |
@@ -33,7 +33,7 @@ var k8sBinDir = flag.String("k8s-bin-dir", "", "Directory containing k8s kubelet | |||
var buildTargets = []string{ | |||
"cmd/kubelet", | |||
"test/e2e_node/e2e_node.test", | |||
"vendor/github.com/onsi/ginkgo/ginkgo", | |||
"github.com/onsi/ginkgo/ginkgo", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change is validated in CI and LGTM. (avoiding a real LGTM on first pass to let someone more familiar with the rest of our build system to review other parts. but they make sense to me too, thanks for the note on vendor/ support)
/milestone v1.25 holding to make sure this doesn't disrupt GOPATH / go workspace work @thockin is exploring. building in module mode would enable using buildinfo to get things like the built-in kustomize version from build module info, which sig-cli was looking into last release |
This definitely impacts my branch. That said, this is way smaller, so it probably makes sense to get this in first, if it is ready |
/retest |
Yeah, this is ready. All the changes (dropping vendor qualification, switching builds to module mode and ensuring they are building using local sources) are moving in the right direction, so I think this is a net positive. /retest |
/retest |
1 similar comment
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will rebase my workspaces on this.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, thockin 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 |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Targeted change to build with module mode enabled to be able to use buildinfo added in go1.18
Which issue(s) this PR fixes:
xref #82531
Special notes for your reviewer:
Does this PR introduce a user-facing change?