Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Running clang-tidy-review outside of container #122

Open
bwrsandman opened this issue May 10, 2024 · 4 comments
Open

Running clang-tidy-review outside of container #122

bwrsandman opened this issue May 10, 2024 · 4 comments

Comments

@bwrsandman
Copy link
Contributor

bwrsandman commented May 10, 2024

I wanted to post this in discussions but it's not enabled for the repo.

Here's how I am able to run the clang-tidy review script directly on the github runner environments without needing the ubuntu container.
Why would you do this?

  1. Saves time spent each run fetching the docker and installing packages with apt
  2. You can take better advantage of github caches
  3. You can run this on any of the runners that github provides or that you host yourself.
  4. This works on different CI workflows than github actions
  5. You can use any version of clang-tidy that is available to you in this environment.
  6. Overall seems to run much faster
name: Clang Tidy
on:
  pull_request_target:  # Required to post comments on PR from forks
jobs:
  clang-tidy-review:
    runs-on: ubuntu-latest
    if: startsWith(github.event_name, 'pull_request')  # Posting a review only makes sense on PRs
    steps:
      # Repo config for pull_request_target
      - uses: actions/checkout@v3
        with:
          ref: ${{ github.event.pull_request.head.ref }}
          repository: ${{ github.event.pull_request.head.repo.full_name }}
          submodules: recursive

      # Generate compilation database and make sure to have the source and all third party headers available

      # Checkout the review source for the python package
      - name: Checkout clang-tidy-review
        uses: actions/checkout@v4
        with:
          repository: ZedThree/clang-tidy-review
          ref: master
          path: clang-tidy-review

      - name: Install LLVM and Clang
        uses: KyleMayes/install-llvm-action@v2
        with:
          version: '17'

     - uses: actions/setup-python@v5
       with:
         python-version: '3.12'
         cache: 'pip'

      # Install the python package and run review
      - name: Non-docker clang-tidy-check
        run: |
          clang-tidy --version
          pip install ${{github.workspace}}/clang-tidy-review/post/clang_tidy_review
          review --token=${{ secrets.GITHUB_TOKEN }} --repo=${{ github.event.pull_request.head.repo.full_name }} --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 }}

     # Upload the review for the next job
      - 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

  # Second part of the split job
  clang-tidy-comments:
    runs-on: ubuntu-latest
    needs: clang-tidy-review
    steps:
      - name: Checkout clang-tidy-review
        uses: actions/checkout@v4
        with:
          repository: ZedThree/clang-tidy-review
          ref: master
          path: clang-tidy-review

     # Download all the reviews
      - 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

     - uses: actions/setup-python@v5
       with:
         python-version: '3.12'
         cache: 'pip'

      # Again install and run the python package to post the review
      - name: Non-docker clang-tidy-check
        run: |
          clang-tidy --version
          pip install ${{github.workspace}}/clang-tidy-review/post/clang_tidy_review
          cp ubuntu-latest/clang-tidy-review-metadata.json .
          post --token=${{ secrets.GITHUB_TOKEN }} --repo=${{ github.event.pull_request.head.repo.full_name }} ubuntu-latest/clang-tidy-review-output.json macos-latest/clang-tidy-review-output.json windows-latest/clang-tidy-review-output.json
@ZehMatt
Copy link

ZehMatt commented May 12, 2024

I initially used also clang-tidy-review but ended up creating my own that works almost the same as this one for the reasons you just pointed out, you can check it out here https://github.com/ZehMatt/clang-tidy-annotations

I hope this doesn't come over as self advertisement but I figured this is exactly what this issue is about.

@bwrsandman
Copy link
Contributor Author

Thanks for sharing!
Correct me if I'm wrong but annotations don't give you the option to add fixes to a batch in the PR's web interface, is that right?

@ZedThree
Copy link
Owner

This is really nice, thanks @bwrsandman! I need to investigate properly, but I think it's possible to have composite actions (e.g. https://docs.github.com/en/actions/creating-actions/creating-a-composite-action). That looks like we could then use your workflow here and export it as a single action for users?

@ZedThree
Copy link
Owner

ZedThree commented Nov 7, 2024

I think this is the way forward for this action. It would solve the issue of not being able to use the latest clang-tidy through KyleMayes/install-llvm-action.

It would also solve the issue of needing to install packages again inside our container
-- assuming the user has already set up their environment to configure their package and already get compile_commands.json. On the other hand, if they rely on us to install packages + run cmake, then we'll need to still handle that.

To make @bwrsandman's initial workflow a bit nicer, we could either release clang-tidy-review on PyPI, or just install it straight from github.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants