| Message ID | 20241122150337.417243-1-david.marchand@redhat.com |
|---|---|
| State | Superseded |
| Delegated to: | Eelco Chaudron |
| Headers | show |
| Series | [ovs-dev] github: Skip clang-analyze when reference generation fails. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 22 Nov 2024, at 16:03, David Marchand wrote: > By assuming that OVS maintainers never push changes that breaks > compilation in GHA (which seems to be the case so far), it seems natural > to assume that generating the reference for clang analyzer should always > work. > > If generating this reference fails, it is likely due to a change external > to OVS code, and not due to the series being tested (though this series > gets flagged with an error in patchwork). > > Such a situation is often hit in the dpdk-latest branch when some DPDK > API change breaks OVS compilation and no OVS fix is merged yet. > > Split the clang analyzer check in two jobs: > - a clang-cache job, responsible for computing a cache key of the > reference code and compiling a reference build, > - a clang-analyzer job, which depends on the former job, responsible for > compiling the current patch and comparing the result against the > reference, > > The cache generation won't be reported as a failure at the OVS build step > (using continue-on-error:). > If such a failure happens, the cache-analyzer job is skipped. > > Fixes: d662eee09724 ("ci: Add clang-analyze to GitHub actions.") > Signed-off-by: David Marchand <david.marchand@redhat.com> This change looks good to me, with the minor exception of the cache stage name. It suggests that the cache is for Clang rather than the analyze phase. I suggest the following change, which I can apply at commit time if you agree: --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -332,7 +332,7 @@ jobs: name: logs-linux-${{ join(matrix.*, '-') }} path: logs.tgz - build-clang-cache: + build-clang-analyze-cache: needs: build-dpdk env: dependencies: | @@ -341,7 +341,7 @@ jobs: CC: clang DPDK: dpdk CLANG_ANALYZE: true - name: clang-cache + name: clang-analyze-cache outputs: key: ${{ steps.cache_key.outputs.key }} outcome: ${{ steps.build_base.outcome }} @@ -446,10 +446,10 @@ jobs: run: ./.ci/linux-build.sh build-clang-analyze: - needs: [build-dpdk, build-clang-cache] + needs: [build-dpdk, build-clang-analyze-cache] if: > - needs.build-clang-cache.outputs.outcome == 'success' || - needs.build-clang-cache.outputs.outcome == 'skipped' + needs.build-clang-analyze-cache.outputs.outcome == 'success' || + needs.build-clang-analyze-cache.outputs.outcome == 'skipped' env: dependencies: | automake bc clang-tools libnuma-dev libunbound-dev libunwind-dev \ @@ -474,7 +474,7 @@ jobs: uses: actions/cache/restore@v4 with: path: base-clang-analyzer-results - key: ${{ needs.build-clang-cache.outputs.key }} + key: ${{ needs.build-clang-analyze-cache.outputs.key }} - name: set up python uses: actions/setup-python@v5
diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 6501f23d26..bfd11f39b1 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -336,7 +336,7 @@ jobs: name: logs-linux-${{ join(matrix.*, '-') }} path: logs.tgz - build-clang-analyze: + build-clang-cache: needs: build-dpdk env: dependencies: | @@ -345,7 +345,10 @@ jobs: CC: clang DPDK: dpdk CLANG_ANALYZE: true - name: clang-analyze + name: clang-cache + outputs: + key: ${{ steps.cache_key.outputs.key }} + outcome: ${{ steps.build_base.outcome }} runs-on: ubuntu-24.04 timeout-minutes: 30 @@ -416,35 +419,86 @@ jobs: key: ${{ steps.cache_key.outputs.key }} - name: set up python + if: steps.clang_cache.outputs.cache-hit != 'true' uses: actions/setup-python@v5 with: python-version: ${{ env.python_default }} - name: get cached dpdk-dir + if: steps.clang_cache.outputs.cache-hit != 'true' uses: actions/cache/restore@v4 with: path: dpdk-dir key: ${{ needs.build-dpdk.outputs.dpdk_key }} - name: update APT cache + if: steps.clang_cache.outputs.cache-hit != 'true' run: sudo apt update || true - name: install common dependencies + if: steps.clang_cache.outputs.cache-hit != 'true' run: sudo apt install -y ${{ env.dependencies }} - name: prepare + if: steps.clang_cache.outputs.cache-hit != 'true' run: ./.ci/linux-prepare.sh - name: build base reference + id: build_base if: steps.clang_cache.outputs.cache-hit != 'true' + continue-on-error: true run: ./.ci/linux-build.sh - - name: save cache - uses: actions/cache/save@v4 - if: steps.clang_cache.outputs.cache-hit != 'true' + build-clang-analyze: + needs: [build-dpdk, build-clang-cache] + if: > + needs.build-clang-cache.outputs.outcome == 'success' || + needs.build-clang-cache.outputs.outcome == 'skipped' + env: + dependencies: | + automake bc clang-tools libnuma-dev libunbound-dev libunwind-dev \ + libssl-dev libtool libxdp-dev llvm-dev + CC: clang + DPDK: dpdk + CLANG_ANALYZE: true + name: clang-analyze + runs-on: ubuntu-24.04 + timeout-minutes: 30 + + steps: + - name: checkout + uses: actions/checkout@v4 + + - name: update PATH + run: | + echo "$HOME/bin" >> $GITHUB_PATH + echo "$HOME/.local/bin" >> $GITHUB_PATH + + - name: check for analyzer result cache + uses: actions/cache/restore@v4 with: path: base-clang-analyzer-results - key: ${{ steps.cache_key.outputs.key }} + key: ${{ needs.build-clang-cache.outputs.key }} + + - name: set up python + uses: actions/setup-python@v5 + with: + python-version: ${{ env.python_default }} + + - name: get cached dpdk-dir + uses: actions/cache/restore@v4 + with: + path: dpdk-dir + key: ${{ needs.build-dpdk.outputs.dpdk_key }} + + - name: update APT cache + run: sudo apt update || true + + - name: install common dependencies + run: sudo apt install -y ${{ env.dependencies }} + + - name: prepare + run: ./.ci/linux-prepare.sh - name: build run: ./.ci/linux-build.sh
By assuming that OVS maintainers never push changes that breaks compilation in GHA (which seems to be the case so far), it seems natural to assume that generating the reference for clang analyzer should always work. If generating this reference fails, it is likely due to a change external to OVS code, and not due to the series being tested (though this series gets flagged with an error in patchwork). Such a situation is often hit in the dpdk-latest branch when some DPDK API change breaks OVS compilation and no OVS fix is merged yet. Split the clang analyzer check in two jobs: - a clang-cache job, responsible for computing a cache key of the reference code and compiling a reference build, - a clang-analyzer job, which depends on the former job, responsible for compiling the current patch and comparing the result against the reference, The cache generation won't be reported as a failure at the OVS build step (using continue-on-error:). If such a failure happens, the cache-analyzer job is skipped. Fixes: d662eee09724 ("ci: Add clang-analyze to GitHub actions.") Signed-off-by: David Marchand <david.marchand@redhat.com> --- .github/workflows/build-and-test.yml | 66 +++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 6 deletions(-)