system:update:finish script might be run before system:install has been run

shopware / production

Shopware 6 production template

https://shopware.com

175 stars 152 forks source link

system:update:finish script might be run before system:install has been run #158

Closed AndreasA closed 1 year ago

AndreasA commented 2 years ago

PHP Version

8.1

Shopware Version

6.4.14.0

Expected behaviour

Only run system:update:finish after Shopware has been installed, e.g. checking lock file again. Sanest might be to check if a DB can be reached (e.g. ping) and certain tables exist?

Actual behaviour

The script is might be run prior to Shopware installation being finished. This can result in database connection errors etc.

How to reproduce

Add a .env file with defaults.
Add a .env.local with APP_ENV=test
Add .env.test with a DATABASE_URL for tests.
Run composer install and do not skip scripts.
Fails with database connection errors.

This is also the case with .env.dev etc. that might be in the repository, in case a user/developer has not finished Shopoware setup.

shyim commented 1 year ago

The user has to run them manually with the new template. https://github.com/shopware/docs/pull/760/files#diff-2525a2629f134228fc7d55615d5fbb517990de2c6ff9de903d5522d9fd25acf7R150

AndreasA commented 1 year ago

@shyim OK but:

how is the plugin loader selected? I guess that is only done for the console part? If so, it does not have an option for composer plugin loader or disable extensions as the current template does.
how do you load the $context variable? if I understand it correctly, this is basically $_SERVER (more or less) by default which means that resolving DATABASE_URL and other variables in those files, currently bypasses the EnvironmentHelper which bypasses the transformers?
- Or do you use a custom runtime that uses the environment helper correctly?
- Though I guess as long as you only check if the value exists, it is fine as the transformer would not be called for anyempty variable anyway. and in the Kernel etc. it would be used again.

AndreasA commented 1 year ago

@shyim Can the new recipes already be used as they are? if so, ref ref=flex/main should always be used, independent of Shopware version as the version is determined differently, isn't it?

shyim commented 1 year ago

You need atleast Shopware 6.4.15.0. Yes flex/main is for all versions

AndreasA commented 1 year ago

@shyim I guess this https://github.com/shopware/production/issues/158#issuecomment-1290165299 does not matter as you only check db url variable existence which should be fine without using the environment helper at all as that one only transforms variables. otherwise the transform would need to be called manually instead of using getVariable but ok.