search

joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.79k stars 3.65k forks source link

[5.0] Symfony php80 and php81 polyfills not present in packages but on a git clone #40290

Closed richard67 closed 1 year ago

richard67 commented 1 year ago

Steps to reproduce the issue

  1. On a clean, current 5.0-dev branch in a git clone, run composer install and npm ci.
  2. Check if folders "libraries/vendor/symfony/polyfill-php80" and "libraries/vendor/symfony/polyfill-php81" exist. Result: The folders exist,
  3. Make a new installation.
  4. In backend, go to the database checker "System - Maintenance - Database".
  5. Select the record for the CMS and use the "Update Structure" button.

Expected result

Nothing happens.

Actual result

Blank page. In PHP error log: PHP Fatal error: Uncaught Error: Failed opening required '/home/richard/lamp/public_html/joomla-cms-5.0-dev/libraries/vendor/composer/../symfony/polyfill-php80/bootstrap.php'

System information (as much as possible)

PHP 8.1, 5.0-dev branch on a git clone.

Additional comments

With PR #39968 , the symphony polyfills "polyfill-php80" and "polyfill-php81" were added as explicit dependencies to 4.2-dev so they are packaged into the 4.2.9 installation and update packages.

On 5.0-dev these polyfills should not be necessary because 5.0 requires PHP 8.1. And these polyfills are also not in the installation and update packages for 5.0-dev, that's ok and that's why they have been added to the list of deleted files in script.php with PR #40246 .

But on a git clone when running composer install it seems that they are added as an indirect dependency by some other dependency.

And so they are also added to the autoloader file "libraries/vendor/composer/autoload_real.php".

If you now use the database checker's "Update Structure" button, it also runs the deletion of files and folders in script.php, and this breaks autoloading.

This does not happen when you are not on a git clone but use the latest 5.0-dev nightly build installation package and make a new installation with that package.

richard67 commented 1 year ago

@Fedik Do you have an idea how we can fix that?

degobbis commented 1 year ago

I can confirm this.

richard67 commented 1 year ago

Of course we could fix that by removing the polyfills from the deleted files and folders lists in script.php so they will not be deleted on update (or when using the "Update Structure" button), but I think we should fix the real issue that these polyfills and another one for php72 are still used by some development dependencies on a git clone.

Fedik commented 1 year ago

~I suspect the dependecy was removed somewhere while upmerging.~ ~I not sure what the plan about PHP support for j5.~

Ah, I wrong understood, so error with files update. We probably should not remove files from /library/vendor, but replace whole folder while update? Since we manage it with composer. Hmhm, not very good idea.

How it was managed in past when dependency removed, or it a first case in our history? :)

HLeithner commented 1 year ago

The libraries/vendor folder doesn't exists in git. in case I would suspect that a 3rd party dependency requires the polyfill..

brianteeman commented 1 year ago

we have had something similar to this before with ldap. can you check and see if it is resolved by using composer require no-dev as it is only dev requirements that is loading these. You can see which ones in the composer lock. I think there is a command line option that will tell you as well

HLeithner commented 1 year ago

I created a PR which should fix this #40293

Fedik commented 1 year ago

The libraries/vendor folder doesn't exists in git

I think the the issue "more fun". Because: From one side: we should not remove anything installed by composer while joomla update. From other side: we need to remove left over files, that not in use.

HLeithner commented 1 year ago

I'm closing this since we have a PR @richard67 @degobbis can you test https://github.com/joomla/joomla-cms/pull/40293 please