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

Is it possible to change the PHP version that XHGui runs on? #2439

Closed
garethredwire opened this issue Apr 13, 2021 · 42 comments
Closed

Is it possible to change the PHP version that XHGui runs on? #2439

garethredwire opened this issue Apr 13, 2021 · 42 comments

Comments

@garethredwire
Copy link

Hi,

XHGui will only run up to PHP 7.3 yet the default for VVV is currently php 7.4

I know how to change the PHP versions for each website, but how can we change the version for XHGui so that it works?

Thanks in advance.

@welcome
Copy link

welcome bot commented Apr 13, 2021

Thanks for opening your first issue here! Be sure to follow the issue template and include your OS/Vagrant/VVV versions! Don't forget you can get support in the VVV slack at https://varyingvagrantvagrants.org/docs/en-US/slack/

VVV
Join the VVV Slack Workspace

@tomjn
Copy link
Member

tomjn commented Apr 13, 2021

Do you know what prevents it from running on 7.3? There's no mechanism for this at the moment

@tomjn
Copy link
Member

tomjn commented Apr 13, 2021

Looks like XHGUI uses a very old version of the slim framework which doesn't support 7.4

@garethredwire
Copy link
Author

Hi Tom, "Do you know what prevents it from running on 7.3?" Do you mean, what's preventing VVV from running as default no 7.3 or XHGUI?

Ultimately I'd like VVV nginx to run the XHGUI on 7.3 but I'm not sure how to do that since there's no where to specify which version it uses, or am I missing something?

@tomjn
Copy link
Member

tomjn commented Apr 14, 2021

You would need to add the PHP 7.3 utility then modify the nginx configs to force the dashboard domain to use 7.3. There are no instructions for how to do this, I recommend against doing this.

Or, help finish xhgui's upgrade of the slim library from v2 to v3. This is the preferred solution. It's unclear what the remaining blockers or tasks are on the xhgui end:

perftools/xhgui#333

@Mte90
Copy link
Member

Mte90 commented Apr 14, 2021

You would need to add the PHP 7.3 utility then modify the nginx configs to force the dashboard domain to use 7.3. There are no instructions for how to do this, I recommend against doing this.

Or, help finish xhgui's upgrade of the slim library from v2 to v3. This is the preferred solution. It's unclear what the remaining blockers or tasks are on the xhgui end:

perftools/xhgui#333

Indeed also xhgui uses the default php version of VVV.

Why you want to change the php version for xhgui? there are some reasons?

@tomjn
Copy link
Member

tomjn commented Apr 14, 2021

@Mte90 xhgui depends on slim/slim but uses an ancient version which is broken on 7.4

@garethredwire
Copy link
Author

I've already added PHP 73 into my core and I've added the following line into my xhgui.conf by SSHing onto the virtual machine.

set $upstream php73;

I've restarted nginx and and PHP FPM, however when I do phpinfo() from inside xhgui it says it's still running php7.4

Am I missing something?

@Mte90
Copy link
Member

Mte90 commented Apr 14, 2021

well you need a provision in this way the nginx conf file with your changes is applied.
I am not sure if that a provision will reset the changes you are doing...

@garethredwire
Copy link
Author

Exactly that, I thought reprovision would reset my config change. I will try. Thanks.

@garethredwire
Copy link
Author

  • I added set $upstream php73; into vagrant/provision/utilities/core/tideways/nginx.conf
  • Executed vagrant reload --provision
  • vagrant ssh onto the machine, checked the xhgui.conf and it does have set $upstream php73; still present
  • Yet the phpinfo() returned by http://xhgui.vvv.test/ still says it's running PHP 7.4

@tomjn
Copy link
Member

tomjn commented Apr 15, 2021

Because provisioning reset your changes before the file got installed. We always hard reset the utility checkout so that people don't get stuck on ancient versions due to accidental changes.

@tomjn
Copy link
Member

tomjn commented Apr 15, 2021

Have you contacted the xhgui maintainers yet? If nobody says they have the issue, then upgrading to 7.4 will be considered a lower priority at their end.

@kmgalanakis
Copy link

kmgalanakis commented Apr 29, 2021

OK, it wasn't that hard to fix this, even though I think it's a temporary thing as I think this fix is going to go away when you provision. So here is what I did:

  • Install the php73 utility along with all the utilities related to tideways and xhgui.
  • SSH your VVV machine.
  • Run sudo nano custom-utilities/xhgui.conf
  • Adjust this as seen below
    image
  • Run sudo systemctl restart nginx.service
  • You should be good to go.

