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, gha: Add Windows jobs based on Linux image #1398

Merged
merged 3 commits into from
Aug 18, 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
39 changes: 9 additions & 30 deletions .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ task:
- env:
CC: clang-snapshot
test_script:
- ./ci/cirrus.sh
- ./ci/ci.sh
<< : *CAT_LOGS

task:
Expand All @@ -111,7 +111,7 @@ task:
- env:
CC: clang --target=i686-pc-linux-gnu -isystem /usr/i686-linux-gnu/include
test_script:
- ./ci/cirrus.sh
- ./ci/ci.sh
<< : *CAT_LOGS

task:
Expand All @@ -130,7 +130,7 @@ task:
test_script:
# https://sourceware.org/bugzilla/show_bug.cgi?id=27008
- rm /etc/ld.so.cache
- ./ci/cirrus.sh
- ./ci/ci.sh
<< : *CAT_LOGS

task:
Expand All @@ -150,7 +150,7 @@ task:
- env: {}
- env: {EXPERIMENTAL: yes, ASM: arm32}
test_script:
- ./ci/cirrus.sh
- ./ci/ci.sh
<< : *CAT_LOGS

task:
Expand All @@ -167,7 +167,7 @@ task:
ELLSWIFT: yes
CTIMETESTS: no
test_script:
- ./ci/cirrus.sh
- ./ci/ci.sh
<< : *CAT_LOGS

task:
Expand All @@ -184,28 +184,7 @@ task:
ELLSWIFT: yes
CTIMETESTS: no
test_script:
- ./ci/cirrus.sh
<< : *CAT_LOGS

task:
<< : *LINUX_CONTAINER
env:
WRAPPER_CMD: wine
WITH_VALGRIND: no
ECDH: yes
RECOVERY: yes
SCHNORRSIG: yes
ELLSWIFT: yes
CTIMETESTS: no
matrix:
- name: "x86_64 (mingw32-w64): Windows (Debian stable, Wine)"
env:
HOST: x86_64-w64-mingw32
- name: "i686 (mingw32-w64): Windows (Debian stable, Wine)"
env:
HOST: i686-w64-mingw32
test_script:
- ./ci/cirrus.sh
- ./ci/ci.sh
<< : *CAT_LOGS

# Sanitizers
Expand Down Expand Up @@ -249,7 +228,7 @@ task:
HOST: i686-linux-gnu
CC: i686-linux-gnu-gcc
test_script:
- ./ci/cirrus.sh
- ./ci/ci.sh
<< : *CAT_LOGS

# Memory sanitizers
Expand All @@ -276,7 +255,7 @@ task:
ECMULTWINDOW: 2
CFLAGS: "-fsanitize=memory -g -O3"
test_script:
- ./ci/cirrus.sh
- ./ci/ci.sh
<< : *CAT_LOGS

task:
Expand All @@ -292,7 +271,7 @@ task:
SCHNORRSIG: yes
ELLSWIFT: yes
test_script:
- ./ci/cirrus.sh
- ./ci/ci.sh
<< : *CAT_LOGS

task:
Expand Down
37 changes: 37 additions & 0 deletions .github/actions/run-in-docker-action/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
name: 'Run in Docker with environment'
description: 'Run a command in a Docker container, while passing explicitly set environment variables into the container.'
inputs:
dockerfile:
description: 'A Dockerfile that defines an image'
required: true
tag:
description: 'A tag of an image'
required: true
command:
description: 'A command to run in a container'
required: true
runs:
using: "composite"
steps:
- uses: docker/setup-buildx-action@v2
with:
# See: https://github.com/moby/buildkit/issues/3969.
driver-opts: |
network=host

- uses: docker/build-push-action@v4
with:
context: .
file: ${{ inputs.dockerfile }}
tags: ${{ inputs.tag }}
load: true
cache-from: type=gha

- # Tell Docker to pass environment variables in `env` into the container.
run: >
docker run \
$(echo '${{ toJSON(env) }}' | jq -r 'keys[] | "--env \(.) "') \
--volume ${{ github.workspace }}:${{ github.workspace }} \
--workdir ${{ github.workspace }} \
${{ inputs.tag }} bash -c "${{ inputs.command }}"
shell: bash
78 changes: 77 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,82 @@ env:
EXAMPLES: 'yes'

jobs:
docker_cache:
name: "Build Docker image"
runs-on: ubuntu-latest
steps:
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v2
with:
# See: https://github.com/moby/buildkit/issues/3969.
driver-opts: |
network=host

- name: Build container
uses: docker/build-push-action@v4
with:
file: ./ci/linux-debian.Dockerfile
tags: linux-debian-image
cache-from: type=gha
cache-to: type=gha,mode=min

mingw_debian:
name: ${{ matrix.configuration.job_name }}
runs-on: ubuntu-latest
needs: docker_cache

env:
WRAPPER_CMD: 'wine'
WITH_VALGRIND: 'no'
ECDH: 'yes'
RECOVERY: 'yes'
SCHNORRSIG: 'yes'
ELLSWIFT: 'yes'
CTIMETESTS: 'no'

strategy:
fail-fast: false
matrix:
configuration:
- job_name: 'x86_64 (mingw32-w64): Windows (Debian stable, Wine)'
env_vars:
HOST: 'x86_64-w64-mingw32'
- job_name: 'i686 (mingw32-w64): Windows (Debian stable, Wine)'
env_vars:
HOST: 'i686-w64-mingw32'

steps:
- name: Checkout
uses: actions/checkout@v3

- name: CI script
env: ${{ matrix.configuration.env_vars }}
uses: ./.github/actions/run-in-docker-action
with:
dockerfile: ./ci/linux-debian.Dockerfile
tag: linux-debian-image
command: >
git config --global --add safe.directory ${{ github.workspace }} &&
./ci/ci.sh

- run: cat tests.log || true
if: ${{ always() }}
- run: cat noverify_tests.log || true
if: ${{ always() }}
- run: cat exhaustive_tests.log || true
if: ${{ always() }}
- run: cat ctime_tests.log || true
if: ${{ always() }}
- run: cat bench.log || true
if: ${{ always() }}
- run: cat config.log || true
if: ${{ always() }}
- run: cat test_env.log || true
if: ${{ always() }}
- name: CI env
run: env
if: ${{ always() }}
Comment on lines +103 to +119
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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.)

Copy link
Contributor

Choose a reason for hiding this comment

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

use a yaml template to de-duplicate this?

Copy link
Member Author

@hebasto hebasto Aug 14, 2023

Choose a reason for hiding this comment

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

It is not supported for now, unfortunately.

See: actions/runner#1182.


macos-native:
name: "x86_64: macOS Ventura"
# See: https://github.com/actions/runner-images#available-images.
Expand Down Expand Up @@ -80,7 +156,7 @@ jobs:

- name: CI script
env: ${{ matrix.env_vars }}
run: ./ci/cirrus.sh
run: ./ci/ci.sh

- run: cat tests.log || true
if: ${{ always() }}
Expand Down
File renamed without changes.