laravel-shift / workbench-issues

Issue tracking for the Shift Workbench desktop app.
https://laravelshift.com/workbench
2 stars 0 forks source link

php/switch-to-match warnings and then failed to run #6

Closed ejunker closed 3 years ago

ejunker commented 3 years ago

Desktop:

Describe the bug I ran the php/switch-match task and got some warnings and then it failed. It seemed to take a long time but this is my first time running a task that uses Rector so maybe they are always slower. I am wondering if it is scanning all files instead of just *.php files. In the warning message you can see it could not read rr which is the RoadRunner server from when I was testing Laravel Octane. I am not even sure why it would need to read that file. I am not sure if the warnings are the reason it failed or just coincidence.

Warning: sha1_file(/project/rr): Failed to open stream: Operation not permitted in /root/.composer/vendor/rector/rector/packages/Caching/Detector/ChangedFilesDetector.php on line 92
Warning: sha1_file(/project/rr): Failed to open stream: Operation not permitted in /root/.composer/vendor/rector/rector/packages/Caching/Detector/ChangedFilesDetector.php on line 92
Failed to run rector: convert-switch-to-match: #0 phar:///opt/shift/workbench/src/Tasks/Workbench/ConvertSwitchToMatch.php(18): Shift\Tasks\Workbench\ConvertSwitchToMatch->rector('/opt/shift/reso...')
#1 phar:///opt/shift/workbench/workbench.php(89): Shift\Tasks\Workbench\ConvertSwitchToMatch->perform()
#2 /opt/shift/workbench(9): require('phar:///opt/shi...')
#3 {main}
{"event":"step-failed","data":{"step":"phpswitch-to-match","runtime":"632.194897"}}
jasonmccreary commented 3 years ago

@ejunker, the Rector backed tasks are indeed slower as they run static analysis on your full project. We've tried to limit the files scanned, but there might be come kind of additional file extension filter we could add.

I'm also not sure this was the ultimate reason for the failure though. Let us make some changes and push a patch to see if we can get this one a bit farther along.

ejunker commented 3 years ago

It does not seem to be specific to the php/switch-to-match task as I also ran the php/nullsafe-operator task and got the same warnings and error.

jasonmccreary commented 3 years ago

It's not. Nearly of the PHP tasks are backed by Rector. So whatever fix we make will hopefully both of these.

jasonmccreary commented 3 years ago

@ejunker, could you tell me a little more about this rr? I don't think Rector is trying to scan it as PHP so much as iterating over it. Makes me think it might be a symlink or some kind of binary file…

ejunker commented 3 years ago
❯ ls -l rr
--wxrw--wt 1 dhi dhi 35998032 Apr  6 18:56 rr

Strange that I do not have read permissions.

❯ file rr
rr: sticky writable, executable, regular file, no read permission

As for where the file came from, I was using Laravel Octane and it downloaded automatically

https://laravel.com/docs/8.x/octane#installation

RoadRunner is powered by the RoadRunner binary, which is built using Go. The first time you start a RoadRunner based Octane server, Octane will offer to download and install the RoadRunner binary for you.

I removed the file and the warnings went away though it still failed

Failed to run rector: convert-switch-to-match: #0 phar:///opt/shift/workbench/src/Tasks/Workbench/ConvertSwitchToMatch.php(18): Shift\Tasks\Workbench\ConvertSwitchToMatch->rector('/opt/shift/reso...')
#1 phar:///opt/shift/workbench/workbench.php(89): Shift\Tasks\Workbench\ConvertSwitchToMatch->perform()
#2 /opt/shift/workbench(9): require('phar:///opt/shi...')
#3 {main}
{"event":"step-failed","data":{"step":"phpswitch-to-match","runtime":"598.887001"}}

I wonder if it is running out of memory or timing out somehow. Most of the runs are about 600 seconds so maybe there is a 10 minute timeout. Unfortunately the output does not say why it failed. If there was a debug or verbose mode I could run with that to get more info.

jasonmccreary commented 3 years ago

The permissions would explain the warnings. But yeah, ultimately seems unrelated to the task failing.

Given the time it's taking it makes me wonder how large the project is or if there are other PHP libraries within the project, yet outside the vendor folder.

We don't have a debug mode at this time. We'll likely need to build one given the various projects and tasks we're trying to support.

In the meantime, if you are willing, you could try to run same task/project within the cloud-based Workbench. That would give me a little more debugging at where it might be failing.

ejunker commented 3 years ago

I did a build with the web version and it failed. https://laravelshift.com/account/build/507 but it also says Build #1608

jasonmccreary commented 3 years ago

There was a little more information in the build log from the server. Looks like there is an undefined class for Faker\Provider\Base within app/Providers/FakerServiceProvider.php.

Since these tools perform full static analysis on your files, all of them would need to be without parse/runtime errors.

ejunker commented 3 years ago

Thanks for your time to look into this. I installed rector/rector 0.11.32 and ran it on my app directory and expected to see the same error that I got when running the Shift workbench but it ran just fine.

❯ vendor/bin/rector process app

 [OK] 9 files have been changed by Rector

Here is my rector.php:

<?php

declare(strict_types=1);

use Rector\Php74\Rector\Property\TypedPropertyRector;
use Rector\Set\ValueObject\SetList;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;

return static function (ContainerConfigurator $containerConfigurator): void {
    // get parameters
    $parameters = $containerConfigurator->parameters();

    // Define what rule sets will be applied
    //$containerConfigurator->import(SetList::DEAD_CODE);

    // get services (needed for register a single rule)
    $services = $containerConfigurator->services();

    // register a single rule
    $services->set(\Rector\Php80\Rector\Switch_\ChangeSwitchToMatchRector::class);
};

I'm not sure why I don't see the error unless you are using a different version of rector or if it is due to a different config or command line to run rector. I'm not sure what to do next. I don't want to take up your time, but if you want to investigate this further just let me know if you want me to try anything else.

jasonmccreary commented 3 years ago

@ejunker yeah, let's definitely get to the bottom of it as it's likely affecting others too. If you could email support (support@laravelshift.com), we can set up a time to try and run this together.

jasonmccreary commented 3 years ago

@ejunker, there are some updates in the latest path release (0.3.2) which may help with the issue. Please try again running some of the PHP tasks again on your project and let us know if they continue to fail.

ejunker commented 3 years ago

@jasonmccreary I updated to the latest version and now it runs much faster (266s) but it still got an error. When I was testing with Rector I had doubts about which directories it was scanning based on the Option:SKIP set in the config when combined with directories specified on the command line. It seemed like it may be a bug in Rector. I'll try to do some more testing to see if I can narrow down where the problem is.

jasonmccreary commented 3 years ago

Were you running that latest version (0.4.2? We've since pushed more changed to hopefully improve these tasks.