Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix zend_fibers.c build with ZEND_FIBER_UCONTEXT #7773

Closed
wants to merge 1 commit into from

Conversation

psumbera
Copy link
Contributor

Avoids (Solaris SPARC) issue:
zend_fibers.c:77:9: error: unknown type name 'ucontext_t'

Avoids (Solaris SPARC) issue:
  zend_fibers.c:77:9: error: unknown type name 'ucontext_t'
@cmb69
Copy link
Member

cmb69 commented Dec 14, 2021

Thank you for the PR! This looks right, but should likely target PHP-8.1.

@cmb69 cmb69 requested a review from trowski December 14, 2021 11:09
@psumbera
Copy link
Contributor Author

Thank you for the PR! This looks right, but should likely target PHP-8.1.

I don't know the process. I was expecting it should go to 'master' first. Should I create new pull request against 'PHP-8.1' branch?

@cmb69
Copy link
Member

cmb69 commented Dec 14, 2021

Should I create new pull request against 'PHP-8.1' branch?

No, that's not necessary. The "merger" can apply the patch to PHP-8.1, and merge into master from there.

However, generally, a PR is supposed to target the lowest branch to which it is applicable; see also https://github.com/php/php-src/blob/master/CONTRIBUTING.md#pull-requests.

@trowski trowski changed the base branch from master to PHP-8.1 December 14, 2021 16:38
@trowski trowski changed the base branch from PHP-8.1 to master December 14, 2021 16:38
@@ -32,6 +32,10 @@
# include <valgrind/valgrind.h>
#endif

#ifdef ZEND_FIBER_UCONTEXT
# include <ucontext.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be moved below to line 76

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it's not possible. Line 76 is in struct { }.

@cmb69 cmb69 closed this in 069bbf3 Dec 20, 2021
@cmb69
Copy link
Member

cmb69 commented Dec 20, 2021

Thanks again!

@andypost
Copy link
Contributor

andypost commented Jan 4, 2022

@trowski please close https://bugs.php.net/bug.php?id=81689 as this PR fixed it

@trowski
Copy link
Member

trowski commented Jan 4, 2022

@andypost Oh, interesting, I thought ppc64le would have used asm and not ucontext. Further, strange that you weren't seeing a similar error to the OP. Well, glad it's fixed.

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

Successfully merging this pull request may close these issues.

4 participants