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

[Parallel] Enable parallel on downgrade build on not started with ^(vendor|utils) #1621

Merged
merged 4 commits into from Jan 4, 2022

Conversation

samsonasik
Copy link
Member

@samsonasik samsonasik commented Jan 4, 2022

@TomasVotruba this PR enabling parallel on downgrade build on directory list not start with ^(vendor|utils), with create extended config with enable parallel, because when directory list start with vendor or utils, it currently got notice error like in PR #1613 and #1610

I tested locally and it seems working ok:

Screen Shot 2022-01-04 at 19 43 18

Screen Shot 2022-01-04 at 19 45 03

@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba it is ready for review.

@TomasVotruba
Copy link
Member

I've just merged #1610

Could you check what effect it has on this PR?

@jtojnar jtojnar mentioned this pull request Jan 4, 2022
@samsonasik
Copy link
Member Author

@TomasVotruba enabling vendor for parallel seems got the following error now:

[NOTE] Downgrading 'vendor/symfony/console' directory


                                                                                                                        
 [ERROR] Could not process "" file, due to:                                                                             
         "Child process error: PHP Fatal error:  Declaration of                                                         
         Rector\Core\DependencyInjection\Loader\ConfigurableCallValuesCollectingPhpFileLoader::load($resource, ?string  
         $type = null): void must be compatible with                                                                    
         Symfony\Component\DependencyInjection\Loader\PhpFileLoader::load($resource, $type = null) in                   
         src/DependencyInjection/Loader/ConfigurableCallValuesCollectingPhpFileLoader.php:26                            
         Fatal error: Declaration of                                                                                    
         Rector\Core\DependencyInjection\Loader\ConfigurableCallValuesCollectingPhpFileLoader::load($resource, ?string  
         $type = null): void must be compatible with                                                                    
         Symfony\Component\DependencyInjection\Loader\PhpFileLoader::load($resource, $type = null) in                   
         src/DependencyInjection/Loader/ConfigurableCallValuesCollectingPhpFileLoader.php:26                            
         ".                                                                                                             
                                                                                                                        

                                                                                                                        
 [ERROR] Could not process "" file, due to:                                                                             
         "Child process error: PHP Fatal error:  Declaration of                                                         
         Rector\Core\DependencyInjection\Loader\ConfigurableCallValuesCollectingPhpFileLoader::load($resource, ?string  
         $type = null): void must be compatible with                                                                    
         Symfony\Component\DependencyInjection\Loader\PhpFileLoader::load($resource, $type = null) in                   
         src/DependencyInjection/Loader/ConfigurableCallValuesCollectingPhpFileLoader.php:26                            
         Fatal error: Declaration of                                                                                    
         Rector\Core\DependencyInjection\Loader\ConfigurableCallValuesCollectingPhpFileLoader::load($resource, ?string  
         $type = null): void must be compatible with                                                                    
         Symfony\Component\DependencyInjection\Loader\PhpFileLoader::load($resource, $type = null) in                   
         src/DependencyInjection/Loader/ConfigurableCallValuesCollectingPhpFileLoader.php:26                            
         ".                                                                                                             
                                                                                                                        

                                                                                                                        
 [ERROR] Could not process "" file, due to:                                                                             
         "Child process error: PHP Fatal error:  Declaration of                                                         
         Rector\Core\DependencyInjection\Loader\ConfigurableCallValuesCollectingPhpFileLoader::load($resource, ?string  
         $type = null): void must be compatible with                                                                    
         Symfony\Component\DependencyInjection\Loader\PhpFileLoader::load($resource, $type = null) in                   
         src/DependencyInjection/Loader/ConfigurableCallValuesCollectingPhpFileLoader.php:26                            
         Fatal error: Declaration of                                                                                    
         Rector\Core\DependencyInjection\Loader\ConfigurableCallValuesCollectingPhpFileLoader::load($resource, ?string  
         $type = null): void must be compatible with                                                                    
         Symfony\Component\DependencyInjection\Loader\PhpFileLoader::load($resource, $type = null) in                   
         src/DependencyInjection/Loader/ConfigurableCallValuesCollectingPhpFileLoader.php:26                            
         ".                                                                                                             
                                                                                                                        

                                                                                                                        
 [ERROR] Could not process "" file, due to:                                                                             
         "Child process error: PHP Fatal error:  Declaration of                                                         
         Rector\Core\DependencyInjection\Loader\ConfigurableCallValuesCollectingPhpFileLoader::load($resource, ?string  
         $type = null): void must be compatible with                                                                    
         Symfony\Component\DependencyInjection\Loader\PhpFileLoader::load($resource, $type = null) in                   
         src/DependencyInjection/Loader/ConfigurableCallValuesCollectingPhpFileLoader.php:26                            
         Fatal error: Declaration of                                                                                    
         Rector\Core\DependencyInjection\Loader\ConfigurableCallValuesCollectingPhpFileLoader::load($resource, ?string  
         $type = null): void must be compatible with                                                                    
         Symfony\Component\DependencyInjection\Loader\PhpFileLoader::load($resource, $type = null) in                   
         src/DependencyInjection/Loader/ConfigurableCallValuesCollectingPhpFileLoader.php:26    

@TomasVotruba
Copy link
Member

TomasVotruba commented Jan 4, 2022

I see. The parallel cannot be used in downgrade, as one process downgrades a /vendor file and the other process is already working with invalid type declarations.

We should upgrade the rector.php downgrade config to make it non-parallel.

@TomasVotruba
Copy link
Member

