On 5 Apr 2024, at 12:14, Ilya Maximets wrote:
> On 1/11/24 00:08, Eelco Chaudron wrote: >> This patch identifies new static analysis issues during a GitHub action >> run and reports them. The process involves analyzing the changes introduced >> in the current commit and comparing them to those in the preceding commit. >> >> However, there are two cases when the GitHub push action runner does not >> provide enough details to determine the preceding commit. These cases are >> a new branch or a forced push. The strategy for these exceptions is to >> find the first common commit on any upstream branch, and use that. >> >> An example error output might look like this: >> >> error level: +0 -0 no changes >> warning level: +2 +0 >> New issue "deadcode.DeadStores Value stored to 'remote' is never read" >> (1 occurrence) >> file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:86 >> New issue "unix.Malloc Potential leak of memory pointed to by 'remote'" >> (1 occurrence) >> file:///home/runner/work/ovs/ovs/vswitchd/ovs-vswitchd.c:95 >> note level: +0 -0 no changes >> all levels: +2 +0 >> >> Signed-off-by: Eelco Chaudron <[email protected]> >> --- >> >> changes in v2: >> - When it's a new branch, it compares it to the HEAD of the default branch. >> >> changes in v3: >> - Include the clang version as part of the cache >> - Change the way it looks for the 'default' branch so it will work >> for patch branches. >> - Also compare to the base branch for forced commits. >> >> changes in v4: >> - No longer look for a default branch, but consume all patches >> from the current author. >> >> changes in v5: >> - Addressed Ilya's comments. >> - Checkout upstream branch and find common point to base delta on. >> >> .ci/linux-build.sh | 30 +++++++ >> .ci/linux-prepare.sh | 2 +- >> .github/workflows/build-and-test.yml | 113 +++++++++++++++++++++++++++ >> 3 files changed, 144 insertions(+), 1 deletion(-) >> >> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh >> index 90581c10b..4589a8ba2 100755 >> --- a/.ci/linux-build.sh >> +++ b/.ci/linux-build.sh >> @@ -50,6 +50,31 @@ function build_ovs() >> make ${JOBS} >> } >> >> +function clang_analyze() >> +{ >> + [ -d "./base-clang-analyzer-results" ] && cache_build=false \ >> + || cache_build=true >> + if [ "$cache_build" = true ]; then >> + # If this is a cache build, proceed to the base branch's directory. >> + pushd base_ovs_main >> + fi; >> + >> + configure_ovs $OPTS >> + >> + make clean >> + scan-build -o ./clang-analyzer-results -sarif --use-cc=${CC} make >> ${JOBS} >> + >> + if [ "$cache_build" = true ]; then >> + # Move results, so it will be picked up by the cache. >> + mv ./clang-analyzer-results ../base-clang-analyzer-results >> + popd >> + else >> + # Only do the compare on the none cache builds. >> + sarif --check note diff ./base-clang-analyzer-results \ >> + ./clang-analyzer-results >> + fi; >> +} >> + >> if [ "$DEB_PACKAGE" ]; then >> ./boot.sh && ./configure --with-dpdk=$DPDK && make debian >> mk-build-deps --install --root-cmd sudo --remove debian/control >> @@ -117,6 +142,11 @@ fi >> >> OPTS="${EXTRA_OPTS} ${OPTS} $*" >> >> +if [ "$CLANG_ANALYZE" ]; then >> + clang_analyze >> + exit 0 >> +fi >> + >> if [ "$TESTSUITE" = 'test' ]; then >> # 'distcheck' will reconfigure with required options. >> # Now we only need to prepare the Makefile without sparse-wrapped CC. >> diff --git a/.ci/linux-prepare.sh b/.ci/linux-prepare.sh >> index c28b6819a..5028bdc44 100755 >> --- a/.ci/linux-prepare.sh >> +++ b/.ci/linux-prepare.sh >> @@ -23,7 +23,7 @@ cd .. >> # https://github.com/pypa/pip/issues/10655 >> pip3 install --disable-pip-version-check --user wheel >> pip3 install --disable-pip-version-check --user \ >> - flake8 'hacking>=3.0' netaddr pyparsing sphinx setuptools >> + flake8 'hacking>=3.0' netaddr pyparsing sarif-tools sphinx setuptools >> >> # Install python test dependencies >> pip3 install -r python/test_requirements.txt >> diff --git a/.github/workflows/build-and-test.yml >> b/.github/workflows/build-and-test.yml >> index 710757693..f5858fdbe 100644 >> --- a/.github/workflows/build-and-test.yml >> +++ b/.github/workflows/build-and-test.yml >> @@ -254,6 +254,119 @@ jobs: >> name: logs-linux-${{ join(matrix.*, '-') }} >> path: logs.tgz >> >> + build-clang-analyze: >> + needs: build-dpdk >> + env: >> + dependencies: | >> + automake bc clang-tools libbpf-dev libnuma-dev libpcap-dev \ >> + libunbound-dev libunwind-dev libssl-dev libtool llvm-dev >> + CC: clang >> + DPDK: dpdk >> + CLANG_ANALYZE: true >> + name: clang-analyze >> + runs-on: ubuntu-22.04 >> + timeout-minutes: 30 >> + >> + steps: >> + - name: checkout >> + uses: actions/checkout@v3 >> + with: >> + fetch-depth: 0 >> + >> + - name: get base branch sha >> + id: base_branch >> + env: >> + BASE_SHA: ${{ github.event.pull_request.base.sha }} >> + EVENT_BEFORE: ${{ github.event.before }} >> + FORCED_PUSH: ${{ github.event.forced }} >> + run: | >> + if [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then >> + echo "sha=$BASE_SHA" >> $GITHUB_OUTPUT >> + else >> + if [ "$EVENT_BEFORE" = "0000000000000000000000000000000000000000" >> ] \ >> + || [ "$FORCED_PUSH" = true ]; then >> + BASE_SHA=HEAD~1 >> + MIN_DISTANCE=1000 >> + git remote add upstream https://github.com/openvswitch/ovs.git >> + git fetch upstream >> + for upstream_head in $(git ls-remote --heads upstream main >> master dpdk-latest branch-2.17 branch-[3456789]* | cut -f 1); do >> + CURR_BASE=$(git merge-base ${upstream_head} HEAD 2>/dev/null) >> + if [ ${CURR_BASE} ]; then >> + DISTANCE=$(git log --oneline ${CURR_BASE}..HEAD | wc -l); >> + if test ${MIN_DISTANCE} -gt ${DISTANCE}; then >> + BASE_SHA=${CURR_BASE} >> + MIN_DISTANCE=${DISTANCE} >> + fi >> + fi >> + done >> + echo "sha=$BASE_SHA" >> $GITHUB_OUTPUT >> + else >> + echo "sha=$EVENT_BEFORE" >> $GITHUB_OUTPUT >> + fi >> + fi >> + >> + - name: checkout base branch >> + env: >> + BASE_SHA: ${{ steps.base_branch.outputs.sha }} >> + run: | >> + cp -r $(pwd)/. /tmp/base_ovs_main && mv /tmp/base_ovs_main ./ >> + cd $(pwd)/base_ovs_main >> + git checkout ${BASE_SHA} >> + >> + - name: update PATH >> + run: | >> + echo "$HOME/bin" >> $GITHUB_PATH >> + echo "$HOME/.local/bin" >> $GITHUB_PATH >> + >> + - name: generate cache key >> + id: cache_key >> + run: | >> + ver=$(${CC} -v 2>&1 | grep ' version ' | \ >> + sed 's/.*version \([0-9]*\.[0-9]*\.[0-9]*\).*/\1/g') >> + echo "key=${CC}-${ver}-analyze-$(git -C base_ovs_main rev-parse >> HEAD)" \ >> + >> $GITHUB_OUTPUT >> + >> + - name: check for analyzer result cache >> + id: clang_cache >> + uses: actions/cache@v3 >> + with: >> + path: base-clang-analyzer-results >> + key: ${{ steps.cache_key.outputs.key }} >> + >> + - name: set up python >> + uses: actions/setup-python@v4 >> + with: >> + python-version: '3.9' >> + >> + - name: get cached dpdk-dir >> + uses: actions/cache/restore@v3 >> + 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 base reference >> + if: steps.clang_cache.outputs.cache-hit != 'true' >> + run: ./.ci/linux-build.sh > > FWIW, this is not working if the base requires different version of DPDK. > Not sure what the solution would be though, except for the obvious disabling > DPDK for this job. This email slipped through the cracks… Did not think of this corner case :( I would like to keep DPDK, as packet handling is different, but maybe we could disable only if the base versions differ. Let me add a TODO item, and I will get back to this a bit later. //Eelco >> + >> + - name: save cache >> + uses: actions/cache/save@v3 >> + if: steps.clang_cache.outputs.cache-hit != 'true' >> + with: >> + path: base-clang-analyzer-results >> + key: ${{ steps.cache_key.outputs.key }} >> + >> + - name: build >> + run: ./.ci/linux-build.sh >> + >> build-osx: >> env: >> CC: clang _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
