-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] Use relative paths in templates paths cache #19687
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
[FrameworkBundle] Use relative paths in templates paths cache #19687
Conversation
@@ -23,20 +23,25 @@ class TemplateLocator implements FileLocatorInterface | |||
{ | |||
protected $locator; | |||
protected $cache; | |||
protected $rootDir; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be private I think.
6aec93e
to
2bb53d3
Compare
Travis failure is not related (PHP 7.1 segfault). |
$path = $this->locator->locate($template->getPath(), $currentPath); | ||
|
||
if ($this->rootDir && 0 === strpos($path, $this->rootDir)) { | ||
return $this->rootDir.($this->cache[$key] = './'.substr($path, strlen($this->rootDir))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip: As the length of the rootDir doesn't change at runtime you can cache it as a property or static local variable.
5276db5
to
4f601e8
Compare
LGTM. |
4f601e8
to
6f6139c
Compare
👍 |
@@ -32,7 +32,7 @@ class TemplateLocator implements FileLocatorInterface | |||
*/ | |||
public function __construct(FileLocatorInterface $locator, $cacheDir = null) | |||
{ | |||
if (null !== $cacheDir && is_file($cache = $cacheDir.'/templates.php')) { | |||
if (null !== $cacheDir && file_exists($cache = $cacheDir.'/templates.php')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file_exists is faster and is_file have no advantage over it here.
👍 |
Thank you @tgalopin. |
…s cache (tgalopin) This PR was merged into the 3.2-dev branch. Discussion ---------- [FrameworkBundle] Use relative paths in templates paths cache | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #3079 | License | MIT | Doc PR | - This implements the usage of relative paths instead of absolute ones in `var/cache/*/templates.php`, important for ability to build the cache in a different context than where it will be used. This PR transforms the following `templates.php`: ``` php <?php return array ( ':default:index.html.twig' => '/home/tgalopin/www/symfony-standard/app/Resources/views/default/index.html.twig', '::base.html.twig' => '/home/tgalopin/www/symfony-standard/app/Resources/views/base.html.twig', ); ``` Into: ``` php <?php return array ( ':default:index.html.twig' => __DIR__.'/../../../app/Resources/views/default/index.html.twig', '::base.html.twig' => __DIR__.'/../../../app/Resources/views/base.html.twig', ); ``` I also added tests for the TemplateCachePathsWarmer and improved tests for the TemplateLocator. Commits ------- 6f6139c [FrameworkBundle] Use relative paths in templates paths cache
This implements the usage of relative paths instead of absolute ones in
var/cache/*/templates.php
, important for ability to build the cache in a different context than where it will be used.This PR transforms the following
templates.php
:Into:
I also added tests for the TemplateCachePathsWarmer and improved tests for the TemplateLocator.