Hey!

To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done?

Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review.

Cheers!

Carsonbot

Can you please remove everything from #62249 so that we can have only the patch about your proposal?

@nicolas-grekas done!

Okay I was thinking that in 7.4 we never define now kernel.share_dir aslong APP_SHARE_DIR is not defined.

But as you stated thats not the case its always there now.
#62204 (comment)
But can be removed via APP_SHARE_DIR=false, if I understand correctly?

As we still fallback to getCacheDir? Which I had expected would not longer be the case my suggestion, won't work or would require lot of more magic logic, so I'm lost.

I personally would have make the share dir opt-in and defined it in the recipe. That would also make bundles automatically fail as the share dir is then not defined automatically for all bundles running tests against the Kernel.php. Was thinking that was what @Seldaek suggested. I think this way we maybe run into issue that bundles use %kernel.share_dir% and so make problems in multi region setups @Seldaek mentioned.

be removed via APP_SHARE_DIR=false, if I understand correctly? As we still fallback to getCacheDir ?

Yes, then No, see https://github.com/symfony/symfony/pull/62204/files#diff-ee9b2c16aec8aa80f67e6b3925791d7b092fc097651bcc2df21b70e7dc8bef12R130

@alexander-schranz we want the cache.app pools to have a working configuration by default in a new project. That's why the default is not to have no share directory (locally, any directory is shared among your single running server by definition). And we cannot make cache.app default to a Redis storage to avoid using the share dir.

@stof Understand make sense. Then its definitly good choice to use kernel.share_dir for our storage and indexes. As the rule also make sense there if you multi region like for caches using redis, you want for indexes use Elasticsearch and for file storage a S3 or similar.

Alternative thought:

it might be safer to have per-env by default
this doesn't prevent use cases that need to be env-less: those could use %kernel.share_dir%/../storage for example

@nicolas-grekas that would end in the strange directory we had before in the discussion:

 - var/share
    - dev
       - app_cache
       - http_cache
    - indexes (none env dir)
    - prod
       - app_cache
       - http_cache
    - stage
       - app_cache
       - http_cache
    - storage (none env dir)

Yes of course. However, I don't see this as "strange" :)
Having share split per env by default might be safer.

Okay, you need to maintain it if you see it as safer I understand it, we will discuss it what we do on our side in @sulu if we maybe keep our own parameters and directories, as %kernel.share_dir%/../ is to unsafe for my point of view.

But one question will Symfony Cloud also keep the APP_ENV in mind when set the new APP_SHARE_DIR? So its APP_SHARE_DIR = var/share/{$APP_ENV}?

APP_SHARE_DIR = var/share/{$APP_ENV}?

The env var won't contain the env since it's the responsibility of the kernel to add it when desired - that's only a default when using MicroKernelTrait.

@nicolas-grekas okay Thx, that would atleast avoid us using the %kernel.share_dir%/../storage and we could go with %env(APP_SHARE_DIR)%/storage if we decide to go with var/share.