autoupgrade
autoupgrade copied to clipboard
PrestaShop
Unify calls to list directory contents
| Questions | Answers |
|---|---|
| Description? | They are several ways to list the content of a directory, and some issues were met. This PR adds tests and plugs all the calls to a same method. |
| Type? | refacto |
| BC breaks? | Nope |
| Deprecations? | Nope |
| Fixed ticket? | Need to find it |
| Sponsor company | PrestaShopCorp |
| How to test? | Copy of files during upgrades, backups and restores work the same as before. |
To Do list
- [x] ~Ignore modules during upgrade~ Reverted
- [x] Remove duplicated method listing files in UpgradeFiles.php
- [x] Test generation of files list for a rollback
- [x] Test generation of files list for a backup
Known issues
- These issues will be reported because the module is not copied anymore during upgrade
[INTERNAL] /var/www/html/modules/autoupgrade/vendor/composer/ClassLoader.php line 571 - include(/var/www/html/vendor/composer/../../modules/ps_distributionapiclient/ps_distributionapiclient.php): failed to open stream: No such file or directory
[INTERNAL] /var/www/html/modules/autoupgrade/vendor/composer/ClassLoader.php line 571 - include(): Failed opening '/var/www/html/vendor/composer/../../modules/ps_distributionapiclient/ps_distributionapiclient.php' for inclusion (include_path='/var/www/html/vendor/pear/pear_exception:/var/www/html/vendor/pear/console_getopt:/var/www/html/vendor/pear/pear-core-minimal/src:/var/www/html/vendor/pear/archive_tar:.:/usr/local/lib/php')
This will be fixed when the upgrade of modules is improved. In the meantime ignoring the whole modules/ folder has been reverted.
Issue found with the current version
Guys please make sure we don't reintroduce some issues previously fixed, like - https://github.com/PrestaShop/autoupgrade/pull/651 🙏
Guys please make sure we don't reintroduce some issues previously fixed, like - #651 🙏
The test I'm adding is supposed to cover the case you are talking about in your link, even if we're talking about modules instead of install.
The file of expected files to upgrade has a modules folder we can find deep in the tree. The root modules folder has been ignored, but this one remains. https://github.com/PrestaShop/autoupgrade/blob/05d7e94986c5c0803b9ba6cf1481e2fe09843ff4/tests/fixtures/listOfFiles-ReleaseExample-upgrade-with-theme.json#L21-L25
There is another case that could be covered though, where the case can be different (i.e install vs Installer). I'll have a look at it too.
One idea @Quetzacoalt91, do you think it would be to use this opportunity to fix one annoying behavior? The current logic copies all of the native modules in the zip to the install. Maybe we could copy them only if the folder already exists?
cc @MatShir and @kpodemski
One idea @Quetzacoalt91, do you think it would be to use this opportunity to fix one annoying behavior? The current logic copies all of the native modules in the zip to the install. Maybe we could copy them only if the folder already exists?
cc @MatShir and @kpodemski
There are a few improvement we'd like to add in the upgrade process, something a bit smarter when we deal about modules (i.e avoiding rollback while copying, check if a module is new in the new release vs a simple upgrade). This PR makes us ready to stop copying the modules folder once we have something ready to improve module management during an upgrade.