ci: Add a clang-ast workflow
FYI we have https://github.com/nginx/clang-ast/tree/unit
Ah I see you have forked this repo :) Maybe shoot a PR for changes so we keep it in one place.
PR opened.
- Switch to the unit branch of nginx/clang-ast now that the needed fix has been merged, thanks to @thresheek
$ git range-diff 83557d99...3304aaf4
1: 83557d99 ! 1: 3304aaf4 ci: Add a clang-ast workflow
@@ .github/workflows/clang-ast.yaml (new)
+
+ - name: Checkout and build clang-ast
+ run: |
-+ git clone https://github.com/ac000/clang-ast -b unit-stdc++17
++ git clone https://github.com/nginx/clang-ast -b unit
+ cd clang-ast
+ make
+
Should test be included in this check? it's mostly Python code, and I am not aware of any difference it could make in the output Unit binary
The pytests have no bearing on this.
However we do have the C tests as enabled with --tests.
I was in two minds whether to include them or not.
Having tried running the plugin on the tests, it did actually find a problem (in the test code)
src/test/nxt_base64_test.c:90:13: error: number of arguments for nxt_log_s::handler mismatches query string 'nxt_base64_decode() test "%V" failed', expected 1 args, got 0 args
90 | nxt_log_alert(thr->log, "nxt_base64_decode() test \"%V\" failed");
| ^
src/nxt_log.h:81:9: note: expanded from macro 'nxt_log_alert'
81 | _log_->handler(NXT_LOG_ALERT, _log_, __VA_ARGS__); \
| ^
1 error generated.
So, OK, let's include them..
It could also be expanded out to include language modules...
Can you clarify the desired outcome to this PR?
This was a check that used to run on our buildbot infrastructure, this is simply running it here now.
I am not sure if this is supposed to be a solution to the code formatting or if this is to check some aspect of the build in each branch.
This has diddly-squat to do with code-formatting...
- Run this over the C tests also
- We need to use -add-plugin in order to have the object files created
- Add a fix for an issue it caught in the tests
$ git range-diff 3304aaf4...dad4727a
-: -------- > 1: cfb52003 src/test: Fix missing parameter to nxt_log_alert() in nxt_base64_test()
1: 3304aaf4 ! 2: dad4727a ci: Add a clang-ast workflow
@@ .github/workflows/clang-ast.yaml (new)
+
+ - name: Checkout and build clang-ast
+ run: |
-+ git clone https://github.com/nginx/clang-ast -b unit
++ git clone https://github.com/nginx/clang-ast.git -b unit
+ cd clang-ast
+ make
+
@@ .github/workflows/clang-ast.yaml (new)
+ - name: Build Unit
+ run: |
+ make -j4 \
-+ EXTRA_CFLAGS="-Xclang -load -Xclang clang-ast/ngx-ast.so -Xclang -plugin -Xclang ngx-ast" \
++ EXTRA_CFLAGS="-Xclang -load -Xclang clang-ast/ngx-ast.so -Xclang -add-plugin -Xclang ngx-ast" \
+ NXT_SHARED_LOCAL_LINK=: build/lib/libnxt.so
++
++ - name: Build C tests
++ run: |
++ EXTRA_CFLAGS="-Xclang -load -Xclang clang-ast/ngx-ast.so -Xclang -add-plugin -Xclang ngx-ast" \
++ tests
Need to actually configure and build the tests!
$ git range-diff dad4727a...107055ed
1: dad4727a ! 1: 107055ed ci: Add a clang-ast workflow
@@ .github/workflows/clang-ast.yaml (new)
+ make
+
+ - name: Configure Unit
-+ run: ./configure --cc=clang --openssl --debug
++ run: ./configure --cc=clang --openssl --debug --tests
+
+ - name: Build Unit
+ run: |
@@ .github/workflows/clang-ast.yaml (new)
+
+ - name: Build C tests
+ run: |
-+ EXTRA_CFLAGS="-Xclang -load -Xclang clang-ast/ngx-ast.so -Xclang -add-plugin -Xclang ngx-ast" \
-+ tests
++ make -j4 \
++ EXTRA_CFLAGS="-Xclang -load -Xclang clang-ast/ngx-ast.so -Xclang -add-plugin -Xclang ngx-ast" \
++ tests
- Set the clang-ast plugin arguments at configure time
$ git range-diff 107055e...f04bc68d
1: 107055ed ! 1: f04bc68d ci: Add a clang-ast workflow
@@ .github/workflows/clang-ast.yaml (new)
+ make
+
+ - name: Configure Unit
-+ run: ./configure --cc=clang --openssl --debug --tests
++ run: ./configure --cc=clang --cc-opt="-Xclang -load -Xclang clang-ast/ngx-ast.so -Xclang -add-plugin -Xclang ngx-ast" --openssl --debug --tests
+
+ - name: Build Unit
+ run: |
-+ make -j4 \
-+ EXTRA_CFLAGS="-Xclang -load -Xclang clang-ast/ngx-ast.so -Xclang -add-plugin -Xclang ngx-ast" \
-+ NXT_SHARED_LOCAL_LINK=: build/lib/libnxt.so
++ make -j4 NXT_SHARED_LOCAL_LINK=: build/lib/libnxt.so
+
+ - name: Build C tests
-+ run: |
-+ make -j4 \
-+ EXTRA_CFLAGS="-Xclang -load -Xclang clang-ast/ngx-ast.so -Xclang -add-plugin -Xclang ngx-ast" \
-+ tests
++ run: make -j4 tests
- Add coverage for the language modules (except go and wasm-wasi-component, neither of which are using clang)
$ git range-diff f04bc68d...9fcf6021
1: f04bc68d ! 1: 9fcf6021 ci: Add a clang-ast workflow
@@ Metadata
## Commit message ##
ci: Add a clang-ast workflow
- This does compile time type-checking using a clang-plugin. It was run as
- part of buildbot.
+ This does compile-time type and argument checking using a clang-plugin.
+ It was run as part of buildbot.
- Link: <https://github.com/ac000/clang-ast/tree/unit-stdc%2B%2B17>
+ This covers unitd, src/test and the php, perl, python, ruby, wasm, java
+ and nodejs language modules/support.
+
+ It doesn't cover Go as that doesn't build anything with clang (uses
+ cgo) or wasm-wasi-component as that uses rustc.
+
+ Link: <https://github.com/nginx/clang-ast/tree/unit>
Signed-off-by: Andrew Clayton <[email protected]>
## .github/workflows/clang-ast.yaml (new) ##
@@ .github/workflows/clang-ast.yaml (new)
+ - name: Install tools/deps
+ run: |
+ apt-get -y update
-+ apt-get -y install git llvm-dev libclang-dev clang make \
-+ libssl-dev libpcre2-dev
++ apt-get -y install git wget curl llvm-dev libclang-dev clang make \
++ libssl-dev libpcre2-dev libperl-dev \
++ libphp-embed php-dev python3-dev libpython3-dev \
++ ruby-dev openjdk-17-jdk npm
++ npm install -g node-gyp
+
+ - uses: actions/checkout@v4
+
@@ .github/workflows/clang-ast.yaml (new)
+ run: ./configure --cc=clang --cc-opt="-Xclang -load -Xclang clang-ast/ngx-ast.so -Xclang -add-plugin -Xclang ngx-ast" --openssl --debug --tests
+
+ - name: Build Unit
-+ run: |
-+ make -j4 NXT_SHARED_LOCAL_LINK=: build/lib/libnxt.so
++ run: make -j4 unitd
+
+ - name: Build C tests
+ run: make -j4 tests
++
++ - name: Build Perl language module
++ run: ./configure perl && make -j4 perl
++
++ - name: Build PHP language module
++ run: ./configure php && make -j4 php
++
++ - name: Build Python language module
++ run: ./configure python --config=python3-config && make -j4 python3
++
++ - name: Build Ruby language module
++ run: ./configure ruby && make -j4 ruby
++
++ - name: Build Java support
++ run: ./configure java && make -j4 java
++
++ - name: Build Nodejs support
++ run: ./configure nodejs && make node-local-install DESTDIR=node
++
++ - name: Build wasm language module
++ run: |
++ wget -q -O- https://github.com/bytecodealliance/wasmtime/releases/download/v26.0.0/wasmtime-v26.0.0-x86_64-linux-c-api.tar.xz | tar -xJf -
++ ./configure wasm --include-path=wasmtime-v26.0.0-x86_64-linux-c-api/include --lib-path=wasmtime-v26.0.0-x86_64-linux-c-api/lib --rpath && make wasm
- Drop the src/test fix, that will go in via https://github.com/nginx/unit/pull/1481
$ git range-diff 9fcf6021...c9181663
1: cfb52003 < -: -------- src/test: Fix missing parameter to nxt_log_alert() in nxt_base64_test()
2: 9fcf6021 = 1: c9181663 ci: Add a clang-ast workflow
Rebase with master
$ git range-diff c9181663...1e345b34
-: -------- > 1: 85f21b7c Use nxt_nitems() instead of sizeof() for strings (arrays)
-: -------- > 2: 9f6f4866 src/test: Fix missing parameter to nxt_log_alert() in nxt_base64_test()
1: c9181663 = 3: 1e345b34 ci: Add a clang-ast workflow