oxsecurity / megalinter

🦙 MegaLinter analyzes 50 languages, 22 formats, 21 tooling formats, excessive copy-pastes, spelling mistakes and security issues in your repository sources with a GitHub Action, other CI tools or locally.
https://megalinter.io
GNU Affero General Public License v3.0
1.95k stars 237 forks source link

GithubStatusReporter is not working for PR #1279

Closed ankitbko closed 2 years ago

ankitbko commented 2 years ago

https://github.com/megalinter/megalinter/blob/b9f17545cf5391b20c97a9410f72081da3077d17/megalinter/reporters/GithubStatusReporter.py#L40

I see commit is read from "GITHUB_SHA" which is merge commit in case of PR and the git is in detached state. For PR we need to find the HEAD commit for the PR and use that commit.

nvuillam commented 2 years ago

Do you have an example repository where the issue happens ?

ankitbko commented 2 years ago

The repo is private with self hosted agent but I can share some screenshots.

Here is the commit for which the action ran on PR image

Below is the git checkout step. You can see it merging 744a7b2a with main branch and creating a merge commit 3a2bf9a. image

The merge commit 3a2bf9a is what is assigned as GITHUB_SHA image

And the status is being set on merge commit 3a2bf9a image

ankitbko commented 2 years ago

The above is expected behavior by github action - https://github.community/t/github-sha-not-the-same-as-the-triggering-commit/18286

I went through source code of couple of other actions I am using and they are getting HEAD commit of PR instead of relying on GITHUB_SHA if the action trigger was PR.

https://github.com/EnricoMi/publish-unit-test-result-action/blob/master/python/publish_unit_test_results.py#L135 https://github.com/5monkeys/cobertura-action/blob/717143c6305698f49686e4def6c1156671b3499a/src/action.js#L304

I think something different but on the similar line was done for GithubCommentReporter, i.e not relying on GITHUB_SHA for PR triggered action https://github.com/megalinter/megalinter/pull/526/commits/d1fc69f64481e27e0e142d66c8d8f8147b91b7d2#diff-81c879afcb1f3042d4dc87997faa89145d5891aa9a9ceeee89a5313268b4c0a1R194

nvuillam commented 2 years ago

Thanks for the great analysis !

I'm short on time these days... would you like to make a PR ?

ankitbko commented 2 years ago

@nvuillam So I have done a quick fix for this but I dont know how to run Megalinter from forked repo to test this code. Can you help me with testing this?

nvuillam commented 2 years ago

I retrieved your updates in a local branch and made a test PR -> https://github.com/megalinter/megalinter/runs/5279839076?check_suite_focus=true It will generate a docker image that we'll be able to use for tests :)

ankitbko commented 2 years ago

Thanks a lot. There are 2 minor linting error. Would appreciate if you could fix it in your branch and rebuild the PR

github-actions[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

Yann-J commented 1 year ago

Hey, came across this stale issue, and was facing the same (build running from Azure Devops, but code in Github).

I worked around this with a hack to determine the PR head commit hash based on the comment on the merge commit, like this:

      if [[ "${BUILD_REASON}" == "PullRequest" ]]; then
        echo "This is a PR build, scanning delta only, and posting check to PR head instead of local merge commit"
        export VALIDATE_ALL_CODEBASE=false
        # Hack to determine the commit hash of the PR's head, from the comment on the local merge commit
        export GITHUB_SHA=$(git log -1 --pretty=%B | sed -e 's/^Merge //' -e 's/ into.*//')
      else
        echo "This is a regular build"
        export VALIDATE_ALL_CODEBASE=false=true
        export GITHUB_SHA=${BUILD_SOURCEVERSION}
      fi
Kurt-von-Laven commented 1 year ago

Thanks for sharing what worked for you! Did you mean VALIDATE_ALL_CODEBASE=true instead of VALIDATE_ALL_CODEBASE=false=true?

Yann-J commented 1 year ago

Thanks for sharing what worked for you! Did you mean VALIDATE_ALL_CODEBASE=true instead of VALIDATE_ALL_CODEBASE=false=true?

Haha yeah, good catch, indeed!

For anyone in a similar situation (code in Github but build in Azure devops), I'm also sharing here the exact working Megalinter invocation, to show how Azure variables map to what ML reporters expects:

      if [[ "${BUILD_REASON}" == "PullRequest" ]]; then
        echo "This is a PR build, scanning delta only"
        export VALIDATE_ALL_CODEBASE=false
        export GITHUB_SHA=$(git log -1 --pretty=%B | sed -e 's/^Merge //' -e 's/ into.*//')
      else
        echo "This is a regular build"
        export VALIDATE_ALL_CODEBASE=true
        export GITHUB_SHA=${BUILD_SOURCEVERSION}
      fi

      docker run -v "$(System.DefaultWorkingDirectory):/tmp/lint" \
        -e GITHUB_TOKEN \
        -e VALIDATE_ALL_CODEBASE \
        -e APPLY_FIXES=none \
        -e GITHUB_STATUS_REPORTER=true \
        -e GITHUB_COMMENT_REPORTER=true \
        -e CI=true \
        -e GITHUB_REF=$(Build.SourceBranch) \
        -e GITHUB_REPOSITORY=$(Build.Repository.ID) \
        -e GITHUB_TARGET_URL="$(System.CollectionUri)/$(System.TeamProject)/_build/results?buildId=$(Build.BuildId)&view=logs&j=$(System.JobId)&t=$(System.TaskInstanceId)" \
        -e GITHUB_SHA \
        -e LOG_LEVEL=debug \
        oxsecurity/megalinter-cupcake:v6