-
Notifications
You must be signed in to change notification settings - Fork 101
CLID-476: Fixes for cross-build errors #1303
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
base: main
Are you sure you want to change the base?
Conversation
aguidirh
left a comment
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.
Hi @dorzel,
Thanks for the PR. I tested make cross-build and it worked fine as expected.
I added only one concern I have to the PR.
Makefile
Outdated
|
|
||
| cross-build-linux-amd64: | ||
| +@GOOS=linux GOARCH=amd64 $(MAKE) "$(GO_BUILD_FLAGS)" --no-print-directory build GO_BUILD_BINDIR=$(GO_BUILD_BINDIR)/linux-amd64 | ||
| +@CGO_ENABLED=0 GOOS=linux GOARCH=amd64 $(MAKE) --no-print-directory build GO_BUILD_BINDIR=$(GO_BUILD_BINDIR)/linux-amd64 |
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.
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.
@aguidirh Ah I see, if it is an issue I can try to modify this to work with CGO enabled as well.
|
@dorzel: This pull request references CLID-476 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@dorzel: This pull request references CLID-476 which is a valid jira issue. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
Ok, I think the latest changes solve it nicely. The conditional assignment allows for:
|
|
/retest |
|
/test v1-e2e |
|
/retest |
|
@dorzel: all tests passed! Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
aguidirh
left a comment
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.
| GO_BUILD_PACKAGES := ./cmd/oc-mirror | ||
| GO_PACKAGE = github.com/openshift/oc-mirror/v2 | ||
|
|
||
| # Default to CGO_ENABLED=0 for cross-builds, but allow override from command line |
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.
nit: it would be nice to add in the comment the reason why it is needed to disable CGO for cross-builds.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aguidirh, dorzel 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 |
Description
I noticed when cross-building for ppc64le that make was erroring while trying to pass GO_BUILD_FLAGS as positional arguments. Cross-builds were also failing with CGO due to unknown ASM instructions while translating the assembly.
Also set GO_BUILD_BINDIR in the build step so one can run cross-builds first, without having already run
make build. (It was assuming v1/bin/oc-mirror existed regardless of the value of GO_BUILD_BINDIR).Github / Jira issue:
None
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
make clean
make tidy
make cross-build
make build
Expected Outcome
Please describe the outcome expected from the tests.
cross-builds properly build and populate /bin without errors.