cli usage

[rgubby]

I include F3 in a project, but then have an independent script that happens to use the same autoloader, it pulls in F3 (even though I don't need it) and causes this error to appear:

Undefined index: path
Fatal error: Undefined index: HALT

The problem happens here: https://github.com/bcosca/fatfree/blob/master/lib/base.php#L1893

It's because in CLI mode, $uri['path'] doesn't exist. Maybe something like this would do the trick:

if (PHP_SAPI!='cli'){
    $path=preg_replace('/^'.preg_quote($base,'/').'/','',$uri['path']);
} else{
    $path=preg_replace('/^'.preg_quote($base,'/').'/','','/');
}

[xfra35]

In CLI mode, $uri['path'] should exist. Did you provide the URI to the script?

Syntax in CLI mode is php index.php /my/uri.

[rgubby]

I'm using the script like this:

php scripts/script.php argument1 argument2

I'm not using F3 for this particular script, it's a utility script that is being used to import data into models. It's part of the repo, it just shouldn't be handled at all by F3.
Because $uri['path'] isn't defined, it dies because the HALT flag is set to true.

The common thing between the F3 controlled pages and my script, is an include file that references the composer autoloader:

require $appRoot.'/vendor/autoload.php';

I'm using composer in this script for other libraries and not f3 (so turning off composer is not an option). Clearly f3 dieing because $uri['path'] isn't set on the command line is a pretty obvious bug.

[xfra35]

Maybe you can clarify which version of PHP you're using as well as an example of argument1 throwing the error?

The parse_url function is not free of bugs.. for example https://bugs.php.net/bug.php?id=54180

[sgpinkus]

but then have an independent script that happens to use the same autoloader

HI. Can you clarify what autoloader are you using?

[rgubby]

Version of PHP:

$ php -v
PHP 5.3.10-1ubuntu3.10 with Suhosin-Patch (cli) (built: Feb 28 2014 23:14:25) 
Copyright (c) 1997-2012 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2012 Zend Technologies

I'm using the composer autoloader - I've only brought in F3 via composer and it's being loaded up via autoloader_files.php.

In my example, both the web route (using F3) and the back end script use the same include, which references the composer autoloader. In my web route, I .htaccess files to index.php - my back end script is a completely separate php file and I don't actually want to use F3 for it.

[sgpinkus]

Regardless of whether there is some issue with argument parsing, I think the F3 autoloader is also an issue. Its instantiating of Base when its not necessarily needed. The autoloading needs to be rewritten. Known issue - see #741.

There maybe some other legit issue going on here with args. Not sure.

[xfra35]

I'm not familiar with composer but judging from the docs and from the project composer.json I understand that base.php is required automatically (instead of being autoloaded).

That could explain why base.php is called even though scripts/script.php doesn't require it.

Any composer user to confirm that behaviour?

[sgpinkus]

@xfra35 I can confirm that is what is happening.

[bcosca]

The question is: why should an autoloader care if an object is instantiated, or for that same reason, should an autoloader care if a PHP constant suddenly appeared? It shouldn't. It should only care about loading the class... whatever happens next is up to the app. Not Composer, not the framework. Where does the responsibility lie?

[xfra35]

As I understand it, the current composer.json is defined in such a way that F3 is automatically required before we even call it. In other words, when using Composer, people get an autoloader + an automatic require('lib/base.php'). This seems to be the issue here, since @rgubby is using a script which doesn't need F3 but has it automatically loaded anyway.

This behavior was introduced in #721.

[rgubby]

I'm ok (kind of) with it requiring lib/base.php - but having it halt because a variable isn't set, I'm not so ok with.

Ignoring the fact that it's automatically requiring the file, I should be able to use other scripts and not use F3 if I don't want to. My view is that if I have a command line script to run in the same directory as the project I'm using F3 (I can't be the only one doing this, right?), then F3 should have a responsibility to not break the script because of a missing variable - missing because it's come from the CLI and not handled.

[xfra35]

The thing is that the framework works perfectly in CLI mode. So the error comes from parse_url choking on something. Could be a PHP bug.. could be caused by your argument1. Hence my request to see it.

[xfra35]

In order to help debug this issue, you could add a var_dump($_SERVER) in base.php just after this line and tell us what it prints out. The criticial values here are argc/argv.

[KOTRET]

for me the whole issue reads like this:
using any component referencing Base forces the autoloader to load base.php which will create a new instance of Base because of the last line in the file. This throws the error (remember: this was in __construct()... In this case i agree: it's an issue and the cause is convenience.

[xfra35]

using any component referencing Base

He's not using Base at all.

[KOTRET]

so then why the class is included / required? the constructor does not load by any magic

[rgubby]

Here's argv:

["argv"]=>
  array(3) {
    [0]=>
    string(46) "src/scripts/test.php"
    [1]=>
    string(18) "http://example.com"
    [2]=>
    string(5) "users"
  }

The first param is a URL - which doesn't have a slash at the end, which means parse_url doesn't have a path.

A defensive check to make sure it exists would do the trick. Or stop base.php being loaded - seeing as I'm not using it at all then that would make the most sense to me.

[ikkez]

@KOTRET because of this line: https://github.com/bcosca/fatfree/blob/master/composer.json#L16

[KOTRET]

yes, but in general requiring a file does not mean to instantiate the class in it. This has been done for convenience if F3 is the starter app. So in this case the last line in base.php has to be removed.

[xfra35]

@KOTRET when would one need to require base.php without instantianting the class?

@rgubby you're right about parse_url not filling path on http://example.com. But I disagree with the need to modify the framework for this case. The framework expects a URI and nothing else, so that's a good thing to throw an error in this situation.

Couldn't there be another way to configure composer.json to avoid base.php being automatically loaded?

[xfra35]

Couldn't files be replaced by classmap?

"autoload": {
    "classmap": ["lib/"]
}

I just gave it a try and it works fine: framework libraries are auto-loaded only when necessary.

[KOTRET]

@xfra35

$f3 = require('path/to/base.php');

already creates an instance. In this case its more convenient instead of

require('path/to/base.php');
$f3 = new \Base();

using an autoloader you can call $f3 = new \Base(); alone, which will create TWO instances if im correctly: reuquiring the file itself will create one (because of line #L2931), which will be dumped as not used, and the \new Base() will create another one.

back to the topic this means: as soon as the file gets required, it will start the framework, its irrelevant if you instantiate Base yourself.

[xfra35]

its irrelevant if you instantiate Base yourself

On this point: the correct way of invoking the framework is Base::instance() so new Base() should be forbidden. The framework should probably throw an error in such a situation, but at the very least there should be a mention about it in the docs, I realized that it's nowhere mentioned.

Anyway this doesn't relate to the current issue, which is:

without Composer:

$f3=require('lib/base.php');//<-- base.php explicitly required and loaded

with Composer:

require('vendor/autoload.php');//<-- base.php NOT explicitly required BUT loaded anyway

[KOTRET]

ah okay, then your suggestion is correct: as of the documentation the usage of autoload files will require the file:

If you want to require certain files explicitly on every request then you can use the 'files' autoloading mechanism. This is useful if your package includes PHP functions that cannot be autoloaded by PHP.

Example:

{
    "autoload": {
        "files": ["src/MyLibrary/functions.php"]
    }
}

[xfra35]

Exactly. Also a side effect is that other libraries are not autoloaded (template.php, db/sql.php etc..).

I think that classmap fixes it all.

[KOTRET]

@xfra35 you are right with Base::instance(), forgot that =) but what about the two instances of the framework (one is thrown away)?

[xfra35]

What do you mean? If new Base is not called, there won't ever be two instances.

[KOTRET]

read the last line in base.php, this will create one instance automatically on requiring the file?
So this will do:

1. can has a new Base::instance() plz?
2. Autoloader, please load the class Base
3. Autoloader looks for base.php
4. Autoloader require base.php
   1. requiring base.php will create an instance (as defined in the last line!)
   2. the instance will do stuff as defined in constructor <--- imho this is the root cause (of the topic here)
5. here is your new instance: $f3 = Base::instance()

[xfra35]

I still don't get it. The last line is return Base::instance() not return new Base().

You can call Base::instance() 1000 times, only one instance will be created.

[KOTRET]

The bug is the last line itself. This will create an instance of Base, no matter if you use it later!

[xfra35]

I feel like this conversation is looping ;)

Indeed it will create an instance, but that won't result in having two instances at any time.

[KOTRET]

@xfra35 : the problem is that the error is thrown in the constructor, so it DOES matter if it will create an instance or not.
My assumption with two instances was wrong, yes, but overall this is still correct: as soon as you require base.php, it will create Base::instance()

[xfra35]

Ok. But why would anybody require base.php and not instantiate Base? The problem is not about base.php auto-instantiating, it's about Composer auto-requiring base.php when nobody expects it.

[KOTRET]

it is expected as of the composer entry you mentioned ^^ The bug itself will be fixed with the suggestion, but the general discussion i initiated will remain.
Just because i require a file, i do not expect that loading it will create an instance automatically. This is (imho) a (bad) sideeffect and should be removed. It just saves a one-liner...

[ikkez]

@xfra35 so composers "Classmap" option somehow make the require of the base class lazy loading? can you confirm that, and that this CLI issue should be solved with it?

[xfra35]

From my tests yes.

@rgubby maybe you can change that line in composer.json, re-run composer install and confirm that it helps?

[rgubby]

@xfra35 I can confirm that this is working as expected.

My script is now running exactly as I want it to and does not halt. I've also noticed that a PHP notification I was hiding with php.ini settings, which appeared due to strict checking in F3 has now gone again.

Ignoring the fact that I'm removing certain types of PHP error messages, it's a good thing that F3 isn't trying to interfere with this.

Thanks for getting to the bottom of it!