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

Drop PHP 7.4 support #7924

Merged
merged 11 commits into from
Sep 23, 2023
Merged

Drop PHP 7.4 support #7924

merged 11 commits into from
Sep 23, 2023

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Sep 11, 2023

Description
See #6921
Supersedes #6922

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added the 4.5 label Sep 11, 2023
@kenjis kenjis marked this pull request as draft September 11, 2023 07:55
@kenjis kenjis marked this pull request as ready for review September 11, 2023 08:04
@kenjis kenjis added the refactor Pull requests that refactor code label Sep 11, 2023
@michalsn
Copy link
Member

I don't know when v4.5 will be released, but currently, PHP 7.4 is still used by more than 20% of 4.4 users.

https://packagist.org/packages/codeigniter4/framework/php-stats#4.4

@kenjis
Copy link
Member Author

kenjis commented Sep 11, 2023

The release of 4.5 will be after the release of PHP 8.3, no matter how soon.
By then it will be more than a year since PHP 7.4 EOL.

@kenjis
Copy link
Member Author

kenjis commented Sep 13, 2023

auto-label failed.

Run prince-chrismc/label-merge-conflicts-action@v3
🔎 Gather data for Pull Request #7924
🏷️ Updating labels
  #7924 has a mismatching SHA's
  Labeling #7924...
  Error: GraphqlResponseError: Request failed due to following response errors:
   - Resource not accessible by integration

https://github.com/codeigniter4/CodeIgniter4/actions/runs/6167549712/job/16739374912

@ddevsr
Copy link
Collaborator

ddevsr commented Sep 13, 2023

@kenjis By https://github.com/prince-chrismc/label-merge-conflicts-action#faq---how-do-i-fix-resource-not-accessible-by-integration

We already set minimum permission

permissions:
  issues: write
  pull-requests: write

@datamweb
Copy link
Contributor

I don't know, but isn't there a difference between github_token: ${{ secrets.GITHUB_TOKEN }} and github_token: ${{ github.token }}?

@ddevsr
Copy link
Collaborator

ddevsr commented Sep 13, 2023

@datamweb I agree,

Different :

- uses: prince-chrismc/label-merge-conflicts-action@v3
        with:
          conflict_label_name: 'stale'
          github_token: ${{ github.token }}
uses: actions/checkout@v4
        with:
          repository: codeigniter4/api
          token: ${{ secrets.ACCESS_TOKEN }}

Parameter with.token or with.github_token is true configuration?

https://github.com/codeigniter4/CodeIgniter4/blob/develop/.github/workflows/deploy-apidocs.yml#L40
https://github.com/codeigniter4/CodeIgniter4/blob/develop/.github/workflows/label-conflict.yml#L24

@kenjis
Copy link
Member Author

kenjis commented Sep 13, 2023

They are the same.

At the start of each workflow job, GitHub automatically creates a unique GITHUB_TOKEN secret to use in your workflow. You can use the GITHUB_TOKEN to authenticate in the workflow job.
...
The token is also available in the github.token context. For more information, see "Contexts."
https://docs.github.com/en/actions/security-guides/automatic-token-authentication#about-the-github_token-secret

@MGatner
Copy link
Member

MGatner commented Sep 13, 2023

Any ideas on the other failure? I will try rerunning.

PHP Fatal error: Uncaught Error: Class "Config\Autoload" not found in /home/runner/work/CodeIgniter4/CodeIgniter4/system/Test/Mock/MockAutoload.php:16

@kenjis
Copy link
Member Author

kenjis commented Sep 14, 2023

Upload coverage results to Coveralls / coveralls

Run vendor/bin/phpcov merge --clover build/logs/clover.xml build/cov
  vendor/bin/phpcov merge --clover build/logs/clover.xml build/cov
  shell: /usr/bin/bash -e {0}
  env:
    COMPOSER_PROCESS_TIMEOUT: 0
    COMPOSER_NO_INTERACTION: 1
    COMPOSER_NO_AUDIT: 1
    COMPOSER_CACHE_FILES_DIR: /home/runner/.cache/composer/files
phpcov 8.2.1 by Sebastian Bergmann.

PHP Fatal error:  Uncaught Error: Class "Config\Autoload" not found in /home/runner/work/CodeIgniter4/CodeIgniter4/system/Test/Mock/MockAutoload.php:16
Stack trace:
#0 /home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/php-code-coverage/src/CodeCoverage.php(578): include_once()
#1 /home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/php-code-coverage/src/CodeCoverage.php(176): SebastianBergmann\CodeCoverage\CodeCoverage->processUncoveredFilesFromFilter()
#2 /home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/php-code-coverage/src/Node/Builder.php(44): SebastianBergmann\CodeCoverage\CodeCoverage->getData()
#3 /home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/php-code-coverage/src/CodeCoverage.php(136): SebastianBergmann\CodeCoverage\Node\Builder->build()
#4 /home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/php-code-coverage/src/Report/Clover.php(53): SebastianBergmann\CodeCoverage\CodeCoverage->getReport()
#5 /home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpcov/src/cli/Command.php(88): SebastianBergmann\CodeCoverage\Report\Clover->process()
#6 /home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpcov/src/cli/MergeCommand.php(84): SebastianBergmann\PHPCOV\Command->handleReports()
#7 /home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpcov/src/cli/Application.php(47): SebastianBergmann\PHPCOV\MergeCommand->run()
#8 /home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpcov/phpcov(30): SebastianBergmann\PHPCOV\Application->run()
#9 /home/runner/work/CodeIgniter4/CodeIgniter4/vendor/bin/phpcov(119): include('...')
#10 {main}
  thrown in /home/runner/work/CodeIgniter4/CodeIgniter4/system/Test/Mock/MockAutoload.php on line 16
Generating code coverage report in Clover XML format ... 
Error: Process completed with exit code 255.

https://github.com/codeigniter4/CodeIgniter4/actions/runs/6167549719/job/16751771845?pr=7924

@kenjis
Copy link
Member Author

kenjis commented Sep 14, 2023

Created an issue #7941

@kenjis
Copy link
Member Author

kenjis commented Sep 17, 2023

It seems unethical to continue to support an unmaintained PHP version.

If someone needs using PHP 7.4, s/he can use v4.4.
There are no vulnerabilities reported and the framework itself seems quite secure,
and is still maintained until v4.5 is released.

@michalsn
Copy link
Member

Personally, I'm not against abandoning PHP 7.4. However, if we draw the line based on the officially supported version of PHP and not what our users actually use, then supporting PHP 8.0 after November 26, 2023, will also be "unethical".

What's more... according to composer stats, the percentage of users using 8.0 is smaller than those using 7.4.

If we want to say "Use CI 4.4 if you need support for an unsupported version of PHP," then it would be more natural to introduce PHP 8.1 as a minimum version for CI 4.5 release.

@kenjis
Copy link
Member Author

kenjis commented Sep 19, 2023

I would like to merge this.
Approve if you don't oppose.

@kenjis kenjis requested review from MGatner and ddevsr September 19, 2023 05:10
@kenjis
Copy link
Member Author

kenjis commented Sep 23, 2023

No one objects, so I merge.

@kenjis kenjis merged commit 760dbf4 into codeigniter4:4.5 Sep 23, 2023
55 checks passed
@kenjis kenjis deleted the drop-php-7.4-CI45 branch September 23, 2023 11:49
@kenjis kenjis mentioned this pull request Sep 23, 2023
4 tasks
@MGatner
Copy link
Member

MGatner commented Sep 27, 2023

I'm good with this.

@michalsn I am generally in favor of keeping our supported versions in line with PHP, while also recognizing driving factors. Often time is just our biggest bottleneck - we haven't always been compatible with new versions of PHP on their release, which is a much higher priority. On the bottom end we are motivated to drop versions that prevent us from adopting features. The feature set of PHP 7.4 => 8 is way more enticing than 8.0 => 8.1, so while I agree it would be better to keep consistency there's also less motivation among all the work competing for priority.

@kenjis kenjis mentioned this pull request Oct 13, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.5 refactor Pull requests that refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants