Non redistributable does not build everything, plus aarch64 #12333
Conversation
tdawson
force-pushed
the
2016-12-multi-arch-non-redistribute
branch
from
f5cd18d
to
a12e23c
Compare
|
[test] |
| @@ -12,6 +12,11 @@ if [[ -n "${OS_ONLY_BUILD_PLATFORMS-}" ]]; then | |||
| filtered+=("${platform}") | |||
| fi | |||
| done | |||
| if [[ ${OS_ONLY_BUILD_PLATFORMS} == "linux/ppc64le" ]]; then | |||
There was a problem hiding this comment.
We should handle these two cases by making sure "${OS_CROSS_COMPILE_PLATFORMS}" contains these architectures when we can build for them, as we are doing in common.sh
There was a problem hiding this comment.
I was debating about that. But the problem would then be that ppc64le and aarch64 would be built each time someone did a cross-compile.
When building rpm's that do a cross compiled, we are already building for linux/i386, which we then just throw away. I was trying to not have 2 more long builds that get thrown away 99% of the time.
There was a problem hiding this comment.
I still feel like appending these architectures to $OS_CROSS_COMPILE_PLATFORMS, the list of possible platforms, and using $OS_ONLY_BUILD_PLATFORMS, the platform whitelist, is the correct approach. We should look at the places we call the cross-compile scripts and make sure we are passing in the platform whitelist correctly.
| @@ -20,6 +25,8 @@ OS_BUILD_PLATFORMS=("${platforms[@]}") | |||
| host_platform=$(os::build::host_platform) | |||
| if [[ $host_platform == "linux/ppc64le" ]]; then | |||
| OS_GOFLAGS_LINUX_PPC64LE="-tags=gssapi" os::build::build_binaries "${OS_CROSS_COMPILE_TARGETS[@]}" | |||
| elif [[ $host_platform == "linux/arm64" ]]; then | |||
There was a problem hiding this comment.
We can't assume Bash 3.4+ so we don't have associative arrays, so the generic approach in common.sh uses eval, but I still prefer it to these types of if/elif blocks.
There was a problem hiding this comment.
I'm ok with whatever you feel is best. I used the elif because it was used several other places. It is also more generic so in my mind it should work the same in more variations of bash.
There was a problem hiding this comment.
Oh, whoops, I mis-read this slightly. We can just do this:
local platform_goflags_envvar; platform_goflags_envvar="OS_GOFLAGS_$(echo ${platform} | tr '[:lower:]/' '[:upper:]_')"
declare "${platform_goflags_envvar}=-tags=gssapi"
os::build::build_binaries "${OS_CROSS_COMPILE_TARGETS[@]}"If we wanted to support zsh or something we could use typeset but we are fine with bash.
There was a problem hiding this comment.
Sorry for the delay, but my aarch64 testing area is having issues.
On x86_64 this works if you get rid of the 'local platform_goflags_envvar;' So it looks like this.
platform_goflags_envvar="OS_GOFLAGS_$(echo ${host_platform} | tr '[:lower:]/' '[:upper:]_')"
declare "${platform_goflags_envvar}=-tags=gssapi"
os::build::build_binaries "${OS_CROSS_COMPILE_TARGETS[@]}"If it passes the aarch64 test, I'll update this pull request with it in.
There was a problem hiding this comment.
Oh, of course. We're not in a function so local doesn't make sense. That sounds good.
Add ppc64le and arm64 to filtered list Simplify OS_GOFLAGS
tdawson
force-pushed
the
2016-12-multi-arch-non-redistribute
branch
from
a12e23c
to
c88f7b8
Compare
|
I was able to do a test on ppc64le. Although it fails in the golang build part, it ran the hack/build-cross.sh script correctly. But I won't ask for a merge or anything until get this fully tested on an aarch64 machine. |
|
@openshift-bot, the last build failed from the following flakes:
re[test] |
|
Evaluated for origin test up to c88f7b8 |
|
continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/12697/) (Base Commit: 15872c0) |
|
Origin Action Required: Pull request cannot be automatically merged, please rebase your branch from latest HEAD and push again |
openshift-bot
added
the
needs-rebase
|
Due to work on the hack scripts, this pull request will need to be rewritten. I might do a completely different pull request for it. But I'm leaving this open for now. If I do a new pull request, I'll link to it from here. |
|
Due to the number of changes in hack scripts, it was easier to create a new pull request on the latest code. |
|
Again, another rebase where it was easier to start a new pull request. |
If you set the non-redistributable bit to 0, it will not build everything.
This not only saves time, but helps for multi-arches that might not be able to do cross compiling.
This also allows origin rpms to be built on aarch64