search

codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.38k stars 1.9k forks source link

Debug Toolbar error not found tpl error and fix #2275

Closed demortx closed 5 years ago

demortx commented 5 years ago

Describe the bug NOT FOUND (/var/www/www-root/data/www/site.com/system/Debug/Toolbar/Views/_database.tpl) -> real name _database.tpl.php

Fix _database.tpl.php copy and rename _database.tpl and etc files

CodeIgniter 4 version 4.0.0-rc.2

Affected module(s) Which package or class is the bug in, if known.

CRITICAL - 2019-09-27 11:33:34 --> Invalid file: {0}

0 /var/www/www-root/data/www/site.com/system/View/Parser.php(156): CodeIgniter\Exceptions\FrameworkException::forInvalidFile('') -> NOT FOUND (/var/www/www-root/data/www/site.com/system/Debug/Toolbar/Views/_database.tpl) -> real name _database.tpl.php

1 /var/www/www-root/data/www/site.com/system/Debug/Toolbar/Views/toolbar.tpl.php(139): CodeIgniter\View\Parser->render('_files.tpl')

2 /var/www/www-root/data/www/site.com/system/Debug/Toolbar.php(495): include('/var/www/www-ro...')

3 /var/www/www-root/data/www/site.com/system/Debug/Toolbar.php(453): CodeIgniter\Debug\Toolbar->format(Array, 'html')

4 /var/www/www-root/data/www/site.com/app/Config/Events.php(41): CodeIgniter\Debug\Toolbar->respond()

5 [internal function]: CodeIgniter\Events\Events::Config{closure}()

6 /var/www/www-root/data/www/site.com/system/Events/Events.php(187): call_user_func(Object(Closure))

7 /var/www/www-root/data/www/site.com/system/CodeIgniter.php(226): CodeIgniter\Events\Events::trigger('pre_system')

8 /var/www/www-root/data/www/site.com/public_html/index.php(45): CodeIgniter\CodeIgniter->run()

9 {main}

Context

dayrui commented 5 years ago

/system/View/Parser.php error, bug

`$fileExt = pathinfo($view, PATHINFO_EXTENSION); $view = empty($fileExt) ? $view . '.php' : $view; // allow Views as .html, .tpl, etc (from CI3)

    // Was it cached?
    if (isset($options['cache']))
    {
        $cacheName = $options['cache_name'] ?? str_replace('.php', '', $view);

        if ($output = cache($cacheName))
        {
            $this->logPerformance($start, microtime(true), $view);
            return $output;
        }
    }

    $file = $this->viewPath . $view;`
dayrui commented 5 years ago

fix $view = $view . '.php'; $file = $this->viewPath . $view;

MGatner commented 5 years ago

I noticed this today as well. @dayrui I'm not sure where your fix ix supposed to go - could you send over a PR? Or clarify? Did you mean to add before line 146:

$view = $view . '.php';

So the updated version is:

    $view = $view . '.php';
    $file = $this->viewPath . $view;

I think that doesn't fix the problem of allowing other extensions, just compound extensions.

MGatner commented 5 years ago

I think changing the file extensions in system/Debug/Toolbar/Views from .tpl.php to just .tpl might be the way to go, now that we support non-PHP files.

MGatner commented 5 years ago

That works for everything except toolbar.tpl.php which appears to be included separately and needs the compound extension.

InsiteFX commented 5 years ago

I' am also getting an Error

MGatner commented 5 years ago

@jim-parry We might need to consider a hotfix or the Toolbar isn’t going to work until RC.3. Could you or @lonnieezell check out #2280 and #2281 and decide on an approach, then we can at least get dev patched?

jim-parry commented 5 years ago

My preference would be to have them have just a ".tpl" file extension. That would let us eliminate a phpunit filter and keep them out of the testing more cleanly.

However, I don't where these are all used. and I would be surprised if it's just a matter of renaming them. I did a search in my IDE, and found Commands/Sessions/CreateMigration and Debug/Toolbar. Is that all that uses them?

If we are thinking a hotfix, the cleanest would be to have all the changes in the one PR, which would then be based on master, not develop.

jim-parry commented 5 years ago

Something niggly that might be impacting the toolbar: Debug/Toolbar/Views/toolbarloader.js.php has messed up quoting, lines 78-79

MGatner commented 5 years ago

That's what #2280 is. It passed all tests, FWIW. The template in CreateMigration is unrelated to the toolbar, so I think changing it would be good but not as part of this potential hotfix. I can re-submit #2280 off master.

michalsn commented 5 years ago

In CreateMigration we're using full file name with extension, so it's all good.

jim-parry commented 5 years ago

The tests don't break, but there is no Debug/ToolbarTest, so we wouldn't know :( CreateMigration references a different template, not the toolbar ones, so we are good there (for unit testing) I would think that renamikng that template would make sense later, for consistency.

toolbar.tpl.php is referenced in Toolbar::495. That would need to be changed as part of #2280.

jim-parry commented 5 years ago

It sounds like we are on the same page, and online at the same time. Weird, but effective!

MGatner commented 5 years ago

toolbar.tpl.php is actually a PHP file and is included directly rather than passed through the parser. I'm not sure why it has the .tpl extension but I'd feel more comfortable leaving it out of this for now.

Re: tests I have been using the toolbar with renamed templates in my dev environment and can confirm that all collectors work and the toolbar displays.

MGatner commented 5 years ago

Okay #2283 sent - does nothing except rename the actual templates for collector views. PR is for master, not sure how you want to handle that and if to release.

jim-parry commented 5 years ago

This would affect the codeigniter4, appstarter and framework distributions. I will merge & update the releases.

lonnieezell commented 5 years ago

I’m late to the party ok this one but the bigger question I have is why don’t .pho extensions work anymore? I know we had the recent PR that allowed different extensions on the parser but didn’t realize that killed the ability to use the current extension.

Sent from my iPhone

On Sep 28, 2019, at 9:13 AM, Project lead, CodeIgniter notifications@github.com wrote:

 This would affect the codeigniter4, appstarter and framework distributions. I will merge & update the releases.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jim-parry commented 5 years ago

@lonnieezell See #2283 - Matthew explained that in the "fix"

MGatner commented 5 years ago

@michalsn has #2281 which took the approach of pointing the code back to the files. I went with renaming the files to match the code, mostly because they are actually template files and not PHP code.

jim-parry commented 5 years ago

@lonnieezell I need to hotfix the framework distro as is. Does is make sense to include this change, and redo all three distros and the codebase repo? If so, is it ok to just re-release them, or should be do an "rc.2b"?

lonnieezell commented 5 years ago

Probably better to give it a new release name I would think.

Sent from my iPhone

On Sep 28, 2019, at 9:52 AM, Project lead, CodeIgniter notifications@github.com wrote:

 @lonnieezell I need to hotfix the framework distro as is. Does is make sense to include this change, and redo all three distros and the codebase repo? If so, is it ok to just re-release them, or should be do an "rc.2b"?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jim-parry commented 5 years ago

Agreed. I had to do that before too. coedigniter4 and framework distros affected. will post & tweet

MGatner commented 5 years ago

Resolved by 8ba099e