Skip to content

Migrate GH Actions macos-10.05 to macos-11 #9087

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

Merged
merged 1 commit into from
Aug 4, 2022

Conversation

zeriyoshi
Copy link
Contributor

@zeriyoshi zeriyoshi commented Jul 21, 2022

@zeriyoshi zeriyoshi requested review from iluuu1994 and Girgias July 21, 2022 15:04
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.

LGTM if the build passes

@iluuu1994
Copy link
Member

@zeriyoshi I think macOS 11 disallows sudo by default, you might be able to just remove it from make install in push.yml and nightly.yml.

@zeriyoshi
Copy link
Contributor Author

@iluuu1994
Thanks!

I did look at the logs, it appears that many libraries are already installed. Do I remove these from brew.yaml? Or leave it for clarity?

https://github.com/php/php-src/runs/7451974041

@devnexen
Copy link
Member

devnexen commented Jul 21, 2022

I did look at the logs, it appears that many libraries are already installed. Do I remove these from brew.yaml? Or leave it for clarity?

That depends, the system libs are not updated as often as the brew counterparts nor they are necessarily the same e.g.

dcarlier@Davids-MacBook-Pro php-src % openssl version LibreSSL 2.8.3 dcarlier@Davids-MacBook-Pro php-src % /opt/homebrew/opt/openssl/bin/openssl version OpenSSL 3.0.5 5 Jul 2022 (Library: OpenSSL 3.0.5 5 Jul 2022)
I would recommend to keep some

@iluuu1994
Copy link
Member

@zeriyoshi I think my previous assumption was wrong, sudo is still needed. Not sure if the error was related.

@zeriyoshi
Copy link
Contributor Author

@devnexen
OK, I revert cleanup commit. thx!

@iluuu1994
I succeeded by adding make with brew. The install problem is solved, but the test results are puzzling. I will investigate further.

https://github.com/php/php-src/runs/7452896461

@zeriyoshi
Copy link
Contributor Author

zeriyoshi commented Jul 21, 2022

The problem could not be resolved during waking hours.
I will go to bed once. sorry.

For speedy fixes, please disregard this PR and correct the problem if it can be resolved.

@Ayesh
Copy link
Member

Ayesh commented Jul 21, 2022

Linking to the relevant changelog entry: https://github.blog/changelog/2022-07-20-github-actions-the-macos-10-15-actions-runner-image-is-being-deprecated-and-will-be-removed-by-8-30-22/

@cmb69
Copy link
Member

cmb69 commented Jul 27, 2022

Any progress here? It seems that our macOS jobs are being cancelled for a while, so upgrading to macOS 11 appears to be somewhat urgent (also for the PHP-8.0 and PHP-8.1 branches).

@zeriyoshi
Copy link
Contributor Author

@cmb69
I have been on hold because I could not figure out the cause of my current problem.
I was going to try to find the cause on my Intel Mac over the weekend.

CI is cancelled, but works fine except for the GH's takedown schedule.

I think the size of zend_long is wrong, any ideas?

https://github.com/php/php-src/runs/7453701653

@cmb69
Copy link
Member

cmb69 commented Jul 27, 2022

I think the size of zend_long is wrong, any ideas?

Are they possibly running on a different architecture? We might need to cater to that:

php-src/Zend/zend_long.h

Lines 25 to 28 in a398a2f

/* This is the heart of the whole int64 enablement in zval. */
#if defined(__x86_64__) || defined(__LP64__) || defined(_LP64) || defined(_WIN64)
# define ZEND_ENABLE_ZVAL_LONG64 1
#endif

@TimWolla
Copy link
Member

@devnexen perhaps 👆?

@iluuu1994
Copy link
Member

@cmb69

Are they possibly running on a different architecture?

Doesn't sound like this has changed.

https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners

  • Hardware specification for macOS virtual machines:
    • 3-core CPU (x86_64)

@devnexen
Copy link
Member

I can definitely reproduce the error on rosetta mode in my mac M1.

devnexen added a commit to devnexen/php-src that referenced this pull request Jul 27, 2022
Disable long 64 bits support and disable zend_string_equal_val assembly
implementation then as instructions are not longer understood.
@zeriyoshi
Copy link
Contributor Author

@devnexen
Can you tell me the package and toolchain version you put in brew for reference?
I tried to reproduce on macOS 11 on Intel but could not reproduce the problem.

@zeriyoshi
Copy link
Contributor Author

Perhaps it is incorrect to disable ZEND_ENABLE_ZVAL_LONG64. This will cause zend_long to be 32-bit even on 64-bit architectures.

@devnexen
Copy link
Member

devnexen commented Jul 28, 2022

Or maybe it happens only in virtual environment ? after all, rosetta is just x86 64 emulation (it can t handle AVX instructions for example), I no longer have my old intel macos to confirm.

@devnexen
Copy link
Member

@devnexen Can you tell me the package and toolchain version you put in brew for reference? I tried to reproduce on macOS 11 on Intel but could not reproduce the problem.

Same brew packages and I use mostly clang from xcode, however I m running monterey.

@nikic
Copy link
Member

nikic commented Jul 28, 2022

This is likely related to ZEND_DVAL_TO_LVAL_CAST_OK. Can you try force disabling it?

@nikic
Copy link
Member

nikic commented Jul 28, 2022

I see similar failures when doing a release build with clang on Linux.

@zeriyoshi
Copy link
Contributor Author

zeriyoshi commented Aug 1, 2022

@devnexen @nikic
I tried everything on weekend, but could not reproduce it in the end.
I will check about the clang. Thanks.

@zeriyoshi zeriyoshi force-pushed the migrate_gh_actions_macos11 branch from b3b0704 to 2dfb543 Compare August 1, 2022 05:17
@zeriyoshi
Copy link
Contributor Author

zeriyoshi commented Aug 1, 2022

After confirming that it works properly, create a separate PR.

@zeriyoshi
Copy link
Contributor Author

  • clang is not updated with CommandLineTools updates
  • However, it will be updated after Xcode is installed.
  • I did not have Xcode installed on my Intel Mac

The above seems to be the conclusion.

On the other hand, I will create a separate PR as it turns out that the problem requires a change in implementation to be resolved.

@zeriyoshi zeriyoshi force-pushed the migrate_gh_actions_macos11 branch from f3a778a to 8532bad Compare August 4, 2022 15:23
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Great, now it works. Thank you!

I suggest to apply that PHP 8.0, and to merge upwards.

@zeriyoshi
Copy link
Contributor Author

Thank you for suggesting!
I will it after CI succeeded :)

@zeriyoshi zeriyoshi force-pushed the migrate_gh_actions_macos11 branch from 8532bad to 1fb8599 Compare August 4, 2022 17:21
@zeriyoshi zeriyoshi changed the base branch from master to PHP-8.0 August 4, 2022 17:21
@zeriyoshi zeriyoshi merged commit a807092 into php:PHP-8.0 Aug 4, 2022
@zeriyoshi zeriyoshi deleted the migrate_gh_actions_macos11 branch August 4, 2022 17:29
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.

7 participants