TimWolla / docker-adminer

Database management in a single PHP file

https://hub.docker.com/_/adminer/

157 stars 69 forks source link

Update to PHP 8 #85

Closed TimWolla closed 3 years ago

zzjin commented 3 years ago

need any update to merge?

TimWolla commented 3 years ago

PHP 8 does not bring any benefit for this Docker image, because it is not meant to run custom software. It however possibly introduces breakage. Thus I plan on sitting on this PR for a little longer to make sure any PHP bugs and Adminer incompatibilities are fixed.

guilliamxavier commented 2 years ago

It seems that Adminer is still not fully compatible with PHP 8, see e.g. https://github.com/vrana/adminer/pull/429, I also have a regression where tables status is all empty cells...

TimWolla commented 2 years ago

@guilliamxavier There's also #108. I was hoping to sit that out, because at least #108 is a not-as-severe issue. But Adminer returning incorrect or missing information is bad of course. I'll try to get a revert done this evening.

guilliamxavier commented 2 years ago

My bad, the "tables status is all empty cells" regression is not because of PHP 8 but Adminer 4.8.1, as downgrading to 4.8.0 fixes it 😖

guilliamxavier commented 2 years ago

Wait... I inspected https://github.com/vrana/adminer/compare/v4.8.0...v4.8.1?w=1 but didn't see what could have caused the regression, then I realized that the Docker image adminer:4.8.0 was based on php:7.4 🤦‍♂️ (to be sure of the cause, one should try Adminer 4.8.0 on PHP 8.0 and/or Adminer 4.8.1 on PHP 7.4...)

TimWolla commented 2 years ago

110

guilliamxavier commented 2 years ago

Thanks for the revert. After a docker pull adminer I now have Adminer 4.8.1 on PHP 7.4.24 and I confirm that the regression is gone 🙂 (now I would like to report the bug to Adminer but their GitHub doesn't accept issues, requires a SourceForge account...)