image

@Mte90
Copy link
Member

Mte90 commented Apr 29, 2021

I guess that this won't be kept with a provision.

@kmgalanakis
Copy link

kmgalanakis commented Apr 29, 2021

That's true, but if you want to make it "persistent" you can also do this instead:

  • Install the php73 utility along with all the utilities related to tideways and xhgui.
  • Edit vagrant/provision/utilities/core/tideways/nginx.conf.
  • Change fastcgi_pass value from php to php73
  • You are good to go!

What do you think?

@tomjn
Copy link
Member

tomjn commented Apr 29, 2021 via email

@kmgalanakis
Copy link

We hard reset that folder on provision so that you always get the latest version of the repo. The change will be undone on the next provision

Yes but what if this change happens to the nginx.conf file that the provisioned will use?

I tested it myself and it seems that the change survives the provision.

@Mte90
Copy link
Member

Mte90 commented Apr 29, 2021

yes the only solution is a parameter for the xhgui provision that let you to change it.

The changes is kept because you broken the git repo so you won't get updates as a git pull will create an error.

@tomjn
Copy link
Member

tomjn commented Apr 29, 2021

@Mte90 I would veto such a parameter, we should not fix another projects failing by trying to make it configurable. The moment xhgui fixes their problem the feature becomes useless and misleading.

The fix for this needs to happen in the xhgui project.

@kmgalanakis @garethredwire neither of you have raised an issue over at xhgui or commented on the issues or PR's. Their maintainer probably thinks this issue only impacts a small number of people and is low priority

Yes but what if this change happens to the nginx.conf file that the provisioned will use?

That file is tracked by git, it will be hard reset back to what the remote repo has on the next provision. If that is not what happens then that is a bug.

@tomjn
Copy link
Member

tomjn commented Apr 29, 2021

What needs to happen:

  • someone needs to write a PR to change it in the repo, not a local change that gets destroyed on every provision
  • everybody who has this issue needs to make it known on the xhgui projects relevant issue When I update PHP to 7.4. Slim reports an error. perftools/xhgui#278
  • test out the PR so the maintainer has data to work with. They may have fixed it already! Who knows, nobody tested it and told him.

@tomjn
Copy link
Member

tomjn commented Apr 29, 2021

As for the original question if there is a way to configure VVV to use PHP 7.3 for xhgui, no there is not, and we will not be adding one. If xhgui is broken then a flag to change things is not the solution. Fixing it is the solution.

@Mte90
Copy link
Member

Mte90 commented Apr 29, 2021

@Mte90 I would veto such a parameter, we should not fix another projects failing by trying to make it configurable. The moment xhgui fixes their problem the feature becomes useless and misleading.

I agree with you I didn't explained well. My was a way to keep that version with provision but altering the files will always have issues on provision.

@tomjn
Copy link
Member

tomjn commented Apr 29, 2021

if everyone using xhgui has to make this change, then we should just make this change

@tomjn
Copy link
Member

tomjn commented Apr 29, 2021

Varying-Vagrant-Vagrants/vvv-utilities#89 tries to run the 7.3 provisioner and force XHGui to use 7.3, it needs testing

@garethredwire
Copy link
Author

Thanks for your work on this. I can confirm PHP 7.3 is now being loaded for Xhgui.

It now hits a new error when examining a profile record. I'll post it here so you can tell me if it's something that I need to fix or that needs updating in the provision (I'm aware that this should probably have it's own bug thread):

XHGui\Searcher\PdoSearcher::getForUrl not implemented for PDO

You should check the following things:

Ensure that Mongo has been started
That config/config.php has the right connection information.
That the cache/ directory has the correct permissions.

