[Parallel] Handle Spaced root project main script on parallel process by samsonasik · Pull Request #4813 · rectorphp/rector-src

samsonasik · 2023-08-18T15:24:23Z

@paulbalandan @sh1hab this is test for rectorphp/rector#8005

I copied existing test so I probablly missing the expecation assertion as currently shows:

'/usr/bin/php8.1' 'C:\Users\P\Desktop\Web Dev\vendor\bin\rector' 'packages-tests/Parallel/Command/WorkerCommandLineFactoryTest.php' worker --port 2000 --identifier 'identifier' 'src' --output-format 'json' --no-ansi
.'/usr/bin/php8.1' 'C:\Users\P\Desktop\Web Dev\vendor\bin\rector' 'packages-tests/Parallel/Command/WorkerCommandLineFactoryTest.php' worker --port 2000 --identifier 'identifier' 'src' --output-format 'json' --no-ansi
.'/usr/bin/php8.1' 'C:\Users\P\Desktop\Web Dev\vendor\bin\rector' 'packages-tests/Parallel/Command/WorkerCommandLineFactoryTest.php' worker --debug --port 2000 --identifier 'identifier' 'src' --output-format 'json' --no-ansi

Fixes rectorphp/rector#8005

samsonasik · 2023-08-18T15:31:55Z

Step to reproduce error locally:

download https://github.com/rectorphp/rector/files/12210766/spaced.prj.sample.zip
extract, then

cd spaced\ prj\ sample/
cd target

➜  target git:(main) vendor/bin/rector      
 0/1 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░]   0%{"fatal_errors":["Cannot load resource \"/Users/samsonasik/www/spaced\"."]}
                                                                                                                        
 [ERROR] Could not process some files, due to:                                                                          
         "Child process error: ".                                                                                       ```

samsonasik · 2023-08-18T15:43:56Z

@paulbalandan it seems double quoted is needed for generated main script:

➜  rector-src git:(spaced-main-script) mkdir foo\ bar
➜  rector-src git:(spaced-main-script) cd "foo bar"
➜  foo bar git:(spaced-main-script)

paulbalandan · 2023-08-18T16:16:07Z

@paulbalandan it seems double quoted is needed for generated main script:
➜  rector-src git:(spaced-main-script) mkdir foo\ bar
➜  rector-src git:(spaced-main-script) cd "foo bar"
➜  foo bar git:(spaced-main-script)

Yes, single quotes won't work

samsonasik · 2023-08-18T16:23:39Z

@paulbalandan On linux, single quote can go to target dir:

➜  ~ mkdir foo\ bar
➜  ~ cd 'foo bar'
➜  foo bar

so the issue is somewhere on extracting space ...

samsonasik · 2023-08-19T09:47:53Z

For side note: this issue is also happen even on Linux and macOs

samsonasik · 2023-09-14T07:33:26Z

The issue seems deep on https://github.com/reactphp/child-process, when I replace:

/Users/samsonasik/www/spaced prj sample/target/rector.php

on

rector-src/packages/Parallel/Command/WorkerCommandLineFactory.php

Lines 96 to 113 in 2384073

    
                       /** 
        
                        * On parallel, the command is generated with `--config` addition 
        
                        * Using escapeshellarg() to ensure the --config path escaped, even when it has a space. 
        
                        * 
        
                        * eg: 
        
                        *    --config /path/e2e/parallel with space/rector.php 
        
                        * 
        
                        * that can cause error: 
        
                        * 
        
                        *    File /rector-src/e2e/parallel\" was not found 
        
                        * 
        
                        * the escaped result is: 
        
                        * 
        
                        *    --config '/path/e2e/parallel with space/rector.php' 
        
                        * 
        
                        * tested in macOS and Ubuntu (github action) 
        
                        */ 
        
                       $workerCommandArray[] = escapeshellarg((string) $input->getOption(Option::CONFIG));

into:

/Users/samsonasik/www/spaced\ prj\ sample/target/rector.php

it processed as:

-/Users/samsonasik/www/spaced\ prj\ sample/target/rector.php
+/Users/samsonasik/www/spaced\\ prj\\ sample/target/rector.php

staabm · 2023-09-14T07:35:23Z

maybe open a new issue in the component to discuss it?

samsonasik · 2023-09-14T07:52:12Z

The issue seems on `--config '/path/to/rector.php' that the "root" project contains space usage, if that can be trimmed to only relative path, it probably should can be resolved, trying with manually:

-$workerCommandArray[] = \escapeshellarg((string) $input->getOption(Option::CONFIG));
+$workerCommandArray[] = 'rector.php';

solve the issue, so the condition should be:

is the config root project is equal with project root? If yes, use relative path
otherwise, back to real path

samsonasik · 2023-09-14T08:01:02Z

@staabm @paulbalandan I tried manually using this change seems solve the issue:

            $config = (string) $input->getOption(Option::CONFIG);
            $filePathHelper = new \Rector\Core\FileSystem\FilePathHelper(new \RectorPrefix202307\Symfony\Component\Filesystem\Filesystem());

            $workerCommandArray[] = escapeshellarg($filePathHelper->relativePath(realpath($config)));

I will push a commit.

samsonasik · 2023-09-14T08:16:51Z

@paulbalandan @staabm @sh1hab it should be working ok now, I will merge it and let you test 5 minutes after it merged:

3 minutes on downgrade
1-2 minutes to get updated to packagist

to test this, update with the following command:

composer config minimum-stability dev
composer config prefer-stable true
composer require --dev rector/rector:dev-main

paulbalandan · 2023-09-14T08:27:29Z

@samsonasik

$ composer info -D
codeigniter/coding-standard     v1.7.8           Official Coding Standards for CodeIgniter based on PHP CS Fixer
codeigniter/phpstan-codeigniter v1.1.3.70400     CodeIgniter extensions and rules for PHPStan
ergebnis/composer-normalize     2.36.0           Provides a composer plugin for normalizing composer.json.
fakerphp/faker                  v1.23.0          Faker is a PHP library that generates fake data for you.
kint-php/kint                   5.0.7            Kint - debugging tool for PHP developers
laminas/laminas-escaper         2.12.0           Securely and safely escape HTML, HTML attributes, JavaScript, CSS, and URLs
mikey179/vfsstream              v1.6.11          Virtual file system to mock the real file system in unit tests.
nexusphp/cs-config              v3.15.0          A factory for custom rulesets for PHP CS Fixer.
nexusphp/tachycardia            v1.4.0           Detects slow running tests in your PHPUnit-driven test suites.
php-coveralls/php-coveralls     v2.6.0           PHP client library for Coveralls API
phpstan/extension-installer     1.3.1            Composer plugin for automatic installation of PHPStan extensions
phpstan/phpstan                 1.10.34          PHPStan - PHP Static Analysis Tool
phpstan/phpstan-strict-rules    1.5.1            Extra strict and opinionated rules for PHPStan
phpunit/phpcov                  8.2.1            CLI frontend for php-code-coverage
phpunit/phpunit                 9.6.12           The PHP Unit Testing framework.
predis/predis                   v2.2.2           A flexible and feature-complete Redis client for PHP.
psr/log                         1.1.4            Common interface for logging libraries
rector/rector                   dev-main 5f78db9 Instant Upgrade and Automated Refactoring of any PHP code
vimeo/psalm                     5.15.0           A static analysis tool for finding errors in PHP applications

$ vendor/bin/rector process system
Could not open input file: C:\Users\P\Desktop\Web

samsonasik · 2023-09-14T08:29:55Z

@paulbalandan hm.., that's strange, I can't reproduce on macOS:

➜  target git:(main) ✗ pwd
/Users/samsonasik/www/spaced prj sample/target

➜  target git:(main) ✗ vendor/bin/rector process --clear-cache --dry-run
 1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
1 file with changes
===================

1) src/Foo.php:0

    ---------- begin diff ----------
@@ @@
 <?php 

-final class Foo
+final readonly class Foo
 {
-  private readonly int $bar = 1;
+  private int $bar = 1;
 }
    ----------- end diff -----------

Applied rules:
 * ReadOnlyClassRector (https://wiki.php.net/rfc/readonly_classes)


                                                                                                                        
 [OK] 1 file would have changed (dry-run) by Rector

samsonasik · 2023-09-14T08:33:43Z

@paulbalandan could you probably check if there is something missing on

rector-src/packages/Parallel/Command/WorkerCommandLineFactory.php

Lines 114 to 115 in d188b67

    
           $config = (string) $input->getOption(Option::CONFIG); 
        
           $workerCommandArray[] = escapeshellarg($this->filePathHelper->relativePath($config));

it probably some tweak on escapeshellarg vs escapeshellcmd on windows env.

thank you.

samsonasik · 2023-09-14T08:35:13Z

@paulbalandan is changing to escapeshellcmd this working?

-$workerCommandArray[] = \escapeshellarg($this->filePathHelper->relativePath($config));
+$workerCommandArray[] = \escapeshellcmd($this->filePathHelper->relativePath($config));

or probably you can check what's wrong there on windows OS :)

paulbalandan · 2023-09-14T08:44:17Z

escapeshellcmd does not work either. Does rector prevent xdebug breakpoints?

staabm · 2023-09-14T08:44:35Z

escapeshellcmd does not work either. Does rector prevent xdebug breakpoints?

yes, you need to pass --xdebug

staabm · 2023-09-14T08:45:33Z

* is the config root project is equal with project root? If yes, use relative path

* otherwise, back to real path

this still is just a workaround. I think report a problem to the upstream component is the way to solve the underlying problem, or at least get better feedback on how to fix it on our end

samsonasik · 2023-09-14T08:56:09Z

@staabm sure, I created issue for that:

Don't replace generated command from \ to \\ reactphp/child-process#104

samsonasik · 2023-09-14T08:56:21Z

@staabm sure, I created issue for that:

Don't replace generated command from \ to \\ reactphp/child-process#104

paulbalandan · 2023-09-14T08:58:43Z

Okay, some weird changes.

I used relative path for $mainScript

/**
     * @param class-string<Command> $mainCommandClass
     */
    public function create(string $mainScript, string $mainCommandClass, string $workerCommandName, InputInterface $input, string $identifier, int $port) : string
    {
+       $mainScript = $this->filePathHelper->relativePath($mainScript);
        $commandArguments = \array_slice($_SERVER['argv'], 1);
        $args = \array_merge([\PHP_BINARY, $mainScript], $commandArguments);

Run vendor/bin/rector process system (Cache cleared)

$ vendor/bin/rector process system --clear-cache
Could not open input file: C:\Users\P\Desktop\Web

Run vendor/bin/rector process system --xdebug (Cache cleared) with no set breakpoints

$ vendor/bin/rector process system --xdebug --clear-cache
 437/437 [============================] 100%

                                                                                                                        
 [OK] Rector is done!

samsonasik · 2023-09-14T09:02:44Z

@paulbalandan interesting, you may need to debug in here:

rector-src/src/Console/ConsoleApplication.php

Lines 45 to 50 in d188b67

    
           $isXdebugAllowed = $input->hasParameterOption('--xdebug'); 
        
           if (! $isXdebugAllowed) { 
        
               $xdebugHandler = new XdebugHandler('rector'); 
        
               $xdebugHandler->check(); 
        
               unset($xdebugHandler); 
        
           }

samsonasik · 2023-09-14T09:13:45Z

@paulbalandan I am testing on macOS, seems working ok:

# here spaced root project 

➜  CodeIgniter4 git:(develop) ✗ pwd
/Users/samsonasik/www/new spaced/CodeIgniter4

➜  CodeIgniter4 git:(develop) ✗ vendor/bin/rector process system 
 437/437 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 [OK] Rector is done!

I don't have windows env so I will check on linux next.

samsonasik · 2023-09-14T09:22:29Z

@paulbalandan I am testing on linux, seems working ok:

# here spaced root project 

➜  CodeIgniter4 git:(develop) ✗ pwd
/home/abdul/www/new spaced/CodeIgniter4


➜  CodeIgniter4 git:(develop) ✗ vendor/bin/rector process system 
 437/437 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

                                                                                
 [OK] Rector is done!

paulbalandan · 2023-09-14T09:34:02Z

@samsonasik I have isolated the issue to be from the terminal used.

Your fix seems to be correct. However, the terminal used will be the problem for Windows.

cmd => ok
pwsh (Powershell 7) - nope
bash on windows - ok

However, I am not sure why this is the case 🤷

samsonasik · 2023-09-14T09:38:59Z

@paulbalandan thank you 👍 , well, at least working on other terminal (cmd, bash on windows), I think we can add note as known drawbacks on README.md for running paralle under pwsh (Powershell 7):

https://github.com/rectorphp/rector-src/blob/main/build/target-repository/README.md#known-drawbacks

could you write a PR for it? Thank you :)

paulbalandan · 2023-09-15T02:16:20Z

@samsonasik see #5022

samsonasik requested a review from TomasVotruba as a code owner August 18, 2023 15:24

samsonasik force-pushed the spaced-main-script branch from eda078f to f56ba27 Compare August 18, 2023 15:25

samsonasik marked this pull request as draft August 18, 2023 15:53

samsonasik commented Aug 18, 2023

View reviewed changes

Comment thread packages/Parallel/Command/WorkerCommandLineFactory.php Outdated

samsonasik added 4 commits September 14, 2023 15:02

[Parallel] Handle Spaced root project main script on parallel process

ad67047

update main script example

ae24d73

try using double quoted for main script

db366c1

use relative path for --config

41fd69e

samsonasik force-pushed the spaced-main-script branch from a32fb72 to 41fd69e Compare September 14, 2023 08:12

use relative path for --config

95e0765

samsonasik marked this pull request as ready for review September 14, 2023 08:13

samsonasik merged commit 0e869b9 into main Sep 14, 2023

samsonasik deleted the spaced-main-script branch September 14, 2023 08:16

paulbalandan mentioned this pull request Sep 15, 2023

Add note on using pwsh on Windows with parallel mode #5022

Merged

SimonFrings mentioned this pull request Dec 22, 2023

Don't replace generated command from \ to \\ reactphp/child-process#104

Closed

samsonasik mentioned this pull request Oct 25, 2024

Execution on Windows rectorphp/rector#8871

Closed

Uh oh!

Conversation

samsonasik commented Aug 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samsonasik commented Aug 18, 2023

Uh oh!

samsonasik commented Aug 18, 2023

Uh oh!

Uh oh!

paulbalandan commented Aug 18, 2023

Uh oh!

samsonasik commented Aug 18, 2023

Uh oh!

samsonasik commented Aug 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samsonasik commented Sep 14, 2023

Uh oh!

staabm commented Sep 14, 2023

Uh oh!

samsonasik commented Sep 14, 2023

Uh oh!

samsonasik commented Sep 14, 2023

Uh oh!

samsonasik commented Sep 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paulbalandan commented Sep 14, 2023

Uh oh!

samsonasik commented Sep 14, 2023

Uh oh!

samsonasik commented Sep 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samsonasik commented Sep 14, 2023

Uh oh!

paulbalandan commented Sep 14, 2023

Uh oh!

staabm commented Sep 14, 2023

Uh oh!

staabm commented Sep 14, 2023

Uh oh!

samsonasik commented Sep 14, 2023

Uh oh!

samsonasik commented Sep 14, 2023

Uh oh!

paulbalandan commented Sep 14, 2023

Uh oh!

samsonasik commented Sep 14, 2023

Uh oh!

samsonasik commented Sep 14, 2023

Uh oh!

samsonasik commented Sep 14, 2023

Uh oh!

paulbalandan commented Sep 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samsonasik commented Sep 14, 2023

Uh oh!

paulbalandan commented Sep 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

samsonasik commented Aug 18, 2023 •

edited

Loading

samsonasik commented Aug 19, 2023 •

edited

Loading

samsonasik commented Sep 14, 2023 •

edited

Loading

samsonasik commented Sep 14, 2023 •

edited

Loading

paulbalandan commented Sep 14, 2023 •

edited

Loading