Skip to content

[FrameworkBundle] generate preload.php in src/ to make opcache.preload predictable #38063

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
Sep 7, 2020

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Sep 4, 2020

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

As seen in symfonycorp/cloud-templates#15, having the path of the preload file vary by env+debug makes configuring PHP.ini settings impossible.

This PR dump a new preload.php file in src/ when cache:clear is called.
This makes the path predictable.

This is submitted as a bugfix because the current behavior is barely usable without this change.

@nicolas-grekas
Copy link
Member Author

Recipe update at symfony/recipes#817

@nicolas-grekas
Copy link
Member Author

Doc update: symfony/symfony-docs#14176

Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

Thank you Nicolas 🥰😍

@nicolas-grekas nicolas-grekas changed the title [FrameworkBundle] generate .preload.php in project_dir to make opcache.preload predictable [FrameworkBundle] generate preload.php in src/ to make opcache.preload predictable Sep 7, 2020
@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Sep 7, 2020

Updated to src/preload.php

@fabpot
Copy link
Member

fabpot commented Sep 7, 2020

Thank you @nicolas-grekas.

@fabpot fabpot merged commit d441d86 into symfony:4.4 Sep 7, 2020
@nicolas-grekas nicolas-grekas deleted the preload-project-dir branch September 7, 2020 07:19
xabbuh added a commit to symfony/symfony-docs that referenced this pull request Sep 7, 2020
This PR was merged into the 4.4 branch.

Discussion
----------

Fix path to preload file

According to symfony/symfony#38063

Commits
-------

b830395 Fix path to preload file
@deflock
Copy link

deflock commented Sep 7, 2020

@nicolas-grekas sorry, I'm not sure, but will this work on environments with read-only src/?

This was referenced Sep 27, 2020
@alexander-schranz
Copy link
Contributor

alexander-schranz commented Sep 28, 2020

@nicolas-grekas any way to disable this behaviour? We currently have some issues with the preloading in sulu and this file should not be generated in our case.

Also I personally think cache:clear should nothing write in the src as its a generated cache file so I think a better place in my opinion would be var folder e.g.: var/.preload.php, maybe we can make a container parameter to make it configurable? Alternative to var I would think that config/.preload.php would maybe be a better place because I think its more similar to the config/bundles.php.

If the file stays at src/ or any folder in the git versioning, I think it would be better to create this file over a symfony/flex recipe so I can edit it the way I want it instead of creating it on every cache:clear.

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Sep 28, 2020

var/ and config/ don't exist from the pov of the code. The code knows only about a cache-dir, a project-dir and the directory where the Kernel is found (by reflection).
Putting it anywhere else would need extra knowledge that is not present today.

But why is generating this file an issue? You didn't explain it. If preloading doesn't work with sulu, just don't use it?

I think it would be better to create this file over a symfony/flex recipe so I can edit it the way I want it instead of creating it on every cache:clear.

That'd mean hardcoding the env in the path (since the path contains "prod"). Doable, but this would need a reason (the current way has the benefit of being unconditional, thus quite easy to configure by default by hosters).

@nicolas-grekas
Copy link
Member Author

Please note that if you think this should be discussed seriously, a closed PR is not the place as it will be forgotten.

@alexander-schranz
Copy link
Contributor

@nicolas-grekas oh sorry you are correct an own issue would be better created one here: #38334. Mostly the problem is on our side that its misleading devs using sulu that preloading is now possible over this file generated by symfony but its currently not in sulu so we would like to avoid generating of it until preloading is supported.

nicolas-grekas added a commit to nicolas-grekas/symfony that referenced this pull request Sep 29, 2020
…rc/ to make opcache.preload predictable (nicolas-grekas)"

This reverts commit d441d86, reversing
changes made to 043e7c3.
fabpot added a commit that referenced this pull request Sep 30, 2020
…in src/ to make opcache.preload predictable" (nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

Revert "bug #38063 [FrameworkBundle] generate preload.php in src/ to make opcache.preload predictable"

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #38334
| License       | MIT
| Doc PR        | -

As discussed in the linked issue, let's replace this with a recipe:
symfony/recipes#825

TL;DR, these PRs replace `src/.preload.php` (generated) by `config/preload.php` (committed).

Commits
-------

662fcff Revert "bug #38063 [FrameworkBundle] generate preload.php in src/ to make opcache.preload predictable (nicolas-grekas)"
fabpot added a commit that referenced this pull request Sep 30, 2020
* 4.4:
  Revert "bug #38063 [FrameworkBundle] generate preload.php in src/ to make opcache.preload predictable (nicolas-grekas)"
  [PhpUnitBridge] Fix class_alias() for PHPUnit\Framework\Error\Error
fabpot added a commit that referenced this pull request Sep 30, 2020
* 5.1:
  [FrameworkBundle] Add Mailjet definition
  Revert "bug #38063 [FrameworkBundle] generate preload.php in src/ to make opcache.preload predictable (nicolas-grekas)"
  [PhpUnitBridge] Fix class_alias() for PHPUnit\Framework\Error\Error
This was referenced Oct 4, 2020
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.

8 participants