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

[FrameworkBundle] Fix for Controller DEPRECATED when using composer --optimized #30993

Merged
merged 1 commit into from Apr 12, 2019

Conversation

Projects
None yet
7 participants
@aweelex
Copy link
Contributor

aweelex commented Apr 7, 2019

Q A
Branch? 4.2
Bug fix? Yes
New feature? No
BC breaks? No
Deprecations? No
Tests pass? Yes
Fixed tickets ---
License MIT

Using composer --optimize-autoload causes console cache:clear (without warmup) to give DEPRECATED error, that stays in profiler.

I moved @trigger_error from beggining of the file to Controller __consctruct method.

@GuilhemN

This comment has been minimized.

Copy link
Contributor

GuilhemN commented Apr 8, 2019

If this is accepted I think we should rely on the DebugClassLoader (see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Debug/DebugClassLoader.php#L283) instead of the constructor which isn't safe (it may be overriden).

[FrameworkBundle] Fix Controller deprecated when using composer --opt…
…imized

Update Controller.php
Update Controller.php
@aweelex

This comment has been minimized.

Copy link
Contributor Author

aweelex commented Apr 8, 2019

If this is accepted I think we should rely on the DebugClassLoader (see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Debug/DebugClassLoader.php#L283) instead of the constructor which isn't safe (it may be overriden).

I think it's ok to remove @trigger_error at all.

@nicolas-grekas nicolas-grekas added this to the 4.2 milestone Apr 8, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Apr 8, 2019

Why is this triggered? What does trigger it? What is specific to Controller that makes it special vs other classes that have the call?

@aweelex

This comment has been minimized.

Copy link
Contributor Author

aweelex commented Apr 8, 2019

Why is this triggered? What does trigger it? What is specific to Controller that makes it special vs other classes that have the call?

I don't sure why it happens, but you can test it by yourself. If you will use composer install --optimize-autoloader, var/cache/dev/srcApp_KernelDevDebugContainerDeprecations.log will store this DEPRECATION. It's wrong behavior.

And if you using Monolog, it will print Deprecation in profiler, even if Controller is not used.

@nicolas-grekas
Copy link
Member

nicolas-grekas left a comment

I inspected the reason why this is needed: FrameworkExtension calls addAnnotatedClassesToCompile with '**\\Controller\\', which means that when the class map is dumped, all the matching classes are parsed for annotations at cache warm-up. Controller is one of them - and there is no way to exclude it. We could certainly find a way to exclude it, but that'd be a new feature.
The current patch is way more pragmatic for the use case.
👍

@fabpot

fabpot approved these changes Apr 12, 2019

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Apr 12, 2019

Thank you @aweelex.

@fabpot fabpot merged commit 2ae2fd8 into symfony:4.2 Apr 12, 2019

1 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Apr 12, 2019

bug #30993 [FrameworkBundle] Fix for Controller DEPRECATED when using…
… composer --optimized (aweelex)

This PR was merged into the 4.2 branch.

Discussion
----------

[FrameworkBundle] Fix for Controller DEPRECATED  when using composer --optimized

| Q | A |
| --- | --- |
| Branch? | 4.2 |
| Bug fix? | Yes |
| New feature? | No |
| BC breaks? | No |
| Deprecations? | No |
| Tests pass? | Yes |
| Fixed tickets | --- |
| License | MIT |

Using `composer --optimize-autoload` causes `console cache:clear` (without warmup) to give DEPRECATED error, that stays in profiler.

I moved `@trigger_error` from beggining of the file to Controller __consctruct method.

Commits
-------

2ae2fd8 [FrameworkBundle] Fix Controller deprecated when using composer --optimized

@aweelex aweelex deleted the aweelex:4.2 branch Apr 12, 2019

@fabpot fabpot referenced this pull request Apr 16, 2019

Merged

Release v4.2.6 #31125

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.