Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Add Kernel::getProjectDir() #22315

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 7, 2017
Merged

Add Kernel::getProjectDir() #22315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions 2 src/Symfony/Component/HttpKernel/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ CHANGELOG
3.3.0
-----

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

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
Member Author

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 deprecate root_dir.

Copy link
Contributor

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 a trigger_error()?

Copy link
Contributor

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 👍

* Deprecated `Kernel::getEnvParameters()`
* Deprecated the special `SYMFONY__` environment variables
* added the possibility to change the query string parameter used by `UriSigner`
Expand Down
26 changes: 26 additions & 0 deletions 26 src/Symfony/Component/HttpKernel/Kernel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ abstract class Kernel implements KernelInterface, TerminableInterface
protected $startTime;
protected $loadClassCache;

private $projectDir;
Copy link
Contributor

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 is private and not protected like $rootDir. I need to override this in my kernel and can't really do at runtime…

Copy link
Contributor

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

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 by getKernelParameters. My kernel does not know the path at construct time, so I would need to override multiple methods for no reason…

Copy link
Contributor

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 change private $projectDir to protected $projectDir so it matches protected $rootDir.

Copy link
Member

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

@leofeyer leofeyer May 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

Choose a reason for hiding this comment

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

See #22728 for alternative


const VERSION = '3.3.0-DEV';
const VERSION_ID = 30300;
const MAJOR_VERSION = 3;
Expand All @@ -80,6 +82,7 @@ public function __construct($environment, $debug)
$this->environment = $environment;
$this->debug = (bool) $debug;
$this->rootDir = $this->getRootDir();
$this->projectDir = $this->getProjectDir();
$this->name = $this->getName();

if ($this->debug) {
Expand Down Expand Up @@ -306,6 +309,28 @@ public function getRootDir()
return $this->rootDir;
}

/**
* Gets the application root dir (path of the project's composer file).
*
* @return string The project root dir
*/
public function getProjectDir()
{
if (null === $this->projectDir) {
Copy link
Contributor

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

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

$r = new \ReflectionObject($this);
$dir = $rootDir = dirname($r->getFileName());
while (!file_exists($dir.'/composer.json')) {
if ($dir === dirname($dir)) {
return $this->projectDir = $rootDir;
}
$dir = dirname($dir);
}
$this->projectDir = $dir;
}

return $this->projectDir;
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -553,6 +578,7 @@ protected function getKernelParameters()
return array_merge(
array(
'kernel.root_dir' => realpath($this->rootDir) ?: $this->rootDir,
'kernel.project_dir' => realpath($this->projectDir) ?: $this->projectDir,
'kernel.environment' => $this->environment,
'kernel.debug' => $this->debug,
'kernel.name' => $this->name,
Expand Down
4 changes: 2 additions & 2 deletions 4 src/Symfony/Component/HttpKernel/KernelInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ public function getEnvironment();
public function isDebug();

/**
* Gets the application root dir.
* Gets the application root dir (path of the project's Kernel class).
*
* @return string The application root dir
* @return string The Kernel root dir
*/
public function getRootDir();

Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.