Skip to content
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

.env files processed twice, booting not consistent between web and CLI #16638

Closed
brusch opened this issue Feb 19, 2024 · 18 comments · Fixed by #16652
Closed

.env files processed twice, booting not consistent between web and CLI #16638

brusch opened this issue Feb 19, 2024 · 18 comments · Fixed by #16652

Comments

@brusch
Copy link
Member

brusch commented Feb 19, 2024

Improvement description

Pimcore is using Symfony Runtime for bootstrapping.
Symfony runtime is using DotEnv (when available) and does parsing .env file.

Pimcore is also using DotEnv here:

(new Dotenv())->bootEnv(PIMCORE_PROJECT_ROOT . '/.env');

So `.env' file are getting processed twice during bootstrapping.

Also booting is not consistent between CLI and web.
For web Bootstrap::bootstrap() is called before the callback:
https://github.com/pimcore/demo/blob/6978c2d36e9f580e22654e9970aa411487f320e0/public/index.php#L23

however for CLI, it's called inside the callback as part of startupCli():
https://github.com/pimcore/demo/blob/6978c2d36e9f580e22654e9970aa411487f320e0/bin/console#L36

This could cause very weird behavior, especially when doing some stuff in config/pimcore/startup.php, e.g. overriding env variables, which are then again overridden from values in .env.

@kingjia90
Copy link
Contributor

kingjia90 commented Feb 22, 2024

Hope i got the issue right, it seems that by adding $_ENV['APP_RUNTIME_OPTIONS'] = ['disable_dotenv' => true]; to config/pimcore/startup.php will be fixing it.

The "second time" that env get processed is done by /vendor/autoload_runtime.php
image

See also https://github.com/symfony/symfony/blob/635a9f1bcc48386e5013bcf0454b5857d407cc78/src/Symfony/Component/Runtime/SymfonyRuntime.php#L38

If it's the right fix, it's something that we need to add to the documentation or set by default?

@brusch
Copy link
Member Author

brusch commented Feb 22, 2024

To be consistent I'd do the following:

  1. Move Bootstrap::bootstrap(); in index.php into the callback function, then it's consistent with bin/console.
    This is something that we'd need to include in upgrade notes, since this change needs to be made in the projects repo.
  2. I'd remove self::prepareEnvVariables(); as it's covered by Runtime

Shouldn't break anything right?

@kingjia90
Copy link
Contributor

kingjia90 commented Feb 22, 2024

Thank you for the feedback!

On index.php and CLI seem that the suggested change would be fine and safe so far.

Looks like it would be breaking the install script instead

Bootstrap::defineConstants();

since it's not loading the runtime one
if (file_exists($a = getcwd() . '/vendor/autoload.php')) {

So if i got correctly, prepareEnvVariables was more for PIMCORE_COMPOSER_PATH to be changed by the value set in env before defining the Constant or something like that

@brusch
Copy link
Member Author

brusch commented Feb 26, 2024

@kingjia90
prepareEnvVariables() comes from a time where Symfony Runtime didn't exist 😉 Since it's private I'd assume it can go completely.

Regarding the installer:
I'd refactor the installer to use Runtime as well - so that we're consistent - would make sense, right?

@kingjia90
Copy link
Contributor

Thank you for the feedback, i've applied the requested change to #16652 and seems working fine so far

@danielerne
Copy link
Contributor

After Updating to 11.2.0 .env variables are not set anymore.
Has this change caused this problem? If I downgrade to 11.1.5 it works again.

Could this be that our project was migrated from Pimcore 10?

Our index.php - looks like this:

use Pimcore\Tool;
use Symfony\Component\Debug\Debug;
use Symfony\Component\HttpFoundation\Request;

include __DIR__ . "/../vendor/autoload.php";

\Pimcore\Bootstrap::setProjectRoot();
\Pimcore\Bootstrap::bootstrap();

$request = Request::createFromGlobals();

// set current request as property on tool as there's no
// request stack available yet
Tool::setCurrentRequest($request);

/** @var \Pimcore\Kernel $kernel */
$kernel = \Pimcore\Bootstrap::kernel();

// reset current request - will be read from request stack from now on
Tool::setCurrentRequest(null);

$response = $kernel->handle($request);
$response->send();

$kernel->terminate($request, $response);

@jdreesen
Copy link
Contributor

You should probably update your index.php (and bin/console) to use Symfony Runtime.

See: https://github.com/pimcore/skeleton/blob/11.x/public/index.php

@blankse
Copy link
Contributor

blankse commented Mar 14, 2024

@brusch @kingjia90 Should be added to the upgrade notes if Symfony Runtime is required now in Pimcore 11.
https://pimcore.com/docs/platform/Pimcore/Installation_and_Upgrade/Updating_Pimcore/V10_to_V11/

@brusch
Copy link
Member Author

brusch commented Mar 15, 2024

@danielerne @jdreesen @blankse plz have a look at #16796 - i's kind of a poor mans solution, but should do the job for backwards compatibility. Thanks for your feedback 😊

@dpfaffenbauer
Copy link
Contributor

@brusch Is Pimcore 11.1 compatible with runtime? I am having the issue to support both in CoreShop for testing purposes.

@brusch
Copy link
Member Author

brusch commented Mar 15, 2024

@dpfaffenbauer yeah, should be, since parts were using it already before, see: https://github.com/pimcore/skeleton/pull/183/files
This whole topic was just about making it consistent throughout all packages.

@Cruiser13
Copy link
Contributor

Cruiser13 commented Apr 3, 2024

@brusch we just updated to pimcore 11.2.2 and now APP_ENV=prod from our .env file is ignored by CLI.
php bin/console about will output:

Type App\Kernel
Environment dev
Debug false

We adjusted our index.php to the one from skeleton: https://github.com/pimcore/skeleton/blob/11.x/public/index.php

Is this intended behaviour?

@jdreesen
Copy link
Contributor

jdreesen commented Apr 3, 2024

Did you also adjust your bin/console? => https://github.com/pimcore/skeleton/blob/11.x/bin/console

@Cruiser13
Copy link
Contributor

Thank you @jdreesen totally missed that. Added it to the docs.

@brusch
Copy link
Member Author

brusch commented Apr 5, 2024

@Cruiser13 I'm wondering why you're experiencing the issue ... how did your bin/console before?
Because acutally bin/console didn't change recently, see:
https://github.com/pimcore/skeleton/commits/11.x/bin/console

I'd assume you missed the upgrade of the file earlier 😉

@Cruiser13
Copy link
Contributor

@brusch you are right, looks like we missed pimcore/skeleton@b1075d6 before. Worked fine until the update though.
I guess we need to have a closer look at the skeleton project on updates as some changes do not seem to be documented.

@patrick-rudolph-umusic
Copy link

I'm getting the same error when running tests. How do I need to setup my tests/bootstrap.php in order to work again? The one used in skeleton does not help

@agdrs
Copy link

agdrs commented Apr 19, 2024

I'm getting the same error when running tests. How do I need to setup my tests/bootstrap.php in order to work again? The one used in skeleton does not help

@patrick-rudolph-umusic
We faced the same issue and fixed it with following tests/bootstrap.php (based on the standard symfony bootstrap.php)

declare(strict_types=1);

use Pimcore\Bootstrap;
use Symfony\Component\Dotenv\Dotenv;

include __DIR__ . '/../vendor/autoload_runtime.php';

if (method_exists(Dotenv::class, 'bootEnv')) {
    (new Dotenv())->bootEnv(dirname(__DIR__).'/.env');
}

Bootstrap::setProjectRoot();
Bootstrap::bootstrap();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants