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

Composer 2.2, running composer update uses incorrect composer source file somehow #10401

Closed
hostep opened this issue Dec 28, 2021 · 25 comments
Closed
Labels
Milestone

Comments

@hostep
Copy link

hostep commented Dec 28, 2021

Hi

This is something weird that I encountered after upgrading to composer 2.2.1
I have 2 different Magento2 projects, they both run the same version of Magento (2.3.7-p2) but have different custom dependencies.
In one project I have no problem using composer 2.2.1, but in the other project, I get this error when I try to run composer update:

PHP Fatal error:  Uncaught Error: Call to undefined method Composer\Installer::setPlatformRequirementFilter() in phar://composer.phar/src/Composer/Command/UpdateCommand.php:239
Stack trace:
#0 phar://composer.phar/vendor/symfony/console/Command/Command.php(245): Composer\Command\UpdateCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#1 phar://composer.phar/vendor/symfony/console/Application.php(835): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#2 phar://composer.phar/vendor/symfony/console/Application.php(185): Symfony\Component\Console\Application->doRunCommand(Object(Composer\Command\UpdateCommand), Object(Symfony\Component\Console\Input\A in phar://composer.phar/src/Composer/Command/UpdateCommand.php on line 239

So it can't find that setPlatformRequirementFilter method in Composer\Installer. Which is strange because it exists in the phar file.
Magento2 does come with composer/composer as dependency as well, which is currently at version 2.0.13 and can't be updated further for some reason I haven't looked into yet (in both projects).
That version of composer does not have that setPlatformRequirementFilter method yet.
So my guess is that while executing composer update, somehow composer tries to load Composer\Installer from vendor/composer/composer instead of from the composer.phar file. But only on one project and not in the other, which is very strange ...

Any ideas?

@hostep
Copy link
Author

hostep commented Dec 28, 2021

Update, ok confirmed my suspicion, I edited the vendor/composer/composer/src/Composer/Installer.php file and added:

    public function setPlatformRequirementFilter()
    {
        die('test');
    }

And now executing composer update results in test being outputted.

So that's a very bad bug, composer shouldn't use the source files from inside vendor/composer/composer but from the composer.phar file that I'm executing.

@hostep
Copy link
Author

hostep commented Dec 28, 2021

Okay I can reproduce this on a vanilla Magento 2.3.7-p2 installation.
Steps to reproduce:

  1. If you don't have authentication keys for repo.magento.com, create a new account on https://account.magento.com/ and create a set of authentication keys and set these up in your local environment
  2. Use composer 2.2.1
  3. Run:
$ composer create-project --no-install --repository-url=https://repo.magento.com/ magento/project-community-edition=2.3.* ./some-directory`
$ cd some-directory
$ composer install
# wait a bit until installation is finished
$ composer update
PHP Fatal error:  Uncaught Error: Call to undefined method Composer\Installer::setPlatformRequirementFilter() in phar://composer.phar/src/Composer/Command/UpdateCommand.php:239
Stack trace:
#0 phar://composer.phar/vendor/symfony/console/Command/Command.php(245): Composer\Command\UpdateCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#1 phar://composer.phar/vendor/symfony/console/Application.php(835): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#2 phar://composer.phar/vendor/symfony/console/Application.php(185): Symfony\Component\Console\Application->doRunCommand(Object(Composer\Command\UpdateCommand), Object(Symfony\Component\Console\Input\A in phar://composer.phar/src/Composer/Command/UpdateCommand.php on line 239

@Seldaek
Copy link
Member

Seldaek commented Dec 28, 2021

Yeah, requiring composer/composer itself is a pretty bad idea usually. I don't know why it's required in your case, but I'd say updating it to a version equivalent to your composer.phar, or running Composer exclusively from vendor/bin/composer would be the best way to get around this. We cannot really ignore the vendor dir as it is needed for plugins.

@Seldaek
Copy link
Member

Seldaek commented Dec 28, 2021

To update it in the broken project, try composer --no-plugins --no-scripts update composer/composer

@Seldaek Seldaek added this to the 2.2 milestone Dec 28, 2021
@hostep
Copy link
Author

hostep commented Dec 28, 2021

Yeah that's not going to work I'm afraid. Magento always runs behind with updating their dependencies, and their super slow release cycle isn't helping either (1 release per 6 months).

