-
Notifications
You must be signed in to change notification settings - Fork 451
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
Enhance makefile tools binaries to be system and arch independent #9589
Conversation
/retest-required |
/cc @timebertt |
/retest-required |
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 will break our precompiled tools (ref). When they are imported again (ref) the files would be copied to something like ./hack/tools/bin/linux-amd64/linux-amd64
I think the unit-tests are failing because golangci-lint
does not find the library at the place where it is expected (.golangci-lint.yaml). I don't know if it is possible to use the sub folder here too.
Now, by default the os-arch specific dir will be used, in pre-configured environment the non os-arch specific dir will be used. |
/retest-required |
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.
/retest
/remove-lgtm |
I might need to rebase because of the tests failing like https://prow.gardener.cloud/view/gs/gardener-prow/pr-logs/pull/gardener_gardener/9589/pull-gardener-e2e-kind-ha-multi-zone-upgrade/1780228296198328320#1:build-log.txt%3A1694-1697
|
d614e64
to
619b90f
Compare
/test pull-gardener-e2e-kind |
/test pull-gardener-e2e-kind-ha-multi-zone |
1 similar comment
/test pull-gardener-e2e-kind-ha-multi-zone |
/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.
/lgtm
LGTM label has been added. Git tree hash: 417359dd209bd3cf010a0bce8aaaf89f95b34d2f
|
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.
Nice improvement!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: oliver-goetz, timebertt 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 |
/retest |
How to categorize this PR?
/area dev-productivity
/kind enhancement
What this PR does / why we need it:
Enhance makefile tools binaries to be system and arch independent
Sometimes, I am mounting the source code in a dev container and running various make commands from there, hence the system could differ (
linux
vsdarwin
andarm64
vsamd64
). This results in failure because darwin cannot run linux binaries and vise versa, to fix this I have to manually delete binaries one by one or the entire folder which is time consuming. By using dedicated folder for each system and arch, each target will get own set of binaries and the collision will be avoided.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Release note: