Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix the commit email filters to exclude personal branches #7

Merged
merged 2 commits into from
Dec 5, 2023
Merged

Fix the commit email filters to exclude personal branches #7

merged 2 commits into from
Dec 5, 2023

Conversation

jroelofs
Copy link
Contributor

@jroelofs
Copy link
Contributor Author

I don't know how to test this.

@jroelofs
Copy link
Contributor Author

jroelofs commented Dec 1, 2023

cc: @tstellar

@tstellar
Copy link
Contributor

tstellar commented Dec 4, 2023

In order to test this, I need to get a webhook event from a commit to one of the personal branches. Maybe someone could create a test stacked PR that demonstrates the bug.

@jroelofs
Copy link
Contributor Author

jroelofs commented Dec 4, 2023

IIUC any commit to a personal branch would trigger it. This stack triggers it every time I do spr diff --all --update-message

tstellar
tstellar reviewed Dec 4, 2023
View reviewed changes
AWS/Lambda/CommitEmailer/commit_emailer.py Outdated
Comment on lines 179 to 180
if event['ref'].startswith('refs/heads/users/'):
break
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check should be moved to the beginning of the function and should cause an early exit with a status code like we do here: https://github.com/llvm/llvm-admin/blob/main/AWS/Lambda/PREmailer/pr_emailer.py#L268

@tstellar
Copy link
Contributor

tstellar commented Dec 4, 2023

You can use this patch and input for debugging:

debug.patch.txt
spr.json

@jroelofs
Copy link
Contributor Author

jroelofs commented Dec 4, 2023

Mind pasting the contents of that json blob? I'm getting a permissions error.

@tstellar
Copy link
Contributor

tstellar commented Dec 4, 2023 •
edited 

{
  "ref": "refs/heads/users/vitalybuka/spr/nfcasan-replace-asaninitedensure_asan_inited-with-tryasaninitfromrtl",
  "before": "038d0f7a565d26b5a81020a62b804a5968ea6c66",
  "after": "d9f50eb2f4d07a61eec2b68518500de59f4f34ec",
  "repository": {
    "id": 75821432,
    "node_id": "MDEwOlJlcG9zaXRvcnk3NTgyMTQzMg==",
    "name": "llvm-project",
    "full_name": "llvm/llvm-project",
    "private": false,
    "owner": {
      "name": "llvm",
      "email": null,
      "login": "llvm",
      "id": 17149993,
      "node_id": "MDEyOk9yZ2FuaXphdGlvbjE3MTQ5OTkz",
      "avatar_url": "https://avatars.githubusercontent.com/u/17149993?v=4",
      "gravatar_id": "",
      "url": "https://api.github.com/users/llvm",
      "html_url": "https://github.com/llvm",
      "followers_url": "https://api.github.com/users/llvm/followers",
      "following_url": "https://api.github.com/users/llvm/following{/other_user}",
      "gists_url": "https://api.github.com/users/llvm/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/llvm/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/llvm/subscriptions",
      "organizations_url": "https://api.github.com/users/llvm/orgs",
      "repos_url": "https://api.github.com/users/llvm/repos",
      "events_url": "https://api.github.com/users/llvm/events{/privacy}",
      "received_events_url": "https://api.github.com/users/llvm/received_events",
      "type": "Organization",
      "site_admin": false
    },
    "html_url": "https://github.com/llvm/llvm-project",
    "description": "The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.",
    "fork": false,
    "url": "https://github.com/llvm/llvm-project",
    "forks_url": "https://api.github.com/repos/llvm/llvm-project/forks",
    "keys_url": "https://api.github.com/repos/llvm/llvm-project/keys{/key_id}",
    "collaborators_url": "https://api.github.com/repos/llvm/llvm-project/collaborators{/collaborator}",
    "teams_url": "https://api.github.com/repos/llvm/llvm-project/teams",
    "hooks_url": "https://api.github.com/repos/llvm/llvm-project/hooks",
    "issue_events_url": "https://api.github.com/repos/llvm/llvm-project/issues/events{/number}",
    "events_url": "https://api.github.com/repos/llvm/llvm-project/events",
    "assignees_url": "https://api.github.com/repos/llvm/llvm-project/assignees{/user}",
    "branches_url": "https://api.github.com/repos/llvm/llvm-project/branches{/branch}",
    "tags_url": "https://api.github.com/repos/llvm/llvm-project/tags",
    "blobs_url": "https://api.github.com/repos/llvm/llvm-project/git/blobs{/sha}",
    "git_tags_url": "https://api.github.com/repos/llvm/llvm-project/git/tags{/sha}",
    "git_refs_url": "https://api.github.com/repos/llvm/llvm-project/git/refs{/sha}",
    "trees_url": "https://api.github.com/repos/llvm/llvm-project/git/trees{/sha}",
    "statuses_url": "https://api.github.com/repos/llvm/llvm-project/statuses/{sha}",
    "languages_url": "https://api.github.com/repos/llvm/llvm-project/languages",
    "stargazers_url": "https://api.github.com/repos/llvm/llvm-project/stargazers",
    "contributors_url": "https://api.github.com/repos/llvm/llvm-project/contributors",
    "subscribers_url": "https://api.github.com/repos/llvm/llvm-project/subscribers",
    "subscription_url": "https://api.github.com/repos/llvm/llvm-project/subscription",
    "commits_url": "https://api.github.com/repos/llvm/llvm-project/commits{/sha}",
    "git_commits_url": "https://api.github.com/repos/llvm/llvm-project/git/commits{/sha}",
    "comments_url": "https://api.github.com/repos/llvm/llvm-project/comments{/number}",
    "issue_comment_url": "https://api.github.com/repos/llvm/llvm-project/issues/comments{/number}",
    "contents_url": "https://api.github.com/repos/llvm/llvm-project/contents/{+path}",
    "compare_url": "https://api.github.com/repos/llvm/llvm-project/compare/{base}...{head}",
    "merges_url": "https://api.github.com/repos/llvm/llvm-project/merges",
    "archive_url": "https://api.github.com/repos/llvm/llvm-project/{archive_format}{/ref}",
    "downloads_url": "https://api.github.com/repos/llvm/llvm-project/downloads",
    "issues_url": "https://api.github.com/repos/llvm/llvm-project/issues{/number}",
    "pulls_url": "https://api.github.com/repos/llvm/llvm-project/pulls{/number}",
    "milestones_url": "https://api.github.com/repos/llvm/llvm-project/milestones{/number}",
    "notifications_url": "https://api.github.com/repos/llvm/llvm-project/notifications{?since,all,participating}",
    "labels_url": "https://api.github.com/repos/llvm/llvm-project/labels{/name}",
    "releases_url": "https://api.github.com/repos/llvm/llvm-project/releases{/id}",
    "deployments_url": "https://api.github.com/repos/llvm/llvm-project/deployments",
    "created_at": 1481103573,
    "updated_at": "2023-12-04T19:06:14Z",
    "pushed_at": 1701717193,
    "git_url": "git://github.com/llvm/llvm-project.git",
    "ssh_url": "git@github.com:llvm/llvm-project.git",
    "clone_url": "https://github.com/llvm/llvm-project.git",
    "svn_url": "https://github.com/llvm/llvm-project",
    "homepage": "http://llvm.org",
    "size": 2311069,
    "stargazers_count": 23062,
    "watchers_count": 23062,
    "language": null,
    "has_issues": true,
    "has_projects": false,
    "has_downloads": true,
    "has_wiki": false,
    "has_pages": false,
    "has_discussions": false,
    "forks_count": 9224,
    "mirror_url": null,
    "archived": false,
    "disabled": false,
    "open_issues_count": 21151,
    "license": {
      "key": "other",
      "name": "Other",
      "spdx_id": "NOASSERTION",
      "url": null,
      "node_id": "MDc6TGljZW5zZTA="
    },
    "allow_forking": true,
    "is_template": false,
    "web_commit_signoff_required": false,
    "topics": [

    ],
    "visibility": "public",
    "forks": 9224,
    "open_issues": 21151,
    "watchers": 23062,
    "default_branch": "main",
    "stargazers": 23062,
    "master_branch": "main",
    "organization": "llvm",
    "custom_properties": {

    }
  },
  "pusher": {
    "name": "vitalybuka",
    "email": "vitalybuka@gmail.com"
  },
  "organization": {
    "login": "llvm",
    "id": 17149993,
    "node_id": "MDEyOk9yZ2FuaXphdGlvbjE3MTQ5OTkz",
    "url": "https://api.github.com/orgs/llvm",
    "repos_url": "https://api.github.com/orgs/llvm/repos",
    "events_url": "https://api.github.com/orgs/llvm/events",
    "hooks_url": "https://api.github.com/orgs/llvm/hooks",
    "issues_url": "https://api.github.com/orgs/llvm/issues",
    "members_url": "https://api.github.com/orgs/llvm/members{/member}",
    "public_members_url": "https://api.github.com/orgs/llvm/public_members{/member}",
    "avatar_url": "https://avatars.githubusercontent.com/u/17149993?v=4",
    "description": "This is the LLVM organization on GitHub for the LLVM Project: a collection of modular and reusable compiler and toolchain technologies."
  },
  "sender": {
    "login": "vitalybuka",
    "id": 1531415,
    "node_id": "MDQ6VXNlcjE1MzE0MTU=",
    "avatar_url": "https://avatars.githubusercontent.com/u/1531415?v=4",
    "gravatar_id": "",
    "url": "https://api.github.com/users/vitalybuka",
    "html_url": "https://github.com/vitalybuka",
    "followers_url": "https://api.github.com/users/vitalybuka/followers",
    "following_url": "https://api.github.com/users/vitalybuka/following{/other_user}",
    "gists_url": "https://api.github.com/users/vitalybuka/gists{/gist_id}",
    "starred_url": "https://api.github.com/users/vitalybuka/starred{/owner}{/repo}",
    "subscriptions_url": "https://api.github.com/users/vitalybuka/subscriptions",
    "organizations_url": "https://api.github.com/users/vitalybuka/orgs",
    "repos_url": "https://api.github.com/users/vitalybuka/repos",
    "events_url": "https://api.github.com/users/vitalybuka/events{/privacy}",
    "received_events_url": "https://api.github.com/users/vitalybuka/received_events",
    "type": "User",
    "site_admin": false
  },
  "created": false,
  "deleted": false,
  "forced": false,
  "base_ref": null,
  "compare": "https://github.com/llvm/llvm-project/compare/038d0f7a565d...d9f50eb2f4d0",
  "commits": [
    {
      "id": "5bc391a7c96a58929794ec1dc664b966c0a03d09",
      "tree_id": "2a4de3491a1a0c2dbf9af900725a31cbd0706cbf",
      "distinct": false,
      "message": "[SelectionDAG] Use getVectorElementPointer in DAGCombiner::replaceStoreOfInsertLoad. (#74249)\n\nThis ensures we clip the index to be in bounds of the vector we are\r\ninserting into. If the index is out of bounds the results of the insert\r\nelement is poison. If we don't clip the index we can write memory that\r\nwas not part of the original store.\r\n\r\nFixes #74248.",
      "timestamp": "2023-12-04T11:11:37-08:00",
      "url": "https://github.com/llvm/llvm-project/commit/5bc391a7c96a58929794ec1dc664b966c0a03d09",
      "author": {
        "name": "Craig Topper",
        "email": "craig.topper@sifive.com",
        "username": "topperc"
      },
      "committer": {
        "name": "GitHub",
        "email": "noreply@github.com",
        "username": "web-flow"
      },
      "added": [

      ],
      "removed": [

      ],
      "modified": [
        "llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp",
        "llvm/test/CodeGen/Mips/msa/basic_operations.ll",
        "llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert.ll",
        "llvm/test/CodeGen/X86/pr59980.ll"
      ]
    },
    {
      "id": "d9f50eb2f4d07a61eec2b68518500de59f4f34ec",
      "tree_id": "d08c0caab31ba8789c82bc3b98d60c30dddd254a",
      "distinct": true,
      "message": "[𝘀𝗽𝗿] landed version\n\nCreated using spr 1.3.4",
      "timestamp": "2023-12-04T11:13:10-08:00",
      "url": "https://github.com/llvm/llvm-project/commit/d9f50eb2f4d07a61eec2b68518500de59f4f34ec",
      "author": {
        "name": "Vitaly Buka",
        "email": "vitalybuka@google.com",
        "username": "vitalybuka"
      },
      "committer": {
        "name": "Vitaly Buka",
        "email": "vitalybuka@google.com",
        "username": "vitalybuka"
      },
      "added": [

      ],
      "removed": [

      ],
      "modified": [
        "llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp",
        "llvm/test/CodeGen/Mips/msa/basic_operations.ll",
        "llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert.ll",
        "llvm/test/CodeGen/X86/pr59980.ll"
      ]
    }
  ],
  "head_commit": {
    "id": "d9f50eb2f4d07a61eec2b68518500de59f4f34ec",
    "tree_id": "d08c0caab31ba8789c82bc3b98d60c30dddd254a",
    "distinct": true,
    "message": "[𝘀𝗽𝗿] landed version\n\nCreated using spr 1.3.4",
    "timestamp": "2023-12-04T11:13:10-08:00",
    "url": "https://github.com/llvm/llvm-project/commit/d9f50eb2f4d07a61eec2b68518500de59f4f34ec",
    "author": {
      "name": "Vitaly Buka",
      "email": "vitalybuka@google.com",
      "username": "vitalybuka"
    },
    "committer": {
      "name": "Vitaly Buka",
      "email": "vitalybuka@google.com",
      "username": "vitalybuka"
    },
    "added": [

    ],
    "removed": [

    ],
    "modified": [
      "llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp",
      "llvm/test/CodeGen/Mips/msa/basic_operations.ll",
      "llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert.ll",
      "llvm/test/CodeGen/X86/pr59980.ll"
    ]
  }
}

@jroelofs
Copy link
Contributor Author

jroelofs commented Dec 4, 2023

Thanks for the test case! 9cdc63d seems to work.

@tstellar tstellar merged commit 6c70f06 into llvm:main Dec 5, 2023
@tstellar
Copy link
Contributor

tstellar commented Dec 5, 2023

This has been deployed, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tstellar tstellar tstellar left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Non-main changes are sent to llvm-commits
2 participants
@jroelofs @tstellar