Successful lint with PHP startup errors should exit with 0 not 254

[Seldaek]

I am not sure exactly what the problem is, but parallel-lint exits with "Error in skip-linting.php process" on php 8, you can check the full output at https://travis-ci.org/github/Seldaek/monolog/jobs/689943262#L395

[jrfnl]

@Seldaek Thanks for reporting this.

I've just had a quick look and as far as I can see, the scan finishes without any actual issues. (it scanned the 197/197 files)

The error output you are seeing starts with "Error in skip-linting.php process\nError output:" and shows the warnings about Redis and MongoDB you also see in several other places in the Travis script output. Those are unrelated to PHP Parallel Lint, Parallel Lint just comes across them and saves them to show at the end.

[Seldaek]

Oh I see, thanks. I misinterpreted the output as the warnings for redis/mongo showed up so many times.

I'd argue if linting succeeds the process should exit with 0 though as a php misconfiguration like this one is a problem indeed but nothing to do with the linter and it's not its job to complain about it :) Outputting it for info would be enough IMO.

[jrfnl]

I'd argue if linting succeeds the process should exit with 0 though as a php misconfiguration like this one is a problem indeed but nothing to do with the linter and it's not its job to complain about it :) Outputting it for info would be enough IMO.

I'd agree, but am not sure it's that easy (haven't looked into it in detail yet though).

IIRC the output received from the PHP linter is all collected and saved to memory for display at the end of the run. It does distinguish between syntax errors to be displayed and "other" errors, but to change the error code, the code would need to differentiate between the PHP startup errors and any potential "other" errors. Not sure how easy that will be to do without using some sort of unreliable text recognition, which would start breaking as soon as the wording of the message would be changed again in PHP (as it has for numerous errors over the years). If this was a specific type of catchable error, it would be easier, but it's a plain PHP warning.

[jrfnl]

Currently trying to see if there is an PHP ini setting which can be used to suppress these errors when starting the various processes.... if anyone knows of one... feel free to chip in.

[jrfnl]

Hmm... display_startup_errors might do the trick...

[jrfnl]

@Seldaek Would you be willing to do a test run with the Monolog repo using this branch: https://github.com/jrfnl/PHP-Parallel-Lint/tree/feature/33-ignore-startup-errors ?

I suspect that may fix it, but it needs testing as I haven't been able to reproduce the issue locally (Windows with too many PHP versions confusing things).

[Seldaek]

I think all you need is an extension=invalid/path/to/ext.dll line in your php.ini file to trigger a similar kind of error. I can try a bit later though if that doesn't do it for you.

[jrfnl]

@Seldaek I tried that (extension=ifx) and while I did get the error for running the command in the first place, I couldn't get it to show for the subprocesses. As I said: running on Windows with too many PHP versions, so the subprocess might be using a different PHP version than the one I use to call the command. Tried making the change in several of the php.ini files, including the usual suspects for the PHP version my system defaults to, but haven't had any luck.

[Seldaek]

Doesn't seem to work https://travis-ci.org/github/Seldaek/monolog/jobs/690069478

[jrfnl]

Darn... Thanks for testing. Back to the drawing board I guess...

[jrfnl]

Been running some tests and the weirdest thing is that the fix seems to work fine with --no-colors, but not without it.... guess I'll have to do more & deeper digging to figure this one out....

[jrfnl]

@Seldaek I think I might have got a working solution: https://travis-ci.com/github/jrfnl/monolog/jobs/339134575

[Seldaek]

Cool thanks

[c0dehulk]

I've just run into this issue today with an incompatibility between Dynatrace and PHP 7.0 (an unrelated problem we're working), which is reported as a startup error.

Thanks for doing the work and finding the solution. Do you have a timeline for when this fix is likely to make it to packagist?

[jrfnl]

@c0dehulk No clue. Depends on when the maintainer takes a look (which I'm not).

[grogy]

Merged, thank you