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

[8.x] Fix require fails if is_file cached by opcache #41614

Merged
merged 1 commit into from Mar 22, 2022
Merged

[8.x] Fix require fails if is_file cached by opcache #41614

merged 1 commit into from Mar 22, 2022

Conversation

Tofandel
Copy link
Contributor

@Tofandel Tofandel commented Mar 22, 2022

Fixes #41613

In the case where opcache.enable_file_override is enabled, the is_file call might return true because it's cached until the end of the script so the require will fail if it was unlinked during the execution which is the case when running the command artisan config:cache
This just changes is_file to file_exists which is the method to use before a require and is not affected by this issue

@driesvints
Copy link
Member

It's best that you explain what you're doing here or your PR will be closed (see the PR template when submitting PR's).

@driesvints driesvints changed the title fix: require fails if is_file cached by opcache [9.x] Fix require fails if is_file cached by opcache Mar 22, 2022
@driesvints
Copy link
Member

Also if this also exists in 8.x it should be sent in to that branch: https://laravel.com/docs/9.x/contributions#which-branch

@Tofandel Tofandel changed the base branch from 9.x to 8.x March 22, 2022 12:15
@Tofandel Tofandel changed the base branch from 8.x to 9.x March 22, 2022 12:15
@Tofandel
Copy link
Contributor Author

It does exist in 8.x so, I'll rebase the PR

@driesvints driesvints changed the title [9.x] Fix require fails if is_file cached by opcache [8.x] Fix require fails if is_file cached by opcache Mar 22, 2022
@driesvints
Copy link
Member

The main issue here probably is that you're enabling opcache on the CLI which I don't see a use case for.

@Tofandel
Copy link
Contributor Author

Tofandel commented Mar 22, 2022

I know but it's not always in our power to change this with some hosts because both the cli and php-fpm must have the same config file which would in turn impact performance on the front

@Tofandel Tofandel changed the base branch from 9.x to 8.x March 22, 2022 12:32
@driesvints
Copy link
Member

Why did you change the branch again? Can you also update the PR title when you do?

@driesvints driesvints changed the title [8.x] Fix require fails if is_file cached by opcache [9.x] Fix require fails if is_file cached by opcache Mar 22, 2022
@Tofandel Tofandel changed the title [9.x] Fix require fails if is_file cached by opcache [8.x] Fix require fails if is_file cached by opcache Mar 22, 2022
@Tofandel
Copy link
Contributor Author

Sure, sorry

It's on 8.x, I messed up the first time

@Tofandel
Copy link
Contributor Author

After some testing, no need for try catch, file_exists fixes the issue entirely

@taylorotwell taylorotwell merged commit d53b1ad into laravel:8.x Mar 22, 2022
QWp6t added a commit to roots/acorn that referenced this pull request Apr 5, 2022
QWp6t added a commit to roots/acorn that referenced this pull request Apr 5, 2022
hackhero920 added a commit to hackhero920/acorn that referenced this pull request Aug 15, 2023
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.

php artisan config:cache error
3 participants