ci: Add missing amd64 to win64-cross task #28295

pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2308-ci-win-amd- changing 2 files +4 −4

maflcko commented at 12:05 PM on August 18, 2023: member

Currently the task will fail if run on non-x86_64.

Fix this by adding the missing amd64, similar to

https://github.com/bitcoin/bitcoin/blob/7bf078f2b7d4a0339d053144b4fb35fe020dac25/ci/test/00_setup_env_i686_multiprocess.sh#L11
DrahtBot commented at 12:05 PM on August 18, 2023: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Tests on Aug 18, 2023
ci: Add missing amd64 to win64-cross task
Also, do the same for android, which also fails.
fa56d17a4b
in ci/test/00_setup_env_win64.sh:10 in 79f0cc12b1 outdated
```
   7 |  
   8 |  export LC_ALL=C.UTF-8
   9 |  
  10 |  export CONTAINER_NAME=ci_win64
  11 | -export CI_IMAGE_NAME_TAG=ubuntu:22.04  # Check that Jammy can cross-compile to win64
  12 | +export CI_IMAGE_NAME_TAG="docker.io/amd64/ubuntu:22.04"  # Check that Jammy can cross-compile to win64
```
hebasto commented at 12:11 PM on August 18, 2023:

Same change in cirrus.sh: https://github.com/bitcoin/bitcoin/blob/7bf078f2b7d4a0339d053144b4fb35fe020dac25/.cirrus.yml#L233 ?
maflcko commented at 12:13 PM on August 18, 2023:

In theory yes, but in practise, no, because:

Cirrus already runs on amd64, so this won't change anything.

The config will be removed in another pull, so changing it will cause merge conflicts.
hebasto commented at 12:17 PM on August 18, 2023:

Does the following log:

[#3](/bitcoin-bitcoin/3/) [internal] load metadata for docker.io/library/ubuntu:jammy

look OK?
maflcko commented at 12:23 PM on August 18, 2023:

Yes, the log looks green
hebasto commented at 12:35 PM on August 18, 2023:

I was expecting to see:

[#3](/bitcoin-bitcoin/3/) [internal] load metadata for docker.io/amd64/ubuntu:22.04

The PR description claims that this is a bugfix, but changes do not affect CI builds at all, according to the logs.

I'm confused a bit about that.
maflcko commented at 12:38 PM on August 18, 2023:

It is possible to run the CI locally. For example, on a local riscv64 machine you can type:

MAKEJOBS="-j1" FILE_ENV="./ci/test/00_setup_env_win64.sh" ./ci/test_run_all.sh

And then observe the failure.

The failure is fixed by this patch.
hebasto commented at 12:46 PM on August 18, 2023:

Currently the task will fail if run on non-x86_64.

I expect it still fail with this PR with a non-x86_64 Cirrus worker.

maflcko commented at 12:50 PM on August 18, 2023:

I expect it still fail with this PR with a non-x86_64 Cirrus worker.

No, it shouldn't, unless the switch is done incorrectly. If you switch to a non-x86_64 Cirrus-hosted worker (you shouldn't do that), you should also change the docker label to match this one at the same time.
maflcko force-pushed on Aug 18, 2023
maflcko commented at 12:24 PM on August 18, 2023: member

Force pushed to do the same for android
hebasto approved
hebasto commented at 1:03 PM on August 18, 2023: member

ACK fa56d17a4b868f42fa45bea0d1f0c78de75e7838
fanquake merged this on Aug 18, 2023
fanquake closed this on Aug 18, 2023
maflcko deleted the branch on Aug 18, 2023
Frank-GER referenced this in commit db22afe4e9 on Sep 8, 2023
bitcoin locked this on Aug 17, 2024

Type	Reviewers
ACK	hebasto

Contributors

Labels

ci: Add missing amd64 to win64-cross task #28295

Reviews