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

PHP incompatibility on PHP > 8? #140

Open
mattange opened this issue Jan 15, 2023 · 19 comments
Open

PHP incompatibility on PHP > 8? #140

mattange opened this issue Jan 15, 2023 · 19 comments

Comments

@mattange
Copy link

hi

I found when trying to start v 2.00 (at the related commit) the following when running via docker that this error pops up in console and nothing really shows up on the webpage.

Is there a maximum requirement on PHP? using PHP-FPM with docker-compose. worked as a charm with 8.1 and 1.61
Let me know if you need to know anything else.
As usual - thanks for the great work

rompr-php-1    | NOTICE: PHP message: PHP Fatal error:  Uncaught TypeError: Unsupported operand types: string + int in /ROMPR/includes/prefs.class.php:584
rompr-php-1    | Stack trace:
rompr-php-1    | #0 /ROMPR/backends/sql/sqlite/init_database.class.php(858): prefs::upgrade_host_defs()
rompr-php-1    | #1 /ROMPR/index.php(80): init_database->check_sql_tables()
rompr-php-1    | #2 {main}
rompr-php-1    |   thrown in /ROMPR/includes/prefs.class.php on line 584
rompr-php-1    | 172.23.0.3 -  15/Jan/2023:23:20:06 +0000 "GET /index.php" 500
@fatg3erman
Copy link
Owner

There should be no limit on PHP version, I'm running 8.1 myself though not in docker but I doubt that's relevant.

The error on that line number is where it is trying to add 2 to a variable. The variable is a string but it should be a numeric string so that should work.

Would you be able to post the contents of prefs/prefs.var here? That file does contain some passwords so if you'd rather not do that then the variable of interest is 'http_port_for_mopidy' which ought to look something like this:

"http_port_for_mopidy";s:4:"6680"

@mattange
Copy link
Author

This is what i get in the current config.

....."http_port_for_mopidy";s:0:"";s:10:"multihosts"....

I am one of the crazy ones that have multiple Mopidy targets (2) on same machine (and other ones on other machines) and is NOT using the HTTP interface for those as - as i had discussed in the past with you - i think that meant i could not use them as they have different HTTP ports (but only one could be defined for all of the targets in the past).

Maybe i should reopen the old docker container and remove temporarily the other players and then add them back?
That way i can put a value i suppose in that field for the HTTP port?

Let me know if you think this cunning plan should work!

matteo

@mattange
Copy link
Author

i think i solved that via the cunning plan. now i can reach Rompr as before and settings appear there!
yay - although i have not tried to play anything yet as it's late here!!

by converse once i connect - and tried as recommended keeping the prefs and albumart folders - this is what i get on the top right of the page once the connection is successful

image

Should i perhaps kill the albumart folder and start from scratch?

@fatg3erman
Copy link
Owner

OK this is making sense now. I'd forgotten to account for the fact that the websocket port might not be a number. In version 2 you can now set the websocket port individually for each player, and I'd recommend you do it.

The second error is odd, I'm not seeing that. But the fact that you're seeing it in the UI suggests that your PHP installation is in 'production' mode rather than 'development' mode - check your php.ini, display_errors should be Off and log_errors should be On.

As for what the error means, it means I have to rewrite some code because PHP have decided I can't do a thing any more. Grr.

@fatg3erman
Copy link
Owner

If you're struggling to get rid of that error, you can edit util_classes/basealbumimage.class.php and add these lines above the definition of public function __construct

	// Remember to keep albumart_translator in uifunctions.js in step with this

	// We can no longer use dynamic properties so we have to declare all these here.
	protected $artist;
	protected $album;
	protected $key;
	protected $source;
	protected $file;
	protected $base64data;
	protected $mbid;
	protected $albumpath;
	protected $albumuri;
	protected $trackuri;
	protected $dbimage;

@mattange
Copy link
Author

Thanks!
so I have used production settings as my php is really out of docker default image and i just use the production-php.ini file

FROM php:8.2.1-fpm
COPY --from=mlocati/php-extension-installer /usr/bin/install-php-extensions /usr/bin/
RUN install-php-extensions gd intl\
        && apt-get update \
        && apt-get install -y imagemagick procps \
        && mv "$PHP_INI_DIR/php.ini-production" "$PHP_INI_DIR/php.ini"
COPY ./rompr-config.ini $PHP_INI_DIR/conf.d/

the last config only has one setting modified vs. production.
You can see default production at [https://github.com/php/php-src/blob/master/php.ini-production](Github url)

I'll let you handle that change as i am more than happy to wait for that!!
Once you mark this as resolved i'll pick up the latest commit :-)

@fatg3erman
Copy link
Owner

I've alreday fixed it on the develop branch but it won't make it to main until I do another release, which will be some months away.

@fatg3erman
Copy link
Owner

Fixed in 2.06

@franperr
Copy link

franperr commented Sep 13, 2023

Hi, I have installed version 2.06 and PHP 8.2, the message about "creation of dynamic properties" still appears for 3 variables. Adding them to the list remove the message.

public $baseimage;
public $image_downloaded;
public $images;

@franperr
Copy link

$basepath must also be added to be able to update podcasts.

@mattange
Copy link
Author

mattange commented Sep 19, 2023 via email

@franperr
Copy link

franperr commented Sep 19, 2023

This error was silent on the interface, I found it out by activating the logs. Try activating the logs to see if it is the same problem.

@fatg3erman
Copy link
Owner

Thanks for reporting. I'll do a 2.07 release in the next day or so to add this fix.

Apple podcasts are working for me. Which podcast were you searching for? It could be another PHP 8 error, but I'm developing using PHP 8.1. I really need to see a debug log.

@fatg3erman fatg3erman reopened this Sep 23, 2023
@jonzobot
Copy link

jonzobot commented Mar 10, 2024

Hello,

I have unsuccessfully tried to host Rompr 2.10 on a FreeBSD 14.0 box with PHP 8.3, I also tried PHP 8.1.

Both PHP versions throw an error: "Rompr Backend Daemon Not Running".

Reviewing the docs, I tried to run the following manually:

$ /usr/local/bin/php rompr_backend.php

It looks like it is failing out:

PHP Fatal error: Uncaught TypeError: fclose(): Argument #1 ($stream) must be of type resource, bool given in /usr/local/www/apache24/data/includes/prefs.class.php:352

@fatg3erman
Copy link
Owner

I have never tried FreeBSD but I can guess what the error is.

The error message you're showing me means your prefs folder is not writable by PHP when you run the command by hand. This is an oversight in the docs, I very much doubt it's the actual problem.

The backend daemon is a process that is spawned automatically when you first try to load RompR. The method for spawning the process depends on the OS. On Linux I have to do one thing, on MacOS I have to do another. At a guess I'd say that FreeBSD needs the same treatment as MacOS, but I'm not detecting FreeBSD so it's defaulting to the Linux method.
You might have some luck running the backend daemon as a system process, but I don't know what method FreeBSD uses to do that (eg on Linux I've documented how to do it using systemd).
I have no knowledge of FreeBSD I'm afraid so you're on your own here.

@jonzobot
Copy link

jonzobot commented Mar 11, 2024

OK I think I'm closer. For anyone else running FreeBSD running into this issue:

I didn't realize running "php rompr_backend.php" from the webserver directory (for me, /usr/local/www/apache24/data) was necessary to get it to work, without failing out.

Once I got it to work and run properly, it still wasn't recognized by RompR which, delving into the code, was a result of RompR not seeing the backend in the list of running processes (internally, /includes/functions.php runs ps as a process and greps for (working directory)/rompr_backend.php in order to figure out if it's running).

So the following is a quick and dirty (and probably, subject to much improvement) rc.d init file:

/usr/local/etc/rc.d/romprbackend:

# PROVIDE: romprbackend
# REQUIRE: LOGIN cleanvar sshd apache24
# KEYWORD: shutdown

. /etc/rc.subr

name="romprbackend"
rcvar="romprbackend_enable"

romprbackend_chdir="/usr/local/www/apache24/data/"

command="/usr/sbin/daemon"
command_args="-r -f -u www /usr/local/bin/php /usr/local/www/apache24/data/rompr_backend.php"

load_rc_config $name
: ${romprbackend_enable:=no}
: ${romprbackend_msg="Nothing started."}

run_rc_command "$1"

This requires first running:

# sysrc romprbackend_enable="YES"

This has issues shutting down cleanly but it is at least up and running, and I can play with the Rompr interface now.

PS:

I realize this has gotten quite a bit off the original topic, as PHP is working fine for me now.

@fatg3erman
Copy link
Owner

Thanks for that, I'll add it to the docs when I remember.

The backend is responsible for starting an romonitor.php process for each of your players, you should check that is running, otherwise some features won't work correctly.

@jonzobot
Copy link

jonzobot commented Mar 12, 2024

Thanks. Yes, romonitor.php is running, the backend service autoruns as it's supposed to on jail restart, and everything looks like it's running smoothly. Thanks again.

@fatg3erman
Copy link
Owner

That's cool. The only thing you'll need to remember to do is to restart your backend service whenever you update to a new release of RompR. It is possible that refreshing the browser after an update might kill the previous process, but it won't be able to start it again. This might get messy if database changes are needed. I'll need to look into it in a bit more depth before I do a new release to make sure it's all safe. Thanks for reporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants