Impovement #17696 Upgrade dependencies where possible without BC break
Changes in this pull request
Resolves #17696
Additional info
The file models/Document/Editable/Date.php requires a temporary workaround due to Carbon deprecating the formatLocalized method. Once support for the outputFormat option is dropped, this workaround will no longer be necessary.
The entries in the composer.json file under the require-dev section have been sorted. Additionally, phpstan/phpstan has been upgraded to version 1.12.6.
Review Checklist
- [ ] Target branch (
11.4for bug fixes, others11.x) - [ ] Tests (if it's testable code, there should be a test for it - get help)
- [ ] Docs (every functionality needs to be documented, see here)
- [ ] Migration incl.
install.sql(e.g. if the database schema changes, ...) - [ ] Upgrade notes (deprecations, important information, migration hints, ...)
- [ ] Label
- [ ] Milestone
@fashxp The Codeception tests are failing for PHP 8.1 because the scheb/2fa-bundle requires PHP 8.2 or higher. In this case, should we downgrade the version of scheb/2fa-bundle to 6.x, or rather drop support for PHP 8.1?
Is it not possible to support scheb/2fa-bundle v6 and v7?
I think this should be done for all libs, if possible, to ease upgrades for Pimcore users.
Is it not possible to support
scheb/2fa-bundlev6 and v7?I think this should be done for all libs, if possible, to ease upgrades for Pimcore users.
This is possible as long as both versions are compatible in the areas used by Pimcore and its extensions. As for scheb/2fa-bundle, both versions should be fine as long as no one is using scheb_two_factor.google.window. https://github.com/scheb/2fa/blob/7.x/UPGRADE.md#6x-to-7x
If you use a library in your project, you should add it as a dependency in the composer.json of your project to have control over its update.
This allows you to determine when the next version will be used and adapt the project code accordingly beforehand.
If you use a library in your project, you should add it as a dependency in the
composer.jsonof your project to have control over its update.This allows you to determine when the next version will be used and adapt the project code accordingly beforehand.
You’re right. However, in the case of libraries used in Pimcore's core, you need to consider the core's compatibility with both versions. For example, assuming that Pimcore is currently using scheb/2fa-bundle version 5, when updating from 5.x to 6.x and declaring both versions as compatible in composer, it might be necessary to rewrite part of Pimcore's code responsible for authentication and maintain compatibility with both solutions (due to significant changes between 5.x and 6.x).
In the case of comparing 6.x and 7.x (2fa-bundle), the differences are not as drastic, which in my opinion allows for both versions to be used.
Sure, Pimcore needs to take care of supporting both version if both are allowed.
Another thing. endroid/qr-code requirement should be moved to pimcore/admin-ui-classic-bundle. It is not used anywhere else.
@michalananapps I just did a test installation and created a class with a date range.
On saving I encounter the following problem which seems to be related to the nesbot/carbon update.
That is a BC-Break since there is no available solution in myclabs/deep-copy since the latest version is 1.12. and I tested it with that version.
Timestamp: Wed Nov 06 2024 10:48:58 GMT+0100 (Central European Standard Time)
Status: 500 | Internal Server Error
URL: /admin/object/save?task=version
Method: PUT
Message: Cannot modify readonly property DatePeriod::$start
Trace:
in /var/www/html/vendor/myclabs/deep-copy/src/DeepCopy/DeepCopy.php:267
#0 /var/www/html/vendor/myclabs/deep-copy/src/DeepCopy/DeepCopy.php(267): ReflectionProperty->setValue(Object(Carbon\CarbonPeriod), Object(Carbon\Carbon))
#1 /var/www/html/vendor/myclabs/deep-copy/src/DeepCopy/DeepCopy.php(214): DeepCopy\DeepCopy->copyObjectProperty(Object(Carbon\CarbonPeriod), Object(ReflectionProperty))
#2 /var/www/html/vendor/myclabs/deep-copy/src/DeepCopy/DeepCopy.php(150): DeepCopy\DeepCopy->copyObject(Object(Carbon\CarbonPeriod))
#3 /var/www/html/vendor/myclabs/deep-copy/src/DeepCopy/DeepCopy.php(267): DeepCopy\DeepCopy->recursiveCopy(Object(Carbon\CarbonPeriod))
#4 /var/www/html/vendor/myclabs/deep-copy/src/DeepCopy/DeepCopy.php(214): DeepCopy\DeepCopy->copyObjectProperty(Object(Pimcore\Model\DataObject\Testclass), Object(ReflectionProperty))
#5 /var/www/html/vendor/myclabs/deep-copy/src/DeepCopy/DeepCopy.php(150): DeepCopy\DeepCopy->copyObject(Object(Pimcore\Model\DataObject\Testclass))
#6 /var/www/html/vendor/myclabs/deep-copy/src/DeepCopy/DeepCopy.php(95): DeepCopy\DeepCopy->recursiveCopy(Object(Pimcore\Model\DataObject\Testclass))
#7 /var/www/html/dev/pimcore/pimcore/models/Version.php(217): DeepCopy\DeepCopy->copy(Object(Pimcore\Model\DataObject\Testclass))
#8 /var/www/html/dev/pimcore/pimcore/models/Version.php(156): Pimcore\Model\Version->marshalData(Object(Pimcore\Model\DataObject\Testclass))
#9 /var/www/html/dev/pimcore/pimcore/models/Element/AbstractElement.php(607): Pimcore\Model\Version->save()
#10 /var/www/html/dev/pimcore/pimcore/models/DataObject/Concrete.php(268): Pimcore\Model\Element\AbstractElement->doSaveVersion(NULL, false, true, false)
#11 /var/www/html/dev/pimcore/pimcore/models/DataObject/Concrete.php(206): Pimcore\Model\DataObject\Concrete->saveVersion(false, false, NULL)
#12 /var/www/html/dev/pimcore/pimcore/models/DataObject/AbstractObject.php(552): Pimcore\Model\DataObject\Concrete->update(true, Array)
#13 /var/www/html/dev/pimcore/pimcore/models/DataObject/Concrete.php(637): Pimcore\Model\DataObject\AbstractObject->save(Array)
#14 /var/www/html/vendor/pimcore/admin-ui-classic-bundle/src/Controller/Admin/DataObject/DataObjectController.php(1428): Pimcore\Model\DataObject\Concrete->save()
#15 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(181): Pimcore\Bundle\AdminBundle\Controller\Admin\DataObject\DataObjectController->saveAction(Object(Symfony\Component\HttpFoundation\Request))
#16 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(76): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#17 /var/www/html/vendor/symfony/http-kernel/Kernel.php(197): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#18 /var/www/html/vendor/symfony/runtime/Runner/Symfony/HttpKernelRunner.php(35): Symfony\Component\HttpKernel\Kernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#19 /var/www/html/vendor/autoload_runtime.php(29): Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run()
#20 /var/www/html/public/index.php(19): require_once('/var/www/html/v...')
#21 {main}
Quality Gate passed
Issues
3 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
@michalananapps something went wrong with the merge. There are a lot of commits that have nothing to do with the actual PR.
Regarding to your latest commit, our minimum Version of PHP is 8.1
8.2.0 The [DatePeriod::INCLUDE_END_DATE](https://www.php.net/manual/en/class.dateperiod.php#dateperiod.constants.include-end-date) constant and include_end_date property have been added.
So we cannot use DatePeriod since the tests are failing.
@mattamon Sorry. I've made mistake with last rebase. I've created new PR on clean branch: https://github.com/pimcore/pimcore/pull/17799
In second branch I've also fixed PHP 8.1 compatibility.
Closing in favor of https://github.com/pimcore/pimcore/pull/17799