10000 Preload broken with cache:warmup command · Issue #36793 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Preload broken with cache:warmup command #36793

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.

Sign up for GitHub

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

Jump to bottom
Closed
ovrflo opened this issue May 12, 2020 · 5 comments · Fixed by #36796
Closed

Preload broken with cache:warmup command #36793

ovrflo opened this issue May 12, 2020 · 5 comments · Fixed by #36796
Labels
Bug Status: Needs Review

Comments

@ovrflo
Copy link
Contributor
ovrflo commented May 12, 2020 •
edited
Loading

Symfony version(s) affected: 5.1.0-BETA1

Description
The generated preload file throws PHP Fatal error: Cannot declare class Proxies\__CG__\App\Entity\SomeEntityName, because the name is already in use. From what I can tell, the doctrine proxies are listed twice in the preload file. I tried debugging it and it looks like there are 2 pieces of code responsible for the bug:

  1. https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/Kernel.php#L576
  2. https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Command/CacheWarmupCommand.php#L83

When running cache:warmup the kernel is building the container and calling the cache warmers and appending to the preload file. After the kernel built the container, the cache:warmer command is also calling the cache:warmers and append to the preload file. This is how doctrine proxies get appended twice.

How to reproduce
It should be reproducible with symfony/demo

rm -rf var/cache/prod
APP_ENV=prod php bin/console cache:warmup

Possible Solution
I'd gladly submit a PR, but I have no idea how to fix this one.

Additional context
I also hit the issue reported in #35759. I had to set container.dumper.inline_class_loader=true in order for my preload file to be generated. I'm running PHP 7.4.5.

Edit
I worked around the issue by skipping cache:warmup and just calling bin/console (this way, the kernel does it's thing, but the cache:warmup command won't append again).

Also, I was apparently wrong about setting container.dumper.inline_class_loader=true. That will generate include calls for hot path services and it will throw errors with preloading because all those files are already loaded. I have set container.dumper.inline_class_loader=false and it fixed the issue. I don't know exactly why it wasn't generating the preload file before setting it to true.
@ovrflo ovrflo added the Bug label May 12, 2020
@nicolas-grekas
Copy link
Member

Thanks for the report, can you provide a small reproducing app please?

@ovrflo
Copy link
Contributor Author
ovrflo commented May 12, 2020

Hi, I just tried to do that and failed miserably :) I created a new symfony/website-skeleton with 5.1.0, added 2 entities and called cache:warmup. It did not reproduce. I'll try debugging it further and get back to you when I have more info (and hopefully something reproducible).

Thanks !

@ovrflo
Copy link
Contributor Author
ovrflo commented May 12, 2020

Back. Managed to reproduce it in https://github.com/ovrflo/symfony-preload-bug
Steps I followed:

  1. symfony new --full --version=stable sf510
  2. change in composer json all versions to 5.1.*
  3. force latest recipes with composer symfony:recipes:install --force -v
  4. add 2 entities (Post, Comment)
  5. rm -rf var/cache/prod && APP_ENV=prod bin/console cache:warmup -v
  6. cat var/cache/prod/App_KernelProdContainer.preload.php

Steps to reproduce:

git clone https://github.com/ovrflo/symfony-preload-bug.git sfbug
cd sfbug
composer install
rm -rf var/cache/prod && APP_ENV=prod bin/console cache:warmup -v
cat var/cache/prod/App_KernelProdContainer.preload.php

Is there anything else I can do to help ? I could help with a PR, but I don't know how to approach the bug. This feels like an issue with the design. Yeah, sure, I might be able to submit a PR with one or two extra checks to prevent this from happening, but that wouldn't be ok in the long run.

Thanks !

@nicolas-grekas
Copy link
Member

Thanks, that makes understanding the issue very easy!

Would you mind sending a PR to replace those require by require_once?
That would save us from preventing duplicates, which might be difficult.

@ovrflo
Copy link
Contributor Author
ovrflo commented May 12, 2020

Sure ! I'll have a PR soon :)

Thanks !

ovrflo added a commit to ovrflo/symfony that referenced this issue May 12, 2020
@ovrflo
bug symfony#36793 [DI][Preload] Use require_once instead of require w…
a53d126 
…hen appending cache warmer-returned files to preload file.
@ovrflo ovrflo mentioned this issue May 12, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Status: Needs Review
Projects
None yet
Milestone
No milestone
3 participants
@nicolas-grekas @ovrflo @carsonbot
0