Skip to content

Conversation

andypost
Copy link
Contributor

@andypost andypost commented Aug 29, 2025

Proposed changes

The diable_classes is deprecated since PHP 8.5, see php/php-src@f4e2e91

Making this build conditional also require to change tests

Checklist

Before creating a PR, run through this checklist and mark each as complete:

@andypost
Copy link
Contributor Author

also I disabled following tests

		and not test_php_application_disable_classes \
		and not test_php_application_disable_classes_user \

@osokin osokin mentioned this pull request Aug 30, 2025
5 tasks
@ac000
Copy link
Member

ac000 commented Sep 1, 2025

Can we limit the #ifdeferry to the nxt_php_disable() function itself?

@andypost
Copy link
Contributor Author

andypost commented Sep 1, 2025

Yes, it could be cleaner to limit ifdef to the function only but there's no zend structure anymore so yoou need to change calling code anyway.

src/nxt_php_sapi.c:715:33: error: use of undeclared identifier 'zend_disable_class'

Another option could be deprecation of disable_classes in php mod as it will be removed in php 9 anyway so this setting will gone someday...

So ATM it's not clear which way is preferable, moreover disabled_functions also full of ifdeffery(

@ac000
Copy link
Member

ac000 commented Sep 2, 2025

It was just a thought.

I will probably squeeze this into 1.35.0 (which is likely to be the
final release for the foreseeable future...)

@ac000
Copy link
Member

ac000 commented Sep 3, 2025

@andypost

Seeing as I don't have permissions to update this branch, can you rebase
with master and make the commit message this

php: Fix building with 8.5

Link: <https://github.com/php/php-src/commit/f4e2e91d4b6d28448104500819b68edf58bd263c>
Signed-off-by: Andy Postnikov <apostnikov@gmail.com>

Thanks!

@ac000
Copy link
Member

ac000 commented Sep 3, 2025

Actually make the commit message

php: Fix building with 8.5

Closes: https://github.com/nginx/unit/issues/1646
Link: <https://github.com/php/php-src/commit/f4e2e91d4b6d28448104500819b68edf58bd263c>
Signed-off-by: Andy Postnikov <apostnikov@gmail.com>

(Note: no <> on the closes link as GitHub still doesn't recognise that)

@andypost
Copy link
Contributor Author

andypost commented Sep 3, 2025

doing

Closes: nginx#1646
Link: <php/php-src@f4e2e91>
Signed-off-by: Andy Postnikov <apostnikov@gmail.com>
@andypost
Copy link
Contributor Author

andypost commented Sep 3, 2025

ready

EDIT: somehow I can grant permission allowing maintainers to allow edits(

@ac000 ac000 merged commit de14dc7 into nginx:master Sep 3, 2025
25 checks passed
@ac000
Copy link
Member

ac000 commented Sep 3, 2025

Thanks!

@andypost andypost deleted the 1646-php85 branch September 4, 2025 00:09
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.

2 participants