Skip to content

Conversation

arnaud-lb
Copy link
Member

No description provided.

@arnaud-lb arnaud-lb force-pushed the alpine-push branch 2 times, most recently from 2cd8a63 to 36498b1 Compare October 2, 2025 19:01
Comment on lines +38 to +48
runs-on: ubuntu-22.04
container:
image: 'alpine:3.20.1'
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 will modernize the push and nightly Alpine jobs in a separate PR.

The latest Alpine version has Clang 20, so we can have at least one job that runs the tailcall VM.

Copy link
Contributor

Choose a reason for hiding this comment

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

@arnaud-lb
Copy link
Member Author

@TimWolla I think this needs to target 8.1, but the job should not run in branches lower than 8.4 as the test suite will not pass. What would be the best way to skip lower branches?

@TimWolla
Copy link
Member

TimWolla commented Oct 2, 2025

I think this needs to target 8.1

Why do you think so? Just target 8.4 then. Some differences in CI between the branches is normal and expected.

@iluuu1994
Copy link
Member

We only keep the root.yml and nightly.yml files synced between branches. push.yml may freely diverge.

I'm not sure if this build is really necessary though? I haven't seen many failures for alpine in nightly that were missed when merging. My thinking is, it's not worth running dozens of jobs daily to prevent a merge that needs correction every few weeks/months. Push will never be able to find all issues.

But I'll leave that at your discretion.

@arnaud-lb
Copy link
Member Author

We only keep the root.yml and nightly.yml files synced between branches. push.yml may freely diverge.

Ah thank you, I had understood that all workflows were maintained in 8.1.

I'm not sure if this build is really necessary though?

@bukka anticipated this would be useful while he works on streams. I will leave the PR open for now, @bukka feel free to merge it.

@bukka
Copy link
Member

bukka commented Oct 5, 2025

Yeah I might be hitting this more for low level networking stuff so it's useful for me.

@bukka bukka changed the base branch from master to PHP-8.4 October 5, 2025 21:19
@bukka bukka changed the base branch from PHP-8.4 to master October 5, 2025 21:19
@bukka bukka changed the base branch from master to PHP-8.4 October 5, 2025 21:25
@bukka bukka marked this pull request as ready for review October 5, 2025 21:25
@bukka bukka requested a review from TimWolla as a code owner October 5, 2025 21:25
@iluuu1994
Copy link
Member

The alternative is to run it conditionally, e.g. when adding some keyword to the commit message. This would be useful for other jobs as well, though in that case it might make sense to try to combine nightly with push somehow.

@bukka
Copy link
Member

bukka commented Oct 6, 2025

Maybe but I don't have any time for it and would like to have it in pipeline tested. Fixing nightly failures is slower and also having UBSAN is good. Btw this job caught two issues in my single PR fixed in e68396c and a3c14d6 which took extra of my time to get fixed.

Triggering extra job (or adding some special commit message or even PR message) for each of my PR would be quite annoying as I would need to do it constantly. But I agree that it might be useful for other jobs that are not that useful.

@bukka bukka merged commit d34278a into php:PHP-8.4 Oct 6, 2025
10 checks passed
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.

5 participants