How to use with a public repo?

[skrobinson]

I would like to add StaticAnalysis as an Action to run on PR in a public repository (see https://github.com/p-ranav/argparse/pull/148#issuecomment-967472187). Is this possible?

The PR log seems to say that the action does not have permission to write to the repo to add a comment.

[JacobDomagala]

Hmm, I haven't seen this type of error before (I've been only using it on my own repositories). For now you can run the action with force_console_print input set to true, but it won't create the comments (it will only print the result to GitHub's action console log, see https://github.com/JacobDomagala/StaticAnalysis#non-pull-request)

[skrobinson]

but it won't create the comments

The comments are a very nice feature of SA and I'd be sorry to see comments remain not working with public repos.

I have an idea that I believe is safe because SA does not build or run PR-controlled code.

1. Add pull_request_target as a supported event.
2. Checkout PR HEAD into a subdirectory.

name: Static analysis

on: [pull_request_target]

jobs:
  static_analysis:
    runs-on: ubuntu-latest

    steps:
    - name Checkout target
      uses: actions/checkout@v2

    - name Checkout PR tree
      uses: actions/checkout@v2
      with:
        repository: ${{ github.event.pull_request.repository.full_name }}
        ref: ${{ github.event.pull_request.sha }}
        path: pr_tree

    - name: Run static analysis
      uses: JacobDomagala/StaticAnalysis@master
      with:
        exclude_dir: lib
        init_script: init_script.sh
        root_dir: pr_tree

$ ls -1RF
.github/
lib/
pr_tree/
src/
README.md
test/
...
pr_tree/.github/
pr_tree/lib/
pr_tree/src/
pr_tree/README.md
pr_tree/test/
...

The key idea is that the SA tools configuration is in .github and it analyzes sources in pr_tree. What won't work with the current SA is specifying a root directory.

Is this a feature you would be interested in adding?

[JacobDomagala]

Yes, I will have to fix that eventually, I wasn't aware of that issue before.

[JacobDomagala]

Ok, I was able to fix the issue with comments from forks (https://github.com/JacobDomagala/StaticAnalysis/pull/30), but there's another issue that I don't think I'll be able to resolve.

The code snippets (or perma-links) work only on the same repository, meaning when you create PR from fork, and there're any issues found, the permalink to said file(s) will appear as link, not as a code snippet (source)

This type of permanent link will render as a code snippet only in the repository it originated in. In other repositories, the permalink code snippet will render as a URL.

I'll try to see whether it's something that can be somehow worked around.

[skrobinson]

Are you seeing bot comment boxes with snippet URLs and the associated warning (or note)? Does clicking the URL open the PR head at the file with highlighted lines? If so, that's a good start and may be all that is possible with the current GH Actions API.

There is a 3+ year issue asking for this to be resolved. If GH does include a cross-repo code snippet feature in the future, SA is in a good position to use it.

Thank you for your work adding this feature.

P.S. I just noticed that code snippets do seem to work with review comments, that code should also originate in the PR head. Maybe this is an angle to investigate?

[JacobDomagala]

Are you seeing bot comment boxes with snippet URLs and the associated warning (or note)? Does clicking the URL open the PR head at the file with highlighted lines?

Yes. This is a test PR created from forked repo, that I'm using to test my changes.

I just noticed that code snippets do seem to work with review comments, that code should also originate in the PR head. Maybe this is an angle to investigate?

Initially I was thinking about generating the code block (using the Markdown) with the hyperlink to the source file. Something like :

---

Issue found in source.cpp

int main(int /*argc*/, char** argv){
    int unused = 0;
    return 0;
}

!Line: 2 - style: Variable 'unused' is assigned a value that is never used. [unreadVariable]

---

But your idea could be easier to implement, I'll see if it works. Thanks!

[JacobDomagala]

Update: I ended up actually using my approach (creating code snippet using Markdown code-block), as it's easier to implement. The PR is close to being done, I need to double check the changes and update the documentation, so this should be done around weekend.

[skrobinson]

I'm not a fan of "<---- HERE", but it's not a problem, just a preference. Have you considered using a diff (rather than cpp) block with !line or @@line@@?

Example:

!   int anotherUnused;
}

int main(int /*argc*/, char** argv){
    int unused = 0;

Also, I looked over your recent commits on make-it-work-on-fork and my other questions/comments went away as I delved deeper. This new feature is coming together nicely. Thank you.

[JacobDomagala]

Ok, I'm merging #30, as it should work properly.