contao / manager-bundle

[READ-ONLY] Contao Manager Bundle
GNU Lesser General Public License v3.0
17 stars 10 forks source link

.env files are ignored when using the symfony binary #87

Closed m-vo closed 4 years ago

m-vo commented 4 years ago

If $_SERVER['APP_ENV'] ist set, loading of the .env files is currently prevented in our Kernel:

https://github.com/contao/manager-bundle/blob/b5b9a22225a1cda6aed3d9337f645d11efbe1cc9/src/HttpKernel/ContaoKernel.php#L380-L383

This leads to a bad dx when using the symfony binary to execute the console ( as a wrapper for php to get a consistent version) because the symfony binary is apparently setting $_SERVER['APP_ENV'] = 'prod' by default:

symfony php vendor/bin/contao-console # <-- won't process .env / .env.local
php vendor/bin/contao-console         # <-- will process .env / .env.local

In a Symfony 5 application (with flex) the .env files are processed anyways, also the default bootstrapping (see recipe) for Symfony 4 doesn't contain this check.

I'd really like the managed edition and flex behave the same way in this case. Although I don't really know what the correct behavior should be.

Toflar commented 4 years ago

Symfony Flex does default to dev, the Managed Edition to prod. Both for very good reasons.

leofeyer commented 4 years ago

I doubt that we can do anything about it, because we do need this check in case real env variables are used. @aschempp /cc

m-vo commented 4 years ago

I don't really get why the ME cannot behave the same as a flex app? Imo this doesn't have to do with the defaults but maybe I'm missing something. :smile:

Toflar commented 4 years ago

I have already described why it doesn‘t work?

leofeyer commented 4 years ago

It is still confusing. Maybe we can improve the DX here?

leofeyer commented 4 years ago

As discussed in Mumble on August 27th, we should always load the .env file (even if APP_ENV is set), because DotEnv will no longer overwrite existing variables.

leofeyer commented 4 years ago

See https://github.com/contao/contao/pull/2208