Python fixes for running on multiple platforms and outside of docker #103

bwrsandman · 2023-11-25T01:19:31Z

This PR is part of an attempt to run clang-tidy-review outside of docker.

The reason why we would want to do this is better control over github runner setup (packages, commands, environment).

Additionally, it grants the ability to run on non-linux platforms which allows the runner to analyze platform specific files/headers/libraries not available on linux in a docker.

The removal of docker also can speed up the analysis of clang-tidy. In my tests, docker runs for 4 minutes while linux without docker runs for 2, though macos and windows take upwards of 10 minutes.

Main fixes:

Use subprocess.run with args list instead of string. This removes the need for shlex and helps running on windows.
Convert the line filter path separators from \ to / on windows. This fixes running on windows.
The split workflow post step returns as an error code, the number of unique clang-tidy warnings. This allows a non-github runner to detect a non-perfect PR.
The split workflow post step allows multiple reviews as input. This allows multiple reviews on different platforms to be posted as one.

In short:

# Run review with token, repo, pr number, clang-tidy (from PATH) and
# with split workflow
review --token=${{ secrets.GITHUB_TOKEN }} \
       --repo=${{ github.repository }} \
       --pr=${{ github.event.pull_request.number }} \
       --clang_tidy_binary=clang-tidy \
       --build_dir=cmake-build-presets/ninja-multi-vcpkg \
       --config_file=.clang-tidy \
       --split_workflow=True

# Run the post step with each review
post --token=${{ secrets.GITHUB_TOKEN }} \
     --repo=${{ github.repository }} \
     ubuntu-latest/clang-tidy-review-output.json \
     macos-latest/clang-tidy-review-output.json \
     windows-latest/clang-tidy-review-output.json

In long:

name: Clang Tidy Review without Docker
on:
  pull_request:
jobs:
  clang-tidy-review: # Runs on each platform
    runs-on: ${{ matrix.os }}
    strategy:
      matrix:
        os: [ ubuntu-latest, macos-latest, windows-latest ]
    steps:
      - uses: actions/checkout@v3
      - name: Get clang-tidy in path
        run: |
          ln -s $(brew --prefix llvm@15)/bin/clang-tidy ~/bin/clang-tidy
          echo "export PATH=$PATH:~/bin" >> $GITHUB_ENV
        if: startsWith(matrix.os, 'macos')
      - name: Configure the source code and genereate compilation database json
      # ...
      - name: Checkout clang-tidy-review
        uses: actions/checkout@v4
        with:
          repository: bwrsandman/clang-tidy-review
          ref: windows_shell_fix
          path: clang-tidy-review
      - name: Non-docker clang-tidy-check
        run: |
          # Install clang-tidy-review python package and dependencies
          python3 -m pip install pip -U && python3 -m pip install ${{github.workspace}}/clang-tidy-review/post/clang_tidy_review
          # Run review with token, repo, pr number, clang-tidy (from PATH) and with split workflow
          review --token=${{ secrets.GITHUB_TOKEN }} \
                 --repo=${{ github.repository }} \
                 --pr=${{ github.event.pull_request.number }} \
                 --clang_tidy_binary=clang-tidy \
                 --build_dir=cmake-build-presets/ninja-multi-vcpkg \
                 --config_file=.clang-tidy \
                 --split_workflow=True
        env:
          USER: ${{ github.event.pull_request.user.login }} # for TODO fixes to include right name  
      - name: Upload metadata and review for next step to pick up
        uses: actions/upload-artifact@v3
        with:
          name: ClangTidyReviewOutputs-${{matrix.os}}
          path: |
            clang-tidy-review-metadata.json
            clang-tidy-review-output.json
            clang_tidy_review.yaml

  clang-tidy-comments: # Only needs one runner
    runs-on: ubuntu-latest
    needs: clang-tidy-review
    steps:
      - name: Checkout clang-tidy-review
        uses: actions/checkout@v4
        with:
          repository: bwrsandman/clang-tidy-review
          ref: windows_shell_fix
          path: clang-tidy-review
      - uses: actions/download-artifact@v3
        with:
          name: ClangTidyReviewOutputs-ubuntu-latest
          path: ubuntu-latest
      - uses: actions/download-artifact@v3
        with:
          name: ClangTidyReviewOutputs-macos-latest
          path: macos-latest
      - uses: actions/download-artifact@v3
        with:
          name: ClangTidyReviewOutputs-windows-latest
          path: windows-latest
      - name: Non-docker clang-tidy-check
        run: |
          # Install clang-tidy-review python package and dependencies
          python3 -m pip install pip -U && python3 -m pip install ${{github.workspace}}/clang-tidy-review/post/clang_tidy_review
          cp ubuntu-latest/clang-tidy-review-metadata.json . # copy one metadata for the PR number
          # Run the post step with each review
          post --token=${{ secrets.GITHUB_TOKEN }} \
               --repo=${{ github.repository }} \
               ubuntu-latest/clang-tidy-review-output.json \
               macos-latest/clang-tidy-review-output.json \
               windows-latest/clang-tidy-review-output.json

bwrsandman · 2024-01-09T16:56:20Z

@ZedThree Could you have a look?

ZedThree · 2024-01-09T17:27:50Z

@bwrsandman Apologies, I missed this at the time! Will take a look this week.