search

oxsecurity / megalinter

🦙 MegaLinter analyzes 50 languages, 22 formats, 21 tooling formats, excessive copy-pastes, spelling mistakes and security issues in your repository sources with a GitHub Action, other CI tools or locally.
https://megalinter.io
GNU Affero General Public License v3.0
1.9k stars 232 forks source link

[v4.46] PHP 8.0 not supported #788

Closed llaville closed 3 years ago

llaville commented 3 years ago

Describe the bug Side effects may be found when using Mega-Linter GA with other Github Actions that use PHP Platform

To Reproduce

  1. Use a GA workflow with multiple steps, including https://github.com/ramsey/composer-install to install Composer dependencies of a PHP project (for example)
  2. Use a Mega-Linter PHP flavor step (like nvuillam/mega-linter/flavors/php@v4)
  3. Be sure to have PHP_PHPSTAN linter enabled in project mode to avoid issue #725) for example
  4. Run GA workflow
  5. See error in screenshot; full GA report available at https://github.com/llaville/php-compatinfo-db/runs/3729299344?check_suite_focus=true

Expected behavior Be able to configure Mega-Linter to accept PHP versions other than PHP 7.x or a better catching error about PHP platform

Screenshots

phpstan_php8

Additional context Mega-Linter v4.46 only support PHP 7 (confirmed by https://github.com/nvuillam/mega-linter/blob/v4.46.0/megalinter/descriptors/php.megalinter-descriptor.yml#L7-L19)

llaville commented 3 years ago

As workaround we can use shivammathur/setup-php GA as specified https://github.com/ramsey/composer-install#usage For example in the same context project as previous : https://github.com/llaville/php-compatinfo-db/commit/8db3e4b5162496568a8aea72443afe3d9a6bb126

And all run fine, as expected ! See on full report https://github.com/llaville/php-compatinfo-db/runs/3729982777?check_suite_focus=true#step:6:3402 or just screenshot

phpstan_01299

nvuillam commented 3 years ago

Maybe you'd like to make a PR to update php install instructions so PHP8 is natively handled ? :)

llaville commented 3 years ago

Does it make sense for you to keep major PHP versions as different flavor ? Please vote ! That's mean a php7 and php8 instead of just a php

llaville commented 3 years ago

@nvuillam Finally, after study carefully source code of Mega-Linter, there is no need to have a different flavor for PHP 7 and PHP 8, as previously suggested.

Solution is to use ability to override executable and make it available at runtime with a new env var as follow PHP_BUILTIN_CLI_EXECUTABLE: "/usr/bin/php8", Default will be None and used linter name as executable.

And of course https://github.com/nvuillam/mega-linter/blob/v4.46.0/megalinter/descriptors/php.megalinter-descriptor.yml#L7-L19 will be updated to add php8 and modules both with existing php7 copies

If you are agree with it, a PR may follows

It's not just a POC, but a real possibility, I've tested. See following screenshot

php7_8

nvuillam commented 3 years ago

Go ! :)