Skip to content

[HttpKernel] Add $kernel->getBuildDir() to separate it from the cache directory #36515

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

Merged
merged 1 commit into from
Aug 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ protected function createContainer(array $data = []): ContainerBuilder
return new ContainerBuilder(new ParameterBag(array_merge([
'kernel.bundles' => ['FrameworkBundle' => 'Symfony\\Bundle\\FrameworkBundle\\FrameworkBundle'],
'kernel.cache_dir' => __DIR__,
'kernel.build_dir' => __DIR__,
'kernel.container_class' => 'kernel',
'kernel.project_dir' => __DIR__,
], $data)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ private function createContainer()
{
$container = new ContainerBuilder(new ParameterBag([
'kernel.cache_dir' => __DIR__,
'kernel.build_dir' => __DIR__,
'kernel.charset' => 'UTF-8',
'kernel.debug' => true,
'kernel.project_dir' => __DIR__,
Expand Down
7 changes: 7 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Command/AboutCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int
/** @var KernelInterface $kernel */
$kernel = $this->getApplication()->getKernel();

if (method_exists($kernel, 'getBuildDir')) {
$buildDir = $kernel->getBuildDir();
} else {
$buildDir = $kernel->getCacheDir();
}

$rows = [
['<info>Symfony</>'],
new TableSeparator(),
Expand All @@ -76,6 +82,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
['Debug', $kernel->isDebug() ? 'true' : 'false'],
['Charset', $kernel->getCharset()],
['Cache directory', self::formatPath($kernel->getCacheDir(), $kernel->getProjectDir()).' (<comment>'.self::formatFileSize($kernel->getCacheDir()).'</>)'],
['Build directory', self::formatPath($buildDir, $kernel->getProjectDir()).' (<comment>'.self::formatFileSize($buildDir).'</>)'],
['Log directory', self::formatPath($kernel->getLogDir(), $kernel->getProjectDir()).' (<comment>'.self::formatFileSize($kernel->getLogDir()).'</>)'],
new TableSeparator(),
['<info>PHP</>'],
Expand Down
28 changes: 24 additions & 4 deletions src/Symfony/Bundle/FrameworkBundle/Command/CacheClearCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,23 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$io = new SymfonyStyle($input, $output);

$kernel = $this->getApplication()->getKernel();
$realBuildDir = $kernel->getContainer()->getParameter('kernel.build_dir');
$realCacheDir = $kernel->getContainer()->getParameter('kernel.cache_dir');
// the old cache dir name must not be longer than the real one to avoid exceeding
// the maximum length of a directory or file path within it (esp. Windows MAX_PATH)
$oldBuildDir = substr($realBuildDir, 0, -1).('~' === substr($realBuildDir, -1) ? '+' : '~');
$oldCacheDir = substr($realCacheDir, 0, -1).('~' === substr($realCacheDir, -1) ? '+' : '~');
$fs->remove($oldCacheDir);
$fs->remove([$oldBuildDir, $oldCacheDir]);

if (!is_writable($realBuildDir)) {
throw new RuntimeException(sprintf('Unable to write in the "%s" directory.', $realBuildDir));
}
if (!is_writable($realCacheDir)) {
throw new RuntimeException(sprintf('Unable to write in the "%s" directory.', $realCacheDir));
}

$io->comment(sprintf('Clearing the cache for the <info>%s</info> environment with debug <info>%s</info>', $kernel->getEnvironment(), var_export($kernel->isDebug(), true)));
$this->cacheClearer->clear($realBuildDir);
Copy link
Contributor

@ogizanagi ogizanagi Aug 20, 2020

Choose a reason for hiding this comment

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

Shouldn't this command avoid removing the build dir by default? What about adding a --build option?
For the ones not overriding KernelInterface::getBuildDir(), this option won't do anything more.
But for the ones overriding, you might want to call this command to clear the read-write cache, but not the read-only build dir, right?

Copy link
Member

Choose a reason for hiding this comment

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

As a first step, we must clear everything like today.

$this->cacheClearer->clear($realCacheDir);

// The current event dispatcher is stale, let's not use it anymore
Expand Down Expand Up @@ -155,17 +161,31 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}
}

if ($oldBuildDir) {
$fs->rename($realBuildDir, $oldBuildDir);
} else {
$fs->remove($realBuildDir);
}
if ($oldCacheDir) {
$fs->rename($realCacheDir, $oldCacheDir);
} else {
$fs->remove($realCacheDir);
}
$fs->rename($warmupDir, $realCacheDir);
// Copy the content of the warmed cache in the build dir
$fs->copy($realCacheDir, $realBuildDir);

if ($output->isVerbose()) {
$io->comment('Removing old cache directory...');
$io->comment('Removing old build and cache directory...');
}

try {
$fs->remove($oldBuildDir);
} catch (IOException $e) {
if ($output->isVerbose()) {
$io->warning($e->getMessage());
}
}
try {
$fs->remove($oldCacheDir);
} catch (IOException $e) {
Expand All @@ -184,7 +204,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
return 0;
}

private function warmup(string $warmupDir, string $realCacheDir, bool $enableOptionalWarmers = true)
private function warmup(string $warmupDir, string $realBuildDir, bool $enableOptionalWarmers = true)
{
// create a temporary kernel
$kernel = $this->getApplication()->getKernel();
Expand All @@ -207,7 +227,7 @@ private function warmup(string $warmupDir, string $realCacheDir, bool $enableOpt

// fix references to cached files with the real cache directory name
$search = [$warmupDir, str_replace('\\', '\\\\', $warmupDir)];
$replace = str_replace('\\', '/', $realCacheDir);
$replace = str_replace('\\', '/', $realBuildDir);
foreach (Finder::create()->files()->in($warmupDir) as $file) {
$content = str_replace($search, $replace, file_get_contents($file), $count);
if ($count) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1561,6 +1561,7 @@ protected function createContainer(array $data = [])
'kernel.bundles' => ['FrameworkBundle' => 'Symfony\\Bundle\\FrameworkBundle\\FrameworkBundle'],
'kernel.bundles_metadata' => ['FrameworkBundle' => ['namespace' => 'Symfony\\Bundle\\FrameworkBundle', 'path' => __DIR__.'/../..']],
'kernel.cache_dir' => __DIR__,
'kernel.build_dir' => __DIR__,
'kernel.project_dir' => __DIR__,
'kernel.debug' => false,
'kernel.environment' => 'test',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ private function createContainer($sessionStorageOptions)
$container = new ContainerBuilder();
$container->setParameter('kernel.bundles_metadata', []);
$container->setParameter('kernel.cache_dir', __DIR__);
$container->setParameter('kernel.build_dir', __DIR__);
$container->setParameter('kernel.charset', 'UTF-8');
$container->setParameter('kernel.container_class', 'cc');
$container->setParameter('kernel.debug', true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ protected function setUp(): void
$this->container->register('twig', 'Twig\Environment')->addArgument(new Reference('twig_loader'))->setPublic(true);
$this->container->setParameter('kernel.bundles', []);
$this->container->setParameter('kernel.cache_dir', __DIR__);
$this->container->setParameter('kernel.build_dir', __DIR__);
$this->container->setParameter('kernel.debug', false);
$this->container->setParameter('kernel.project_dir', __DIR__);
$this->container->setParameter('kernel.charset', 'UTF-8');
Expand Down
30 changes: 20 additions & 10 deletions src/Symfony/Component/HttpKernel/Kernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ public function getContainer()
*/
public function setAnnotatedClassCache(array $annotatedClasses)
{
file_put_contents(($this->warmupDir ?: $this->getCacheDir()).'/annotations.map', sprintf('<?php return %s;', var_export($annotatedClasses, true)));
file_put_contents(($this->warmupDir ?: $this->getBuildDir()).'/annotations.map', sprintf('<?php return %s;', var_export($annotatedClasses, true)));
}

/**
Expand All @@ -333,6 +333,15 @@ public function getCacheDir()
return $this->getProjectDir().'/var/cache/'.$this->environment;
}

/**
* Gets the build directory.
*/
public function getBuildDir(): string
{
// Returns $this->getCacheDir() for backward compatibility
return $this->getCacheDir();
Copy link
Contributor Author

@mnapoli mnapoli Apr 21, 2020

Choose a reason for hiding this comment

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

Here is the core of the PR :)

For backward compatibility, I return the current cache directory.

In future versions, this could be for example var/build. In the meantime, users can override this method to set it to a different path, for example /tmp.

}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -419,14 +428,14 @@ protected function getContainerBaseClass()
/**
* Initializes the service container.
*
* The cached version of the service container is used when fresh, otherwise the
* The built version of the service container is used when fresh, otherwise the
* container is built.
*/
protected function initializeContainer()
{
$class = $this->getContainerClass();
$cacheDir = $this->warmupDir ?: $this->getCacheDir();
$cache = new ConfigCache($cacheDir.'/'.$class.'.php', $this->debug);
$buildDir = $this->warmupDir ?: $this->getBuildDir();
$cache = new ConfigCache($buildDir.'/'.$class.'.php', $this->debug);
$cachePath = $cache->getPath();

// Silence E_WARNING to ignore "include" failures - don't use "@" to prevent silencing fatal errors
Expand All @@ -448,7 +457,7 @@ protected function initializeContainer()
$oldContainer = \is_object($this->container) ? new \ReflectionClass($this->container) : $this->container = null;

try {
is_dir($cacheDir) ?: mkdir($cacheDir, 0777, true);
is_dir($buildDir) ?: mkdir($buildDir, 0777, true);

if ($lock = fopen($cachePath.'.lock', 'w')) {
flock($lock, LOCK_EX | LOCK_NB, $wouldBlock);
Expand Down Expand Up @@ -533,8 +542,8 @@ protected function initializeContainer()
if ($collectDeprecations) {
restore_error_handler();

file_put_contents($cacheDir.'/'.$class.'Deprecations.log', serialize(array_values($collectedLogs)));
file_put_contents($cacheDir.'/'.$class.'Compiler.log', null !== $container ? implode("\n", $container->getCompiler()->getLog()) : '');
file_put_contents($buildDir.'/'.$class.'Deprecations.log', serialize(array_values($collectedLogs)));
file_put_contents($buildDir.'/'.$class.'Compiler.log', null !== $container ? implode("\n", $container->getCompiler()->getLog()) : '');
}
}

Expand Down Expand Up @@ -570,7 +579,7 @@ protected function initializeContainer()
$preload = array_merge($preload, (array) $this->container->get('cache_warmer')->warmUp($this->container->getParameter('kernel.cache_dir')));
}

if ($preload && method_exists(Preloader::class, 'append') && file_exists($preloadFile = $cacheDir.'/'.$class.'.preload.php')) {
if ($preload && method_exists(Preloader::class, 'append') && file_exists($preloadFile = $buildDir.'/'.$class.'.preload.php')) {
Preloader::append($preloadFile, $preload);
}
}
Expand All @@ -597,7 +606,8 @@ protected function getKernelParameters()
'kernel.project_dir' => realpath($this->getProjectDir()) ?: $this->getProjectDir(),
'kernel.environment' => $this->environment,
'kernel.debug' => $this->debug,
'kernel.cache_dir' => realpath($cacheDir = $this->warmupDir ?: $this->getCacheDir()) ?: $cacheDir,
'kernel.build_dir' => realpath($buildDir = $this->warmupDir ?: $this->getBuildDir()) ?: $buildDir,
'kernel.cache_dir' => realpath($this->getCacheDir()) ?: $this->getCacheDir(),
'kernel.logs_dir' => realpath($this->getLogDir()) ?: $this->getLogDir(),
'kernel.bundles' => $bundles,
'kernel.bundles_metadata' => $bundlesMetadata,
Expand All @@ -615,7 +625,7 @@ protected function getKernelParameters()
*/
protected function buildContainer()
{
foreach (['cache' => $this->warmupDir ?: $this->getCacheDir(), 'logs' => $this->getLogDir()] as $name => $dir) {
foreach (['cache' => $this->getCacheDir(), 'build' => $this->warmupDir ?: $this->getBuildDir(), 'logs' => $this->getLogDir()] as $name => $dir) {
if (!is_dir($dir)) {
if (false === @mkdir($dir, 0777, true) && !is_dir($dir)) {
throw new \RuntimeException(sprintf('Unable to create the "%s" directory (%s).', $name, $dir));
Expand Down
8 changes: 8 additions & 0 deletions src/Symfony/Component/HttpKernel/KernelInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
*
* It manages an environment made of application kernel and bundles.
*
* @method string getBuildDir() Returns the build directory - not implementing it is deprecated since Symfony 5.2.
* This directory should be used to store build artifacts, and can be read-only at runtime.
* Caches written at runtime should be stored in the "cache directory" ({@see KernelInterface::getCacheDir()}).
*
* @author Fabien Potencier <fabien@symfony.com>
*/
interface KernelInterface extends HttpKernelInterface
Expand Down Expand Up @@ -121,6 +125,10 @@ public function getStartTime();
/**
* Gets the cache directory.
*
* Since Symfony 5.2, the cache directory should be used for caches that are written at runtime.
* For caches and artifacts that can be warmed at compile-time and deployed as read-only,
* use the new "build directory" returned by the {@see getBuildDir()} method.
*
* @return string The cache directory
*/
public function getCacheDir();
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/HttpKernel/RebootableInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ interface RebootableInterface
/**
* Reboots a kernel.
*
* The getCacheDir() method of a rebootable kernel should not be called
* while building the container. Use the %kernel.cache_dir% parameter instead.
* The getBuildDir() method of a rebootable kernel should not be called
* while building the container. Use the %kernel.build_dir% parameter instead.
*
* @param string|null $warmupDir pass null to reboot in the regular cache directory
* @param string|null $warmupDir pass null to reboot in the regular build directory
*/
public function reboot(?string $warmupDir);
}