deprecated-packages / symplify

[DISCONTINUED] Check split packages in their own repositories :)
MIT License
622 stars 190 forks source link

`[ERROR] Child process error` with no actual information about error #4071

Closed Wirone closed 2 years ago

Wirone commented 2 years ago

ℹ️ ECS 10.2.5 (no parallel enabled)

I have weird error in CI that I found hard to work with. But before that, simple background:

So I'm working on simplifying this process (remove exports, some of not needed xargs etc.), prepared MR with large diff and I got this:

$ printf "%d PHP files after filtering out ignored ones\n" $(echo "$GIT_DIFF_CHANGED_PHP_FILES" | awk 'NF' | wc -l)
3330 PHP files after filtering out ignored ones

$ (echo "${GIT_DIFF_CHANGED_PHP_FILES:-ecs.php}" | xargs -r vendor/bin/ecs check) || (echo 'To fix errors automatically run locally and commit: composer cs:develop-diff-fix' && exit 1)
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
 [ERROR] Child process error:                                                   
    0/1181 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░]   0%
  120/1181 [▓▓░░░░░░░░░░░░░░░░░░░░░░░░░░]  10%
  180/1181 [▓▓▓▓░░░░░░░░░░░░░░░░░░░░░░░░]  15%
  240/1181 [▓▓▓▓▓░░░░░░░░░░░░░░░░░░░░░░░]  20%
  360/1181 [▓▓▓▓▓▓▓▓░░░░░░░░░░░░░░░░░░░░]  30%
  480/1181 [▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░░░░░░░░]  [40]
  600/1181 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░░░░░]  [50]
  720/1181 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░░░]  60%
  840/1181 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░░░░░░░░]  71%
 1001/1181 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░░░░]  84%
 1121/1181 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓░░]  94%
 1181/1181 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
 [OK] No errors found. Great job - your code is shiny in style!

I was able to debug CI runner and execute those commands up to this error. But it's not xargs fault since there's the same error when I run vendor/bin/ecs check $GIT_DIFF_CHANGED_PHP_FILES. Adding -vvv or --debug does not print any helpful data.

Running vendor/bin/ecs check without explicit paths starts analysis with 25k of files, so the tool is working in general. I know it's not easy case for you, especially I don't have reproducer repo as of now, but I would ask at least for help with finding actual error 🙂

TomasVotruba commented 2 years ago

ECS 10.2.5 (no parallel enabled)

The parallel is enabled by default. Thus "child process error". Where have you disable parallel?


It will be probably some memory or limit exhaustion of provided args. I think input paths might saved somewhere in bigger amount than accepted. Possibly even in the PHP itself. That's my guess for "too long input" errors :) in general this is a bad practice as it will hit limits of system settings.

Personally, I'd try to add 2 CI jobs:

How long does parallel run on 25 k files? In most projects it finished in 10-30 seconds on full project.

Wirone commented 2 years ago

As I said, --debug does not print any info 😉 Did not know parallel is enabled by default, I'll try to disable it and run again.

The reason why we run analysis only on subset of files is that currently we don't have CS for main branch (only MRs) so we don't have cache reference that could be propagated to jobs for MRs and allow analyse only changed files. Also some paths are excluded from analysis (after determining git diff the result is filtered with grep based on regex set in Gitlab CI's settings, so we're able to change analysis context without touching code). In the future it will probably change when we clean up legacy etc.

Anyway, Argument list too long was caused by exporting diff list to variable - also xargs stops working after this 😅 It was fixed by removing export and passing arguments through xargs.

FYI: we have Rector job with almost the same flow (without filtering paths) and it works properly on the same set of modified files. Output is so large that Gitlab has problems with displaying it though 😅

TomasVotruba commented 2 years ago

In saw no --debug in the provided output: xargs -r vendor/bin/ecs check), that's why I noted :)

In the future it will probably change when we clean up legacy etc.

I think the propper way instead of this complex exluding/including would be to apply the standard in single PR. It's a matter of a day or two with safe sets like PSR-12 or split of COMMON even on the worst kind of code :muscle:

TomasVotruba commented 2 years ago

Closing as ECS is not designed to run 3330 files long args. The issue will be somewhere in OS/Gitlab/Process command line construction.

Use ECS with configued paths in config/dir paths args (no more than 100 :D) and it should be ok :+1:

Wirone commented 2 years ago

FYI: I was able to disable parallel on CI Runner (modified ecs.php) and analysis was totally OK:

$ (echo "${GIT_DIFF_CHANGED_PHP_FILES:-ecs.php}" | xargs -r vendor/bin/ecs check) || (echo 'To fix errors automatically run locally and commit: \e[32mcomposer cs:develop-diff-fix\e[0m' && exit 1)

 2149/2149 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 [OK] No errors found. Great job - your code is shiny in style!                 

 1181/1181 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 [OK] No errors found. Great job - your code is shiny in style!                 

So the root cause of the error is parallelisation 😉 But as I said, Rector works OK with such large file list (maybe there is parallel disabled by default?).

TomasVotruba commented 2 years ago

maybe there is parallel disabled by default?.

Yes, we're still testing it :+1:

It unrelated to parallel, but rather to CLI limits. The parallel is using json I/O from reactphp: https://github.com/rectorphp/rector-src/blob/main/packages/Parallel/WorkerRunner.php#L62 There are limits on paths that will crash this IMO.

Wirone commented 2 years ago

Is it possible to print some better information rather than [ERROR] Child process error? From our perspective it's an absolute minimum to do in terms of this problem. Because I was unable to get any helpful information about the root cause 😕

TomasVotruba commented 2 years ago

I would find the place it throws the exceptoin and dump the content before/around that.

Wirone commented 2 years ago

I was rather thinking that ECS could handle those errors better and display helpful information 😉 Does ReactPHP emits error with such data?

TomasVotruba commented 2 years ago

That would require lot of debugging, as I don't have such knolwedge of reactphp. Yet when you find out, any improvment is welcomed :+1: