Islandora / Crayfish

A collection of Islandora microservices, lovingly known as Crayfish.
MIT License
9 stars 29 forks source link

Attempt to get working under PHP 8.0 and 8.1 #161

Closed nigelgbanks closed 1 year ago

nigelgbanks commented 2 years ago

Upgrade symphony dependencies to ^4.4 to match up with Drupal 9.

Gonna test with all the other services at once, will post my results.

PHP is stopping security updates for 7.4 in November.

https://www.php.net/supported-versions.php

codecov[bot] commented 2 years ago

Codecov Report

Base: 75.55% // Head: 75.34% // Decreases project coverage by -0.20% :warning:

Coverage data is based on head (3dee7c6) compared to base (4dca22a). Patch has no changes to coverable lines.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## 3.x #161 +/- ## ============================================ - Coverage 75.55% 75.34% -0.21% - Complexity 159 160 +1 ============================================ Files 6 6 Lines 589 572 -17 ============================================ - Hits 445 431 -14 + Misses 144 141 -3 ``` | [Impacted Files](https://codecov.io/gh/Islandora/Crayfish/pull/161?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Islandora) | Coverage Δ | | |---|---|---| | [...ild\_dir/Recast/src/Controller/RecastController.php](https://codecov.io/gh/Islandora/Crayfish/pull/161/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Islandora#diff-YnVpbGRfZGlyL1JlY2FzdC9zcmMvQ29udHJvbGxlci9SZWNhc3RDb250cm9sbGVyLnBocA==) | `52.89% <0.00%> (-0.73%)` | :arrow_down: | | [...d\_dir/Houdini/src/Controller/HoudiniController.php](https://codecov.io/gh/Islandora/Crayfish/pull/161/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Islandora#diff-YnVpbGRfZGlyL0hvdWRpbmkvc3JjL0NvbnRyb2xsZXIvSG91ZGluaUNvbnRyb2xsZXIucGhw) | `98.21% <0.00%> (-0.18%)` | :arrow_down: | | [...r/Hypercube/src/Controller/HypercubeController.php](https://codecov.io/gh/Islandora/Crayfish/pull/161/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Islandora#diff-YnVpbGRfZGlyL0h5cGVyY3ViZS9zcmMvQ29udHJvbGxlci9IeXBlcmN1YmVDb250cm9sbGVyLnBocA==) | `100.00% <0.00%> (ø)` | | | [build\_dir/Milliner/src/Service/MillinerService.php](https://codecov.io/gh/Islandora/Crayfish/pull/161/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Islandora#diff-YnVpbGRfZGlyL01pbGxpbmVyL3NyYy9TZXJ2aWNlL01pbGxpbmVyU2VydmljZS5waHA=) | `71.80% <0.00%> (+0.56%)` | :arrow_up: | | [...d\_dir/Homarus/src/Controller/HomarusController.php](https://codecov.io/gh/Islandora/Crayfish/pull/161/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Islandora#diff-YnVpbGRfZGlyL0hvbWFydXMvc3JjL0NvbnRyb2xsZXIvSG9tYXJ1c0NvbnRyb2xsZXIucGhw) | `85.71% <0.00%> (+1.40%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Islandora). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Islandora)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

whikloj commented 2 years ago

Hey @nigelgbanks you are welcome to work on this, but I wanted to note that #133 is using Symfony 4.4 for all the microservices. It doesn't currently test PHP 8 but it might be a lower bar.

nigelgbanks commented 2 years ago

Hi @whikloj, thanks for pointing that out, at the moment I'm testing infrastructure changes that require a move to PHP 8.1 I'll probably stick to working against 2.x until 3.x is officially released.

whikloj commented 2 years ago

I understand, I'm just not super confident on when that will happen.

nigelgbanks commented 2 years ago

Just a heads up for 8.1 tests are failing due to guzzle https://github.com/guzzle/psr7/blob/8f0a09ce19adcf28f25ff9d90e7329488501b363/src/StreamWrapper.php#L47 is passing a NULL where it should be FALSE as PHP 8.1 has deprecated this. I'll need to get that sorted first. I tested with patching locally but I don't think we want to maintain a patch, better to fix it at the source.

nigelgbanks commented 2 years ago

So looking in the previous comment it would force me to update chullo which would affect crayfish-commons which would affect the islandora module, this would force users onto Drupal 9.4 and later. So I'll just patch it.

nigelgbanks commented 2 years ago

Can't easily patch it I'll just change the setting to ignore deprecations in the unit tests.

nigelgbanks commented 2 years ago

@whikloj it's finally ready for review :upside_down_face:

whikloj commented 1 year ago

I'm sorry @nigelgbanks, I'm trying to build a vagrant box with 8.0 to test this right now.