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

Try to fix macOS CI #16735

Closed
wants to merge 2 commits into from
Closed

Try to fix macOS CI #16735

wants to merge 2 commits into from

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Nov 9, 2024

No description provided.

@nielsdos
Copy link
Member Author

nielsdos commented Nov 9, 2024

Seems to work on first sight, it passes through configure at least.

@nielsdos nielsdos marked this pull request as ready for review November 9, 2024 10:27
@cmb69
Copy link
Member

cmb69 commented Nov 9, 2024

See PR #11056; in the meantime we dropped installing cURL from brew, but still add it to the pkg-config path. Maybe we should use Homebrew's cURL again; that might help to catch some issues with new cURL releases early. cc @iluuu1994

@nielsdos
Copy link
Member Author

nielsdos commented Nov 9, 2024

Ugh FFS, crash in CI is related to a bugfix that was apparently wrong but we don't have ASAN jobs in 8.2 to catch this (cfr #16736)
EDIT: ah it's a wrong merge, anyway, my point still stands
EDIT 2: fixed

@nielsdos
Copy link
Member Author

nielsdos commented Nov 9, 2024

Wait what, how is it failing now? It passed earlier...

@cmb69
Copy link
Member

cmb69 commented Nov 9, 2024

Wait what, how is it failing now? It passed earlier...

Yeah. It might be instable for whatever reasons. I suggest to brew reinstall curl instead of doing brew install openldap.

@cmb69
Copy link
Member

cmb69 commented Nov 9, 2024

macOS CI is green. Maybe run again to be "sure"?

@nielsdos
Copy link
Member Author

nielsdos commented Nov 9, 2024

OK, retriggering both macOS jobs.

Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

I don't understand why GitHub images change installed packages in "minor" versions... LGTM

@iluuu1994
Copy link
Member

in the meantime we dropped installing cURL from brew

FWIU, curl was not actually installed from brew if it was already installed in the base image, which was likely added at some point. Forcing an installation through brew makes sense to me.

@nielsdos nielsdos closed this in 89e750a Nov 9, 2024
@nielsdos
Copy link
Member Author

nielsdos commented Nov 9, 2024

... And it failed on master ffs

@cmb69
Copy link
Member

cmb69 commented Nov 9, 2024

Ah: https://github.com/php/php-src/actions/runs/11757067559/job/32753899572#step:1:8 vs https://github.com/php/php-src/actions/runs/11756657260/job/32753493390#step:1:8. So depends on which version we get.

Anyhow, we may have the PKG_CONFIG_PATH backwards:

export PKG_CONFIG_PATH="$PKG_CONFIG_PATH:$BREW_OPT/[email protected]/lib/pkgconfig"
export PKG_CONFIG_PATH="$PKG_CONFIG_PATH:$BREW_OPT/curl/lib/pkgconfig"
export PKG_CONFIG_PATH="$PKG_CONFIG_PATH:$BREW_OPT/libffi/lib/pkgconfig"
export PKG_CONFIG_PATH="$PKG_CONFIG_PATH:$BREW_OPT/libxml2/lib/pkgconfig"
export PKG_CONFIG_PATH="$PKG_CONFIG_PATH:$BREW_OPT/libxslt/lib/pkgconfig"
export PKG_CONFIG_PATH="$PKG_CONFIG_PATH:$BREW_OPT/zlib/lib/pkgconfig"
export PKG_CONFIG_PATH="$PKG_CONFIG_PATH:$BREW_OPT/icu4c/lib/pkgconfig"

Should probably look first in BREW_OPT/… (aka preprend to PKG_CONFIG_PATH).

@nielsdos
Copy link
Member Author

nielsdos commented Nov 9, 2024

But why does the version even change?

@iluuu1994
Copy link
Member

@nielsdos It's possible that GitHub doesn't roll out fresh images on all runners immediately.

@cmb69
Copy link
Member

cmb69 commented Nov 9, 2024

I'm currently trying #16741.

@cmb69
Copy link
Member

cmb69 commented Nov 9, 2024

That failed. But what is this

export PKG_CONFIG_PATH="$PKG_CONFIG_PATH:$BREW_OPT/[email protected]/lib/pkgconfig"

Latest Homebrew wants OpenSSL 3.4.0 for curl and ldap. Is there any particular reason why we're still using OpenSSL 1.1 for macOS x64? Hmm, apparently we're not using Homebrew's openssl at all. However,

https://github.com/php/php-src/actions/runs/11757637074/job/32755128612#step:5:428

Is that the native macOS OpenSSL? I'm confused. Ah, thats >= 1.1

Well, let's try at #16741.

PS: actions/runner-images#10817
PPS: epic

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