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.

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

Add an auto-cherry-pick GitHub Action #19021

Merged
merged 6 commits into from
May 17, 2023
Merged

Conversation

thejcannon
Copy link
Member

@thejcannon thejcannon commented May 17, 2023

This adds a new GitHub action that runs after a PR is merged which is labeled "needs-cherrypick" as well as can be run manually.

I've done a decent amount of testing on my fork, but wasn't able to test the actual PR creation (it failed on me not configuring git over there)

@thejcannon thejcannon added the category:internal CI, fixes for not-yet-released features, etc. label May 17, 2023
Copy link
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

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

Woohoo, automation!

build-support/bin/cherry_pick.sh Outdated Show resolved Hide resolved
.github/workflows/auto-cherry-picker.yaml Outdated Show resolved Hide resolved
Comment on lines 43 to 44
git config --local user.email "pantsbuild+github-automation@gmail.com"
git config --local user.name "Worker Pants (Pantsbuild GitHub Automation Bot) "
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is happening in several automations, it might be good to centralise, and I see two options:

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't tried it yet, but I suspect there's just a missing env var to handle it

.github/workflows/auto-cherry-picker.yaml Outdated Show resolved Hide resolved
.github/workflows/auto-cherry-picker.yaml Outdated Show resolved Hide resolved
@thejcannon
Copy link
Member Author

Oh sorry @huonw I just realized this branch wasn't up to date with my latest changes 😬

@@ -25,13 +27,11 @@ if [[ -z $PR_NUM ]]; then
read -r PR_NUM
fi

TARGET_MILESTONE=$2
Copy link
Member Author

Choose a reason for hiding this comment

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

The milestone changes should've been made on the "every milestone" PR, sorry y'all

@@ -51,6 +51,7 @@ COMMIT=$(gh pr view "$PR_NUM" --json mergeCommit --jq '.mergeCommit.oid')
if [[ -z $COMMIT ]]; then
fail "Wasn't able to retrieve merge commit for $PR_NUM."
fi
git fetch https://github.com/pantsbuild/pants "$COMMIT"
Copy link
Member Author

Choose a reason for hiding this comment

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

This means the person running the script doesn't need an up-to-date main, we'll fetch the ref.

Comment on lines +77 to +79
if [[ -n $REMOTE ]]; then
git push -u "$REMOTE" "$BRANCH_NAME"
fi
Copy link
Member Author

Choose a reason for hiding this comment

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

Since the GitHub Action doesn't get to participate in prompting, we need to push the branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume the GitHub cli won't even try to prompt in this case, it'll detect the branch already exists on a remote?

Comment on lines +30 to +31
git config --local user.email "pantsbuild+github-automation@gmail.com"
git config --local user.name "Worker Pants (Pantsbuild GitHub Automation Bot)"
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Sponsor Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Thanks!

Ideally this would be generated by

