Page MenuHomePhorge

Handling PHP deprecations: convert to Setup Issues
Closed, ResolvedPublic

Assigned To
Authored By
avivey
Jul 14 2023, 10:48
Referenced Files
F342059: image.png
Aug 26 2023, 16:59
Tokens
"The World Burns" token, awarded by revi."Like" token, awarded by Matthew."Like" token, awarded by valerio.bozzolan.

Description

Currently, we upgrade Deprecation warnings to Fatal Errors.
So to support current version of PHP (8.2), we're basically hunting down all cases of things that are deprecated in 8.2, to fix them before we can actually use 8.2.
That kind of defeats the purpose of the PHP team deprecating the errors instead of out-right deleting them - that is, these things are deprecated to give applications (us) time to remove them without breaking anything...

And I'm guessing there will be a bunch more deprecations in the next couple of years from PHP.

OTOH, we can't just keep the warnings in the logs and hope we'll eventually get to them - most people ignore the logs.

My proposal, then:

  • Promote "Deprecation" notices into Setup Issues
  • Catch the notice, phlog() it anyway, and add it to a Cache entry (PhabricatorSetupCheck is using cache for the heavy checks)
  • If any user is running into a Deprecated event, they get to continue their work and an Admin will get the Issue lamp
  • In the issue view, we'll show a count of these events, one of the traces, and instructions on how to report them. We'll also explain that these warnings means "Phorge might not be compatible with newer versions of PHP".
  • Restarting the server will clear the warnings cache - we can maybe try to keep them in a database for later retrieval, but they'll also be in the logs.

If we do that, we can basically announce that we support PHP 8.2 already - as far as we know, everything should still work, until version 9 where they are slated to become runtime or compile time errors.

This might also gives us a path towards deprecating Phorge API in the future, to promote a more-stable Extension ecosystem.

Thoughts?


There are several different execution contexts to handle:

  • Web
  • Arcanist
  • Daemons
  • Scripts in phorge/bin/ and phorge/scripts/
  • SSH server-side scripts (ssh-auth and ssh-exec)
  • SSH client-side scripts (ssh-connect - I think this one is usually executed from the web context, when a diffusion request is involving a repository that's hosted in another Phorge host)

Event Timeline

avivey triaged this task as High priority.

I agree that Phorge should not fall over because of a deprecation warning.

(Implementation notice: in both phorge and arc, this is handled in PhutilErrorHandler::handleError(), and the check is simply $num === E_DEPRECATED), and maybe E_USER_DEPRECATED.

E_USER_DEPRECATED is also available since php 5.3, so no compatibility issues.

There's no equivalent to setup issues in arc though, so this would have to be shown to the user immediately.

This is a good plan. Would this be opt-in, e.g. this Phorge instance would be the main one with this on but other installs wouldn’t see this by default?

I was thinking "out out", but only visible to admins - and then, only as a yellow notice at the top of the page:

Admin can mark "ignored" if they don't want to see them.

And I only want to show one or two traces, with maybe a count of how many total have happened - right now, there's still many cases (could be easily 1000s of warnings per week - there are still some common flows that have deprecated things).

D25386 and D25387 provide the big "do not crash on deprecation warning", and are probably enough to declare "experimental support for php 8.2" on their own.

D25388 and the stuff that will follow is all about exposing the deprecations in a way that isn't destructive, but allows/encourages installs to report them as bugs.

Maybe also something for this topic!
We update php yesterday on our WSL2 (Ubuntu 22.04) clients to 8.1.22 and pulled the latest arcanist from git after the commit
Now we facing this problem on starting arcanist:

ubu22@NB-CN0100021:/mnt/c/Users/ncoker/workspace$ arc version
[2023-08-15 05:30:35] EXCEPTION: (CommandException) Command failed with error #-1!
COMMAND
php -d variables_order=E -r 'echo json_encode($_ENV);'

STDOUT
{"SUDO_GID":"1000","LESSOPEN":"| \/usr\/bin\/lesspipe %s","HTTPS_PROXY":"http:\/\/127.0.0.1:3128","MAIL":"\/var\/mail\/ubu22","no_proxy":"localhost, 127.0.0.*, 10.*, 172.*, 192.168.*,.apac.igus.net","NODE_EXTRA_CA_CERTS":"\/usr\/local\/share\/ca-certificates\/RootCA.crt","USER":"ubu22","XDG_SESSION_TYPE":"tty","SHLVL":"1","MOTD_SHOWN":"pam","HOME":"\/home\/ubu22","OLDPWD":"\/home\/ubu22","NO_PROXY":"localhost, 127.0.0.*, 10.*, 172.*, 192.168.*,.apac.igus.net","HUSHLOGIN":"FALSE","DBUS_SESSION_BUS_ADDRESS":"unix:path=\/run\/user\/1000\/bus","WSL_DISTRO_NAME":"ubu22","https_proxy":"http:\/\/127.0.0.1:3128","SUDO_UID":"1000","LOGNAME":"ubu22","WSL_INTEROP":"\/run\/WSL\/8_interop","http_proxy":"http:\/\/127.0.0.1:3128","_":"\/usr\/local\/arcanist\/bin\/arc","XDG_SESSION_CLASS":"user","TERM":"xterm-256color","XDG_SESSION_ID":"c1","PATH":"\/usr\/local\/arcanist\/bin:\/usr\/local\/nodejs\/bin:\/opt\/apache-maven\/bin:\/opt\/java\/openjdk\/bin:\/usr\/local\/sbin:\/usr\/local\/bin:\/usr\/sbin:\... (2,667 more byte(s)) ...

STDERR
(empty) at [<arcanist>/src/future/exec/ExecFuture.php:436]
arcanist(head=master, ref.master=df6c315ace5f)

#0 ExecFuture::raiseResultError(array) called at [<arcanist>/src/future/exec/ExecFuture.php:340]
#1 ExecFuture::resolvex() called at [<arcanist>/src/future/exec/execx.php:17]
#2 execx(string, string) called at [<arcanist>/src/utils/PhutilExecutionEnvironment.php:40]
#3 PhutilExecutionEnvironment::repairMissingVariablesOrder() called at [<arcanist>/support/init/init-script.php:120]
#4 __arcanist_init_script__() called at [<arcanist>/support/init/init-script.php:131]
#5 require_once(string) called at [<arcanist>/support/init/init-arcanist.php:3]
#6 require_once(string) called at [<arcanist>/bin/arc:10]

The command output looks like this:
ubu22@NB-CN0100021:/mnt/c/Users/ncoker/workspace$ php -d variables_order=E -r 'echo json_encode($_ENV);'

{"'WSL_INTEROP":"\/run\/WSL\/8_interop'","'WSL_DISTRO_NAME":"ubu22'","'DISPLAY":":0'","'WT_SESSION":"17a1b7e6-af5d-43fc-b3ca-5e52e8fbded7'","'WT_PROFILE_ID":"{bff75edf-0556-5ab2-8fb0-1bf49d0b713c}'","'WSLENV":"WT_SESSION:BASH_ENV\/u:WT_PROFILE_ID'","'PULSE_SERVER":"unix:\/mnt\/wslg\/PulseServer'","'PWD":"\/home\/ubu22'","'TERM":"xterm-256color'","'WSL2_GUI_APPS_ENABLED":"1'","'XDG_RUNTIME_DIR":"\/mnt\/wslg\/runtime-dir'","'WAYLAND_DISPLAY":"wayland-0'","'HOSTTYPE":"x86_64'","SHELL":"\/bin\/bash","SUDO_GID":"1000","no_proxy":"localhost, 127.0.0.*, 10.*, 172.*, 192.168.*,.apac.igus.net","WSL_DISTRO_NAME":"ubu22","JAVA_HOME":"\/opt\/java\/openjdk","NODE_EXTRA_CA_CERTS":"\/usr\/local\/share\/ca-certificates\/RootCA.crt","SUDO_COMMAND":"\/usr\/sbin\/enter-systemd-namespace ","SUDO_USER":"ubu22","PWD":"\/mnt\/c\/Users\/ncoker\/workspace","LOGNAME":"ubu22","XDG_SESSION_TYPE":"tty","ftp_proxy":"http:\/\/127.0.0.1:3128","MOTD_SHOWN":"pam","HOME":"\/home\/ubu22","LANG":"C.UTF-8","WSL_INTEROP":"\/run\/WSL\/8_interop","LS_COLORS":"rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=00:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arc=01;31:*.arj=01;31:*.taz=01;31:*.lha=01;31:*.lz4=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.tzo=01;31:*.t7z=01;31:*.zip=01;31:*.z=01;31:*.dz=01;31:*.gz=01;31:*.lrz=01;31:*.lz=01;31:*.lzo=01;31:*.xz=01;31:*.zst=01;31:*.tzst=01;31:*.bz2=01;31:*.bz=01;31:*.tbz=01;31:*.tbz2=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.war=01;31:*.ear=01;31:*.sar=01;31:*.rar=01;31:*.alz=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.cab=01;31:*.wim=01;31:*.swm=01;31:*.dwm=01;31:*.esd=01;31:*.jpg=01;35:*.jpeg=01;35:*.mjpg=01;35:*.mjpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.webm=01;35:*.webp=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=00;36:*.au=00;36:*.flac=00;36:*.m4a=00;36:*.mid=00;36:*.midi=00;36:*.mka=00;36:*.mp3=00;36:*.mpc=00;36:*.ogg=00;36:*.ra=00;36:*.wav=00;36:*.oga=00;36:*.opus=00;36:*.spx=00;36:*.xspf=00;36:","https_proxy":"http:\/\/127.0.0.1:3128","LESSCLOSE":"\/usr\/bin\/lesspipe %s %s","XDG_SESSION_CLASS":"user","TERM":"xterm-256color","LESSOPEN":"| \/usr\/bin\/lesspipe %s","USER":"ubu22","NO_PROXY":"localhost, 127.0.0.*, 10.*, 172.*, 192.168.*,.apac.igus.net","FTP_PROXY":"http:\/\/127.0.0.1:3128","DISPLAY":":0","SHLVL":"1","HTTPS_PROXY":"http:\/\/127.0.0.1:3128","HTTP_PROXY":"http:\/\/127.0.0.1:3128","XDG_SESSION_ID":"c1","http_proxy":"http:\/\/127.0.0.1:3128","XDG_RUNTIME_DIR":"\/run\/user\/1000","WSLENV":"WT_SESSION:BASH_ENV\/u:WT_PROFILE_ID","XDG_DATA_DIRS":"\/usr\/local\/share:\/usr\/share:\/var\/lib\/snapd\/desktop","HUSHLOGIN":"FALSE","PATH":"\/usr\/local\/arcanist\/bin:\/usr\/local\/nodejs\/bin:\/opt\/apache-maven\/bin:\/opt\/java\/openjdk\/bin:\/usr\/local\/sbin:\/usr\/local\/bin:\/usr\/sbin:\/usr\/bin:\/sbin:\/bin:\/usr\/games:\/usr\/local\/games:\/usr\/lib\/wsl\/lib:\/mnt\/c\/ProgramData\/Oracle\/Java\/javapath:\/mnt\/c\/WINDOWS\/system32:\/mnt\/c\/WINDOWS:\/mnt\/c\/WINDOWS\/System32\/Wbem:\/mnt\/c\/WINDOWS\/System32\/WindowsPowerShell\/v1.0\/:\/mnt\/c\/WINDOWS\/System32\/OpenSSH\/:\/mnt\/c\/Program Files (x86)\/AdminStudio\/9.0\/Common\/:\/mnt\/c\/Program Files\/PuTTY\/:\/mnt\/c\/Program Files\/Git\/cmd:\/mnt\/c\/Program Files\/Microsoft VS Code\/bin:\/mnt\/c\/Program Files\/dotnet\/:\/mnt\/c\/Users\/ncoker\/AppData\/Local\/Microsoft\/WindowsApps:\/mnt\/c\/Program Files (x86)\/Nmap:\/snap\/bin","SUDO_UID":"1000","DBUS_SESSION_BUS_ADDRESS":"unix:path=\/run\/user\/1000\/bus","MAIL":"\/var\/mail\/ubu22","OLDPWD":"\/home\/ubu22","_":"\/usr\/bin\/php"}

thank you @ncoker, but:

  1. This is not a task to discuss specific errors when trying to run on php 8, and
  2. We don't expect arcanist to work on php 8 yet

You can put your error trace in a new Ponder question, and roll back to php 7 until we announce support for php 8.

You can follow T15064 for updates about specific issues.

Would taking care of the depreciations bump the minimum PHP version required to run Phorge?
looks good to me otherwise

image.png (954×870 px, 87 KB)

Would taking care of the depreciations bump the minimum PHP version required to run Phorge?

Not in itself, but we do plan to bump the minimum to 7.1 (in T15047).

avivey renamed this task from Handling PHP deprecations: Maybe convert to Setup Issues? to Handling PHP deprecations: convert to Setup Issues.Sep 1 2023, 08:54
avivey claimed this task.
avivey removed a project: Discussion Needed.
avivey mentioned this in 2023 week 49.

Unless something broke, this task is now complete.