Skip to content

[FrameworkBundle] Require symfony/debug #32270

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

Closed
wants to merge 1 commit into from

Conversation

fancyweb
Copy link
Contributor

@fancyweb fancyweb commented Jun 28, 2019

Q A
Branch? 4.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

In https://github.com/symfony/symfony/pull/32227/files#diff-9abb1cf74d470e961cc3fb6b0dc3b781L23 the symfony/debug dependency was removed from the HttpKernel component because all the features that were used from the Debug component are now in the ErrorCatcher component.

I just updated the dependencies of an existing project to the 4.4.x-dev versions and got a problem : the symfony/debug package was removed, because the only package that was requiring it was symfony/http-kernel.

Since there is a call to Debug::enable() in all front controllers of existing applications, (cf https://github.com/symfony/recipes/blob/fa6571b2a261e137a302c86e9d112a12fa414480/symfony/framework-bundle/4.2/public/index.php), we still need to require it somewhere. I guess we can't expect thousands of projects to add this dependency manually.

Adding it in the FrameworkBundle seems more logical to me than restoring it in the HttpKernel component since the front controller recipe of flex is for this bundle.

@@ -20,6 +20,7 @@
"ext-xml": "*",
"symfony/cache": "^4.3|^5.0",
"symfony/config": "^4.2|^5.0",
"symfony/debug": "^4.4|^5.0",
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be in the require-dev section instead?

Copy link
Member

Choose a reason for hiding this comment

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

That wouldn't fix the issue then as the dev dependencies are only applied if they are part of the root project's composer.json file.

Copy link
Member

Choose a reason for hiding this comment

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

We should also update the FB recipe (public/index.php) and check by class_exists(Debug::class) too? inline with the bin/console recipe.

Copy link
Member

Choose a reason for hiding this comment

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

So this should be part of the https://github.com/symfony/skeleton/blob/master/composer.json instead? in require-dev?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe with #32270 (comment) we solve the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main problem are the existing applications. Especially because you get a fully white page (500 error) since the ErrorCatcher error handler and exception hander were never registered.

I guess Flex can update the front controller index.php file but is it automatic ? Checking if the class exists would make the error disappear but that would change the default current behavior. That would mean that every project would need to require symfony/debug manually. That would be better but I guess that implies a lot of communication.

AFAIK, Flex cannot add a dependency to the composer.json file right?

Copy link
Member

Choose a reason for hiding this comment

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

Right, It wouldn't solve the problem for actual projects updating to 4.4, but new projects. Meanwhile, let's fix it for the new ones.

@Tobion
Copy link
Contributor

Tobion commented Jun 28, 2019

Something similar was declined in #32004
And I agree. Adding it as an dependency makes it impossible to only require it in require-dev for projects. Can't we add the dependency in the skeleton instead? That would be cleaner, esp. since SF 4.4 is going to be LTS.

@fancyweb
Copy link
Contributor Author

fancyweb commented Jul 1, 2019

@nicolas-grekas
Copy link
Member

👎 , that's a hack.

@fancyweb fancyweb closed this Jul 3, 2019
@fancyweb fancyweb deleted the require-debug branch July 3, 2019 16:00
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.

6 participants