Yeah, requiring composer/composer itself is a pretty bad idea usually. I don't know why it's required in your case

Magento requires it: https://github.com/magento/magento2/blob/2.3.7-p2/composer.json#L37

I can't update composer/composer in this case because a bunch of other dependencies rely on composer/xdebug-handler 1.*

But why is the composer phar/binary not using its own source code, it makes no sense that it would priotise source code in vendor/composer/composer before its own code?
This used to work fine in composer 2.0 or 2.1, it's a new problem introduced in 2.2. You're sure it's not a bug?

@Seldaek
Copy link
Member

Seldaek commented Dec 28, 2021

It's just a "new" problem because we added a new method there so it's not finding it in the code from vendor. The problem has always existed though.

I believe we have already discussed/attempted switching to using internal vendors before project vendors, but the issue is it is sometimes better but sometimes worse. In some cases you may have a plugin requiring the latest symfony/process to function for example, and Composer is typically compatible with a broad range of versions of its dependencies so we can be more flexible than a plugin which does not necessarily expect version X to be loaded.

For the composer/composer files themselves perhaps we could load them exclusively from the phar, but I am not entirely sure if this would be safe or break things for a case like what Magento does here.

What I'd recommend is that you use vendor/bin/composer instead of a phar, if you have composer in the vendor dir already, it'll avoid any kind of conflicts.

@hostep
Copy link
Author

hostep commented Dec 29, 2021

Thanks for the explanation @Seldaek, that's much appreciated!

I wasn't aware of this mechanic and I think I understand it for the dependencies that composer uses. But I don't really understand why it would be necessary for composer's own code.
It means when we update the composer phar to a new version that potentially only a part of the new code gets activated and still older code might be used which is very much not expected in my opinion and might lead to very unpredictable behavior.

Is there a way to try this out, to exclusively use composer/composer code from the phar instead of from the vendor directory? If it turns out this causes problems in certain situations, maybe a new flag can be introduced in the composer.json file so end users can choose which behavior they want to use?

Thanks!

@Seldaek
Copy link
Member

Seldaek commented Dec 29, 2021

There is no way to try this out, and frankly I am not even sure how I'd implement this cleanly.. It'd also end up including files we depend on earlier than the project autoloader is available so it would defeat the point of keeping the project dependencies active IMO. This is a shitty problem I can see that, but there's no easy solution AFAICT.

If dependencies are blocking you from updating composer/composer in vendor due to a composer/xdebug-handler require, I suggest creating issues/PRs on these dependencies to get that resolved.

@stof
Copy link
Contributor

stof commented Dec 29, 2021

I don't understand how that's related to composer/xdebug-handler. composer/xdebug-handler does not depend on composer/composer and so it is not the reason why composer/composer is installed in your vendor directory.

@Seldaek
Copy link
Member

Seldaek commented Dec 29, 2021

The problem is Composer 2.1+ requires xdebug-handler ^2, so if some other dependency is blocked with xdebug-handler ^1 it will prevent updating composer/composer to latest in vendor. Magento is the one depending on composer/composer, but they allow ^2 so that's not blocking anything.

@hostep
Copy link
Author

hostep commented Dec 29, 2021

Okay guys, please ignore this xdebug handler thing. This issue is not a support request, it's a bug report.

Around this remark:

It's just a "new" problem because we added a new method there so it's not finding it in the code from vendor. The problem has always existed though.

I just spend an hour experimenting with various versions of composer.
As a test, I edited the file vendor/composer/composer/src/Composer/Installer.php and added a die('test') statement on the first line of the public static function create(IOInterface $io, Composer $composer) method.

After testing various versions of composer, only versions 2.2.0, 2.2.1 and 2.2.2 outputted test upon running composer update
All versions below 2.2.0 didn't have this behavior.
So this does mean this is a new regression bug introduced in 2.2.0, right?

Now, I also tried to git bisect the composer source code, but because of a weird issue with the Laminas\ZendFrameworkBridge package, I had to skip a bunch of tests.
The output of git bisect gave me this list of possible commits that could have introduced the problem:

If I have to guess, the most likely candidates are either one of these, but I can't say that with 100% confidence:

@Seldaek: is this enough info for you to start considering this as a bug report? Or shall I continue searching to hopefully find even more info?

@hostep
Copy link
Author

hostep commented Dec 29, 2021

Hmm, this might be a bug introduced in laminas/laminas-zendframework-bridge#90
If I return null; in that getClassLoaderFromVendorDirectory method that got introduced there, I get rid of this bug here.

@boesing, @Ocramius: any ideas?
Do you agree this might be a bug in laminas/laminas-zendframework-bridge (I'm using version 1.4.1)? Should I open a new issue over there?

Update: downgrading laminas/laminas-zendframework-bridge to version 1.4.0 solves my issue.
So this is either a bug in laminas/laminas-zendframework-bridge version 1.4.1 or a bug in composer/composer 2.2.0+ or a combination of both I think ...

@Ocramius
Copy link
Contributor

I have no idea :D

@boesing
Copy link
Contributor

boesing commented Dec 29, 2021

I did not even understand what is happening from just reading this issue tbh.
besides this, the laminas package does not require composer/composer, only the plugin API. It also does not call the method mentioned in the issue description.
Due to the lack of source code where this error occurs, its impossible to make correct assumptions.

Could you please add the laminas package temporarily to your projects replace section? this would prevent the installation of the package and the results can be used to narrow it down to the laminas package or not:

{
    "replace": {
        "laminas/laminas-zendframework-bridge": "*"
    }
}

@hostep
Copy link
Author

hostep commented Dec 29, 2021

Sorry, I understand the issue has become quite a mess.
The steps to reproduce are in: #10401 (comment)
Magento comes with a requirement on both composer/composer and laminas/laminas-zendframework-bridge.

The issue is:
When I execute a globally installed composer.phar file (version 2.2.2), it starts executing code inside the phar, but the moment it loads laminas/laminas-zendframework-bridge it starts switching to the source code of composer/composer which is installed in vendor. Probably due to you guys loading in another autoload file in laminas/laminas-zendframework-bridge.

What I expect to happen, is when I run a globally installed composer.phar file, it executes all the code from that file, and doesn't suddenly switch to running source code from an installed composer/composer repository that got installed via composer. Which could potentially have a completely different version than the globally installed composer.phar file.

Hope this makes it a bit more clear?

@Seldaek
Copy link
Member

Seldaek commented Dec 29, 2021

Ok I see.. Maybe you could try this:

composer config prepend-autoloader false
composer dump-autoload

Then try again see if that fixes it?

Possibly you'll need to run these commands with --no-plugins to avoid breakage.

@hostep
Copy link
Author

hostep commented Dec 29, 2021

Interesting trick, that seems to work over here! Even without --no-plugins

But is that only a workaround, or do you believe - as I do - that there is still a bug in here somewhere?
The more I think about it, the more it sounds like a security issue.
A bad actor could in theory create a composer package that has some php classes with the Composer namespace and if he could trick somebody into installing his package, than executing composer could potentially start running the code from that bad actor's package without the user noticing or expecting this (but I guess the same could be said for composer plugins, so it's probably not as bad as I imagine it).

@boesing
Copy link
Contributor

boesing commented Dec 29, 2021

Since laminas/laminas-zendframework-bridge is being abandoned, I doubt there is a way in fixing this issue from our side.
Our component depends on the autoloader which is dumped by composer.
If that leads to issues in composer itself, I'm afraid there is nothing we can do about it.
The zendframework bridge is not even a composer plugin and the main reason why we end up running into problems is that all autoloaders are now required/loaded by composer itself in its installation process.

I think composer needs to somehow isolate itself from being overruled by its own composer dependency. This would be a fix which will solve all this mess I guess. But I think this would also mean a lot of work for composer itself.
Not sure if something like box-project/box would solve these problems.

@Seldaek
Copy link
Member

Seldaek commented Dec 30, 2021

If this package is still causing issues then perhaps I should remove the version check in 54123e4 and simply always exclude it.

That's the reason that downgrading it to 1.4.0 fixes the issue.

Switching the application autoloader to be appended instead of prepended also fixes it, but that usually causes other issues with PHPUnit and co, which is why we default to prepend.

@Seldaek
Copy link
Member

Seldaek commented Dec 30, 2021

OK please confirm if things work with composer self-update --snapshot @hostep

@hostep
Copy link
Author

hostep commented Dec 30, 2021

Confirmed that the snapshot build fixes the issue!

However, this is just a workaround. If another composer module does the same thing we will have the same issue.
I still believe there is a deeper bug here, in that if you run a composer.phar file it shouldn't start loading code from inside vendor/composer/composer in a project.
But it sounds like that's very difficult to solve unfortunately if I understand you correctly.

@Seldaek
Copy link
Member

Seldaek commented Dec 30, 2021

The thing is composer isn't loading it, well it is only loading enough to allow the plug-ins to run. The problem in this case is the plug-in depends on that package which then includes the entire application autoloader and prepends it to the composer one, which ends up messing up the autoload setup.

So yes it's possible some other package would do something equally messed up, but I'm hoping people would notice in time to fix it before it becomes as widespread as laminas/laminas-zendframework-bridge now is. The problem in this case is composer changed the way things are included so that broke the assumptions the package made, which is the only reason I'm willing to workaround the issue here.

@julienloizelet
Copy link

Hi,
just for information, I was facing the same kind of issue with Magento 2.4.3 and composer 2.2.2 :

  • No issue to run composer create --repository=https://repo.magento.com/ magento/project-community-edition m243 2.4.3
  • But then, as I was trying to add a composer repository with composer config repositories.my-repo path my-own-modules/something/, an error was thrown :
PHP Fatal error:  Uncaught TypeError: Argument 3 passed to Composer\Package\Locker::__construct() must be an instance of Composer\Repository\RepositoryManager, instance of Composer\Installer\InstallationManager given, called in phar:///usr/local/bin/composer/src/Composer/Factory.php on line 446 and defined in /var/www/html/vendor/composer/composer/src/Composer/Package/Locker.php:54
Stack trace:
#0 phar:///usr/local/bin/composer/src/Composer/Factory.php(446): Composer\Package\Locker->__construct(Object(Composer\IO\ConsoleIO), Object(Composer\Json\JsonFile), Object(Composer\Installer\InstallationManager), '{\n    "name": "...', Object(Composer\Util\ProcessExecutor))
#1 phar:///usr/local/bin/composer/src/Composer/Factory.php(643): Composer\Factory->createComposer(Object(Composer\IO\ConsoleIO), Array, false, '/var/www/html', true, false)
#2 phar:///usr/local/bin/composer/src/Composer/Console/Application.php(445): Composer\Factory::create(Object(Composer\IO\ConsoleIO), NULL, false, false)
#3 phar:///usr/local/bin/composer/src/Compo in /var/www/html/vendor/composer/composer/src/Composer/Package/Locker.php on line 54

Fatal error: Uncaught TypeError: Argument 3 passed to Composer\Package\Locker::__construct() must be an instance of Composer\Repository\RepositoryManager, instance of Composer\Installer\InstallationManager given, called in phar:///usr/local/bin/composer/src/Composer/Factory.php on line 446 and defined in /var/www/html/vendor/composer/composer/src/Composer/Package/Locker.php:54
Stack trace:
#0 phar:///usr/local/bin/composer/src/Composer/Factory.php(446): Composer\Package\Locker->__construct(Object(Composer\IO\ConsoleIO), Object(Composer\Json\JsonFile), Object(Composer\Installer\InstallationManager), '{\n    "name": "...', Object(Composer\Util\ProcessExecutor))
#1 phar:///usr/local/bin/composer/src/Composer/Factory.php(643): Composer\Factory->createComposer(Object(Composer\IO\ConsoleIO), Array, false, '/var/www/html', true, false)
#2 phar:///usr/local/bin/composer/src/Composer/Console/Application.php(445): Composer\Factory::create(Object(Composer\IO\ConsoleIO), NULL, false, false)
#3 phar:///usr/local/bin/composer/src/Compo in /var/www/html/vendor/composer/composer/src/Composer/Package/Locker.php on line 54

After doing composer self-update --snapshot, the error disappears.

@hostep
Copy link
Author

hostep commented Dec 30, 2021

Thanks @Seldaek for all the information and trying to help out here. It does make sense the way you describe it.

I might have sounded a bit short or annoyed but note that I'm very grateful for the feedback here and the proposed workarounds!

Thanks again!

@Seldaek
Copy link
Member

Seldaek commented Dec 30, 2021

@hostep no worries. you threw me off with the composer/composer require because it was in fact not the root cause of the problem, which is why I thought this was a somewhat bogus issue for so long, but glad we figured it out eventually.

@Seldaek Seldaek added Bug and removed Support labels Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants