Revert "[CI] Make email check workflow fail when author's email is private in Github UI"

[uditagarwal97]

Reverts #148694

The workflow is failing if user's email is not listed publicly on your
GH profile. This is different from not having your email public on
Github (in Github email settings page vs. email field in Github
profile/email settings).

[llvmbot]

@llvm/pr-subscribers-github-workflow

Author: Udit Kumar Agarwal (uditagarwal97)

Changes

Reverts llvm/llvm-project#148694

The workflow is failing if user's email is not listed publicly on your
GH profile. This is different from not having your email public on
Github (in Github email settings page vs. email field in Github
profile/email settings).

---

Full diff: https://github.com/llvm/llvm-project/pull/149186.diff

1 Files Affected:

• (modified) .github/workflows/email-check.yaml (+3-19)

diff --git a/.github/workflows/email-check.yaml b/.github/workflows/email-check.yaml
index 3339b1eed667b..904ad718f97dd 100644
--- a/.github/workflows/email-check.yaml
+++ b/.github/workflows/email-check.yaml
@@ -20,30 +20,14 @@ jobs:
 
       - name: Extract author email
         id: author
-        env:
-          GH_TOKEN: ${{ github.token }}
         run: |
-          # Use Github GraphQL APIs to get the email associated with the PR author because this takes into account the GitHub settings for email privacy.
-          query='
-          query($login: String!) {
-            user(login: $login) {
-              email
-            }
-          }'
-
-          PR_AUTHOR=${{ github.event.pull_request.user.login }}
-
-          email=$(gh api graphql -f login="$PR_AUTHOR" -f query="$query" --jq '.data.user.email')
-          echo "EMAIL_AUTHOR_GH_UI=$email" >> "$GITHUB_OUTPUT"
-
+          git log -1
+          echo "EMAIL=$(git show -s --format='%ae' HEAD~0)" >> $GITHUB_OUTPUT
           # Create empty comment file
           echo "[]" > comments
 
-      # When EMAIL_AUTHOR_GH_UI is NULL, author's email is hidden in GitHub UI.
-      # In this case, we warn the user to turn off "Keep my email addresses private"
-      # setting in their account.
       - name: Validate author email
-        if: ${{ steps.author.outputs.EMAIL_AUTHOR_GH_UI == '' }}
+        if: ${{ endsWith(steps.author.outputs.EMAIL, 'noreply.github.com')  }}
         env:
           COMMENT: >-
             ⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.<br/>

[uditagarwal97]

@sarnex @DavidSpickett @boomanaiden154 @asl FYI

[sarnex]

Modified CI job passed so merging

[DavidSpickett]

FYI @mstorsjo

@uditagarwal97 you say:

This is different from not having your email public on
Github (in Github email settings page vs. email field in Github
profile/email settings).

As I understand that, the API you were using only gives access to the email on the GitHub profile. That is, a publicly displayed email address.

So people who did not have their email hidden for commits and also did not have it listed on their GitHub profile were being told their email was private. Correct?

Do you know if there is an API that would tell us if the email "within" the user's settings is going to be used for a commit? I would guess there's no API to grab the email address itself, as that's reading it without the users' consent (if they choose to use it in a commit, that's their decision).

So without an API for that specific setting, this workflow is basically impossible to do correctly, right? We'd have to go back to the dodgy way of looking at the commits themselves.

(none of this is a criticism btw, thank you for your work to improve this, even though it didn't work out, sometimes that's out of our control and especially so with GitHub)

[uditagarwal97]

@DavidSpickett

FYI @mstorsjo

@uditagarwal97 you say:

This is different from not having your email public on
Github (in Github email settings page vs. email field in Github
profile/email settings).

As I understand that, the API you were using only gives access to the email on the GitHub profile. That is, a publicly displayed email address.

Unfortunately, yes.

So people who did not have their email hidden for commits and also did not have it listed on their GitHub profile were being told their email was private. Correct?

Exactly. Folks who have a visible email address for commits were also getting pinged just because email is not listed publicly on their Github profile.

Do you know if there is an API that would tell us if the email "within" the user's settings is going to be used for a commit? I would guess there's no API to grab the email address itself, as that's reading it without the users' consent (if they choose to use it in a commit, that's their decision).

No, I scanned Github docs pretty extensively but didn't find any API to get user's email settings :(

So without an API for that specific setting, this workflow is basically impossible to do correctly, right? We'd have to go back to the dodgy way of looking at the commits themselves.

One possible option is to get the commit when PR gets merged, get the email associated with the commit, and if it's noreply.github then we drop a comment to the PR about using private email. But, this approach is post-commit and does not prevent users from merging a PR with noreply.github email, still it is better than what we have currently.

Also, at https://github.com/intel/llvm, we have a policy for gatekeepers (folks with merge rights) to double-check the email id before merging a PR and in the past, this has worked pretty good for us. Maybe we can adopt a similar policy for https://github.com/llvm/llvm-project ?

(none of this is a criticism btw, thank you for your work to improve this, even though it didn't work out, sometimes that's out of our control and especially so with GitHub)

[mstorsjo]

One possible option is to get the commit when PR gets merged, get the email associated with the commit, and if it's noreply.github then we drop a comment to the PR about using private email. But, this approach is post-commit and does not prevent users from merging a PR with noreply.github email, still it is better than what we have currently.

Thanks, that sounds like a good idea - certainly better than doing nothing (or having to manually comment about it when encountering it)! It doesn't get the rate of noreply email addresses down to zero, but it should converge towards valid emails for each individual contributor, ideally.

Also, at https://github.com/intel/llvm, we have a policy for gatekeepers (folks with merge rights) to double-check the email id before merging a PR and in the past, this has worked pretty good for us. Maybe we can adopt a similar policy for https://github.com/llvm/llvm-project ?

Like just a bullet point in docs for your development practices? I guess we could add that as well - not sure how much practical effect that has (if people would notice the added documentation and pick up on doing it), but I guess it's better than nothing as well.

[uditagarwal97]

One possible option is to get the commit when PR gets merged, get the email associated with the commit, and if it's noreply.github then we drop a comment to the PR about using private email. But, this approach is post-commit and does not prevent users from merging a PR with noreply.github email, still it is better than what we have currently.

Thanks, that sounds like a good idea - certainly better than doing nothing (or having to manually comment about it when encountering it)! It doesn't get the rate of noreply email addresses down to zero, but it should converge towards valid emails for each individual contributor, ideally.

True. If it's something community would be interested in, I can try implementing this approach over the weekend.

Also, at https://github.com/intel/llvm, we have a policy for gatekeepers (folks with merge rights) to double-check the email id before merging a PR and in the past, this has worked pretty good for us. Maybe we can adopt a similar policy for https://github.com/llvm/llvm-project ?

Like just a bullet point in docs for your development practices? I guess we could add that as well - not sure how much practical effect that has (if people would notice the added documentation and pick up on doing it), but I guess it's better than nothing as well.

Yes, updating the doc and dropping an email in the gatekeepers mailing list (if there is one).