nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.25k stars 322 forks source link

Add GitHub workflows for extra coverage #1223

Closed ac000 closed 1 month ago

ac000 commented 2 months ago

This adds a workflow for building Unit under Fedora Rawhide and Alpine Edge with both GCC and Clang.

These are the development branches from which releases are cut.

This usually consists of the latest versions of software and will hopefully catch new compiler issues and API breakages in the various languages we support.

With Alpine and Clang that also gives us musl libc + clang coverage.

On Alpine we don't build the wasm and wasm-wasi-component modules, mainly as this would require messing around with all the rust stuff and building wasmtime from source (as there's no musl libc based packages) and the wasm module is pretty small, any new compiler issues would hopefully show up in the rest.

We do build the wasm module with gcc and clang on Fedora. But not wasm-wasi-component in the interests of time. Can be added at a later date if deemed necessary.

We don't build the Perl language module on Fedora with clang due to the Fedora (and probably Red Hat) Perl CFLAGS having incompatible with clang flags.

We probably could work around it if we really wanted to, but not sure it's worth it and on Red Hat/Fedora, GCC is the system compiler.

On Alpine we also don't build the nodejs and go language modules as there's nothing that actually gets compiled there and the main reason for building on Alpine is to get musl libc + clang coverage.

We're also not bothering with njs for now... can be revisited at a later date.

Also no pytests, these should be well covered via other workflows for example by running on latest Alpine releases.

arbourd commented 2 months ago

I think this should be covered by @oxpa's self hosted runners, yes? Excluding Alpine of course.

oxpa commented 2 months ago

not really covered: the buildbot workflow should mostly ensure that we can build packages on a target. So I went with a subset of OSs that have unit packages. Fedora rawhide and alpine edge are a bit out of scope in this regards. But we have alpine latest release (3.19 as of today) in the workflow and a worker AMI for fedora 35 (IIRC). So if fedora 35 is good enough - it's doable

ac000 commented 2 months ago

not really covered: the buildbot workflow should mostly ensure that we can build packages on a target. So I went with a subset of OSs that have unit packages. Fedora rawhide and alpine edge are a bit out of scope in this regards. But we have alpine latest release (3.19 as of today) in the workflow and a worker AMI for fedora 35 (IIRC). So if fedora 35 is good enough - it's doable

I do specifically want the dev branches of these, mainly to catch new compiler issues, but also other language stuff... I also want musl libc + clang coverage...

I'm also specifically using distribution packages for everything (except wasmtime, there isn't any).

I also want it to run on pull-requests...

ac000 commented 2 months ago

Added a whitespace issue checker workflow

ac000 commented 2 months ago

These new checks finish in under half the time of the existing CI, so shouldn't have an adverse effect on the testing time...

ac000 commented 2 months ago

Added a Closes: tag...

1:  087c9b4a ! 1:  60a942be Add GitHub workflows for extra coverage
    @@ Commit message
         Also no pytests, these should be well covered via other workflows for
         example by running on latest Alpine releases.

    +    Closes: https://github.com/nginx/unit/issues/949
         Signed-off-by: Andrew Clayton <a.clayton@nginx.com>

      ## .github/workflows/ci-dev-distro-compiler.yaml (new) ##
2:  2651374f = 2:  759ddbfb Add a GitHub workflow to check for whitespace issues
ac000 commented 2 months ago

Update to wasmtime 20.0.0

$ git range-diff 759ddbfb...a0968a79
1:  60a942be ! 1:  035dd2cd Add GitHub workflows for extra coverage
    @@ .github/workflows/ci-dev-distro-compiler.yaml (new)
     +
     +      - name: Install wasmtime
     +        run: |
    -+          wget -O- https://github.com/bytecodealliance/wasmtime/releases/download/v19.0.2/wasmtime-v19.0.2-x86_64-linux-c-api.tar.xz | tar -xJf -
    ++          wget -O- https://github.com/bytecodealliance/wasmtime/releases/download/v20.0.0/wasmtime-v20.0.0-x86_64-linux-c-api.tar.xz | tar -xJf -
     +
     +      - name: configure unit-wasm
    -+        run: ./configure wasm --include-path=wasmtime-v19.0.2-x86_64-linux-c-api/include --lib-path=wasmtime-v19.0.2-x86_64-linux-c-api/lib --rpath
    ++        run: ./configure wasm --include-path=wasmtime-v20.0.0-x86_64-linux-c-api/include --lib-path=wasmtime-v20.0.0-x86_64-linux-c-api/lib --rpath
     +
     +      - name: make unit-wasm
     +        run: make wasm
2:  759ddbfb = 2:  a0968a79 Add a GitHub workflow to check for whitespace issues
ac000 commented 2 months ago
$ git range-diff a0968a79...c318cf3e
1:  035dd2cd ! 1:  cca7ab09 Add GitHub workflows for extra coverage
    @@ .github/workflows/ci-dev-distro-compiler.yaml (new)
     +      - name: Install tools/deps
     +        run: |
     +          dnf -y update
    -+          dnf -y install which wget git gcc make pcre2-devel openssl-devel \
    ++          dnf -y install --setopt=install_weak_deps=False \
    ++              which wget git gcc make pcre2-devel openssl-devel \
     +              python-unversioned-command python3 python3-devel \
     +              php-devel php-embedded perl-devel perl-ExtUtils-Embed \
     +              ruby-devel java-devel nodejs-devel nodejs-npm golang
     +          if [ "${{ matrix.compiler }}" = "clang" ]; then
    -+            dnf -y install clang
    ++            dnf -y install --setopt=install_weak_deps=False clang
     +          fi
     +          npm install -g node-gyp
     +
    @@ .github/workflows/ci-dev-distro-compiler.yaml (new)
     +          if [ "${{ matrix.compiler }}" = "clang" ]; then
     +            ./configure --openssl --cc=clang
     +          else
    -+             ./configure --openssl
    ++            ./configure --openssl
     +          fi
     +
     +      - name: make unit
    @@ .github/workflows/ci-dev-distro-compiler.yaml (new)
     +          if [ "${{ matrix.compiler }}" = "clang" ]; then
     +            ./configure --openssl --cc=clang
     +          else
    -+             ./configure --openssl
    ++            ./configure --openssl
     +          fi
     +
     +      - name: make unit
2:  a0968a79 = 2:  c318cf3e Add a GitHub workflow to check for whitespace issues
ac000 commented 2 months ago

Split the white space check into its own PR.

ac000 commented 1 month ago
 git range-diff cca7ab09...fc0e24c1
1:  cca7ab09 ! 1:  fc0e24c1 Add GitHub workflows for extra coverage
    @@ Commit message
         Add GitHub workflows for extra coverage

         This adds a workflow for building Unit under Fedora Rawhide and Alpine
    -    Edge.
    +    Edge with both GCC and Clang.

         These are the development branches from which releases are cut.

    @@ .github/workflows/ci-dev-distro-compiler.yaml (new)
     +        run: ./configure perl
     +
     +      - name: make unit-perl
    -+        run: |
    -+          if [ "${{ matrix.compiler }}" = "clang" ]; then
    -+            make -j 4 perl EXTRA_CFLAGS=-Qunused-arguments
    -+          else
    -+            make -j 4 perl
    -+          fi
    ++        run: make -j 4 perl
     +
     +      - name: configure unit-ruby
     +        run: ./configure ruby
ac000 commented 1 month ago

Ok, this is now ready for merging.

To recap. This is building Unit under Fedora Rawhide and Alpine Edge with both GCC and Clang. This is mainly to get coverage with the latest toolchain developments but also for upcoming versions of the various language stuff.

This already found a build issue with the Perl language module and Clang.

So, last chance to speak up or forever hold yer whisht! ;)

ac000 commented 1 month ago

Rebased with master

$ git range-diff fc0e24c1...babb154a
 -:  -------- >  1:  237a26aa wasm-wc: Bump the rustls crate from 0.21.10 to 0.21.11
 -:  -------- >  2:  3fbca6ca Fix some trailing whitespace and long lines in the README
 -:  -------- >  3:  e5bc299d configuration: Constify numerous pointers
 -:  -------- >  4:  8f861cf4 Constify a bunch of static local variables
 -:  -------- >  5:  33c978cc php: Constify some local static variables
 -:  -------- >  6:  31cec908 configuration: Constify more pointers
 -:  -------- >  7:  b26c119f Tighten up some string arrays
 -:  -------- >  8:  db3cf3e4 tools: Add unitctl CLI
 -:  -------- >  9:  2c4502f8 tools: Add unitctl section to the README
 -:  -------- > 10:  ff2e0f42 Add a GitHub workflow to check for whitespace issues
 -:  -------- > 11:  5b01bd65 auto/wasm: No need to explicitly set -fno-strict-aliasing now
 -:  -------- > 12:  e2a09c77 Convert 0-sized arrays to true flexible array members
 -:  -------- > 13:  5d1ce5c4 auto, perl: Fix building the Perl language module with clang
 -:  -------- > 14:  6e8f7bbb tools/unitctl: Initial Docker Procedures
 -:  -------- > 15:  818d4ad7 tools/unitctl: API Plumbing for docker deployments
 -:  -------- > 16:  f6989dd6 tools/unitctl: Add Docker deployment functionality
 -:  -------- > 17:  1d237990 tools/unitctl: Add new functionality to README.md and fmt code
 -:  -------- > 18:  4e4d1dd2 tools/unitctl: temporarily ignore issues with autogenerated readme
 -:  -------- > 19:  e61d9e7a tools/unitctl: Readme fixes
 -:  -------- > 20:  787980db tools/unitctl: Improve quality of life on osx
 -:  -------- > 21:  cb03d31e tools/unitctl: Update host_path() to account for OSX special behaviour
 -:  -------- > 22:  6ad1fa34 tools/unitctl: clean up control socket impls
 -:  -------- > 23:  cc9eb8e7 tools/unitctl: enable passing IP addresses to the 'instances new' command
 -:  -------- > 24:  da43f443 java: Update third-party components
 -:  -------- > 25:  05a82294 http: Use consistent target in nxt_h1p_peer_header_send()
 -:  -------- > 26:  87077ec4 http: Ensure REQUEST_URI immutability
 -:  -------- > 27:  eed21785 tests: Change request_uri tests for changed behaviour
 -:  -------- > 28:  00009765 tests: REQUEST_URI variable test with rewrite
 -:  -------- > 29:  6d0880c9 Add unitctl build and release CI
 -:  -------- > 30:  149555db trigger unitctl CI on version tags of existing format
 -:  -------- > 31:  a98acded ci: Add unit testing to unitctl CI workflow
 1:  fc0e24c1 = 32:  babb154a Add GitHub workflows for extra coverage
ac000 commented 1 month ago

Rebased with mailmap branch

$ git range-diff babb154a...30b39bd0
-:  -------- > 1:  44709bea .mailmap: Add an entry for Ava's GitHub address
1:  babb154a = 2:  30b39bd0 Add GitHub workflows for extra coverage