Make CPU cores count more tolerant towards system command errors

[romm]

Running Infection with the new --threads=max option was leading to an error on my local MacOS.

This fix is quite opinionated, if you see a better way of doing this I'd be happy to follow your recommendations. 😉

This is the error that was returned:

$ ./vendor/bin/infection --threads=max -vvv

In ExecException.php line 9:

  [Safe\Exceptions\ExecException]
  An error occured

Exception trace:
  at /some/path/to/app/vendor/thecodingmachine/safe/generated/Exceptions/ExecException.php:9
 Safe\Exceptions\ExecException::createFromPhpError() at /some/path/to/app/vendor/thecodingmachine/safe/generated/exec.php:119
 Safe\shell_exec() at /some/path/to/app/vendor/infection/infection/src/Resource/Processor/CpuCoresCountProvider.php:70
 Infection\Resource\Processor\CpuCoresCountProvider::provide() at /some/path/to/app/vendor/infection/infection/src/Command/RunCommand.php:658
 Infection\Command\RunCommand->getThreadCount() at /some/path/to/app/vendor/infection/infection/src/Command/RunCommand.php:485
 Infection\Command\RunCommand->createContainer() at /some/path/to/app/vendor/infection/infection/src/Command/RunCommand.php:356
 Infection\Command\RunCommand->executeCommand() at /some/path/to/app/vendor/infection/infection/src/Command/BaseCommand.php:75
 Infection\Command\BaseCommand->execute() at /some/path/to/app/vendor/symfony/console/Command/Command.php:308
 Symfony\Component\Console\Command\Command->run() at /some/path/to/app/vendor/symfony/console/Application.php:1002
 Symfony\Component\Console\Application->doRunCommand() at /some/path/to/app/vendor/symfony/console/Application.php:299
 Symfony\Component\Console\Application->doRun() at /some/path/to/app/vendor/symfony/console/Application.php:171
 Symfony\Component\Console\Application->run() at /some/path/to/app/vendor/infection/infection/bin/infection:83
 include() at /some/path/to/app/vendor/bin/infection:120

[maks-rafalko]

Could you please past an error here?

• to better understand what's going on on MacOS
• to make it searchable for future developers

[romm]

Sure, I just edited the description above; unfortunately the message is not really relevant. 😕

[theofidry]

one issue I see with this try/catch is that it's unclear which statement you really expect to throw

[romm]

Well the idea is mostly not to block i.e. Mac/Windows users when the Linux command does fail, to allow the next part of the function to run without trouble.

[theofidry]

I was more referring at the statement causing failure, here I doubt that anything apart from the shell exec is suppose to throw right?

[romm]

Right. Actually if any of the two shell_exec calls fails, it should be caught IMO, no matter which one. WDYT?

[theofidry]

That being said I'd personally find unexpected that you ask several threads and end up with only one... but that's out of the scope of this PR

[romm]

Yes, maybe we could add a notice/warning if all commands failed? WDYT?

[theofidry]

👍 (would do so in a different PR though)

[romm]

When/if this PR is merged I'll submit a new one where we can discuss this. 😉

[sanmai]

In addition to what @theofidry said please consider using the unsafe variant of shell_exec. We may not need to handle the exceptions in the first place.

[romm]

In addition to what @theofidry said please consider using the unsafe variant of shell_exec. We may not need to handle the exceptions in the first place.

Sure, if others agree I see no problem to switch to the native shell_exec function.

[romm]

@sanmai @maks-rafalko After giving it some thought, the safe version of shell_exec is easier to work with in this case: working with try/catch is more readable (IMO) than three conditions (because three shell_exec calls) checking if the result is a correct string. I stay open to changing it if you think that's better, though. :)

[maks-rafalko]

Ok, let's get it merged, seems like we all ok with either safe of unsafe versions. Other comments are not related to this fix.

Thank you @romm