Skip to content

Do not require opcache.preload_user in cli SAPIs #9515

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 3 commits into from
Oct 7, 2022

Conversation

arnaud-lb
Copy link
Member

This is a followup of #9473: opcache.preload_user does not have to be required when the SAPI executes requests and startup as the same user.

This PR changes opcache.preload_user so that it's not required under cli and phpdbg SAPIs.

@arnaud-lb arnaud-lb changed the title Preload user not required Do not require opcache.preload_user in cli SAPIs Sep 9, 2022
@arnaud-lb arnaud-lb force-pushed the preload_user_not_required branch from a605261 to 127ea5c Compare September 9, 2022 14:27
@arnaud-lb arnaud-lb marked this pull request as ready for review September 23, 2022 11:29
@arnaud-lb
Copy link
Member Author

arnaud-lb commented Sep 23, 2022

This one may be a bit hard to review, sorry about that. The diff may be easier when ignoring whitespaces.

The first commit reorganizes accel_finish_startup() to make it easier to introduce the change.

opcache.enable_cli=1
opcache.optimization_level=-1
opcache.preload={PWD}/preload_user.inc
opcache.preload_user={ENV:TEST_NON_ROOT_USER}
Copy link
Member

Choose a reason for hiding this comment

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

I can't actually see this env variable anywhere, is this actually set?

Copy link
Member Author

@arnaud-lb arnaud-lb Sep 30, 2022

Choose a reason for hiding this comment

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

That's right, I will have to define this variable in builds if this is merged

@arnaud-lb arnaud-lb merged commit 2864282 into php:master Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants