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

[MAINTENANCE] Run tests for all supported PHP versions and update dependencies #1455

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

stweil
Copy link
Member

@stweil stweil commented Feb 1, 2025

This fixes two vulnerabilities which were reported by Dependabot.

@sebastian-meyer sebastian-meyer self-requested a review February 1, 2025 15:10
@sebastian-meyer sebastian-meyer added the 🛠 maintenance A task to keep the code up-to-date and manageable. label Feb 1, 2025
Copy link

codecov bot commented Feb 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (b227afc) to head (8bc081e).
Report is 2 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff      @@
##   main   #1455   +/-   ##
============================
============================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sebastian-meyer sebastian-meyer changed the title Allow PHP 8.4 and update dependencies [MAINTENANCE] Allow PHP 8.4 and update dependencies Feb 1, 2025
@sebastian-meyer
Copy link
Member

Hm, I don't like the big span of supported PHP versions, because we do not test all of them. Instead of making the support even wider, I am in favor of narrowing it down to those versions which are supported by both TYPO3 v11 and v12 (i. e. 8.1, 8.2 and 8.3). What do you think?

@sebastian-meyer
Copy link
Member

Also, updating composer.lock for PHP 8.4 breaks tests, because those run on PHP 8.1.

Signed-off-by: Stefan Weil <[email protected]>
This fixes two vulnerabilities which were reported by Dependabot:

- TYPO3 Scheduler Module vulnerable to Cross-Site Request Forgery (High)
- TYPO3 Potential Open Redirect via Parsing Differences (Moderate)

Signed-off-by: Stefan Weil <[email protected]>
Both PHP releases had their end of life long ago.

Signed-off-by: Stefan Weil <[email protected]>
@stweil
Copy link
Member Author

stweil commented Feb 1, 2025

Dropping support for PHP 7.4 and PHP 8.0 is a good idea. I still have to find the reason for the CI failure with PHP 8.1.

@stweil stweil marked this pull request as draft February 2, 2025 07:06
Signed-off-by: Stefan Weil <[email protected]>
@stweil
Copy link
Member Author

stweil commented Feb 2, 2025

Also, updating composer.lock for PHP 8.4 breaks tests, because those run on PHP 8.1.

It looks like exactly this CI test failure occurs from time to time, so it is unrelated to my modifications.

@stweil stweil marked this pull request as ready for review February 2, 2025 08:06
@stweil
Copy link
Member Author

stweil commented Feb 2, 2025

Dropping support for PHP 7.4 and PHP 8.0 is a good idea.

This is done now. So two PHP releases were dropped, PHP 8.4 was added => the span of PHP releases decreased.

The CI test with PHP 8.4 works locally and can be added here as soon as a pending issue is solved.

@sebastian-meyer
Copy link
Member

Thanks for removing support for old PHP versions! It's much cleaner now!

But I still don't think we should add support for PHP 8.4. I am in favor of sticking to those PHP versions which are supported by both TYPO3 versions, v11 and v12. (TYPO3 v11 doesn't support PHP 8.4.) It makes development and testing much easier.

@stweil
Copy link
Member Author

stweil commented Feb 2, 2025

PHP 8.4 is the latest stable release and standard for me on my macBook. I recently switched to a native installation of Kitodo.Presentation there because it makes source code debugging much easier for me. But I can do this using my local fork, of course.

Copy link
Member

@sebastian-meyer sebastian-meyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your work towards PHP v8.4 support is very much appreciated and it's great if it works already! But "official" support should be postponed for the next major version (supporting TYPO3 v12 and v13).

.github/workflows/tests.yml Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
ext_emconf.php Outdated Show resolved Hide resolved
composer.lock Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please re-run composer update with PHP 8.1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please re-run composer update with PHP 8.1.

I created a new pull request #1457 which only updates composer.lock with PHP 8.1.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! But then you have to remove all changes from this pull request to avoid merge conflicts.

@stweil stweil marked this pull request as draft February 2, 2025 15:13
@stweil
Copy link
Member Author

stweil commented Feb 2, 2025

Your work towards PHP v8.4 support is very much appreciated.

Up to now the only "work" was removing the current limitation which only supports up to PHP 8.3.

@sebastian-meyer sebastian-meyer changed the title [MAINTENANCE] Allow PHP 8.4 and update dependencies [MAINTENANCE] Run tests for all supported PHP versions and update dependencies Feb 2, 2025
@sebastian-meyer
Copy link
Member

This is fine! But you marked it as draft.
Just let me know when I can go ahead and merge this pull request!

@stweil
Copy link
Member Author

stweil commented Feb 2, 2025

I marked this PR as draft because it addressed support for PHP 8.4.

@stweil
Copy link
Member Author

stweil commented Feb 2, 2025

The new PR #1458 removes old PHP releases and superseeds similar changes that were part of the PR here.

Increasing the test matrix will be addressed in another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠 maintenance A task to keep the code up-to-date and manageable.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants