[workflows] Delete user branches that are too long #82845
Conversation
Long branch names can cause problems with git checkouts on Windows See https://discourse.llvm.org/t/user-created-branches-in-the-monorepo-are-often-misused/75544/34
|
@llvm/pr-subscribers-github-workflow Author: Tom Stellard (tstellar) ChangesLong branch names can cause problems with git checkouts on Windows See https://discourse.llvm.org/t/user-created-branches-in-the-monorepo-are-often-misused/75544/34 Full diff: https://github.com/llvm/llvm-project/pull/82845.diff 1 Files Affected:
diff --git a/.github/workflows/user-branch-rules.yml b/.github/workflows/user-branch-rules.yml
new file mode 100644
index 00000000000000..ec96d7490cf6aa
--- /dev/null
+++ b/.github/workflows/user-branch-rules.yml
@@ -0,0 +1,33 @@
+name: User Branch Rules
+
+permissions:
+ contents: read
+
+
+on:
+ push:
+ branches:
+ - 'users/**'
+
+
+jobs:
+ user-branch-rules:
+ if: github.repository_owner == 'llvm'
+ permissions:
+ contents: write
+ runs-on: ubuntu-latest
+ steps:
+ - name: Check branch length
+ env:
+ GITHUB_TOKEN: ${{ github.token }}
+ MAX_BRANCH_LENGTH: 200
+
+ run: |
+ if [ `echo "$GITHUB_REF" | wc -c` -gt "$MAX_BRANCH_LENGTH" ]; then
+ curl -s -X DELETE \
+ -H "Accept: application/vnd.github+json" \
+ -H "Authorization: Bearer $GITHUB_TOKEN" \
+ -H "X-GitHub-Api-Version: 2022-11-28" \
+ "https://api.github.com/repos/$GITHUB_REPOSITORY/git/$GITHUB_REF"
+ echo "Branch $GITHUB_REF deleted"
+ fi
|
| - name: Check branch length | ||
| env: | ||
| GITHUB_TOKEN: ${{ github.token }} | ||
| MAX_BRANCH_LENGTH: 200 |
There was a problem hiding this comment.
200 seems too high. Git convention is 80 characters for the subject, users//spr/ is 11 characters, GitHub usernames are limited to 39 characters, that gives 130 characters.
(spr can suffix the commit message with a -1 etc when there are conflicts, but that's rare, and there's enough slack in the above in practice that we can ignore all that, e.g, usernames are likely way less than 39 characters)
There was a problem hiding this comment.
Importantly:
.git/logs/refs/remotes/origin/ is 30 chars, plus NUL, plus <letter>:\ is 34 chars. That leaves just 26 for the path to the checkout if you have a branch that's 200 chars long, but Users\Michael\llvm-project\ exceeds that limit by exactly 1 (as the first name of the right length to come into my head).
There was a problem hiding this comment.
refs/heads/ is also included in the ref name, so that adds another 11 characters. Does spr put the commit subject in the branch name?
There was a problem hiding this comment.
There was a problem hiding this comment.
Can you control the branch name at all or does it always add the commit summary?
There was a problem hiding this comment.
Nope; see the linked issue
There was a problem hiding this comment.
But we already have "The title should be concise. Because all commits are emailed to the list with the first line as the subject, long titles are frowned upon. Short titles also look better in git log." in the developer policy, so this shouldn't be an issue if people actually adhere to our policy... (ha)
There was a problem hiding this comment.
Yeah, the 200 is too long. I was thinking 100, which should be plenty.
There was a problem hiding this comment.
80-100 should be sane upper limit, yes.
There was a problem hiding this comment.
What behavior does this have when someone actually hits it? I assume it runs after the push, so would they just try to go create the PR and have it just not be present? Also if spr is programmatically creating PRs, it may create it before this runs, and I don't think you can delete a branch that is referenced by a PR.
For git push at least, deleting the branch closes the PR. |
Do actions have the ability to make comments on PRs? It would be less user-hostile to have it leave a comment explaining what happened, and why their PR was being closed. |
Yes, we can do that. Can we always guarantee there will be a PR? If not, I was thinking we could create an issue and @mention the user. |
no it's not guaranteed, and probably not if you run the workflow on branch create, then most likely the PR hasn't been opened yet. I think some kind of notification for the user would be good. If we file an issue, that can be pretty spammy for the repo, don't you think? maybe we have a single issue that we @ mention people in the comments? |
|
Is it possible for users to tell spr to create the branch, but not the PR? If it's just a timing issue, we could have the workflow wait for a few minutes until it sees the PR. |
Long branch names can cause problems with git checkouts on Windows See https://discourse.llvm.org/t/user-created-branches-in-the-monorepo-are-often-misused/75544/34