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

Default value of "manager-timeout" setting is incompatible with Composer 2.3+ #44

Closed
soren121 opened this issue Apr 4, 2022 · 2 comments · Fixed by #45
Closed

Default value of "manager-timeout" setting is incompatible with Composer 2.3+ #44

soren121 opened this issue Apr 4, 2022 · 2 comments · Fixed by #45

Comments

@soren121
Copy link
Contributor

soren121 commented Apr 4, 2022

Foxy's documentation describes the "manager-timeout" config option as being a nullable integer, with the default being null. In Composer 2.3, the ProcessExecutor class has been updated, and its setTimeout method now requires an int value for the timeout parameter. This makes Foxy's default configuration incompatible with Composer 2.3+.

Here's the stack trace thrown when using the default configuration with Composer 2.3:

Merging Composer dependencies in the asset package
yarn check v1.22.18
success Folder in sync.
Done in 0.44s.
Updating yarn dependencies

In ProcessExecutor.php line 388:
                                                                                                                                                                                                  
  [TypeError]                                                                                                                                                                                     
  Argument 1 passed to Composer\Util\ProcessExecutor::setTimeout() must be of the type int, null given, called in /home/nicholas/projects/test/vendor/foxy/foxy/Asset/AbstractAssetManage  
  r.php on line 210                                                                                                                                                                               
                                                                                                                                                                                                  

Exception trace:
  at phar:///home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar/src/Composer/Util/ProcessExecutor.php:388
 Composer\Util\ProcessExecutor::setTimeout() at /home/nicholas/projects/test/vendor/foxy/foxy/Asset/AbstractAssetManager.php:210
 Foxy\Asset\AbstractAssetManager->run() at /home/nicholas/projects/test/vendor/foxy/foxy/Solver/Solver.php:103
 Foxy\Solver\Solver->solve() at phar:///home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar/src/Composer/Plugin/PluginManager.php(273) : eval()'d code:205
 Foxy\Foxy_composer_tmp0->solveAssets() at n/a:n/a
 call_user_func() at phar:///home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar/src/Composer/EventDispatcher/EventDispatcher.php:202
 Composer\EventDispatcher\EventDispatcher->doDispatch() at phar:///home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar/src/Composer/EventDispatcher/EventDispatcher.php:125
 Composer\EventDispatcher\EventDispatcher->dispatchScript() at phar:///home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar/src/Composer/Installer.php:372
 Composer\Installer->run() at phar:///home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar/src/Composer/Command/UpdateCommand.php:241
 Composer\Command\UpdateCommand->execute() at phar:///home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar/vendor/symfony/console/Command/Command.php:298
 Symfony\Component\Console\Command\Command->run() at phar:///home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar/vendor/symfony/console/Application.php:1015
 Symfony\Component\Console\Application->doRunCommand() at phar:///home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar/vendor/symfony/console/Application.php:299
 Symfony\Component\Console\Application->doRun() at phar:///home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar/src/Composer/Console/Application.php:334
 Composer\Console\Application->doRun() at phar:///home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar/vendor/symfony/console/Application.php:171
 Symfony\Component\Console\Application->run() at phar:///home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar/src/Composer/Console/Application.php:130
 Composer\Console\Application->run() at phar:///home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar/bin/composer:83
 require() at /home/nicholas/.phpenv/versions/7.4.28/composer/composer.phar:29

A workaround for now is to set a non-null "manager-timeout" value in your composer.json ("300" is the default in Composer):

"config": {
    "foxy": {
        "manager-timeout": 300
    },
    "allow-plugins": {
        "foxy/foxy": true
    }
},
@soren121
Copy link
Contributor Author

soren121 commented Apr 5, 2022

I'll note that Foxy's behavior has always been wrong here-- the docblock for that Composer method has always specified that timeout should be an integer. It's only breaking now because Composer is enforcing that.

I feel like the fix here is to simply not call Composer\Util\ProcessExecutor::setTimeout() if "manager-timeout" is null, thereby keeping Composer's default timeout in place. I can submit a PR for this if that sounds okay.

@francoispluchino
Copy link
Member

The timeout could be null because the ProcessExecutor class uses the Symfony Process component whose timeout may be null, cf. the constructor of the Process class and that the property was not Typed. It was therefore reasonable to consider the PHP doc as incomplete, which often happens, and not as a deliberate choice of the developers.

Now that the property is Typed, it is therefore necessary to use the constant PHP_INT_MAX to obtain a pseudo equivalent to an unlimited timeout. The method Foxy\Config\Config::get() has a second argument to set a default value.

And any contribution is welcome :-)

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

Successfully merging a pull request may close this issue.

2 participants