mmoreram / GearmanBundle

GearmanBundle for Symfony2
MIT License
236 stars 104 forks source link

Symfony 4 getRootDir deprecation #192

Closed ro0NL closed 5 years ago

ro0NL commented 5 years ago

I should have reported this in #190 :sweat: , but i thought the deprecation was coming from our own codebase. It does not, see https://github.com/mmoreram/GearmanBundle/search?q=getrootdir&unscoped_q=getrootdir

The "Symfony\Component\HttpKernel\Kernel::getRootDir()" method is deprecated since Symfony 4.2, use getProjectDir() instead.
daum commented 5 years ago

Will try to get to this later this week sorry for the delay.

daum commented 5 years ago

Trying to figure out the best way to do this, as the bundles configuration for the worker resources is based on relative paths to the kernel itself (https://gearmanbundle.readthedocs.io/en/latest/configuration.html). With the deprecation of getRootDir there isn't a great way that I can tell to reliably locate the directory of the kernel. The standard setups would have it in app (Symfony 3) or src (Symfony4) but someone could have a custom setup where this assumption would fall short.

I'm still trying to figure out if there is reliable way to locate the kernel directory. If there it may be better to do a BC upgrade which requires paths to be relative to the project root rather than the kernel directory.

ro0NL commented 5 years ago

using reflection is the alternative approach now, so your change in #193 looks fine.

However what confuses me ... in SF4 the console binary is located at bin/console.. so if projectDir exists, shouldnt it be $projectDir.'/bin/console some:command then?

$commandLine = method_exists('getProjectDir')
  ? projectDir().'/bin/console'
  : getRootDir().'/console';
daum commented 5 years ago

Yes that is actually an existing bug will be creating a separate issue to address that so the describer properly outputs the path for console.

ro0NL commented 5 years ago

for user configuration, i personally would avoid relative directories.. in yaml users can do

resource:
  - '%kernel.project_dir%/src/Path/To/Stuff'

So perhaps that's something to deprecte on your side :)

daum commented 5 years ago

Yep - I agree would be better to have the FQDN

daum commented 5 years ago

All set in v4.1.2 release.