phpro / grumphp

A PHP code-quality tool
MIT License
4.11k stars 429 forks source link

Custom task not working when parallel option is enabled #1087

Closed ajourquin closed 9 months ago

ajourquin commented 1 year ago
Q A
Version 1.16.0
Bug? yes
New feature? no
Question? no
Documentation? no
Related tickets -

My configuration

grumphp:
    ascii: ~
    parallel:
        enabled: true
    tasks:
        composer_audit:
        composer:
services:
    Vendor\Grumphp\Tasks\ComposerAudit:
        arguments:
            - '@process_builder'
            - '@formatter.raw_process'
            - '@grumphp.util.filesystem'
        tags:
            - { name: grumphp.task, task: composer_audit, priority: 0 }.

Vendor\Grumphp\Tasks\ComposerAudit.php - basically the same content as GrumPHP\Task\Composer.php

<?php

declare(strict_types=1);

namespace Vendor\Grumphp\Tasks;

use GrumPHP\Formatter\ProcessFormatterInterface;
use GrumPHP\Process\ProcessBuilder;
use GrumPHP\Runner\TaskResult;
use GrumPHP\Runner\TaskResultInterface;
use GrumPHP\Task\Config\EmptyTaskConfig;
use GrumPHP\Task\Config\TaskConfigInterface;
use GrumPHP\Task\Context\ContextInterface;
use GrumPHP\Task\Context\GitPreCommitContext;
use GrumPHP\Task\Context\RunContext;
use GrumPHP\Task\TaskInterface;
use GrumPHP\Util\Filesystem;
use Symfony\Component\OptionsResolver\OptionsResolver;

class ComposerAudit implements TaskInterface
{
    protected $processBuilder;

    protected $formatter;

    protected $filesystem;

    public function __construct(
        ProcessBuilder $processBuilder,
        ProcessFormatterInterface $formatter,
        Filesystem $filesystem
    ) {
        $this->config = new EmptyTaskConfig();
        $this->processBuilder = $processBuilder;
        $this->formatter = $formatter;
        $this->filesystem = $filesystem;
    }

    public static function getConfigurableOptions(): OptionsResolver
    {
        $resolver = new OptionsResolver();
        $resolver->setDefaults([
            'file' => './composer.json'
        ]);

        return $resolver;
    }

    public function canRunInContext(ContextInterface $context): bool
    {
        return $context instanceof GitPreCommitContext || $context instanceof RunContext;
    }

    public function run(ContextInterface $context): TaskResultInterface
    {
        $config = $this->getConfig()->getOptions();

        $files = $context->getFiles()
            ->path(pathinfo($config['file'], PATHINFO_DIRNAME))
            ->name(pathinfo($config['file'], PATHINFO_BASENAME));

        if (0 === \count($files)) {
            return TaskResult::createSkipped($this, $context);
        }

        $arguments = $this->processBuilder->createArgumentsForCommand('composer');
        $arguments->add('audit');

        $process = $this->processBuilder->buildProcess($arguments);
        $process->run();

        if (!$process->isSuccessful()) {
            return TaskResult::createFailed($this, $context, $this->formatter->format($process));
        }

        $composerFile = $files->first();
        if ($config['no_local_repository'] && $composerFile && $this->hasLocalRepository($composerFile)) {
            return TaskResult::createFailed($this, $context, 'You have at least one local repository declared.');
        }

        return TaskResult::createPassed($this, $context);
    }

    private function hasLocalRepository(SplFileInfo $composerFile): bool
    {
        $json = $this->filesystem->readFromFileInfo($composerFile);
        $package = json_decode($json, true);

        if (!array_key_exists('repositories', $package)) {
            return false;
        }

        foreach ($package['repositories'] as $repository) {
            if ('path' === ($repository['type'] ?? null)) {
                return true;
            }
        }

        return false;
    }

    public function getConfig(): TaskConfigInterface
    {
        return $this->config;
    }

    public function withConfig(TaskConfigInterface $config): TaskInterface
    {
        $new = clone $this;
        $new->config = $config;

        return $new;
    }
}

Steps to reproduce:

# Generate empty folder
mkdir tmp
cd tmp
git init
echo "vendor" > .gitignore
pbpaste > grumphp.yml
composer require --dev phpro/grumphp

# Your actions
composer require --dev 3f/pygmentize:1.1

# Run GrumPHP:
grumphp run

Result:

GrumPHP is sniffing your code!

Running tasks with priority 0!
==============================

Running task 1/2: composer_audit... ✘
Running task 2/2: composer... ✔

composer_audit
==============

Uncaught TypeError in worker with message "GrumPHP\Runner\TaskHandler\TaskHandler::{closure}(): Argument #1 ($task) must be of type GrumPHP\Task\TaskInterface, __PHP_Incomplete_Class given, called in laravel-serializable-closure://static function (array $parentEnv) use ($task, $runnerContext, $next): \GrumPHP\Runner\TaskResultInterface {
                    $_ENV = array_merge($parentEnv, $_ENV);
                    /** @var TaskResultInterface $result */
                    $result = \Amp\Promise\wait($next($task, $runnerContext));

                    return $result;
                } on line 5" and code "0"; use Amp\Parallel\Worker\TaskFailureError::getOriginalTrace() for the stack trace in the worker
To skip commit checks, add -n or --no-verify flag to commit command

Expected Result:

GrumPHP is sniffing your code!

Running tasks with priority 0!
==============================

Running task 1/2: composer_audit... ✘
Running task 2/2: composer... ✔

composer_audit
==============

+-------------------+----------------------------------------------------------------------------------+
| Package           | 3f/pygmentize                                                                    |
| CVE               | NO CVE                                                                           |
| Title             | Remote Code Execution                                                            |
| URL               | https://github.com/dedalozzo/pygmentize/issues/1                                 |
| Affected versions | <1.2                                                                             |
| Reported at       | 2017-05-15T00:00:00+00:00                                                        |
+-------------------+----------------------------------------------------------------------------------+

Found 1 security vulnerability advisory affecting 1 package:
To skip commit checks, add -n or --no-verify flag to commit command

Note: When 'parallel' option is enabled an error occur. When 'parallel' option is disabled it works correctly.

veewee commented 1 year ago

Hello,

Argument #1 ($task) must be of type GrumPHP\Task\TaskInterface, __PHP_Incomplete_Class given... called in laravel-serializable-closure://static function

When using the parallel mode, the task is being serialized by the laravel/serializable-closure package. As you can see, the result is of type __PHP_Incomplete_Class. This is a special internal class that indicates that PHP was not able to unserialize the serialized version back into a valid PHP class. Most of the time, this is because of an autoloading issue or optionally an issue in your PHP INI configuration.

More info: https://www.php.net/manual/en/function.unserialize.php

You could start debugging the core of the issue here: https://github.com/phpro/grumphp/blob/master/src/Runner/TaskHandler/Middleware/ParallelProcessingMiddleware.php#L64

ajourquin commented 1 year ago

Thanks for the input, i'll take a look

ajourquin commented 1 year ago

Actually, when you install grumphp locally in your project and run it from vendor/bin/grumphp it works fine. ./vendor/bin/grumphp run

When you run grumphp from a global installation it's not working grumphp run

veewee commented 1 year ago

I don't have a clue at the moment. Most of this is how it works internal in amp/parallel. For grumphp global we do load multiple composer autoload files in order to be able to load both project and global classes. So maybe something goes wrong somewhere in there; but I honestly don't know how amp/parallel deals with autoloaders.

veewee commented 9 months ago

Closing because of inactivity. Feel free to reopen if this is still blocking you.