Kernel::getRootDir() is misleading. It returns the path whereAppKernel.php is stored, which isapp/ in Symfony 2/3, butsrc/ in Symfony 4. But most of the time, we are usinggetRootDir() to get the project root dir, so%kernel.root_dir%/../ is a common idiom.

Changing the value ofgetRootDir() would be a hard BC break, so I propose to create a new method,getProjectDir() to "replace"getRootDir() gradually.

added Kernel::getProjectDir()

1f680cc

carsonbot added Status: Needs Review Bug Feature labels

Apr 6, 2017

fabpot mentioned this pull request

Apr 6, 2017

[Question] Where should the old app/Resources/ contents live now?symfony/flex#40

Closed

Copy link

Contributor

Taluu commentedApr 6, 2017•
edited
Loading

~~wouldn't it make sense in 4.0 to useroot_dir instead ?~~ didn't see the part on the BC, so nvm

And it's a bit related (but not so much too...), but having the path to the console binary (if there's one) could be useful too

Copy link

Contributor

linaori commentedApr 6, 2017

Finally! \o/

ro0NL reviewed

Apr 6, 2017

View reviewed changes

src/Symfony/Component/HttpKernel/CHANGELOG.md

		-----

		* Added`kernel.project_dir` and`Kernel::getProjectDir()`
		* Deprecated`kernel.root_dir` and`Kernel::getRootDir()`

Copy link

Contributor

ro0NLApr 6, 2017

Choose a reason for hiding this comment

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

Not happening right?

Copy link

MemberAuthor

fabpotApr 7, 2017

Choose a reason for hiding this comment

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

Virtually at least :) We cannot really display a deprecation as it's used everywhere. But, I want to get rid of it in Symfony Flex, so that in 4, we can really deprecateroot_dir.

Copy link

Contributor

thewilkybarkidApr 7, 2017

Choose a reason for hiding this comment

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

So it should have a@deprecated but not atrigger_error()?

Copy link

Contributor

ro0NLApr 7, 2017

Choose a reason for hiding this comment

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

well.. deprecation !== crashing :) then again migrating it is a burden.. so i see we care a bit more here.

Adding@deprecated sounds like a good idea 👍

nicolas-grekas added this to the3.3 milestone

Apr 7, 2017

nicolas-grekas approved these changes

Apr 7, 2017

View reviewed changes

Copy link

Member

nicolas-grekas left a comment

Choose a reason for hiding this comment

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

👍

fabpot merged commit1f680cc intosymfony:master

Apr 7, 2017

fabpot added a commit that referenced this pull request

Apr 7, 2017

feature#22315Add Kernel::getProjectDir() (fabpot)

7b8409a

This PR was merged into the 3.3-dev branch.Discussion----------Add Kernel::getProjectDir()| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | yes/no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | n/a| License       | MIT| Doc PR        | not yet`Kernel::getRootDir()` is misleading. It returns the path where `AppKernel.php` is stored, which is `app/` in Symfony 2/3, but `src/` in Symfony 4. But most of the time, we are using `getRootDir()` to get the project root dir, so `%kernel.root_dir%/../` is a common idiom.Changing the value of `getRootDir()` would be a hard BC break, so I propose to create a new method, `getProjectDir()` to "replace" `getRootDir()` gradually.Commits-------1f680cc added Kernel::getProjectDir()

fabpot deleted the real-root-dir branch

April 7, 2017 17:17

Copy link

Contributor

curry684 commentedApr 7, 2017•
edited
Loading

Why not introduce a getKernelDir method as well? It would allow deprecating getRootDir via phpDoc today, and for real in 4.0. PhpDoc deprecations are only respected by IDEs and code checkers so that should ease transition.

ghost pushed a commit to lazerball/HitTracker that referenced this pull request

Apr 7, 2017

add new kernel getProjectDir() and project_dir parameter

2d6f3aa

This new parameter and method will appear in Symfony 3.3 as persymfony/symfony#22315We will remove it then.

weaverryan mentioned this pull request

