[HttpKernel] Deprecate usage of getRootDir() and kernel.root_dir #28810

fabpot · 2018-10-10T16:03:58Z

Q	A
Branch?	master
Bug fix?	no
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#24293
License	MIT
Doc PR	n/a

fabpot · 2018-10-10T16:04:22Z

src/Symfony/Bundle/FrameworkBundle/Templating/Helper/CodeHelper.php

     * @param string                   $charset        The charset
     */
-    public function __construct($fileLinkFormat, string $rootDir, string $charset)
+    public function __construct($fileLinkFormat, string $projectDir, string $charset)


We already inject kernel.project_dir in the container definition.

ogizanagi · 2018-10-10T16:25:23Z

You meant kernel.root_dir :)

- Remove internal usage of kernel.project_dir when possible
+ Remove internal usage of kernel.root_dir when possible

fabpot · 2018-10-10T16:31:53Z

indeed, fixed

stof · 2018-10-10T16:40:45Z

src/Symfony/Bundle/FrameworkBundle/Templating/Helper/CodeHelper.php

    protected $rootDir;
+    protected $projectDir;


should the new property be private instead ? I don't think it needs to be an extension point

of course, done

stof · 2018-10-10T16:43:22Z

src/Symfony/Component/HttpKernel/Kernel.php

@@ -572,6 +572,9 @@ protected function getKernelParameters()
        }

        return array(
+            /**
+             * @deprecated since Symfony 3.3


version looks wrong (otherwise, it would already be gone in 4.0).

And for that one, being able to warn when using the parameter looks useful.

version is the right one, it was deprecated in 3.3, but we did not add the deprecation notice and didn't find time/energy/way to make it before 4.0. I can change to 4.2, no big deal.

Moved to 4.2.

fabpot · 2018-10-10T17:30:15Z

Ok, now with the deprecation of the getRootDir() method.

fabpot · 2018-10-10T17:30:42Z

src/Symfony/Component/HttpKernel/Kernel.php

@@ -360,15 +369,15 @@ public function getStartTime()
     */
    public function getCacheDir()
    {
-        return $this->rootDir.'/cache/'.$this->environment;
+        return $this->getProjectDir().'/var/cache/'.$this->environment;


This is the default value in the Symfony Flex recipe.

fabpot · 2018-10-10T17:30:46Z

src/Symfony/Component/HttpKernel/Kernel.php

    }

    /**
     * {@inheritdoc}
     */
    public function getLogDir()
    {
-        return $this->rootDir.'/logs';
+        return $this->getProjectDir().'/var/log';


This is the default value in the Symfony Flex recipe.

src/Symfony/Bundle/FrameworkBundle/Tests/Command/CacheClearCommand/CacheClearCommandTest.php

ro0NL

fixes #24293

src/Symfony/Component/HttpKernel/KernelInterface.php

src/Symfony/Bundle/FrameworkBundle/Command/TranslationUpdateCommand.php

ro0NL

UPGRADE files need to be updated

given the parameters are not deprecated in code (trigger error) i suggest a mention to manually search your code base

for UPGRADE-5.0 these should be listed as BC break (preferably put on top)

src/Symfony/Component/HttpKernel/CHANGELOG.md

ro0NL · 2018-10-16T12:47:22Z

@fabpot can you clarify why deprecating Kernel::getRootDir() but not KernelInterface::getRootDir(), this looks odd.

Do you aim to rename KernelInterface::getRootDir() to getProjectDir() on the interface?

edit: wait.. in 5.0 KernelInterface::getRootDir() will return the project dir at implementation level i guess :) cool 👍

fabpot · 2018-10-16T13:25:56Z

@ro0NL It's more pragmatic than that :) As we are using getRootDir() in our own code (translation commands), having @deprecatedmeans that there is no way to be deprecation-free... unless I'm missing something of course.

chalasr · 2018-10-16T13:32:44Z

IIRC the debug component mutes deprecations from the same vendor so implementing getRootDir() should keep the core deprecation free. @nicolas-grekas am I wrong?

ro0NL · 2018-10-16T13:36:29Z

i was assuming the same yes :)

in 5.0 KernelInterface::getRootDir() will return the project dir at implementation level

should we actually consider this? Does it make sense? We could benefit from it, as we cant add getProjectDir() to the interface. But this means another big step in 5.0 (basically favoring Kernel::getRootDir() again instead of getProjectDir() 😂) Not sure it's worth it actually =/

But then we need to decide what's the purpose of KernelInterface::getRootDir() in 5.0

nicolas-grekas · 2018-10-16T13:44:42Z

IIRC the debug component mutes deprecations from the same vendor

I don't think so - that would prevent detecting when we use our own internal deprecated code actually.

5.0 KernelInterface::getRootDir() will return the project dir

no way, as you spotted :) in fact, getProjectDir() is used externally only once in the core, in AboutCommand. This means there is no need to have it on the interface to me.

we are using getRootDir() in our own code (translation commands),

how will this be implemented in 5.0?
do we want to deprecate using these folders?

fabpot · 2018-10-16T13:50:32Z

The getRootDir usages are for legacy directories that are not relevant in Symfony 4, that we need/can remove in 5.0.

nicolas-grekas · 2018-10-16T13:51:38Z

This means we need to trigger a deprecation whenever these dirs contain anything that we load currently, right?
Anyone willing to give it a try?

fabpot · 2018-10-16T13:54:15Z

It was not possible in 3.4, but we can definitely do it in 4.2.

yceruto · 2018-10-16T13:58:58Z

We talking about these directories %kernel.root_dir%/Resources/views and %kernel.root_dir%/Resources/translations right? I can take TwigBundle by now, maybe translations commands too.

chalasr · 2018-10-16T14:18:15Z

@yceruto I already started the work for translation commands locally :)

…() (nicolas-grekas) This PR was merged into the 4.2-dev branch. Discussion ---------- [HttpKernel] fix deprecating KernelInterface::getRootDir() | Q | A | ------------- | --- | Branch? | 4.2 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Follow up of #28810. Did I get it right? Commits ------- 819f525 [HttpKernel] fix deprecating KernelInterface::getRootDir()

…directories (yceruto) This PR was merged into the 4.2-dev branch. Discussion ---------- [TwigBundle] Deprecating support for legacy templates directories | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | symfony/symfony-docs#10500 go ahead with #28810 (comment) - [x] Fix tests Commits ------- 8b390f3 Deprecating support for legacy templates directories

…lations directory (yceruto) This PR was merged into the 4.2-dev branch. Discussion ---------- [FrameworkBundle] Deprecating support for legacy translations directory | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #28810 (comment) | License | MIT | Doc PR | TODO Commits ------- acdece7 Deprecating support for legacy translations directory

This PR was merged into the 4.2-dev branch. Discussion ---------- [HttpKernel] Fix BC break in Kernel name | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Fix a BC break introduced by #28810 where the name of the kernel where change form the name of the folder containing the kernel class to the name of the folder containing the project. This introduced a bug with deployment processes where the cache warmup is done before moving the application to a folder with a different name and removing the possibility to compile the container (either by moving to a read-only filesystem or by removing the config directory). Commits ------- 872a772 Fix BC break in Kernel name

