Skip to content

Add support for riscv #4079

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ PYIGNORE ?= E128,E241,E402,E501,E722,W503,W504
.PHONY: all check shellcheck flake8 pycheck unittest clean mantle mantle-check install

MANTLE_BINARIES := ore kola plume
KOLET_ARCHES := aarch64 ppc64le s390x x86_64
# Typically just build kolet for the architecture we are on.
KOLET_ARCHES:=$(shell uname -m)

all: bin/coreos-assembler mantle

Expand All @@ -27,12 +28,21 @@ GOARCH:=$(shell uname -m)
export COSA_META_SCHEMA:=$(shell pwd)/src/v1.json
ifeq ($(GOARCH),x86_64)
GOARCH="amd64"
# On x86_64 only we'll build kolet for all the other architectures.
# This is mainly so we can initiate tests against a remote that is
# a different architecture. i.e. x86_64 kola testing against an
# aarch64 EC2 instance.
KOLET_ARCHES=aarch64 ppc64le s390x x86_64
Copy link
Member

Choose a reason for hiding this comment

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

Inconsistent tabs/spaces.

Copy link
Member Author

Choose a reason for hiding this comment

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

will fix

else ifeq ($(GOARCH),aarch64)
GOARCH="arm64"
endif
ifeq ($(GOARCH),riscv64)
# https://github.com/golang/go/issues/72840
CGO_OVERRIDE=CGO_ENABLED=0
endif

bin/coreos-assembler:
cd cmd && go build -mod vendor -o ../$@
cd cmd && $(CGO_OVERRIDE) go build -mod vendor -o ../$@
.PHONY: bin/coreos-assembler

.%.shellchecked: %
Expand Down Expand Up @@ -73,7 +83,7 @@ mantle: $(MANTLE_BINARIES) kolet

.PHONY: $(MANTLE_BINARIES) kolet
$(MANTLE_BINARIES) kolet:
mantle/build cmd/$(basename $@)
KOLET_ARCHES="$(KOLET_ARCHES)" mantle/build cmd/$(basename $@)
Copy link
Member

Choose a reason for hiding this comment

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

Hmm weird, that shouldn't be necessary. It should just get passed through.


mantle-check:
cd mantle && ./test
Expand Down
2 changes: 2 additions & 0 deletions build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ set -x
srcdir=$(pwd)

configure_yum_repos() {
[ "${arch}" == "riscv64" ] && return # No continuous repo for riscv64 yet
local version_id
version_id=$(. /etc/os-release && echo ${VERSION_ID})
# Add continuous tag for latest build tools and mark as required so we
Expand Down Expand Up @@ -102,6 +103,7 @@ install_rpms() {
# and it is very useful to have in the same place/flow as where we do builds/tests related
# to CoreOS.
install_ocp_tools() {
[ "${arch}" == "riscv64" ] && return # No ocp tools for riscv64
# If $OCP_VERSION is defined we'll grab that specific version.
# Otherwise we'll get the latest.
local url="https://mirror.openshift.com/pub/openshift-v4/${arch}/clients/ocp/latest${OCP_VERSION:+-$OCP_VERSION}/openshift-client-linux.tar.gz"
Expand Down
12 changes: 10 additions & 2 deletions mantle/build
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ if [[ $# -eq 0 ]]; then
set -- cmd/* schema
fi

declare -A BASEARCH_TO_GOARCH=([s390x]=s390x [x86_64]=amd64 [aarch64]=arm64 [ppc64le]=ppc64le)
KOLET_ARCHES="${KOLET_ARCHES:-s390x x86_64 aarch64 ppc64le}"
declare -A BASEARCH_TO_GOARCH=([x86_64]=amd64 [aarch64]=arm64 [ppc64le]=ppc64le [riscv64]=riscv64 [s390x]=s390x)
ARCH=$(arch)
KOLET_ARCHES="${KOLET_ARCHES:-$(ARCH)}"
Copy link
Member

Choose a reason for hiding this comment

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

Invalid syntax I think? Also feels like we can just keep that as one line.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will fix in next upload. Not sure what you mean by keep it as one line.

I use the $ARCH var later in the script.


race=
if [ -n "${ENABLE_GO_RACE_DETECTOR:-}" ] && [[ ! "$(uname -m)" =~ "s390" ]]; then
Expand All @@ -26,6 +27,13 @@ ldflags="-X ${REPO_PATH}/version.Version=${version}"
host_build() {
local cmd=$1; shift
echo "Building $cmd"

# Disable CGO if on "riscv64"
# https://github.com/golang/go/issues/72840
local cgo_override="${CGO_ENABLED:-}"
[ "${ARCH}" == "riscv64" ] && cgo_override=0

CGO_ENABLED=${cgo_override} \
Copy link
Member

Choose a reason for hiding this comment

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

It's not clear from quickly looking at the golang docs what CGO_ENABLED= go build ... will do. I think it'd be easier to review if we just only set the env on riscv64.

go build \
-ldflags "${ldflags}" \
-mod vendor \
Expand Down
2 changes: 1 addition & 1 deletion src/cmd-osbuild
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ main() {
fi

case "$basearch" in
"x86_64"|"aarch64"|"s390x"|"ppc64le") ;;
"x86_64"|"aarch64"|"s390x"|"ppc64le"|"riscv64") ;;
*) fatal "$basearch is not supported for this command" ;;
esac

Expand Down
Loading
Loading