I've merged the rectify PR, could you rebase on main?

@samsonasik
Copy link
Member Author

samsonasik commented Jan 4, 2022

@TomasVotruba rebased, I updated for more specific directory that need to be excluded in parallel process 2c46b1e

I added build/config/config-downgrade-parallel.php for parallel process, ref https://github.com/rectorphp/rector-src/pull/1621/files#diff-7818a393147bf199623465fc307c79e32fa62142018c0341644fb067fce670e4

Original build/config/config-downgrade.php is not parallel.

The script updated to not use parallel for the following directory started with:

(vendor/(symfony|symplify|rector)|src|utils)

ref https://github.com/rectorphp/rector-src/pull/1621/files#diff-f86931682f31678797031bd26cc05c1206f8bfc60fc66f4b9ef9a4ca46d5c596

I tried and it seems working.

@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba I think it is ready, let's give it a try?

Comment on lines 37 to 41
if [[ "$directory" =~ ^(vendor/(symfony|symplify|rector)|src|utils).* ]]; then
php -d memory_limit=-1 bin/rector process $directory --config build/config/config-downgrade.php --working-dir $BUILD_DIRECTORY --ansi
else
php -d memory_limit=-1 bin/rector process $directory --config build/config/config-downgrade-parallel.php --working-dir $BUILD_DIRECTORY --ansi
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep the former single command, as this will be very confusing for anyone who wants to downgrade.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I will move config to variable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried updated script to move to variable:

    if [[ "$directory" =~ ^(vendor/(symfony|symplify|rector)|src|utils).* ]]; then
        config="build/config/config-downgrade.php"
    else
        config="build/config/config-downgrade-parallel.php"
    fi

    # --working-dir is needed, so "SKIP" parameter is applied in absolute path of nested directory
    php -d memory_limit=-1 bin/rector process $directory --config $config --working-dir $BUILD_DIRECTORY --ansi

it seems I got error on php 7.4:

➜  rector-build git:(downgrade-parallel) ✗ /opt/homebrew/Cellar/php@7.4/7.4.26_2/bin/php bin/rector -vvv --debug

 [ERROR] syntax error, unexpected 'private' (T_PRIVATE), expecting variable (T_VARIABLE) 

probably I wrote something wrong in the script that possibly make variable not overriden in next iteration?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is strange, still not work even with unset variable in end of each iteration:

for directory in $directories; do
    echo "[NOTE] Downgrading '$directory' directory\n"

    if [[ "$directory" =~ ^(vendor/(symfony|symplify|rector)|src|utils).* ]]; then
        CONFIG_PATH_DOWNGRADE="build/config/config-downgrade.php"
    else
        CONFIG_PATH_DOWNGRADE="build/config/config-downgrade-parallel.php"
    fi

    # --working-dir is needed, so "SKIP" parameter is applied in absolute path of nested directory
    php -d memory_limit=-1 bin/rector process $directory --config $CONFIG_PATH_DOWNGRADE --working-dir $BUILD_DIRECTORY --ansi

    unset CONFIG_PATH_DOWNGRADE
done

@TomasVotruba
Copy link
Member

TomasVotruba commented Jan 4, 2022

I think this might be related to #1613 (comment)
It seems some classes are not loaded with static reflection, but with autoload and new downgraded content. That's why it's broken.

So we should fix that one first, then see if helps here.

@samsonasik
Copy link
Member Author

It seems not working after rebase, even with if else direct:

    if [[ "$directory" =~ ^(vendor/(symfony|symplify|rector)|src|utils).* ]]; then
        php -d memory_limit=-1 bin/rector process $directory --config build/config/config-downgrade.php --working-dir $BUILD_DIRECTORY --ansi
    else
        php -d memory_limit=-1 bin/rector process $directory --config build/config/config-downgrade-parallel.php --working-dir $BUILD_DIRECTORY --ansi
    fi

@samsonasik
Copy link
Member Author

It seems solved with set back Option::PARALLEL back to false when re-read config-downgrade.php 23340be

I updated the script to use variable config.

@samsonasik
Copy link
Member Author

@TomasVotruba it works now 🎉🎉🎉

Screen Shot 2022-01-04 at 22 17 10

@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba I think it is ready.

@samsonasik
Copy link
Member Author

Let's merge it for now to see the diff in the rector/rector as if it already different day, the diff is hard to see.

@samsonasik samsonasik merged commit ac57fe2 into main Jan 4, 2022
@samsonasik samsonasik deleted the downgrade-parallel branch January 4, 2022 15:24
@samsonasik
Copy link
Member Author

samsonasik commented Jan 4, 2022

Ok, the script build/downgrade-rector.sh error:

config;vendor/phpstan/phpdoc-parser/src;vendor/symfony/dependency-injection;vendor/symfony/console;vendor/symfony vendor/psr;vendor/symplify vendor/nikic src;packages;vendor/ssch;rules;utils;bin rector.php;vendor/bin;vendor/clue;vendor/composer;vendor/cweagans;vendor/doctrine;vendor/ergebnis;vendor/evenement;vendor/helmich;vendor/idiosyncratic;vendor/myclabs;vendor/nette;vendor/react;vendor/rector;vendor/sebastian;vendor/tracy;vendor/webmozart
build/downgrade-rector.sh: 36: Syntax error: "(" unexpected (expecting "then")
Error: Process completed with exit code 2.

which working ok locally.

@samsonasik
Copy link
Member Author

I created PR #1625 to solve script downgrade error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants