Add interim GitHub Actions

arbourd commented 4 months ago

This pull-request adds GitHub Actions configuration that allows tests to run on pull-requests and master branches.

Details

Tests are run in parallel per language for speed and reducing flakes
- N number of languages and versions can be set
Module specific tests are set by a string
- go-1.21 will run a job that runs Go tests against Go 1.21, etc
- Adding go-1.20 to the list would add an additional test for Go 1.20.
Module specific tests are run by selecting test/test_${module}*
- Python module runs all tests, including base unit ones, as the Python test suite is used as the default for other tests
Tests complete in about 5 minutes

Caveats

Only unprivileged tests are run
No core dumps
Only ubuntu

ac000 commented 4 months ago

A better way may be something like

$ make -j4 -k 2>errors.txt

Then you can display that file (with a nice big banner or whatever) if there was any errors... plus you get all the errors front and centre.

ac000 commented 4 months ago

If we used kernel style make output, any errors/warnings would stand out much more...

ac000 commented 4 months ago

On Tue, 13 Feb 2024 08:33:46 -0800 Dylan Arbour @.***> wrote:

On Buildbot we use whatever is shipped on that OS (with a notable exception of Node, which we fetch from nodesource repos). This helps us ensure our software is compatible with what is broadly used by respective OS users.

In a past life I worked on / with Ruby, Python and Node applications at scale, all with VMs. In each case we were using some variation of Debian or CentOS with specific versions of the runtimes selected to match the expectation from the developers. I personally have never relied on the default OS' version of a Python to run a production Python workload; Ruby, Node, etc.

My experience is sort of the opposite, we used the OS provided packages where possible. But then I also detest that all these things also bring along their own package management that completely bypasses the OS package manager.

[...]

Lastly, we don't want to interrupt the release process. This implementation doesn't account for that. Because of the limitations of Actions default runners and Ubuntu, we aren't testing for Alpine or FreeBSD.

Not using 'actions' I think, but this is what I do for unit-wasm

name: Builds

on:
  push:
    branches: main
    paths:
      - Makefile
      - shared.mk
      - 'examples/**'
      - 'src/**'
      - '.github/workflows/build_tests.yaml'
  pull_request:
    branches: main
    paths:
      - Makefile
      - shared.mk
      - 'examples/**'
      - 'src/**'
      - '.github/workflows/build_tests.yaml'

jobs:
  # GitHub Currently only supports running directly on Ubuntu,
  # for any other Linux we need to use a container.

  fedora-rawhide:
    runs-on: ubuntu-latest

    container:
      image: fedora:rawhide

    steps:
      - name: Install tools/deps
        run: |
          dnf -y install git wget clang llvm compiler-rt lld make
    wasi-libc-devel cargo rust rust-std-static-wasm32-unknown-unknown
    rust-std-static-wasm32-wasi wget -O-
    https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-20/libclang_rt.builtins-wasm32-wasi-20.0.tar.gz
    | tar --strip-components=1 -xvzf - -C $(dirname $(clang
    -print-runtime-dir))

      - uses: ***@***.***
        with:
          fetch-depth: "0"

      - name: make
        run: make V=1 E=1 all

  debian-testing:
    runs-on: ubuntu-latest

    container:
      image: debian:testing

    steps:
      - name: Install tools/deps
        run: |
          apt-get -y update
          apt-get -y install git curl wget wasi-libc make clang llvm lld
          wget -O-
    https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-20/libclang_rt.builtins-wasm32-wasi-20.0.tar.gz
    | tar --strip-components=1 -xvzf - -C $(dirname $(clang
    -print-runtime-dir)) curl https://sh.rustup.rs -sSf | sh -s -- -y .
    "$HOME/.cargo/env" rustup target add wasm32-wasi wget -O-
    https://github.com/WebAssembly/wasi-sdk/releases/download/wasi-sdk-20/wasi-sysroot-20.0.tar.gz
    | tar -xzf - -C ${RUNNER_TEMP}

      - uses: ***@***.***
        with:
          fetch-depth: "0"

      - name: make
        run: |
          . "$HOME/.cargo/env"
          make WASI_SYSROOT=${RUNNER_TEMP}/wasi-sysroot V=1 E=1 all

That uses Fedora Rawhide and Debian Testing containers. But many many others are available.

[...]

This would also be a very good thing to add, it check for whitespace damage

 name: check-whitespace

# Get the repo with the commits(+1) in the series.
# Process `git log --check` output to extract just the check errors.
# Add a comment to the pull request with the check errors.

on:
  pull_request:
    types: [ opened, synchronize ]

