From cc00362125c7726551d2b6bda85e1a4b17d0bc81 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 14 Jul 2021 22:09:36 +0000 Subject: ci(check-whitespace): stop requiring a read/write token As part of some recent security tightening, GitHub introduced the ability to configure GitHub workflows to be run with a read-only token. This is much more secure, in particular when working in a public repository: While the regular read/write token might be restricted to writing to the current branch, it is not necessarily restricted to access only the current Pull Request. However, the `check-whitespace` workflow threw a wrench into this plan: it _requires_ write access (because it wants to add a PR comment in case of a whitespace issue). Let's just skip that PR comment. The user can always click through to the actual error, even if it is slightly less convenient. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- .github/workflows/check-whitespace.yml | 16 ---------------- 1 file changed, 16 deletions(-) (limited to '.github') diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml index f1483059c7..c53614d603 100644 --- a/.github/workflows/check-whitespace.yml +++ b/.github/workflows/check-whitespace.yml @@ -51,21 +51,5 @@ jobs: if test -n "${log}" then - echo "::set-output name=checkout::"${log}"" exit 2 fi - - - name: Add Check Output as Comment - uses: actions/github-script@v3 - id: add-comment - env: - log: ${{ steps.check_out.outputs.checkout }} - with: - script: | - await github.issues.createComment({ - issue_number: context.issue.number, - owner: context.repo.owner, - repo: context.repo.repo, - body: `Whitespace errors found in workflow ${{ github.workflow }}:\n\n\`\`\`\n${process.env.log.replace(/\\n/g, "\n")}\n\`\`\`` - }) - if: ${{ failure() }} -- cgit v1.2.3 From a066a90db68da5262e81e74a50d18eaeddc6783f Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 14 Jul 2021 22:09:37 +0000 Subject: ci(check-whitespace): restrict to the intended commits During a run of the `check-whitespace` we want to verify that the commits introduced in the Pull Request have no whitespace issues. We only want to look at those commits, not the upstream commits (because the contributor cannot do anything about the latter). However, by using the `-` form in `git log --check`, we run the risk of looking at the wrong commits. The reason is that the `actions/checkout` step does _not_ check out the tip commit of the Pull Request's branch: Instead, it checks out a merge commit that merges that branch into the target branch. For that reason, we already adjust the commit count by incrementing it, but that is not enough: if the upstream branch has newer commits, they are traversed _first_. And obviously we will then miss some of the commits that we _actually_ wanted to look at. Therefore, let's be careful to stop assuming a linear, up to date commit topology in the contributed commits, and instead specify the correct commit range. Unfortunately, this means that we no longer can rely on a shallow clone: There is no way of knowing just how many commits the upstream branch advanced after the commit from which the PR branch branched off. So let's just go with a full clone instead, and be safe rather than sorry (if we have "too shallow" a situation, a commit range `@{u}..` may very well include a shallow commit itself, and the output of `git show --check ` is _not_ pretty). Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- .github/workflows/check-whitespace.yml | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) (limited to '.github') diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml index c53614d603..8c4358d805 100644 --- a/.github/workflows/check-whitespace.yml +++ b/.github/workflows/check-whitespace.yml @@ -12,15 +12,9 @@ jobs: check-whitespace: runs-on: ubuntu-latest steps: - - name: Set commit count - shell: bash - run: echo "COMMIT_DEPTH=$((1+$COMMITS))" >>$GITHUB_ENV - env: - COMMITS: ${{ github.event.pull_request.commits }} - - uses: actions/checkout@v2 with: - fetch-depth: ${{ env.COMMIT_DEPTH }} + fetch-depth: 0 - name: git log --check id: check_out @@ -47,7 +41,7 @@ jobs: echo "${dash} ${etc}" ;; esac - done <<< $(git log --check --pretty=format:"---% h% s" -${{github.event.pull_request.commits}}) + done <<< $(git log --check --pretty=format:"---% h% s" ${{github.event.pull_request.base.sha}}..) if test -n "${log}" then -- cgit v1.2.3