…sys) This PR was submitted for the master branch but it was merged into the 4.2 branch instead (closes #11299). Discussion ---------- [Flex] Tell that `var/logs/` directory was renamed Ref: symfony/symfony#28810 (comment) Commits ------- 2fcd9bc [Flex] Tell that `var/logs/` directory was renamed

…adzki) This PR was submitted for the 5.0 branch but it was squashed and merged into the 4.3 branch instead (closes #12719). Discussion ---------- [minor] remove usage of Kernel::$rootDir from docs As per symfony/symfony#28810 $rootDir has been removed in 5.0 - use getProjectDir in docs instead - small change to be consistent Commits ------- 9da96e1 [minor] remove usage of Kernel::$rootDir from docs

carsonbot added the Status: Needs Review label Oct 10, 2018

fabpot commented Oct 10, 2018

View reviewed changes

fabpot force-pushed the root_dir-removal branch 2 times, most recently from 799ef32 to 086826e Compare October 10, 2018 16:10

fabpot changed the title ~~Remove internal usage of kernel.project_dir when possible~~ Remove internal usage of kernel.root_dir when possible Oct 10, 2018

fabpot force-pushed the root_dir-removal branch from 086826e to ba17d16 Compare October 10, 2018 16:31

stof reviewed Oct 10, 2018

View reviewed changes

fabpot force-pushed the root_dir-removal branch 4 times, most recently from d8188bc to 78416ad Compare October 10, 2018 17:29

fabpot changed the title ~~Remove internal usage of kernel.root_dir when possible~~ [HttpKernel] Deprecate usage of getRootDir() and kernel.root_dir Oct 10, 2018

fabpot commented Oct 10, 2018

View reviewed changes

fabpot force-pushed the root_dir-removal branch from 78416ad to 5129d63 Compare October 10, 2018 17:32

OskarStark reviewed Oct 10, 2018

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Tests/Command/CacheClearCommand/CacheClearCommandTest.php Outdated Show resolved Hide resolved

carsonbot removed the Status: Needs Review label Oct 10, 2018

OskarStark approved these changes Oct 10, 2018

View reviewed changes

carsonbot added the Status: Reviewed label Oct 10, 2018

ogizanagi added the Deprecation label Oct 10, 2018

ro0NL reviewed Oct 11, 2018

View reviewed changes

src/Symfony/Component/HttpKernel/KernelInterface.php Outdated Show resolved Hide resolved

chalasr reviewed Oct 11, 2018

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Command/TranslationUpdateCommand.php Show resolved Hide resolved

fabpot force-pushed the root_dir-removal branch 2 times, most recently from 11c7519 to efffda4 Compare October 11, 2018 11:46

nicolas-grekas added this to the next milestone Oct 14, 2018

fabpot force-pushed the root_dir-removal branch from efffda4 to fb5688e Compare October 15, 2018 15:52

ro0NL reviewed Oct 15, 2018

View reviewed changes

src/Symfony/Component/HttpKernel/CHANGELOG.md Outdated Show resolved Hide resolved

nicolas-grekas mentioned this pull request Oct 16, 2018

[HttpKernel] fix deprecating KernelInterface::getRootDir() #28890

Merged

yceruto mentioned this pull request Oct 16, 2018

[TwigBundle] Deprecating support for legacy templates directories #28891

Merged

1 task

ro0NL mentioned this pull request Oct 17, 2018

[HttpKernel] Added KernelInterface::getProjectDir() symfony/symfony-docs#10503

Closed

yceruto mentioned this pull request Oct 26, 2018

[FrameworkBundle] Deprecating support for legacy translations directory #28997

Merged

jvasseur mentioned this pull request Oct 30, 2018

[HttpKernel] Fix BC break in Kernel name #29032

Merged

nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018

This was referenced Nov 3, 2018

Release v4.2.0-BETA1 #29072

Closed

Release v4.2.0-BETA1 #29073

Merged

fabpot deleted the root_dir-removal branch January 14, 2019 11:01

phansys mentioned this pull request Apr 5, 2019

[Flex] Tell that var/logs/ directory was renamed symfony/symfony-docs#11299

Closed

jzawadzki mentioned this pull request Nov 27, 2019

[minor] remove usage of Kernel::$rootDir from docs symfony/symfony-docs#12719

Merged

This was referenced Jan 1, 2020

Symfony 5 parameter fix doctrine/DoctrineMigrationsBundle#295

Closed

Symfony 5 parameter doc fix schmittjoh/JMSSerializerBundle#783

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HttpKernel] Deprecate usage of getRootDir() and kernel.root_dir #28810

[HttpKernel] Deprecate usage of getRootDir() and kernel.root_dir #28810

fabpot commented Oct 10, 2018 •

edited

Loading

fabpot Oct 10, 2018

ogizanagi commented Oct 10, 2018

fabpot commented Oct 10, 2018

stof Oct 10, 2018

fabpot Oct 10, 2018

stof Oct 10, 2018

fabpot Oct 10, 2018

fabpot Oct 10, 2018

fabpot commented Oct 10, 2018

fabpot Oct 10, 2018

fabpot Oct 10, 2018

ro0NL left a comment

ro0NL left a comment

ro0NL commented Oct 16, 2018 •

edited

Loading

fabpot commented Oct 16, 2018

chalasr commented Oct 16, 2018

ro0NL commented Oct 16, 2018 •

edited

Loading

nicolas-grekas commented Oct 16, 2018 •

edited

Loading

fabpot commented Oct 16, 2018

nicolas-grekas commented Oct 16, 2018 •

edited

Loading

fabpot commented Oct 16, 2018

yceruto commented Oct 16, 2018

chalasr commented Oct 16, 2018

[HttpKernel] Deprecate usage of getRootDir() and kernel.root_dir #28810

[HttpKernel] Deprecate usage of getRootDir() and kernel.root_dir #28810

Conversation

fabpot commented Oct 10, 2018 • edited Loading

Choose a reason for hiding this comment

ogizanagi commented Oct 10, 2018

fabpot commented Oct 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabpot commented Oct 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL left a comment

Choose a reason for hiding this comment

ro0NL left a comment

Choose a reason for hiding this comment

ro0NL commented Oct 16, 2018 • edited Loading

fabpot commented Oct 16, 2018

chalasr commented Oct 16, 2018

ro0NL commented Oct 16, 2018 • edited Loading

nicolas-grekas commented Oct 16, 2018 • edited Loading

fabpot commented Oct 16, 2018

nicolas-grekas commented Oct 16, 2018 • edited Loading

fabpot commented Oct 16, 2018

yceruto commented Oct 16, 2018

chalasr commented Oct 16, 2018

fabpot commented Oct 10, 2018 •

edited

Loading

ro0NL commented Oct 16, 2018 •

edited

Loading

ro0NL commented Oct 16, 2018 •

edited

Loading

nicolas-grekas commented Oct 16, 2018 •

edited

Loading

nicolas-grekas commented Oct 16, 2018 •

edited

Loading