jobs:
  check-whitespace:
    runs-on: ubuntu-latest
    steps:
    - uses: ***@***.***
      with:
        fetch-depth: 0

    - name: git log --check
      id: check_out
      run: |
        log=
        commit=
        while read dash etc
        do
          case "${dash}" in
          "---")
            commit="${etc}"
            ;;
          "")
            ;;
          *)
            if test -n "${commit}"
            then
              log="${log}\n${commit}"
              echo ""
              echo "--- ${commit}"
            fi
            commit=
            log="${log}\n${dash} ${etc}"
            echo "${dash} ${etc}"
            ;;
          esac
        done <<< $(git log --check --pretty=format:"--- %h %s" ${{github.event.pull_request.base.sha}}..)

        if test -n "${log}"
        then
          exit 2
        fi

Just stick it in .github/workflows/check-whitespace.yaml, you can grab it directly from https://raw.githubusercontent.com/nginx/unit-wasm/main/.github/workflows/check-whitespace.yaml

ac000 commented 4 months ago

Hmm, something's gone wrong here (email address wise...)

commit 301728fb74bd912fcd7e69168422004a0c9cd34a
Author:     Dylan Arbour <arbourd@users.noreply.github.com>
AuthorDate: Tue Feb 13 11:39:56 2024 -0500
Commit:     Dylan Arbour <arbourd@users.noreply.github.com>
CommitDate: Tue Feb 20 09:42:17 2024 -0500

    Add a second make call for error logging visibility

commit 87b5aa769e478c766920e83b036aaf5c6db60c8d
Author:     Dylan Arbour <arbourd@users.noreply.github.com>
AuthorDate: Mon Dec 18 18:09:32 2023 -0500
Commit:     Dylan Arbour <arbourd@users.noreply.github.com>
CommitDate: Tue Feb 20 09:42:14 2024 -0500

    Add GitHub Actions

They should probably also be squashed...

ac000 commented 4 months ago

On Tue, 20 Feb 2024 08:30:53 -0800 Dan Callahan @.***> wrote:

@callahad commented on this pull request.

name: Make wasmtime

run: |

make -C pkg/contrib .wasmtime

if: steps.metadata.outputs.module == 'wasm'

Looks like we're at Wasmtime 11 in pkgs/contrib

Hmm, that's pretty old. Any chance of using something more recent? In fact I think I'd be OK with using whatever the latest release is for now...

callahad commented 4 months ago

At your discretion, consider adding a richer commit message / subject line. It could be useful to have some of the caveats spelled out there (mainly, that this is an initial pass and not intended to be comprehensive; the emphasis is on providing a Good Enough smoke test for development purposes)

alejandro-colomar commented 3 months ago

A better way may be something like
$ make -j4 -k 2>errors.txt
Then you can display that file (with a nice big banner or whatever) if there was any errors... plus you get all the errors front and centre.

That doesn't show the errors together with the commands that produced them. Plus, that doesn't solve the races.

ac000 commented 3 months ago

On Wed, 13 Mar 2024 09:38:12 -0700 Alejandro Colomar @.***> wrote:

A better way may be something like
$ make -j4 -k 2>errors.txt
Then you can display that file (with a nice big banner or whatever) if there was any errors... plus you get all the errors front and centre.
That doesn't show the errors together with the commands that produced them. Plus, that doesn't solve the races.

This whole thing is pretty much moot now we have prettified output...

alejandro-colomar commented 3 months ago

On Wed, 13 Mar 2024 09:38:12 -0700 Alejandro Colomar @.***> wrote: > A better way may be something like > > shell > $ make -j4 -k 2>errors.txt > > > Then you can display that file (with a nice big banner or whatever) if there was any errors... plus you get all the errors front and centre. That doesn't show the errors together with the commands that produced them. Plus, that doesn't solve the races. This whole thing is pretty much moot now we have prettified output...

Not really. You're still subject to races. And redirecting stderr still leaves you without the corresponding CC ... next to the error.

So, think that the following may happen:

  CC  foo.o
some error line from compiling foo.o
  CC  bar.o
another error line from compiling foo.o

And if you redirect stderr, you may end up with stuff like the following in error.log:

some error line from compiling foo.o
some error line from compiling bar.o
another error line from compiling foo.o

(and even if you don't happen to see bad output from races, I think having the error lines without the corresponding CC foo.o is a bit unfriendly.)

ac000 commented 3 months ago

On Wed, 13 Mar 2024 09:53:49 -0700 Alejandro Colomar @.***> wrote:

So, think that the following may happen:

  CC  foo.o
some error line from compiling foo.o
  CC bar.o
another error line from compiling foo.o

But the error itself will show where the problem lies...

alejandro-colomar commented 3 months ago

But the error itself will show where the problem lies...

Yeah, but if several error reports are intertwined due to a race, it will be hard to read each of them.

Think of:

error line 1 of foo.o
error line 1 of bar.o
error line 2 of foo.o
error line 2 of bar.o
error line 3 of bar.o
error line 4 of bar.o
error line 3 of foo.o

I see no reasons to avoid -Otarget, especially when it doesn't impose observable performance degradation (I couldn't measure any meaningful degradation in projects that I use often, including the Linux man-pages and shadow).

nginx / unit

Add interim GitHub Actions #1123