Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
richard67 opened this issue Apr 1, 2023 · 9 comments

Comments

@richard67
Copy link
Member

richard67 commented Apr 1, 2023

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
Copy link
Member Author

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

@degobbis
Copy link
Contributor

degobbis commented Apr 1, 2023

I can confirm this.

@richard67
Copy link
Member Author

richard67 commented Apr 1, 2023

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
Copy link
Member

Fedik commented Apr 2, 2023

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
Copy link
Member

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

@brianteeman
Copy link
Contributor

brianteeman commented Apr 2, 2023

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
Copy link
Member

I created a PR which should fix this #40293

@Fedik

This comment was marked as outdated.

@HLeithner
Copy link
Member

I'm closing this since we have a PR @richard67 @degobbis can you test #40293 please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants