ZedThree / clang-tidy-review

Create a pull request review based on clang-tidy warnings
MIT License
88 stars 44 forks source link

no source files found #114

Closed akallabeth closed 7 months ago

akallabeth commented 8 months ago

hi,

I´ve created a workflow as suggested for FreeRDP, see https://github.com/FreeRDP/FreeRDP/pull/9794

now, everything works fine until the actual clang-tidy run, where source files are not detected.

anything I´m missing here?

bwrsandman commented 8 months ago

This check only runs on the diff of the changed files of a PR. No files were found in your PR because your PR didn't change any CPP files only workflow and cmake.

akallabeth commented 7 months ago

@bwrsandman ok, so far so good, but why does that fail the build?

I mean it looks like it fails to collect the reports, but still the result of the build was failure.

Created the following review:
 None
Traceback (most recent call last):
  File "/usr/local/bin/review", line 8, in <module>
No warnings to report, LGTM!
    sys.exit(main())
             ^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/clang_tidy_review/review.py", line 171, in main
    post_review(
  File "/usr/local/lib/python3.11/dist-packages/clang_tidy_review/__init__.py", line 963, in post_review
    pull_request.post_lgtm_comment(lgtm_comment_body)
  File "/usr/local/lib/python3.11/dist-packages/clang_tidy_review/__init__.py", line 223, in post_lgtm_comment
    self.pull_request.create_issue_comment(body)
  File "/usr/local/lib/python3.11/dist-packages/github/PullRequest.py", line 517, in create_issue_comment
    headers, data = self._requester.requestJsonAndCheck(
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/github/Requester.py", line 442, in requestJsonAndCheck
    return self.__check(
           ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/github/Requester.py", line 487, in __check
    raise self.__createException(status, responseHeaders, data)
github.GithubException.GithubException: 403 {"message": "Resource not accessible by integration", "documentation_url": "https://docs.github.com/rest/issues/comments#create-an-issue-comment"}
bwrsandman commented 7 months ago

Try with 0.17.1 instead of 0.14.0. Looks like there's some trouble with the permissions, hopefully the newer versions fix that.

akallabeth commented 7 months ago

@bwrsandman ok, the newer version was a bit clearer with what is failing. still, tried to add permissions: issues: write and still get

Request POST /repos/FreeRDP/FreeRDP/issues/9819/comments failed with 403: Forbidden
No warnings to report, LGTM!
Traceback (most recent call last):
  File "/usr/local/bin/review", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/clang_tidy_review/review.py", line 177, in main
    post_review(
  File "/usr/local/lib/python3.11/dist-packages/clang_tidy_review/__init__.py", line 1102, in post_review
    pull_request.post_lgtm_comment(lgtm_comment_body)
  File "/usr/local/lib/python3.11/dist-packages/clang_tidy_review/__init__.py", line 339, in post_lgtm_comment
    self.pull_request.create_issue_comment(body)
  File "/usr/local/lib/python3.11/dist-packages/github/PullRequest.py", line 437, in create_issue_comment
    headers, data = self._requester.requestJsonAndCheck("POST", f"{self.issue_url}/comments", input=post_parameters)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/github/Requester.py", line 494, in requestJsonAndCheck
    return self.__check(*self.requestJson(verb, url, parameters, headers, input, self.__customConnection(url)))
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/github/Requester.py", line 629, in requestJson
    return self.__requestEncode(cnx, verb, url, parameters, headers, input, encode)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/github/Requester.py", line 726, in __requestEncode
    status, responseHeaders, output = self.__requestRaw(cnx, verb, url, requestHeaders, encoded_input)
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/github/Requester.py", line 760, in __requestRaw
    response = cnx.getresponse()
               ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/github/Requester.py", line 174, in getresponse
    r = verb(
        ^^^^^
  File "/usr/local/lib/python3.11/dist-packages/requests/sessions.py", line 637, in post
    return self.request("POST", url, data=data, json=json, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/requests/sessions.py", line 589, in request
    resp = self.send(prep, **send_kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/requests/sessions.py", line 703, in send
    r = adapter.send(request, **kwargs)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/requests/adapters.py", line 486, in send
    resp = conn.urlopen(
           ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/urllib3/connectionpool.py", line 935, in urlopen
    retries = retries.increment(method, url, response=response, _pool=self)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/github/GithubRetry.py", line 179, in increment
    raise Requester.createException(response.status, response.headers, content)  # type: ignore
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
github.GithubException.GithubException: 403 {"message": "Resource not accessible by integration", "documentation_url": "https://docs.github.com/rest/issues/comments#create-an-issue-comment"}
0s
bwrsandman commented 7 months ago

Left a comment on your PR https://github.com/FreeRDP/FreeRDP/pull/9819#pullrequestreview-1848826368 The message leaves a lot to be desired. I will try to repro on my end.

bwrsandman commented 7 months ago

I'm not able to repro on a repository which already has the test, so I think the issue is that you're running it from a branch which doesn't have the required permissions.

My config looks like this btw. I don't use the cmake option, I generate the databse myself.

# ...
on:
  pull_request_target:
# ...
  clang-tidy:
    needs: build
    runs-on: ubuntu-latest
    if: startsWith(github.event_name, 'pull_request')
    steps:
      - uses: actions/checkout@v3
        with:
          ref: ${{ github.event.pull_request.head.ref }}
          repository: ${{ github.event.pull_request.head.repo.full_name }}
          submodules: recursive
      - name: Download generated compile database
        uses: actions/download-artifact@v3
        with:
          name: openblack-compile-database
          path: cmake-build-presets/ninja-multi-vcpkg
      - name: Add base repo to git config
        run: git remote add upstream ${{ github.event.pull_request.base.repo.html_url }}
        if: startsWith(github.event_name, 'pull_request')
      - uses: ZedThree/clang-tidy-review@v0.17.1
        id: review
        with:
          build_dir: cmake-build-presets/ninja-multi-vcpkg
          exclude: "*ShaderIncluder.h,*json.hpp,*imgui_impl_sdl*,*stb_image_write.h,*dr_*.h"
          config_file: ""       # Don't use a single config file
          clang_tidy_checks: "" # Use checks from .clang-tidy tree
          lgtm_comment_body: "" # Don't post a lgtm message
          annotations: false
      - name: If there are any comments, fail the check
        if: steps.review.outputs.total_comments > 0
        run: exit 1
akallabeth commented 7 months ago

@bwrsandman ok, identified the setting that triggers this:

lgtm_comment_body: ""

and the build succeeds.

bwrsandman commented 7 months ago

That just means it won't try to post anything if all is well. You didn't fix the underlying issue.

What's happening is that your PR comes from your own repo and not the target repo. So you don't have permission to post as the GitHub action bot on the PR.

akallabeth commented 7 months ago

That just means it won't try to post anything if all is well. You didn't fix the underlying issue.

What's happening is that your PR comes from your own repo and not the target repo. So you don't have permission to post as the GitHub action bot on the PR.

assumed as much. but I don´t get it why permissions: issues:write does not fix this? are there some variables missing in the post?

bwrsandman commented 7 months ago

This is my best guess From a security perspective, I don't want any random contributors to be able to use my repo's token. Even if they add a permission. Since you're doing this PR from a fork, you won't have the permissions from that PR.

akallabeth commented 7 months ago

@bwrsandman I´ve merged the workflow now, but still there are issues with the run. did I miss something or why does it still fail?

bwrsandman commented 7 months ago

@ZedThree do you have any idea why he's getting 403 on his project when it comes to posting?

bwrsandman commented 7 months ago

If I compare a job which works with yours, the permissions differ: Works

GITHUB_TOKEN Permissions
  Actions: write
  Checks: write
  Contents: write
  Deployments: write
  Discussions: write
  Issues: write
  Metadata: read
  Packages: write
  Pages: write
  PullRequests: write
  RepositoryProjects: write
  SecurityEvents: write
  Statuses: write

Yours only has read so it would make sense that it can't post anything to the PR.

GITHUB_TOKEN Permissions
  Contents: read
  Metadata: read

Blindly guessing here but you probably need write permissions for PullRequests or Content. And remember that since it's a pull_request_target, your change needs to be in the target branch to run.

akallabeth commented 7 months ago

@bwrsandman thank you, found it. I´ve had copied an example but if I leave out the permissions section altogether it works fine.