def generate() -> dict[Path, str]:
"""Generate all YAML configs with repo-relative paths."""
pr_jobs = test_workflow_jobs([PYTHON37_VERSION], cron=False)
pr_jobs.update(**classify_changes())
for key, val in pr_jobs.items():
if key in {"check_labels", "classify_changes"}:
continue
needs = val.get("needs", [])
if isinstance(needs, str):
needs = [needs]
needs.extend(["classify_changes"])
val["needs"] = needs
if_cond = val.get("if")
not_docs_only = "needs.classify_changes.outputs.docs_only != 'true'"
val["if"] = not_docs_only if if_cond is None else f"({if_cond}) && ({not_docs_only})"
pr_jobs.update(merge_ok(sorted(pr_jobs.keys())))
test_workflow_name = "Pull Request CI"
test_yaml = yaml.dump(
{
"name": test_workflow_name,
"concurrency": {
"group": "${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }}",
"cancel-in-progress": True,
},
"on": {"pull_request": {}, "push": {"branches-ignore": ["dependabot/**"]}},
"jobs": pr_jobs,
"env": global_env(),
},
width=120,
Dumper=NoAliasDumper,
)
test_cron_yaml = yaml.dump(
{
"name": "Daily Extended Python Testing",
# 08:45 UTC / 12:45AM PST, 1:45AM PDT: arbitrary time after hours.
"on": {"schedule": [{"cron": "45 8 * * *"}]},
"jobs": test_workflow_jobs([PYTHON38_VERSION, PYTHON39_VERSION], cron=True),
"env": global_env(),
},
Dumper=NoAliasDumper,
)
ignore_advisories = " ".join(
f"--ignore {adv_id}" for adv_id in CARGO_AUDIT_IGNORED_ADVISORY_IDS
)
audit_yaml = yaml.dump(
{
"name": "Cargo Audit",
"on": {
# 08:11 UTC / 12:11AM PST, 1:11AM PDT: arbitrary time after hours.
"schedule": [{"cron": "11 8 * * *"}],
# Allow manually triggering this workflow
"workflow_dispatch": None,
},
"jobs": {
"audit": {
"runs-on": "ubuntu-latest",
"if": IS_PANTS_OWNER,
"steps": [
*checkout(),
{
"name": "Cargo audit (for security vulnerabilities)",
"run": f"./cargo install --version 0.17.5 cargo-audit\n./cargo audit {ignore_advisories}\n",
},
],
}
},
}
)
cc_jobs, cc_inputs = cache_comparison_jobs_and_inputs()
cache_comparison_yaml = yaml.dump(
{
"name": "Cache Comparison",
# Kicked off manually.
"on": {"workflow_dispatch": {"inputs": cc_inputs}},
"jobs": cc_jobs,
},
Dumper=NoAliasDumper,
)
release_jobs, release_inputs = release_jobs_and_inputs()
release_yaml = yaml.dump(
{
"name": "Record Release Commit",
"on": {
"push": {"tags": ["release_*"]},
"workflow_dispatch": {"inputs": release_inputs},
},
"jobs": release_jobs,
},
Dumper=NoAliasDumper,
)
return {
Path(".github/workflows/audit.yaml"): f"{HEADER}\n\n{audit_yaml}",
Path(".github/workflows/cache_comparison.yaml"): f"{HEADER}\n\n{cache_comparison_yaml}",
Path(".github/workflows/test.yaml"): f"{HEADER}\n\n{test_yaml}",
Path(".github/workflows/test-cron.yaml"): f"{HEADER}\n\n{test_cron_yaml}",
Path(".github/workflows/release.yaml"): f"{HEADER}\n\n{release_yaml}",
}
rather than hand written, but fine as a followup.

.github/workflows/auto-cherry-picker.yaml Show resolved Hide resolved
@thejcannon
Copy link
Member Author

What do we gain from generating? Seems like extra steps for no/little benefit?

@stuhood
Copy link
Sponsor Member

stuhood commented May 17, 2023

What do we gain from generating? Seems like extra steps for no/little benefit?

Type checking, avoiding yaml gotchas, consistency, reuse.

@@ -60,7 +61,8 @@ if [[ -z $CATEGORY_LABEL ]]; then
echo "Couldn't detect category label on PR. What's the label? (E.g., category:bugfix)"
read -r CATEGORY_LABEL
fi
REVIEWERS=$(gh pr view "$PR_NUM" --json reviews --jq '.reviews.[].author.login' | sort | uniq)
REVIEWERS=$(gh pr view "$PR_NUM" --json author --jq '.author.login')
Copy link
Member Author

Choose a reason for hiding this comment

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

I just tried, adding a PR author as a reviewer of their own PR is a no-op, so this is safe to do unconditionally.

Copy link
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

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

Nice

@thejcannon thejcannon merged commit 2a976d5 into pantsbuild:main May 17, 2023
17 checks passed
@thejcannon thejcannon deleted the tryaction branch May 17, 2023 20:47
@stuhood stuhood mentioned this pull request May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:internal CI, fixes for not-yet-released features, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants