Skip to content
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

ci/cirrus: Add native ARM64 jobs #1426

Merged
merged 2 commits into from
Sep 20, 2023
Merged
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
95 changes: 95 additions & 0 deletions .cirrus.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
env:
### cirrus config
CIRRUS_CLONE_DEPTH: 1
### compiler options
HOST:
WRAPPER_CMD:
# Specific warnings can be disabled with -Wno-error=foo.
# -pedantic-errors is not equivalent to -Werror=pedantic and thus not implied by -Werror according to the GCC manual.
WERROR_CFLAGS: -Werror -pedantic-errors
MAKEFLAGS: -j4
BUILD: check
### secp256k1 config
ECMULTWINDOW: auto
ECMULTGENPRECISION: auto
ASM: no
WIDEMUL: auto
WITH_VALGRIND: yes
EXTRAFLAGS:
### secp256k1 modules
EXPERIMENTAL: no
ECDH: no
RECOVERY: no
SCHNORRSIG: no
ELLSWIFT: no
### test options
SECP256K1_TEST_ITERS:
BENCH: yes
SECP256K1_BENCH_ITERS: 2
CTIMETESTS: yes
# Compile and run the tests
EXAMPLES: yes

cat_logs_snippet: &CAT_LOGS
always:
cat_tests_log_script:
- cat tests.log || true
cat_noverify_tests_log_script:
- cat noverify_tests.log || true
cat_exhaustive_tests_log_script:
- cat exhaustive_tests.log || true
cat_ctime_tests_log_script:
- cat ctime_tests.log || true
cat_bench_log_script:
- cat bench.log || true
cat_config_log_script:
- cat config.log || true
cat_test_env_script:
- cat test_env.log || true
cat_ci_env_script:
- env

linux_arm64_container_snippet: &LINUX_ARM64_CONTAINER
env_script:
- env | tee /tmp/env
build_script:
- DOCKER_BUILDKIT=1 docker build --file "ci/linux-debian.Dockerfile" --tag="ci_secp256k1_arm"
- docker image prune --force # Cleanup stale layers
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On a second thought, when using podman, this should probably specify --external, because build images will be considered external.

(No rush to change anything here, though. The hosts will occasionally clean up external images already)

test_script:
- docker run --rm --mount "type=bind,src=./,dst=/ci_secp256k1" --env-file /tmp/env --replace --name "ci_secp256k1_arm" "ci_secp256k1_arm" bash -c "cd /ci_secp256k1/ && ./ci/ci.sh"
Copy link
Contributor

Choose a reason for hiding this comment

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

@maflcko
I think --replace is a podman-only argument. docker run doesn't seem to have it. Should we just rewrite the command to use the "native" podman syntax? I guess there's no point in having an invalid docker command here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, if you prefer to call podman here, you can do that.

Both commands will redirect to podman, currently.

Copy link
Contributor

Choose a reason for hiding this comment

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

On a second thought, we could also prepend docker rm --force ci_secp256k1_arm, which has the same effect as --replace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this should also work. But I haven't fully figured out whether it may be racy if there is a zombie process running in the container.


task:
name: "ARM64: Linux (Debian stable)"
persistent_worker:
labels:
type: arm64
env:
ECDH: yes
RECOVERY: yes
SCHNORRSIG: yes
ELLSWIFT: yes
matrix:
# Currently only gcc-snapshot, the other compilers are tested on GHA with QEMU
- env: { CC: 'gcc-snapshot' }
<< : *LINUX_ARM64_CONTAINER
<< : *CAT_LOGS

task:
name: "ARM64: Linux (Debian stable), Valgrind"
persistent_worker:
labels:
type: arm64
env:
ECDH: yes
RECOVERY: yes
SCHNORRSIG: yes
ELLSWIFT: yes
WRAPPER_CMD: 'valgrind --error-exitcode=42'
SECP256K1_TEST_ITERS: 2
matrix:
- env: { CC: 'gcc' }
- env: { CC: 'clang' }
- env: { CC: 'gcc-snapshot' }
- env: { CC: 'clang-snapshot' }
<< : *LINUX_ARM64_CONTAINER
<< : *CAT_LOGS
10 changes: 7 additions & 3 deletions ci/linux-debian.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,15 @@ RUN apt-get update && apt-get install --no-install-recommends -y \
gcc-i686-linux-gnu libc6-dev-i386-cross libc6-dbg:i386 libubsan1:i386 libasan8:i386 \
gcc-s390x-linux-gnu libc6-dev-s390x-cross libc6-dbg:s390x \
gcc-arm-linux-gnueabihf libc6-dev-armhf-cross libc6-dbg:armhf \
gcc-aarch64-linux-gnu libc6-dev-arm64-cross libc6-dbg:arm64 \
gcc-powerpc64le-linux-gnu libc6-dev-ppc64el-cross libc6-dbg:ppc64el \
gcc-mingw-w64-x86-64-win32 wine64 wine \
gcc-mingw-w64-i686-win32 wine32 \
python3
python3 && \
if ! ( dpkg --print-architecture | grep --quiet "arm64" ) ; then \
apt-get install --no-install-recommends -y \
gcc-aarch64-linux-gnu libc6-dev-arm64-cross libc6-dbg:arm64 ;\
Comment on lines +37 to +38
Copy link
Member

Choose a reason for hiding this comment

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

On an arm64 worker, other cross-toolchains are also not used. Maybe move them into this if block?

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason to keep these packages is that the Docker image can then be used by developers locally and has all the tools available for testing (if they have ARM machines, but this is not unlikely with recent Mac hardware). And okay, yeah, it costs a bit of time on CI, but I assume it's in the range of seconds?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is actually a presumed upstream bug that this list of install needs to be touched. It should not result in a bug to install a package.

fi && \
apt-get clean && rm -rf /var/lib/apt/lists/*

# Build and install gcc snapshot
ARG GCC_SNAPSHOT_MAJOR=14
Expand All @@ -44,7 +48,7 @@ RUN apt-get update && apt-get install --no-install-recommends -y wget libgmp-dev
sha512sum --check --ignore-missing sha512.sum && \
# We should have downloaded exactly one tar.xz file
ls && \
[[ $(ls *.tar.xz | wc -l) -eq "1" ]] && \
[ $(ls *.tar.xz | wc -l) -eq "1" ] && \
tar xf *.tar.xz && \
mkdir gcc-build && cd gcc-build && \
../*/configure --prefix=/opt/gcc-snapshot --enable-languages=c --disable-bootstrap --disable-multilib --without-isl && \
Expand Down
Loading