Closed wwidergoldbsc closed 7 months ago
fashxp
commented
8 months ago Are you sure, you are using the latest version of the demo?
missing table should be created in bootstrap, see https://github.com/pimcore/demo/blob/11.x/dump/data-0-bootstrap.sql#L3183
wwidergoldbsc
commented
8 months ago I'm explicitly running the installer with the --skip-database-data-dump flag to reproduce the scenario in another project, where the setup relies on the pimcore installation working on a clean database.
The bug is not specific to the demo project, I just used it as an example.
kingjia90
commented
8 months ago Confirming the issue and it's surprisingly more related to https://github.com/pimcore/pimcore/issues/16157 rather than the demo install itself. Potentially we just need to add a flag to the command to allow skipping the class rebuild too on https://github.com/pimcore/pimcore/blob/9f9336f0b82e07b5c07d58370d464b9f70e7c5f1/bundles/InstallBundle/src/Installer.php#L446
Internally is a sort of "known issue", see https://github.com/pimcore/ee-pimcore/pull/19#issuecomment-1845337937
wwidergoldbsc
commented
8 months ago We were able to work around this issue by:
pimcore:deployment:classes-rebuild -c by removing the part you mentionedbin/console pimcore:bundle:install PimcorePersonalizationBundle --no-interaction to install the PimcorePersonalizationBundle and avoid the exception above regarding missing tablesbin/console pimcore:deployment:classes-rebuild --create-classes --delete-classes -- without errors this timekingjia90
commented
7 months ago Thank you for the feedback.
I've found a potential solution to this, by swapping the order like in https://github.com/pimcore/pimcore/pull/16729 in combination with install-bundles option or (the step event install_classes), should work, at least by avoiding to manually do the steps you mentioned
/vendor/bin/pimcore-install \
--admin-username=admin --admin-password=admin \
--mysql-database=pimcore \
--no-interaction
--skip-database-data-dump
--install-bundles=PimcorePersonalizationBundleAh wait, it's not intended to work like that, it's for the internal bundles (/bundles) only
https://github.com/pimcore/pimcore/pull/14194/files#diff-4eee5188dd424c74c2215a7595b21d4bde4443e3732f5b86d71484393076c498R62-R72
but i think we can tweak a little there to work for Personalization bundle too
kingjia90
commented
7 months ago @mattamon Do you see any contraindications on doing as https://github.com/pimcore/pimcore/pull/16729?
Would be possible to extend the installable bundle to what the ones that are already enabled, but maybe the installer isn't yet run or so?
eg. by checking if is installable
mattamon
commented
7 months ago @kingjia90 no objections. I guess I was not fully aware of this.
If you want to install an external bundle, you can always add it via the EventSubscriber like we do it here. https://github.com/pimcore/skeleton/blob/11.x/src/EventSubscriber/BundleSetupSubscriber.php
Not sure about checking installable though.
kingjia90
commented
7 months ago @wwidergoldbsc So i've merged https://github.com/pimcore/pimcore/pull/16729 and with the suggestion to use the InstallEvents::EVENT_BUNDLE_SETUP to add the bundles that should be installed before rebuilding the class, it should work around the problem.
This issue is now considered as done, therefore closing for the moment, feel free to re-open (or even better a new one, but maybe not in the demo repository 😄 ) if the problem still occurs.
Thank you again for reporting and testing
matthiashamacher
commented
6 months ago @kingjia90
We've tested it and generally it works. One thing we noticed is that the installBundles() function is not executed if you only have the bundles in the Kernel.php. This is because of these lines of code: InstallCommand.php#L214-L226
It only executes the even subscriber if the --install-bundles option is set in the command. Our workaround is now, to also have the bundle set in that option, then it works. Maybe we should discuss to move the event subscriber execution out of that if? In that case either of those would trigger the bundle installation.
I'm happy to provide a PR for that change, just wanted to get your input first.
kingjia90
commented
6 months ago Thank you for the feedback, glad to hear that it works now.
Could be fine to move the event of that, but the current documentation is quite defined with the current implementations and could be tricky to refactor without BC breaks, what about a new event or so?
matthiashamacher
commented
6 months ago Yeah ok. I had a closer look into the function and I noticed that you don't have to give an option there. Previously we had --install-bundles=PimcorePersonalizationBundle and the bundle again as required bundle in the subscriber. Because of that duplicate notation I thought it might be a good idea to move that. But I now noticed, that you can just use --install-bundles which then triggers the subscriber and also installs the bundles. So I guess its ok then to have it only defined in one place. Maybe there is a use case where you don't want to install the bundles so this would be needed as it is.
Pimcore version
11.1, 11.0.2
Steps to reproduce
Example tested on pimcore demo project:
bin/console doctrine:database:drop --force --no-interactionrunbin/console doctrine:database:create --if-not-existsphp vendor/bin/pimcore-install --mysql-host-socket=db --mysql-username=pimcore --mysql-password=pimcore --mysql-database=pimcore --skip-database-data-dumpActual Behavior
The installer will output
Even though the Pimcore install itself is successful, trying to install bundles manually after this will create and overwrite any class definitions related to the installed bundles, essentially discarding any changes made to bundle classes.
Expected Behavior