ci, gha: Add Windows jobs based on Linux image

hebasto commented at 8:04 am on August 10, 2023: member

Solves one item in #1392 partially.

hebasto cross-referenced this on Aug 10, 2023 from issue ci, gha: Add "x86_64: Linux (Debian stable)" GitHub Actions job by hebasto

hebasto commented at 8:09 am on August 10, 2023: member

@achow101

Could you please explicitly enable the actions as follows:

docker/setup-buildx-action@*
docker/build-push-action@*
addnab/docker-run-action@*

?

In the meantime, reviewers can view the builds in my personal repo: https://github.com/hebasto/secp256k1/actions/runs/5818492202.

real-or-random added the label ci on Aug 10, 2023

in .github/workflows/ci.yml:125 in fe247380bc outdated

120+              SECP256K1_TEST_ITERS="${{ env.SECP256K1_TEST_ITERS }}" \
121+              BENCH="${{ env.BENCH }}" \
122+              SECP256K1_BENCH_ITERS="${{ env.SECP256K1_BENCH_ITERS }}" \
123+              CTIMETESTS="${{ env.CTIMETESTS }}" \
124+              EXAMPLES="${{ env.EXAMPLES }}" \
125+              ${{ matrix.configuration.env_var }}

real-or-random commented at 8:52 am on August 10, 2023:

I suspect this is necessary because addnab/docker-run-action@v3 does not handle env variables: https://github.com/addnab/docker-run-action/pull/23 I wonder if there’s a better way or a different action we could use, or if we could just run docker manually.

But okay, yes, I am starting to see that this approach of using the GHA cache also has its disadvantages.

real-or-random commented at 9:09 am on August 10, 2023:

run docker manually

We could perhaps do something like this (assuming we add a unique prefix our env variables) https://stackoverflow.com/a/61955515

hebasto commented at 0:01 am on August 11, 2023:

Reworked.

real-or-random commented at 9:16 am on August 10, 2023: contributor

In the meantime, reviewers can view the builds in my personal repo: hebasto/secp256k1/actions/runs/5818492202.

Caching doesn’t seem to work properly. I suggest trying to remove the mode=min.

hebasto commented at 9:19 am on August 10, 2023: member

In the meantime, reviewers can view the builds in my personal repo: hebasto/secp256k1/actions/runs/5818492202.

Caching doesn’t seem to work properly.

Which job?

real-or-random commented at 10:03 am on August 10, 2023: contributor

In the meantime, reviewers can view the builds in my personal repo: hebasto/secp256k1/actions/runs/5818492202.

Caching doesn’t seem to work properly.

Which job?

Sorry, all jobs, but I meant to include a link to the job. But I think I was wrong. The “Building container” stages takes ~5min, but all of this time seems to be spent downloading from the cache?! I didn’t expect that it’s that slow.

edit: Fetching from ghcr is similarly slow, ok… https://github.com/hebasto/secp256k1/actions/runs/5783560280/job/15672591894#step:2:54

hebasto commented at 10:11 am on August 10, 2023: member

The “Building container” stages takes ~5min, but all of this time seems to be spent downloading from the cache?!

Correct.

I didn’t expect that it’s that slow.

That is what I meant in #1396 (comment) :)

real-or-random commented at 12:18 pm on August 10, 2023: contributor

Okay! But do you agree with my observation that GHA cache vs. GHCR doesn’t make a significant difference here? Then we can ignore that piece when deciding what to use for Linux tasks.

(Heck, for Linux native tasks with standard gcc and clang, using Docker image at all may be faster… But I’d say let’s worry about optimizations later.)

hebasto commented at 12:30 pm on August 10, 2023: member

Okay! But do you agree with my observation that GHA cache vs. GHCR doesn’t make a significant difference here? Then we can ignore that piece when deciding what to use for Linux tasks.

I agree.

The advantage of GHA cache over GHCR is not requiring write permissions for a workflow.

achow101 commented at 1:09 pm on August 10, 2023: member

Updated the actions settings

hebasto force-pushed on Aug 10, 2023

hebasto commented at 0:00 am on August 11, 2023: member

Reworked. The third-party addnab/docker-run-action has been replaced with own reusable workflow.

hebasto commented at 10:02 am on August 11, 2023: member

@real-or-random

Do you have permissions to re-run workflows and jobs?

real-or-random commented at 10:02 am on August 11, 2023: contributor

Hm, sad to see that it’s already starting to get flaky: https://github.com/bitcoin-core/secp256k1/actions/runs/5827144103/job/15802793632?pr=1398 Perhaps this helps: https://github.com/moby/buildkit/issues/3969

Anyway, I like the changes.

hebasto commented at 10:05 am on August 11, 2023: member

Hm, sad to see that it’s already starting to get flaky: bitcoin-core/secp256k1/actions/runs/5827144103/job/15802793632?pr=1398

From my experience, I would estimate the rate of such kind of failure is about 1 (one) per cent of total runs.

in .github/workflows/ci.yml:75 in dba39d0ac8 outdated

70+      json: ${{ steps.result.outputs.json }}
71+    steps:
72+      - name: Put environment variables into a standard job output
73+        id: result
74+        run: |
75+          echo "json=$(echo '${{ toJSON(env) }}' | jq -c '.')" >> "$GITHUB_OUTPUT"

real-or-random commented at 10:36 am on August 11, 2023:

This still seems a bit overkill to me. Couldn’t we list the env variables just in the mingw_debian job?

What I imagine is something like this:

0mingw_debian:
1  env:
2    - WRAPPER_CMD: 'wine'

Am I right that this doesn’t work, because it actually sets these environment variables, but then it’s not clear to run-in-docker which variables should be passed to the inside of the container?

But what about setting common_env_vars only inside mingw_debian? Something like

0mingw_debian:
1  with:
2      common_env_vars: |
3        WRAPPER_CMD=wine
4        [...]

like this, with a more consistent syntax (using an array of size 1 for common_configuration)

0mingw_debian:
1  strategy:
2    matrix:
3      common_configuration:
4        - common_env_vars:
5           WRAPPER_CMD: 'wine'
6           [...]
7      configuration:
8[...]

hebasto commented at 10:41 am on August 11, 2023:

https://docs.github.com/en/actions/using-workflows/reusing-workflows#limitations:

Any environment variables set in an env context defined at the workflow level in the caller workflow are not propagated to the called workflow.

Similarly, environment variables set in the env context, defined in the called workflow, are not accessible in the env context of the caller workflow. Instead, you must use outputs of the reusable workflow.

hebasto commented at 10:54 am on August 11, 2023:

But what about setting common_env_vars only inside mingw_debian?

It seems working. Going to give it a try.

hebasto commented at 11:10 am on August 11, 2023:

But what about setting common_env_vars only inside mingw_debian?

This works for common_env_vars, but we still need a way to pass global environment variables to the called workflow or duplicate them inside mingw_debian.

hebasto force-pushed on Aug 11, 2023

hebasto commented at 1:24 pm on August 11, 2023: member

Reworked with using a composite action to avoid environment variables hassles.

Using compression=estargz,force-compression=true for caching as suggested by @real-or-random offline.

hebasto commented at 1:36 pm on August 11, 2023: member

The only thing I’m not sure about is that whether PR branches will hit image cache from the master branch.

hebasto commented at 3:01 pm on August 11, 2023: member

The only thing I’m not sure about is that whether PR branches will hit image cache from the master branch.

I’ve just finished this check. It works:

Docker layers caches are re-used from the base/default branch.

real-or-random commented at 3:03 pm on August 11, 2023: contributor

Using compression=estargz,force-compression=true for caching as suggested by @real-or-random offline.

Okay, as @hebasto figured out that doesn’t work with the GHA cache backend for now (see https://docs.docker.com/build/cache/backends/#cache-compression). Well fine.

hebasto force-pushed on Aug 11, 2023

hebasto commented at 3:05 pm on August 11, 2023: member

Using compression=estargz,force-compression=true for caching as suggested by @real-or-random offline.

Okay, as @hebasto figured out that doesn’t work with the GHA cache backend for now (see docs.docker.com/build/cache/backends/#cache-compression). Well fine.

Dropped.

real-or-random commented at 3:13 pm on August 11, 2023: contributor

This looks pretty clean now. Will perform a closer review soon.

One nit: Perhaps it’s now time to rename cirrus.ci to ci.sh.

Some observations:

All the Docker stuff is a bit more complex on GHA than on Cirrus CI, but I still believe Docker is a good idea: It makes it easier to reproduce failures locally, and in principle, it makes it easy to switch between different CI providers. For example, we can easily switch back to Cirrus (self-hosted) runners if we keep the Docker images.
Store/restoring the Docker build cache is still a bit slow. It’s a bit sad that downloading the cache takes almost as long as running the actual CI script. But we may be able to improve this later. And even if we don’t manage to improve, I think the build times are acceptable. (Moreover, the native Windows builds don’t use Docker, so we still get some immediate feedback for the most obvious problems.)

hebasto commented at 3:21 pm on August 11, 2023: member

One nit: Perhaps it’s now time to rename cirrus.ci to ci.sh.

Done.

in .github/actions/run-in-docker-action/action.yml:36 in 133c4e5b90 outdated

31+        load: true
32+        cache-from: type=gha
33+
34+    - run: >
35+        docker run \
36+          $(echo '${{ toJSON(env) }}' | jq -r 'keys[] | "-e \(.)"' | paste -sd " ") \

real-or-random commented at 3:29 pm on August 11, 2023:

0          # Tell Docker to pass environment variables in `env` (which includes those set via `GITHUB_ENV`) into the container.
1          $(echo '${{ toJSON(env) }}' | jq -r 'keys[] | "--env \(.)"' | paste -sd " ") \

hebasto commented at 6:51 pm on August 11, 2023:

Thanks! Updated.

in .github/actions/run-in-docker-action/action.yml:20 in 133c4e5b90 outdated

15+  steps:
16+    - run: |
17+        for var in "$(echo '${{ toJSON(matrix.configuration.env_vars) }}' | jq -r 'to_entries | .[] | "\(.key)=\(.value)"')"; do
18+          echo "$var" >> "$GITHUB_ENV"
19+        done
20+        echo "MAKEFLAGS=-j$(($(nproc) + 1))" >> "$GITHUB_ENV"

real-or-random commented at 3:37 pm on August 11, 2023:

nit: This will probably override MAKEFLAGS if in env_vars. Perhaps it’s a bit cleaner to simply prepend MAKEFLAGS with $(($(nproc) + 1)), and do this inside ci.sh?

hebasto commented at 3:59 pm on August 11, 2023:

To keep the scope of this PR limited, is it OK to just set mingw_debian.env.MAKEFLAGS: '-j3'?

UPD. Or rely on the global env.MAKEFLAGS: '-j4'?

hebasto commented at 6:52 pm on August 11, 2023:

Or rely on the global env.MAKEFLAGS: '-j4'?

Implemented.

in .github/actions/run-in-docker-action/action.yml:2 in 133c4e5b90 outdated

0@@ -0,0 +1,40 @@
1+name: 'CI script in Docker'
2+description: 'Run a CI script in a Docker container'

real-or-random commented at 3:41 pm on August 11, 2023:

0name: 'Run in Docker with environment'
1description: 'Run a command in a Docker container, while passing explicitly set environment variables into the container.'

hebasto commented at 6:52 pm on August 11, 2023:

Thanks! Updated.

in .github/workflows/ci.yml:119 in 133c4e5b90 outdated

106+        if: ${{ always() }}
107+      - run: cat test_env.log || true
108+        if: ${{ always() }}
109+      - name: CI env
110+        run: env
111+        if: ${{ always() }}

real-or-random commented at 3:43 pm on August 11, 2023:

Could this be refactored into an action? Or is this overkill?

hebasto commented at 3:52 pm on August 11, 2023:

Could this be refactored into an action? Or is this overkill?

In that case it won’t show separated entries in the log.

real-or-random commented at 12:17 pm on August 12, 2023:

Ok yes, let’s leave it like this for now.

(I’m not happy with this and it should be reworked in a separate PR. I had introduced this printing of files to make sure that we always have all logs, and I think that’s a good idea in general. But it’s not a good idea to pipe it only to files. We should pipe it to files but also keep stdout/stderr, e.g., using tee. People get confused about this over and over because they can’t find the error messages in the failing tasks etc.)

real-or-random commented at 3:45 pm on August 11, 2023: contributor

ACK mod nits

Do you think it makes sense to retain the Cirrus config commented out, like sometimes done in Core? This would require a bit more work here to make sure it still works now that we split the image.

real-or-random commented at 3:46 pm on August 11, 2023: contributor

@MarcoFalke if you want to have a look

real-or-random cross-referenced this on Aug 11, 2023 from issue ci: Future of CI after Cirrus pricing change by real-or-random

hebasto commented at 3:54 pm on August 11, 2023: member

Do you think it makes sense to retain the Cirrus config commented out, like sometimes done in Core?

We have the commit history for that, don’t we?

hebasto force-pushed on Aug 11, 2023

hebasto commented at 6:50 pm on August 11, 2023: member

Updated 1212e9a556c3e58582f8677ebc2bb8585495acc9 -> 67d6118606c95f2e92e42bd492eed14700c9edfb (pr1398.06 -> pr1398.07, diff):

addressed @real-or-random’s comments

hebasto cross-referenced this on Aug 11, 2023 from issue ci, gha: Run "x86_64: macOS Ventura" job on GitHub Actions by hebasto

bitcoin-core deleted a comment on Aug 11, 2023

hebasto commented at 10:15 pm on August 11, 2023: member

Being rebased on top of this PR, #1396 got a very small diff, btw.

hebasto force-pushed on Aug 12, 2023

hebasto commented at 11:08 am on August 12, 2023: member

Updated 67d6118606c95f2e92e42bd492eed14700c9edfb -> a2a7fc7aa3fb8be68efa126848348daf13bd3bb6 (pr1398.07 -> pr1398.08, diff).

Environment variables from strategy.matrix are passed to steps.*.env directly now.

in .github/workflows/ci.yml:150 in a2a7fc7aa3 outdated

141+      fail-fast: false
142+      matrix:
143+        configuration:
144+          - job_name: 'x86_64 (MSVC): Windows (Debian stable, Wine)'
145+            env_vars:
146+              HOST: 'x86_64-w64-mingw32'

real-or-random commented at 12:13 pm on August 12, 2023:

Is there a reason why you moved this down to configurations as compared to the previous version of the PR?

hebasto commented at 12:20 pm on August 12, 2023:

Is there a reason why you moved this down to configurations as compared to the previous version of the PR?

Yes, the reason is to avoid leaving matrix.configuration.env_vars empty. Otherwise, env: ${{ matrix.configuration.env_vars }} fails.

real-or-random commented at 12:33 pm on August 12, 2023:

Oh okay, hm. What’s the error message? (Just curious, perhaps I have an idea …)

real-or-random commented at 12:43 pm on August 12, 2023:

Have you tried env_vars: {} ? It’s the empty YAML dict, see https://stackoverflow.com/questions/33510094/syntax-for-empty-dictionary-in-yaml

hebasto commented at 1:02 pm on August 12, 2023:

Have you tried env_vars: {} ? It’s the empty YAML dict, see stackoverflow.com/questions/33510094/syntax-for-empty-dictionary-in-yaml

Thank you! It works.

in .github/workflows/ci.yml:170 in a2a7fc7aa3 outdated

163+    steps:
164+      - name: Checkout
165+        uses: actions/checkout@v3
166+
167+      - name: CI script
168+        env: ${{ matrix.configuration.env_vars }}

real-or-random commented at 12:14 pm on August 12, 2023:

Nice! So simple once you see it. :)

hebasto force-pushed on Aug 12, 2023

hebasto commented at 12:25 pm on August 12, 2023: member

Updated a2a7fc7aa3fb8be68efa126848348daf13bd3bb6 -> f4347ce35ea434c031e4a3eae70b36587f39ba77 (pr1398.08 -> pr1398.09, diff):

addressed @real-or-random’s [comment]( #1398 (comment))
dropped superfluous paste command

hebasto force-pushed on Aug 12, 2023

hebasto commented at 1:01 pm on August 12, 2023: member

Updated f4347ce35ea434c031e4a3eae70b36587f39ba77 -> 9ade5743f19f91a3da7ef2b1fe38be0bc6914362 (pr1398.09 -> pr1398.10, diff):

addressed @real-or-random’s comment

MarcoFalke commented at 9:39 am on August 14, 2023: none

This PR relies on GitHub Actions cache rather on GitHub Container Registry (in comparison to #1396).

This works because this repo doesn’t use ccache and thus, the docker images are the only thing in the cache and won’t exceed the 10GB limit?

hebasto commented at 9:59 am on August 14, 2023: member

This PR relies on GitHub Actions cache rather on GitHub Container Registry (in comparison to #1396).

This works because this repo doesn’t use ccache and thus, the docker images are the only thing in the cache and won’t exceed the 10GB limit?

Yes, it does.

As it was pointed by @real-or-random offline, there are alternatives for the GitHub’s cache, for example, the buildjet/cache action.

hebasto commented at 10:03 am on August 14, 2023: member

The actual use of the cache storage is as follows:

in .github/workflows/ci.yml:119 in 9ade5743f1 outdated

111+        if: ${{ always() }}
112+      - run: cat test_env.log || true
113+        if: ${{ always() }}
114+      - name: CI env
115+        run: env
116+        if: ${{ always() }}

MarcoFalke commented at 10:17 am on August 14, 2023:

use a yaml template to de-duplicate this?

hebasto commented at 10:31 am on August 14, 2023:

It is not supported for now, unfortunately.

See: https://github.com/actions/runner/issues/1182.

in ci/linux-debian-wine.Dockerfile:9 in 9ade5743f1 outdated

0@@ -0,0 +1,25 @@
1+FROM debian:stable-slim
2+
3+SHELL ["/bin/bash", "-c"]
4+
5+WORKDIR /root
6+
7+# The "wine" package provides a convenience wrapper that we need
8+RUN dpkg --add-architecture i386 && apt-get update && apt-get install --no-install-recommends -y \
9+        autoconf automake libtool make \

MarcoFalke commented at 10:19 am on August 14, 2023:

nit: you are now caching the common deps twice. Not sure if it is worth it to avoid, but I wanted to mention it.

real-or-random commented at 10:32 am on August 14, 2023:

Hm, I guess it’s okay in this case.

MarcoFalke commented at 10:20 am on August 14, 2023: none

Ok, and on a cache miss, this will build the image for each task in the matrix, instead of only once for each matrix?

real-or-random commented at 10:29 am on August 14, 2023: contributor

This works because this repo doesn’t use ccache and thus, the docker images are the only thing in the cache and won’t exceed the 10GB limit?

Indeed, but we already ran into problems here a few days ago with the 10GB limit.

The docker images in the cache are quite large (see https://github.com/bitcoin-core/secp256k1/actions/caches), so we can fit at most builds in the limit, e.g., one PR + master. When we have multiple PRs updating the Dockerfiles, we see cache misses. I don’t think that’s a big issue. We usually don’t have multiple PRs updating CI. In the worst case, the images needs to be rebuilt more often than necessary, which just adds <30 min of delay to the CI. Moreover, we can reduce the size of the images a bit. Just these the following two should get us multiple GBs back:

removing GCC build files after the build (currently testing this locally, I’ll have a patch soon)
simply using the official https://hub.docker.com/r/sagemath/sagemath for the sage job instead of installing sagemath in our Linux image

However, a conceptual problem is that we rely on the images not being evicted between the “build docker image” job and the actual CI jobs. That contradicts the philosophy of a cache a bit. When we have multiple concurrent CI runs going on, this may fail:

PR1 stores docker image A in cache
PR2 stores docker image B in cache, will evict image A
PR1 tries to run the CI script, but A has been evicted: So A is rebuilt as part of every single job. This is slow, may fail due to insufficient disk space, and may evict B again
…

There’s a possible third way to move the docker image from the build job to the actual CI job: Use artifacts: https://github.com/actions/upload-artifact https://github.com/actions/download-artifact This seems to be the proper way to move data between jobs. Artifacts won’t be evicted automatically, but they’ll be cleaned up after 90 days, so this is probably the sweet spot between GHCR and the GHA cache. We’d still use the GHA cache for caching the docker build, but just the way we transfer the image from one job to another (in the same CI run) would use artifacts.

I’m not entirely sure how to proceed. I think I’d like to see at least one of the two aforementioned issues addressed. If we can reduce the size of the images for now, that will probably good enough. Even if we then have issues with concurrent CI runs, that’s not the end of the world. (One can just re-run manually…) But it’s certainly not convenient, and it’s a bit hacky, so having a reliable way to move images from one job to another (e.g., artifacts) will be better.

real-or-random commented at 10:48 am on August 14, 2023: contributor

Just the following two should get us multiple GBs back:

removing GCC build files after the build (currently testing this locally, I’ll have a patch soon)

simply using the official hub.docker.com/r/sagemath/sagemath for the sage job instead of installing sagemath in our Linux image

This commit reduces the uncompressed size from 9.37GB to 3.55GB. The GHA cache uses compression, but this should still make a large difference.

Cirrus CI will fail now due to Sagemath being unavailable. @hebasto If you want to keep the scope of this PR small, perhaps we should move the Sagemath job to GHA in a separate PR first.

real-or-random commented at 10:51 am on August 14, 2023: contributor

As it was pointed by @real-or-random offline, there are alternatives for the GitHub’s cache, for example, the buildjet/cache action.

Yep, this would give us 20 GB, but after I thought about it, I’d like to avoid it if possible. It adds another infrastructure dependency, we can’t use the nice GHA interface to look at the cache sizes, and it’s not directly compatible with the docker build action. So while we could cache the docker build directory, we’d need to write our own cache logic then (which version to restore etc…).

hebasto cross-referenced this on Aug 14, 2023 from issue ci, gha: Run "SageMath prover" job on GitHub Actions by hebasto

hebasto commented at 1:31 pm on August 14, 2023: member

simply using the official hub.docker.com/r/sagemath/sagemath for the sage job instead of installing sagemath in our Linux image

Done in #1399.

real-or-random commented at 1:56 pm on August 14, 2023: contributor

I’m not entirely sure how to proceed. I think I’d like to see at least one of the two aforementioned issues addressed. If we can reduce the size of the images for now, that will probably good enough. Even if we then have issues with concurrent CI runs, that’s not the end of the world. (One can just re-run manually…)

I’d say let’s first try and see how much space we can gain. If we reduce the images far enough, then this may indeed be good enough, and at least prevent the issues of caused by the actual CI job having to rebuild and then running out of disk space.

edit: This comment and some others above apply more to #1396 than to this PR.

MarcoFalke cross-referenced this on Aug 14, 2023 from issue ci: Run "macOS 11.0 [gui, no tests] [jammy]" job on GitHub Actions by hebasto

in .github/workflows/ci.yml:143 in 9ade5743f1 outdated

138+      AR: '/opt/msvc/bin/x64/lib'
139+      NM: '/opt/msvc/bin/x64/dumpbin -symbols -headers'
140+      # Set non-essential options that affect the CLI messages here.
141+      # (They depend on the user's taste, so we don't want to set them automatically in configure.ac.)
142+      CFLAGS: '-nologo -diagnostics:caret'
143+      LDFLAGS: '-Xlinker -Xlinker -Xlinker -nologo'

real-or-random commented at 1:37 pm on August 15, 2023:

Using MSVC on Linux is still pretty hacky. The reason why I introduced this was simply that we didn’t have Windows runners. We could consider moving it to native Windows (later, in a separate PR).

Advantages:

That would get us rid of the entire business of “installing MSVC” and it would make this Docker image much smaller.

Disadvantages:

We won’t have Dockerfile anymore that people can use locally, e.g., when working on problems that appear only with MSVC.
We’ll probably need some separate CI script for Windows / CMake. Currently, we simply reuse the Linux / autotools script, which is neat.

hebasto commented at 5:14 pm on August 15, 2023:

We’ll probably need some separate CI script for Windows / CMake.

In addition to https://github.com/bitcoin-core/secp256k1/blob/ce765a5b8eb93af589bc7dad2f34da0b8b51557c/.github/workflows/ci.yml#L14-L17?

real-or-random commented at 5:21 pm on August 15, 2023:

I mean we’ll at least need a way to pass config different config options from a matrix to the build systems. (Yes, it shouldn’t be a lot of work. ^^)

hebasto commented at 9:50 pm on August 15, 2023:

Please see #1401.

hebasto force-pushed on Aug 15, 2023

jonasnick commented at 6:34 pm on August 15, 2023: contributor

Concept ACK, thanks @hebasto!

After @real-or-random explained this approach to me I was able to understand what happens without doing too much research into the github actions semantics. In general, it would be nice to have some comments that document what happens (e.g. what exactly is the purpose of docker_wine_cache and what it does if the image already exists), but we can add them over time in follow-up PRs.

hebasto force-pushed on Aug 15, 2023

hebasto cross-referenced this on Aug 15, 2023 from issue ci, gha: Run all MSVC tests on Windows natively by hebasto

hebasto commented at 1:40 pm on August 17, 2023: member

Converting this PR to a draft for now.

Please review #1401 first.

hebasto marked this as a draft on Aug 17, 2023

hebasto force-pushed on Aug 17, 2023

hebasto marked this as ready for review on Aug 17, 2023

hebasto commented at 8:51 pm on August 17, 2023: member

Rebased.

Ready for review.

ci, gha: Add Windows jobs based on Linux image 2b6f9cd546

ci: Remove Windows tasks from Cirrus CI d6281dd008

ci: Rename `cirrus.sh` to more general `ci.sh`

This makes sense in the process of moving stuff to GitHub Actions.

87d35f30c0

hebasto force-pushed on Aug 18, 2023

hebasto commented at 9:58 am on August 18, 2023: member

Rebased.

real-or-random commented at 10:05 am on August 18, 2023: contributor

I believe it’s possible to shave off several minutes from the tasks that run in Docker by setting the docker build driver to docker, see https://github.com/docker/buildx/issues/107#issuecomment-1307929258 ff . This should avoid the long “sending tarball” phases. It can be done by setting driver: docker, see https://github.com/docker/setup-buildx-action#inputs . But let’s experiment with this after this PR. :)

real-or-random approved

real-or-random commented at 10:05 am on August 18, 2023: contributor

ACK 87d35f30c0a322e9b4bc5ee1addc1d0cd463562a

hebasto commented at 11:50 am on August 18, 2023: member

I believe it’s possible to shave off several minutes from the tasks that run in Docker by setting the docker build driver to docker, see docker/buildx#107 (comment) ff . This should avoid the long “sending tarball” phases. It can be done by setting driver: docker, see docker/setup-buildx-action#inputs . But let’s experiment with this after this PR. :)

Unfortunately, GitHub Actions cache does not work with the docker build driver.

jonasnick commented at 12:44 pm on August 18, 2023: contributor

ACK 87d35f30c0a322e9b4bc5ee1addc1d0cd463562a

jonasnick merged this on Aug 18, 2023

jonasnick closed this on Aug 18, 2023

hebasto deleted the branch on Aug 18, 2023

ci, gha: Add Windows jobs based on Linux image #1398