perftools / xhgui

Web interface for XHProf profiling data can store data in MongoDB or PDO database
1.65k stars 341 forks source link

[slim3] Run xhgui in subdir #450

Closed luzip665 closed 2 years ago

luzip665 commented 2 years ago

A little change to be able to run xhgui with slim3 in a subdir.

I need to run xhgui on https://localhost/xhgui/. The gui loads in general. Links work, but css and js are not found because the links are going to / instead of /xhgui

glensc commented 2 years ago

Please commit with sane commit messages, so I could merge the change to intermediate branch

glensc commented 2 years ago

I don't see what problem are you solving here. describe the problem first in PR body!

luzip665 commented 2 years ago

Described in body.

I changed in line 24ff the null handling to set $path_prefix to empty string in case of null. So in url the pathPrefix can directly be used, because $this->router->urlFor allready resolves the subdir. May be the function pathPrefix can be removed if it is only used in staticUrl.

The function pathPrefix does autodetection in case of $this->pathPrefix is an empty string.

glensc commented 2 years ago

If the detection doesn't work, you can just set path.prefix in config.

also, I do not understand anything written in this comment:

luzip665 commented 2 years ago

If I set path.prefix to xhgui in the config, everything is prefixed with this: Starting from http://localhost/xhgui/, Links (in the menubar) are going to http://localhost/xhgui/xhgui/. css to http://localhost/css/.... This happens also in case of path.prefix is null.

Setting path.prefix to empty string results in: Starting from http://localhost/xhgui/, Links (in the menubar) are going to http://localhost/xhgui/xhgui/. css to http://localhost/xhgui/css/....

In the comment above I tried to explain what I did and why in my opinion the autodetection still works.

glensc commented 2 years ago

Compare the html, show the rendered html tag for css. guess the problem is with leading / making requests invalid. I guess I have to test this locally then to see myself.

what adds the second /xghui after the pathPrefix?

glensc commented 2 years ago

Tested locally. having path.prefix=/xhgui, the html is:

<link href="/xhgui/css/xhgui.css" rel="stylesheet" media="screen">

Using docker image provided by this project, i.e this nginx config:

luzip665 commented 2 years ago

I think:

If set path.prefix to null:

luzip665 commented 2 years ago

Tested locally. having path.prefix=/xhgui, the html is:

<link href="/xhgui/css/xhgui.css" rel="stylesheet" media="screen">

Using docker image provided by this project, i.e this nginx config:

What about the link to Longest wall time?

glensc commented 2 years ago

$this->request->getUri()->getBasePath(), which is in my case /xhgui

this seems correct value, since you have installation at /xhgui.

how can I reproduce your setup?

also, to me it seems you want to use path.prefix='' or path.prefix='/' in your config. (remember path.prefix=null means autodetection!)

luzip665 commented 2 years ago

My setup has nothing special. But it uses #333.

In case of path.prefix=null, I get /xhgui/xhgui for non-static urls and /xhgui for static. In case of path.prefix='', I get /xhgui/ for non-static urls and ` for static. In case ofpath.prefix='/xhgui', I get/xhgui/xhguifor non-static urls and/xhgui` for static.

Imo the main problem is in the url function that does the autodetection twice.

glensc commented 2 years ago

Special or not. explain how to reproduce this.

From description seems getBasePath() returns xhgui for you and "" for me and it's not symmetric for static URLs. therefore the inconsistency for you.

your current pull request doesn't fix any problem as you just set the path prefix to an empty string, so the autodetection code is never run.

glensc commented 2 years ago

Added documentation examples how the path prefix must behave:

luzip665 commented 2 years ago

I use an apache with

DocumentRoot /var/www/ Alias /xhgui/ /var/www/xhgui/webroot/

glensc commented 2 years ago

I use an apache with

DocumentRoot /var/www/ Alias /xhgui/ /var/www/xhgui/webroot/

Can you make the reproducer with apache docker image?

luzip665 commented 2 years ago

Further digging bring up that in /vendor/slim/slim/Slim/App.php in Line 381 the router basePath is set to $request->getUri()->getBasePath()

I try to make it reproducibly

luzip665 commented 2 years ago

Reproducing with docker image at tecart/crm_v51. This is one image we work with.

xhgui is bind-mounted to /var/www/xhgui

In the container you need to disable tideways using phpdismod tideways

Then you can access xhgui via http://172.17.0.2/xhgui/

The problem relates only to branch slim-upgrade. 0.20.x works

glensc commented 2 years ago

slim-upgrade branch is not merged, so 0.19.x and 0.20.x branches are the same

glensc commented 2 years ago

Here's why the basepath is set on the router. I don't see it helps

Stack trace:
#0 /var/www/xhgui/vendor/slim/slim/Slim/App.php(381): Slim\\Router->setBasePath('/xhgui')
#1 /var/www/xhgui/vendor/slim/slim/Slim/App.php(297): Slim\\App->process(Object(Slim\\Http\\Request), Object(Slim\\Http\\Response))
#2 /var/www/xhgui/src/Application.php(26): Slim\\App->run()
#3 /var/www/xhgui/webroot/index.php(10): XHGui\\Application->run()
#4 {main}
  thrown in /var/www/xhgui/vendor/slim/slim/Slim/Router.php on line 105
glensc commented 2 years ago

Ok, so the problem is that same basepath added twice, made change that strips from urlFor result:

That replacement PR works in docker image you provided.

glensc commented 2 years ago

Also, please fill in meaningful commit messages if you want your changes to be merged. repeating the same commit message 5 times, isn't it.

Here's some guides:

luzip665 commented 2 years ago

Sorry for that. I'll try! I was still in figuring out where is the difference.