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

Run CI for branch builds with GitHub Actions #11747

Merged
merged 4 commits into from
Mar 19, 2021

Conversation

Eric-Arellano
Copy link
Contributor

We want to run on branch builds to ensure there were no regressions after merging PRs, such as due to racy merges.

@Eric-Arellano Eric-Arellano changed the title Run CI for branch builds with GitHub Actions WIP: Run CI for branch builds with GitHub Actions Mar 19, 2021
[ci skip-rust]
[ci skip-build-wheels]
[ci skip-rust]
[ci skip-build-wheels]
[ci skip-rust]

[ci skip-build-wheels]
@@ -159,12 +182,12 @@ def expose_all_pythons() -> Sequence[Step]:
]


def test_workflow_jobs(python_versions: Sequence[str]) -> Jobs:
def test_workflow_jobs(primary_python_version: str) -> Jobs:
Copy link
Member

Choose a reason for hiding this comment

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

Nit: this was a list because we expect to add 3.9 to cron sometime soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack, although I'm thinking we can add that back once it's ready. There are some unknowns still like that native_engine.so is different across Py38 and Py39, so we need to cache it twice. It's not as simple as setting python_versions to both Py38 and Py39 to run them both in cron.

For now, this makes the Build Wheels code a bit more clear because there, we will always activate 3.7, 3.8, and 3.9 at the same time.

Copy link
Member

Choose a reason for hiding this comment

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

Ack, although I'm thinking we can add that back once it's ready. There are some unknowns still like that native_engine.so is different across Py38 and Py39, so we need to cache it twice. It's not as simple as setting python_versions to both Py38 and Py39 to run them both in cron.

It's a matrix job, so it might be that simple...? 🤞

@Eric-Arellano Eric-Arellano changed the title WIP: Run CI for branch builds with GitHub Actions Run CI for branch builds with GitHub Actions Mar 19, 2021
@Eric-Arellano
Copy link
Contributor Author

Thanks @benjyw and @stuhood for the great infrastructure! This is much better to work with than Travis :)

@Eric-Arellano Eric-Arellano merged commit 0601b23 into pantsbuild:main Mar 19, 2021
@Eric-Arellano Eric-Arellano deleted the branch-builds branch March 19, 2021 22:49
@Eric-Arellano
Copy link
Contributor Author

Interesting. The cancel workflow ends up canceling branch builds if a newer PR is merged while still running. I don't think we want that.

We could fix this according to https://docs.github.com/en/actions/reference/events-that-trigger-workflows#workflow_run by adding a second predicate that the branch cannot == main or 2.*.x. That means that we will still cancel for custom branches like my-branch, which I think is fine? We only care that the official release branches + main build are never canceled.

@benjyw
Copy link
Contributor

benjyw commented Mar 19, 2021

Yeah that sounds right.

Eric-Arellano added a commit that referenced this pull request Mar 20, 2021
Fixes up #11747 so that we always build for `main` and release branches, even if a newer commit lands during a CI run.
@benjyw benjyw mentioned this pull request Mar 20, 2021
@benjyw
Copy link
Contributor

benjyw commented Mar 24, 2021

Looks like this runs the "Daily Extended Python Testing" on the main branches of forks, which we probably don't want or need?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants