ci/cirrus: Add native ARM64 jobs #1426

pull maflcko wants to merge 2 commits into bitcoin-core:master from maflcko:2309-ci-arm- changing 2 files +102 −3
  1. maflcko commented at 12:26 pm on September 20, 2023: none
  2. ci/cirrus: Bring back skeleton .cirrus.yml without jobs 2262d0eaab
  3. maflcko marked this as a draft on Sep 20, 2023
  4. maflcko renamed this:
    2309 ci arm
    ci/cirrus: Add native ARM64 jobs
    on Sep 20, 2023
  5. maflcko force-pushed on Sep 20, 2023
  6. maflcko force-pushed on Sep 20, 2023
  7. maflcko force-pushed on Sep 20, 2023
  8. maflcko force-pushed on Sep 20, 2023
  9. maflcko force-pushed on Sep 20, 2023
  10. maflcko force-pushed on Sep 20, 2023
  11. maflcko force-pushed on Sep 20, 2023
  12. maflcko force-pushed on Sep 20, 2023
  13. maflcko force-pushed on Sep 20, 2023
  14. maflcko force-pushed on Sep 20, 2023
  15. maflcko force-pushed on Sep 20, 2023
  16. maflcko force-pushed on Sep 20, 2023
  17. maflcko force-pushed on Sep 20, 2023
  18. maflcko force-pushed on Sep 20, 2023
  19. maflcko force-pushed on Sep 20, 2023
  20. maflcko commented at 2:35 pm on September 20, 2023: none
    cc @real-or-random Any thoughts on this or bike-shed ideas?
  21. maflcko marked this as ready for review on Sep 20, 2023
  22. maflcko force-pushed on Sep 20, 2023
  23. maflcko force-pushed on Sep 20, 2023
  24. in ci/linux-debian.Dockerfile:39 in 2f31c66cfb outdated
    36-        python3
    37+        python3 && \
    38+        if ! ( dpkg --print-architecture | grep --quiet "arm64" ) ; then \
    39+         apt-get install --no-install-recommends -y \
    40+         gcc-aarch64-linux-gnu libc6-dev-arm64-cross libc6-dbg:arm64 ;\
    41+        fi
    


    real-or-random commented at 4:03 pm on September 20, 2023:

    Strictly speaking, this is unrelated to this PR, but I think it’s good practice/idiom to clean the cache after every apt-get to make the cached layer smaller, so we should add it here

    0        fi && \
    1        apt-get clean && rm -rf /var/lib/apt/lists/*
    
  25. real-or-random commented at 4:05 pm on September 20, 2023: contributor

    Concept ACK

    There are a few lines added in the second commit and removed again in the third commit. I suggest just squashing the two commits.

    This solves one item in #1392, cc @hebasto.

  26. real-or-random added the label ci on Sep 20, 2023
  27. real-or-random added the label assurance on Sep 20, 2023
  28. hebasto commented at 4:30 pm on September 20, 2023: member

    Where is the docker image cache located?

    Is it possible to demonstrate the build_script with unpopulated cache?

  29. maflcko commented at 4:47 pm on September 20, 2023: none

    Where is the docker image cache located?

    It is using the Bitcoin Core persistent workers.

  30. maflcko commented at 4:47 pm on September 20, 2023: none

    Is it possible to demonstrate the build_script with unpopulated cache?

    Yes, you can just pick the task with the longest runtime: https://cirrus-ci.com/task/4555435512954880

  31. in ci/linux-debian.Dockerfile:38 in 2f31c66cfb outdated
    35         gcc-mingw-w64-i686-win32 wine32 \
    36-        python3
    37+        python3 && \
    38+        if ! ( dpkg --print-architecture | grep --quiet "arm64" ) ; then \
    39+         apt-get install --no-install-recommends -y \
    40+         gcc-aarch64-linux-gnu libc6-dev-arm64-cross libc6-dbg:arm64 ;\
    


    hebasto commented at 4:55 pm on September 20, 2023:
    On an arm64 worker, other cross-toolchains are also not used. Maybe move them into this if block?

    real-or-random commented at 5:03 pm on September 20, 2023:
    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?!

    maflcko commented at 5:04 pm on September 20, 2023:
    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.
  32. hebasto approved
  33. hebasto commented at 4:55 pm on September 20, 2023: member
    ACK 2f31c66cfb1faf34397ddb3850adbfdb6836586c.
  34. maflcko commented at 5:00 pm on September 20, 2023: none

    There are a few lines added in the second commit and removed again in the third commit. I suggest just squashing the two commits.

    Who should be the author of the commit then?

  35. real-or-random commented at 5:04 pm on September 20, 2023: contributor

    Who should be the author of the commit then?

    You and add me in Co-authored-by:? Or the other way around, I don’t care. Happy to flip a coin for you if you can’t decide. :)

  36. maflcko force-pushed on Sep 20, 2023
  37. maflcko commented at 5:07 pm on September 20, 2023: none

    You and add me in Co-authored-by:?

    Thx, done

  38. ci/cirrus: Add native ARM64 persistent workers
    Co-authored-by: Tim Ruffing <crypto@timruffing.de>
    fa4d6c76b6
  39. maflcko force-pushed on Sep 20, 2023
  40. real-or-random commented at 5:14 pm on September 20, 2023: contributor

    You and add me in Co-authored-by:?

    Thx, done

    Now it’s authored and co-authored by me. :D

    edit: Nevermind, I’m wrong. For some reason, github doesn’t show your picture here, so I assumed it’s just me.

  41. real-or-random approved
  42. real-or-random commented at 5:22 pm on September 20, 2023: contributor
    ACK fa4d6c76b6dc249d02c926ad75725556614fdddd
  43. hebasto approved
  44. hebasto commented at 5:25 pm on September 20, 2023: member
    re-ACK fa4d6c76b6dc249d02c926ad75725556614fdddd, only last two commits have been squashed since my recent review.
  45. real-or-random cross-referenced this on Sep 20, 2023 from issue ci: Future of CI after Cirrus pricing change by real-or-random
  46. real-or-random merged this on Sep 20, 2023
  47. real-or-random closed this on Sep 20, 2023

  48. real-or-random commented at 11:16 pm on September 20, 2023: contributor
    @MarcoFalke Follow-up question: Can the persistent runner run ARM32 binaries natively? From what I remember, most ARM64 machines can do this, but it depends on the CPU. If yes, we could add jobs tests for this, too.
  49. maflcko commented at 9:08 am on September 21, 2023: none

    Yes:

    0lscpu | grep op-mode
    1CPU op-mode(s):                     32-bit, 64-bit
    
  50. maflcko deleted the branch on Sep 21, 2023
  51. in .cirrus.yml:59 in fa4d6c76b6
    54+    - env | tee /tmp/env
    55+  build_script:
    56+    - DOCKER_BUILDKIT=1 docker build --file "ci/linux-debian.Dockerfile" --tag="ci_secp256k1_arm"
    57+    - docker image prune --force  # Cleanup stale layers
    58+  test_script:
    59+    - 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"
    


    real-or-random commented at 9:23 am on October 26, 2023:
    @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.

    maflcko commented at 10:54 am on October 26, 2023:

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

    Both commands will redirect to podman, currently.


    real-or-random commented at 5:43 pm on October 26, 2023:
    On a second thought, we could also prepend docker rm --force ci_secp256k1_arm, which has the same effect as --replace.

    maflcko commented at 8:33 am on October 27, 2023:
    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.

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-21 11:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me