-
Notifications
You must be signed in to change notification settings - Fork 7.9k
CI: gh-actions - add gh at apt-x64 step #15527
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
Conversation
Wow, that was fast! I was just going to reply: I'm not sure how important testing with Caddyserver is for ASan builds (it seems only a couple of tests would be affected). If it's not that important we could just exclude the setup for that job. @iluuu1994, thoughts? Well, if the But I would prefer to have a clear indication if the setup fails. In #13542, @bukka said:
If the failing downloads are about LINUX_X64_DEBUG_ZTS_ASAN builds, then this issue would be solved, and we could remove the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to me, thanks @Ayesh!
If the failing downloads are about LINUX_X64_DEBUG_ZTS_ASAN builds, then this issue would be solved, and we could remove the continue-on-error: true again.
Ack
Downloading Caddy binaries using |
Ah, this is because ASAN is running in a container (introduced in #12034). Given that an Ubuntu 24.04 image is now available it might make sense to switch the ASAN builds to not run the build in a Docker container instead? Especially since Ubuntu 23.04 is already EOL. |
Technically release downloading or the API could be down even when the actions subsystem or the git service is not. But I agree that this is unlikely. I don't particularly care either way, though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Request changes" for visibility of my first comment (and to remove this from my review requests).
Related: phpGH-13296. ASAN builds use a different base image on GitHub Actions, which does not have the `gh` binary installed. This adds `gh` to the `apt install` list, to make sure it's available on all builds.
92e6a73
to
de45269
Compare
@iluuu1994, thoughts on #15527 (comment)? |
I'll look into dropping docker for the asan build. |
I believe this is now obsolete with 91c0679? |
You are right, I think that commit indeed makes this obsolete. GitHub's own images should have Thank you for the reminder, I will close this now. |
Related: GH-13296.
ASAN builds use a different base image on GitHub Actions, which does not have the
gh
binary installed. This addsgh
to theapt install
list to make sure it's available on all builds.