Apr 7, 2017

New Method: Kernel::getProjectDir()symfony/symfony-docs#7768

Closed

Copy link

Member

weaverryan commentedApr 7, 2017

Docs issue created and PR description updated - let's make sure we keep track of everything :) (docs team has a lot of catching up to do, but I hope we'll at least have a complete issue list to work from!)

xabbuh mentioned this pull request

Apr 20, 2017

[HttpKernel] sync upgrade files#22485

Merged

fabpot added a commit that referenced this pull request

Apr 20, 2017

minor#22485[HttpKernel] sync upgrade files (xabbuh)

59f3f34

This PR was merged into the 3.3-dev branch.Discussion----------[HttpKernel] sync upgrade files| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#22315| License       | MIT| Doc PR        |Commits-------a90e461 sync upgrade files

fabpot mentioned this pull request

May 1, 2017

Release v3.3.0-BETA1#22603

Merged

Tobion reviewed

May 4, 2017

View reviewed changes

src/Symfony/Component/HttpKernel/Kernel.php

		* @return string The project root dir
		*/
		publicfunctiongetProjectDir()
		{

Copy link

Contributor

TobionMay 4, 2017

Choose a reason for hiding this comment

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

This implementation is really strange. It basically assigns the variable twice. Once in the method and once in the constructor.

Copy link

Member

stofMay 4, 2017

Choose a reason for hiding this comment

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

@Tobion the method itself can be overwritten to change the logic (for instance for people wanting to avoid issues with open base dir). But the Kernel still wants to ensure its private property stores the right thing.

And then, the default implementation relies on the property internally to avoid doing the filesystem traversal multiple times

aschempp reviewed

May 8, 2017

View reviewed changes

src/Symfony/Component/HttpKernel/Kernel.php

		protected$startTime;
		protected$loadClassCache;

		private$projectDir;

Copy link

Contributor

aschemppMay 8, 2017

Choose a reason for hiding this comment

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

Is there a reason this isprivate and notprotected like$rootDir. I need to override this in my kernel and can't really do at runtime…

Copy link

Contributor

linaoriMay 8, 2017

Choose a reason for hiding this comment

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

I believe the extension point is the public method, not the property.

Copy link

Contributor

aschemppMay 8, 2017

Choose a reason for hiding this comment

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

Well the issue is that the property is set in__construct and later used bygetKernelParameters. My kernel does not know the path at construct time, so I would need to override multiple methods for no reason…

Copy link

Contributor

leofeyerMay 17, 2017

Choose a reason for hiding this comment

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

@iltar Following your argument,this line should use$this->getProjectDir() then. The current code is inconsistent and I suggest that we changeprivate $projectDir toprotected $projectDir so it matchesprotected $rootDir.

Copy link

Member

stofMay 17, 2017

Choose a reason for hiding this comment

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

Making things protected means we then have to ensure backward compatibility on the access to the property. This is a strong commitment. So we never replace private with protected without a strong use case for it. And if you have one, please open a dedicated issue to discuss it instead of doing it on the merged PR (people are generally looking at opened discussion only when catching up)

Copy link

Contributor

leofeyerMay 17, 2017•
edited
Loading

Choose a reason for hiding this comment

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

See#22727

Copy link

Member

chalasrMay 17, 2017

Choose a reason for hiding this comment

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

See#22728 for alternative

aschempp mentioned this pull request

May 8, 2017

[4.4-beta1] kernel.project_dir parameter points to the manager bundle directorycontao/manager-bundle#28

Closed

xabbuh mentioned this pull request

May 22, 2017

use getProjectDir() when possible#22843

Merged

fabpot added a commit that referenced this pull request

May 23, 2017

minor#22843use getProjectDir() when possible (xabbuh)

222325f

This PR was merged into the 3.3 branch.Discussion----------use getProjectDir() when possible| Q             | A| ------------- | ---| Branch?       | 3.3| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | continues#22315| License       | MIT| Doc PR        |Commits-------a8e298a use getProjectDir() when possible