[FrameworkBundle][HttpKernel] Introduce $buildDir argument to WarmableInterface::warmup to warm read-only artefacts in build_dir

[Okhoshi]

Q	A
Branch?	6.4
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #50357
License	MIT
Doc PR	symfony/symfony-docs#...

See #50357 for the details and the reproduction steps. In this PR, I also moved ConfigBuilderCacheWarmer to use the buildDir argument.

[nicolas-grekas]

As discussed on Slack, I'd suggest adding a string $buildDir = null argument to the existing warmUp method (using a commented argument for BC). We'd pass null when only warming up, and the build-dir when building.

[nicolas-grekas]

Here are some comment, and my review as a patch :)

Now that we have this, I'm wondering about each warmers: are they doing something for build-time or compilte-time? what's their purpose, etc; E.g CompiledClassMetadataFactory added in #29117 by @fbourigault is never wired? We should likely investigate this in a follow up PR this and the others too.

diff --git a/src/Symfony/Bridge/Doctrine/CacheWarmer/ProxyCacheWarmer.php b/src/Symfony/Bridge/Doctrine/CacheWarmer/ProxyCacheWarmer.php
index b148b9347b..baedf9c033 100644
--- a/src/Symfony/Bridge/Doctrine/CacheWarmer/ProxyCacheWarmer.php
+++ b/src/Symfony/Bridge/Doctrine/CacheWarmer/ProxyCacheWarmer.php
@@ -40,9 +40,7 @@ class ProxyCacheWarmer implements CacheWarmerInterface
     }
 
     /**
-     * @param string|null $buildDir directory to put generated source code that can be marked as read-only at runtime.
-     *                              The directory is not provided when only cache_dir should be warmed up.
-     * @return string[] A list of files to preload on PHP 7.4+
+     * @param string|null $buildDir
      */
     public function warmUp(string $cacheDir /* , string $buildDir = null */): array
     {
diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php
index 49fd7f035d..98c281276b 100644
--- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php
+++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php
@@ -35,17 +35,16 @@ abstract class AbstractPhpFileCacheWarmer implements CacheWarmerInterface
     }
 
     /**
-     * @param string|null $buildDir directory to put generated source code that can be marked as read-only at runtime.
-     *                              The directory is not provided when only cache_dir should be warmed up.
-     * @return string[] A list of classes to preload on PHP 7.4+
+     * @param string|null $buildDir
      */
     public function warmUp(string $cacheDir /* , string $buildDir = null */): array
     {
+        $buildDir = 1 < \func_num_args() ? func_get_arg(1) : null;
         $arrayAdapter = new ArrayAdapter();
 
         spl_autoload_register([ClassExistenceResource::class, 'throwOnRequiredClass']);
         try {
-            if (!$this->doWarmUp($cacheDir, $arrayAdapter)) {
+            if (!$this->doWarmUp($cacheDir, $arrayAdapter, $buildDir)) {
                 return [];
             }
         } finally {
@@ -80,7 +79,9 @@ abstract class AbstractPhpFileCacheWarmer implements CacheWarmerInterface
     }
 
     /**
+     * @param string|null $buildDir
+     *
      * @return bool false if there is nothing to warm-up
      */
-    abstract protected function doWarmUp(string $cacheDir, ArrayAdapter $arrayAdapter): bool;
+    abstract protected function doWarmUp(string $cacheDir, ArrayAdapter $arrayAdapter /* , string $buildDir = null */): bool;
 }
diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php
index 279dc4ec9e..20533bb60e 100644
--- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php
+++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AnnotationsCacheWarmer.php
@@ -44,7 +44,10 @@ class AnnotationsCacheWarmer extends AbstractPhpFileCacheWarmer
         parent::__construct($phpArrayFile);
     }
 
-    protected function doWarmUp(string $cacheDir, ArrayAdapter $arrayAdapter): bool
+    /**
+     * @param string|null $buildDir
+     */
+    protected function doWarmUp(string $cacheDir, ArrayAdapter $arrayAdapter /* , string $buildDir = null */): bool
     {
         $annotatedClassPatterns = $cacheDir.'/annotations.map';
 
diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/CachePoolClearerCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/CachePoolClearerCacheWarmer.php
index 7cd02d1cc5..7498a82d1f 100644
--- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/CachePoolClearerCacheWarmer.php
+++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/CachePoolClearerCacheWarmer.php
@@ -37,10 +37,7 @@ final class CachePoolClearerCacheWarmer implements CacheWarmerInterface
         $this->pools = $pools;
     }
 
-    /**
-     * @return string[]
-     */
-    public function warmUp(string $cacheDirectory, string $buildDir = null): array
+    public function warmUp(string $cacheDir, string $buildDir = null): array
     {
         foreach ($this->pools as $pool) {
             if ($this->poolClearer->hasPool($pool)) {
diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ConfigBuilderCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ConfigBuilderCacheWarmer.php
index 12e24bd77c..225fea11b7 100644
--- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ConfigBuilderCacheWarmer.php
+++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ConfigBuilderCacheWarmer.php
@@ -38,11 +38,13 @@ class ConfigBuilderCacheWarmer implements CacheWarmerInterface
     }
 
     /**
-     * @return string[]
+     * @param string|null $buildDir
      */
-    public function warmUp(string $cacheDir, string $buildDir = null): array
+    public function warmUp(string $cacheDir /* , string $buildDir = null */): array
     {
-        if (null === $buildDir) {
+        $buildDir = 1 < \func_num_args() ? func_get_arg(1) : null;
+
+        if (!$buildDir) {
             return [];
         }
 
diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/RouterCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/RouterCacheWarmer.php
index 82a16c01d3..2af9a2fe80 100644
--- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/RouterCacheWarmer.php
+++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/RouterCacheWarmer.php
@@ -34,16 +34,12 @@ class RouterCacheWarmer implements CacheWarmerInterface, ServiceSubscriberInterf
         $this->container = $container;
     }
 
-    /**
-     * @param string|null $buildDir directory to put generated source code that can be marked as read-only at runtime.
-     *                              The directory is not provided when only cache_dir should be warmed up.
-     */
-    public function warmUp(string $cacheDir /* , string $buildDir = null */): array
+    public function warmUp(string $cacheDir, string $buildDir = null): array
     {
         $router = $this->container->get('router');
 
         if ($router instanceof WarmableInterface) {
-            return (array) $router->warmUp($cacheDir);
+            return (array) $router->warmUp($cacheDir, $buildDir);
         }
 
         throw new \LogicException(sprintf('The router "%s" cannot be warmed up because it does not implement "%s".', get_debug_type($router), WarmableInterface::class));
diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php
index 35595474ef..b47a48ce69 100644
--- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php
+++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/SerializerCacheWarmer.php
@@ -39,7 +39,10 @@ class SerializerCacheWarmer extends AbstractPhpFileCacheWarmer
         $this->loaders = $loaders;
     }
 
-    protected function doWarmUp(string $cacheDir, ArrayAdapter $arrayAdapter): bool
+    /**
+     * @param string|null $buildDir
+     */
+    protected function doWarmUp(string $cacheDir, ArrayAdapter $arrayAdapter /* , string $buildDir = null */): bool
     {
         if (!$this->loaders) {
             return true;
diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/TranslationsCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/TranslationsCacheWarmer.php
index d3abbaca5b..39b1444b0e 100644
--- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/TranslationsCacheWarmer.php
+++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/TranslationsCacheWarmer.php
@@ -34,16 +34,16 @@ class TranslationsCacheWarmer implements CacheWarmerInterface, ServiceSubscriber
     }
 
     /**
-     * @param string|null $buildDir directory to put generated source code that can be marked as read-only at runtime.
-     *                              The directory is not provided when only cache_dir should be warmed up.
-     * @return string[]
+     * @param string|null $buildDir
      */
     public function warmUp(string $cacheDir /* , string $buildDir = null */): array
     {
         $this->translator ??= $this->container->get('translator');
 
         if ($this->translator instanceof WarmableInterface) {
-            return (array) $this->translator->warmUp($cacheDir);
+            $buildDir = 1 < \func_num_args() ? func_get_arg(1) : null;
+
+            return (array) $this->translator->warmUp($cacheDir, $buildDir);
         }
 
         return [];
diff --git a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php
index 3119c9942a..224e90985e 100644
--- a/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php
+++ b/src/Symfony/Bundle/FrameworkBundle/CacheWarmer/ValidatorCacheWarmer.php
@@ -39,7 +39,10 @@ class ValidatorCacheWarmer extends AbstractPhpFileCacheWarmer
         $this->validatorBuilder = $validatorBuilder;
     }
 
-    protected function doWarmUp(string $cacheDir, ArrayAdapter $arrayAdapter): bool
+    /**
+     * @param string|null $buildDir
+     */
+    protected function doWarmUp(string $cacheDir, ArrayAdapter $arrayAdapter /* , string $buildDir = null */): bool
     {
         $loaders = $this->validatorBuilder->getLoaders();
         $metadataFactory = new LazyLoadingMetadataFactory(new LoaderChain($loaders), $arrayAdapter);
diff --git a/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php b/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php
index fe89fef6d6..3367ecec2b 100644
--- a/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php
@@ -81,9 +81,7 @@ class Router extends BaseRouter implements WarmableInterface, ServiceSubscriberI
     }
 
     /**
-     * @param string|null $buildDir directory to put generated source code that can be marked as read-only at runtime.
-     *                              The directory is not provided when only cache_dir should be warmed up.
-     * @return string[] A list of classes to preload on PHP 7.4+
+     * @param string|null $buildDir
      */
     public function warmUp(string $cacheDir /* , string $buildDir = null */): array
     {
diff --git a/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php b/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php
index 70f5874307..04b56308f3 100644
--- a/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php
@@ -95,9 +95,7 @@ class Translator extends BaseTranslator implements WarmableInterface
     }
 
     /**
-     * @param string|null $buildDir directory to put generated source code that can be marked as read-only at runtime.
-     *                              The directory is not provided when only cache_dir should be warmed up.
-     * @return string[]
+     * @param string|null $buildDir
      */
     public function warmUp(string $cacheDir /* , string $buildDir = null */): array
     {
diff --git a/src/Symfony/Bundle/SecurityBundle/CacheWarmer/ExpressionCacheWarmer.php b/src/Symfony/Bundle/SecurityBundle/CacheWarmer/ExpressionCacheWarmer.php
index c63c973e41..1cbb681c2d 100644
--- a/src/Symfony/Bundle/SecurityBundle/CacheWarmer/ExpressionCacheWarmer.php
+++ b/src/Symfony/Bundle/SecurityBundle/CacheWarmer/ExpressionCacheWarmer.php
@@ -35,9 +35,7 @@ class ExpressionCacheWarmer implements CacheWarmerInterface
     }
 
     /**
-     * @param string|null $buildDir directory to put generated source code that can be marked as read-only at runtime.
-     *                              The directory is not provided when only cache_dir should be warmed up.
-     * @return string[]
+     * @param string|null $buildDir
      */
     public function warmUp(string $cacheDir /* , string $buildDir = null */): array
     {
diff --git a/src/Symfony/Bundle/TwigBundle/CacheWarmer/TemplateCacheWarmer.php b/src/Symfony/Bundle/TwigBundle/CacheWarmer/TemplateCacheWarmer.php
index 1adabe6636..2ab801130b 100644
--- a/src/Symfony/Bundle/TwigBundle/CacheWarmer/TemplateCacheWarmer.php
+++ b/src/Symfony/Bundle/TwigBundle/CacheWarmer/TemplateCacheWarmer.php
@@ -36,9 +36,7 @@ class TemplateCacheWarmer implements CacheWarmerInterface, ServiceSubscriberInte
     }
 
     /**
-     * @param string|null $buildDir directory to put generated source code that can be marked as read-only at runtime.
-     *                              The directory is not provided when only cache_dir should be warmed up.
-     * @return string[] A list of template files to preload on PHP 7.4+
+     * @param string|null $buildDir
      */
     public function warmUp(string $cacheDir /* , string $buildDir = null */): array
     {
diff --git a/src/Symfony/Component/HttpKernel/CacheWarmer/CacheWarmerAggregate.php b/src/Symfony/Component/HttpKernel/CacheWarmer/CacheWarmerAggregate.php
index 638d9ad2c4..a672956e0f 100644
--- a/src/Symfony/Component/HttpKernel/CacheWarmer/CacheWarmerAggregate.php
+++ b/src/Symfony/Component/HttpKernel/CacheWarmer/CacheWarmerAggregate.php
@@ -48,8 +48,17 @@ class CacheWarmerAggregate implements CacheWarmerInterface
         $this->onlyOptionalsEnabled = $this->optionalsEnabled = true;
     }
 
-    public function warmUp(string $cacheDir, string $buildDir = null, SymfonyStyle $io = null): array
+    /**
+     * @param string|null $buildDir
+     */
+    public function warmUp(string $cacheDir, string|SymfonyStyle $buildDir = null, SymfonyStyle $io = null): array
     {
+        if ($buildDir instanceof SymfonyStyle) {
+            trigger_deprecation('symfony/http-kernel', '6.4', 'Passing a "%s" as second argument of "%s()" is deprecated, pass it as third argument instead, after the build directory.', SymfonyStyle::class, __METHOD__);
+            $io = $buildDir;
+            $buildDir = null;
+        }
+
         if ($collectDeprecations = $this->debug && !\defined('PHPUNIT_COMPOSER_INSTALL')) {
             $collectedLogs = [];
             $previousHandler = set_error_handler(function ($type, $message, $file, $line) use (&$collectedLogs, &$previousHandler) {
@@ -96,8 +105,8 @@ class CacheWarmerAggregate implements CacheWarmerInterface
                 }
 
                 $start = microtime(true);
-                foreach ((array) $warmer->warmUp($cacheDir, null === $buildDir || $warmer->isOptional() ? null : $buildDir) as $item) {
-                    if (is_dir($item) || (str_starts_with($item, \dirname($cacheDir)) && !is_file($item))) {
+                foreach ((array) $warmer->warmUp($cacheDir, $buildDir) as $item) {
+                    if (is_dir($item) || (str_starts_with($item, \dirname($cacheDir)) && !is_file($item)) || ($buildDir && str_starts_with($item, \dirname($buildDir)) && !is_file($item))) {
                         throw new \LogicException(sprintf('"%s::warmUp()" should return a list of files or classes but "%s" is none of them.', $warmer::class, $item));
                     }
                     $preload[] = $item;
diff --git a/src/Symfony/Component/HttpKernel/CacheWarmer/WarmableInterface.php b/src/Symfony/Component/HttpKernel/CacheWarmer/WarmableInterface.php
index 799705e42d..cd051b1add 100644
--- a/src/Symfony/Component/HttpKernel/CacheWarmer/WarmableInterface.php
+++ b/src/Symfony/Component/HttpKernel/CacheWarmer/WarmableInterface.php
@@ -21,8 +21,8 @@ interface WarmableInterface
     /**
      * Warms up the cache.
      *
-     * @param string|null $buildDir directory to put generated source code that can be marked as read-only at runtime.
-     *                              The directory is not provided when only cache_dir should be warmed up.
+     * @param string      $cacheDir Where warm-up artifacts should be stored
+     * @param string|null $buildDir Where read-only artifacts should go; null when called after compile-time
      *
      * @return string[] A list of classes or files to preload on PHP 7.4+
      */
diff --git a/src/Symfony/Component/Translation/DataCollectorTranslator.php b/src/Symfony/Component/Translation/DataCollectorTranslator.php
index bdd0e6ce4d..785e5b89ed 100644
--- a/src/Symfony/Component/Translation/DataCollectorTranslator.php
+++ b/src/Symfony/Component/Translation/DataCollectorTranslator.php
@@ -72,14 +72,14 @@ class DataCollectorTranslator implements TranslatorInterface, TranslatorBagInter
     }
 
     /**
-     * @param string|null $buildDir directory to put generated source code that can be marked as read-only at runtime.
-     *                              The directory is not provided when only cache_dir should be warmed up.
-     * @return string[]
+     * @param string|null $buildDir
      */
     public function warmUp(string $cacheDir /* , string $buildDir = null */): array
     {
+        $buildDir = 1 < \func_num_args() ? func_get_arg(1) : null;
+
         if ($this->translator instanceof WarmableInterface) {
-            return (array) $this->translator->warmUp($cacheDir);
+            return (array) $this->translator->warmUp($cacheDir, $buildDir);
         }
 
         return [];

[fbourigault]

Here are some comment, and my review as a patch :)

Now that we have this, I'm wondering about each warmers: are they doing something for build-time or compilte-time? what's their purpose, etc; E.g CompiledClassMetadataFactory added in #29117 by @fbourigault is never wired? We should likely investigate this in a follow up PR this and the others too.

Sadly, I never finished the framework integration of CompiledClassMetadataFactory :( Sadly I no longer have time to work on this topic anytime soon. It would be awesome if someone want to contribute the integration.

[nicolas-grekas]

please rebase

[nicolas-grekas]

Thank you @Okhoshi.