#0 /srv/www/default/xhgui/src/Searcher/PdoSearcher.php(73): XHGui\Exception\NotImplementedException::notImplementedPdo('XHGui\Searcher\...')
#1 /srv/www/default/xhgui/src/Controller/RunController.php(212): XHGui\Searcher\PdoSearcher->getForUrl('/', Array, Array)
#2 /srv/www/default/xhgui/src/ServiceProvider/RouteProvider.php(90): XHGui\Controller\RunController->url(Object(Slim\Http\Request))
#3 [internal function]: XHGui\ServiceProvider\RouteProvider::XHGui\ServiceProvider{closure}()
#4 /srv/www/default/xhgui/vendor/slim/slim/Slim/Route.php(468): call_user_func_array(Object(Closure), Array)
#5 /srv/www/default/xhgui/vendor/slim/slim/Slim/Slim.php(1355): Slim\Route->dispatch()
#6 /srv/www/default/xhgui/vendor/slim/slim/Slim/Middleware/Flash.php(85): Slim\Slim->call()
#7 /srv/www/default/xhgui/vendor/slim/slim/Slim/Middleware/MethodOverride.php(92): Slim\Middleware\Flash->call()
#8 /srv/www/default/xhgui/vendor/slim/slim/Slim/Middleware/SessionCookie.php(110): Slim\Middleware\MethodOverride->call()
#9 /srv/www/default/xhgui/vendor/slim/slim/Slim/Slim.php(1300): Slim\Middleware\SessionCookie->call()
#10 /srv/www/default/xhgui/src/Application.php(26): Slim\Slim->run()
#11 /srv/www/default/xhgui/webroot/index.php(8): XHGui\Application->run()
#12 {main}

@Mte90
Copy link
Member

Mte90 commented Apr 30, 2021

Uhm maybe on the php side the composer stuff is for another php version (I had a similar problem in the past). I didn't tested so much, I will investigate a bit.

@Mte90
Copy link
Member

Mte90 commented Apr 30, 2021

Working on a fix Varying-Vagrant-Vagrants/vvv-utilities#90, it is not just xhgui to run in nginx with php 7.3 but also installed so you need to remove the folder in www/default/xhgui in this way is installed for that version.

@Mte90
Copy link
Member

Mte90 commented Apr 30, 2021

Ok so it is required to remove www/default/xhgui and www/default/php-profiler and run my pr to get everything working.
So we need probably a way to check the php version used to install xhgui to do it automatically or we leave to the user that part.

@tomjn
Copy link
Member

tomjn commented Apr 30, 2021

It now hits a new error when examining a profile record. I'll post it here so you can tell me if it's something that I need to fix or that needs updating in the provision (I'm aware that this should probably have it's own bug thread):

This was always the case, there's nothing that can be done at our end. What's more the UX is misleading, you're not opening an individual profile, you're opening a list of profiles filtered by that URL

@tomjn
Copy link
Member

tomjn commented Apr 30, 2021

Ok so it is required to remove www/default/xhgui and www/default/php-profiler and run my pr to get everything working.
So we need probably a way to check the php version used to install xhgui to do it automatically or we leave to the user that part.

can you explain? This hasn't been an issue in the past when we've changed PHP version

@Mte90
Copy link
Member

Mte90 commented Apr 30, 2021

On my testing I had to remove that folders or the provision don't work at 100%.
Maybe it is just me.

@tomjn
Copy link
Member

tomjn commented Apr 30, 2021 via email

@garethredwire
Copy link
Author

Unfortunately I tried removing those two folders and provisioning again hit the same error.

@Mte90
Copy link
Member

Mte90 commented Apr 30, 2021

do you have the php73 utility?

@tomjn
Copy link
Member

tomjn commented Apr 30, 2021

@garethredwire which error? If you are referring to XHGui\Searcher\PdoSearcher::getForUrl not implemented for PDO then this has nothing to do with PHP 7.3, and has been the case for years.

@tomjn
Copy link
Member

tomjn commented Apr 30, 2021

@Mte90 the latest tideways utility runs the PHP 7.3 utility provisioner

@tomjn
Copy link
Member

tomjn commented Apr 30, 2021

@garethredwire click on the GET or times in XHGui, not the URL. The method/time link to that run. The URL links to a filter page that needs something not implemented by XHGui's PDO driver

@tomjn
Copy link
Member

tomjn commented Apr 30, 2021

@Mte90 I just reprovisioned without deleting folders and recorded a page profile and viewed it just fine using the develop branch

@garethredwire
Copy link
Author

@garethredwire click on the GET or times in XHGui, not the URL. The method/time link to that run. The URL links to a filter page that needs something not implemented by XHGui's PDO driver

Ah, I assumed they were all the same link. Yes it works fine when clicking on the GET or POST.

Thanks.

@tomjn
Copy link
Member

tomjn commented May 8, 2021

@garethredwire now that the PR to the utilities has been merged, do you still need to do workarounds to use xhgui?

@garethredwire
Copy link
Author

garethredwire commented May 10, 2021 via email

@Mte90 Mte90 closed this as completed May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants