Skip to content

[HttpKernel] Inherit projectDir in the temporary kernel #22889

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

aschempp
Copy link
Contributor

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

Contrary to all other directories defined in the kernel, the getProjectDir method is not copied to the temporary kernel. This can lead to issues if the method is overridden in AppKernel. Unfortunately, the method is not (and obviously cannot be) in the KernelInterface, so method_exists was my only option to see if it can be called.

@jvasseur
Copy link
Contributor

This look like a duplicate of #22872

@aschempp
Copy link
Contributor Author

You're right, though #22872 does not correctly handle the KernelInterface, because the method must not necessarily be there…?

@nicolas-grekas
Copy link
Member

FrameworkBundle 3.3 has a lowest dep on HttpKernel 3.3, which always has the method, so the check is not required.
Closing as duplicate, thanks for the PR anyway.

@aschempp
Copy link
Contributor Author

FrameworkBundle 3.3 has a lowest dep on HttpKernel 3.3, which always has the method, so the check is not required.

Well but my kernel could implement the interface and not extend the Kernel class. I agree that is rather theoretical, but according to the current code the method should expect a Kernel instance and not a KernelInterface.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented May 24, 2017

Hum, you're right. If you care enough about it, I invite you to open a PR against branch 3.3 with the corresponding patch :)

@aschempp
Copy link
Contributor Author

Which one would you prefer, the method check or expect the class?

@nicolas-grekas
Copy link
Member

method check to allow having the feature without extending the Kernel class I think

@aschempp aschempp deleted the cache-clear-projectdir branch May 25, 2017 07:01
@aschempp
Copy link
Contributor Author

see #22921

@stof
Copy link
Member

stof commented May 26, 2017

@nicolas-grekas I also suggest deprecating having a kernel not implementing it, so that we can add the method in the interface in 4.0 (which will be fine as deprecation, as most people should be extending Kernel anyway, or should just be using HttpKernelInterface otherwise)

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.

5 participants