Smarty autoload messes up my environment

[Massimiliano Torromeo]

Hi,
in one of the latest upgrade smarty changed the way it autoloads its
classes with this code:

if (set_include_path(SMARTY_SYSPLUGINS_DIR . PATH_SEPARATOR .
get_include_path()) !== false) {
spl_autoload_extensions('.php,.inc');
$registeredAutoLoadFunctions = spl_autoload_functions();
if (!isset($registeredAutoLoadFunctions['spl_autoload'])) {
spl_autoload_register();
}
} else {
spl_autoload_register('smartyAutoload');
}

The problem is that it changes my include_path and interferes with my
own autoload function by also changing the autoload extensions. In
fact, my application stopped working as soon as I upgraded.
I temporarily solved by replacing the above block with just
spl_autoload_register('smartyAutoload'); , I hope this gets sorted out
because I find it bad practice to interfere with the environment of
tha host application.

Thanks for everything.
Regards.

[Monte Ohrt]

Understood, and I agree, I don't like the idea of Smarty adjusting the include_path without the developer approval. What will happen is the following:

A new flag will be introduced $smarty->use_spl_autoload = false;

This will be false by default, meaning it uses smartyAutoload out of the box. Those that understand spl_autoload and that it adjusts the include_path can enable this flag for a very slight performance gain, making use of the default c-based spl_autoloader.

[Massimiliano Torromeo]

You solution seems perfect to me.
Thank you very much.

Regards.

[Monte Ohrt]

... and we just realize this won't work, so onto plan B, which is revert to the original behavior. Smarty will use the internal smartyAutoloader by default, and it is up to the developer to register their own spl_autoload if they don't like this one.

[dave B.]

Oh it seems to me the problem is in the script.

> >>    spl_autoload_extensions('.php,.inc');

from the phpdocs:
"When calling without an argument, it simply returns the current list
of extensions each separated by comma. "

so maybe it should look something like:
spl_autoload_extensions( trim( '.php,.inc,' .spl_autoload_extensions
(), ',') );

[dave B.]

> because I find it bad practice to interfere with the environment of
> tha host application.

I agree to a certain point but adding a include path to the already
pre-existing include paths is just that:
adding to the environment and keeping the status quo of what is pre-
existing.
The same goes with modifying the autoload extensions by not
overwriting the pre-existing extensions.

To me, it is only *bad* form when you when you *overtake* the
environment, generally thats when scripts start breaking - such as
what was seen in this autoloading code. If every kid plays nicely they
can all play nicely together. ;-)

[Monte Ohrt]

We'll just leave it as an internal autoloader, and if someone wants to integrate something different, that is up to them.

[dave B.]

oh that is a shame that you choose not to fix a simple *incorrect*
implementation to make Smarty faster, more efficient and future
friendly. I wonder how many extra CPU cycles (think energy waste) that
decision causes across the planet. I am sure it is quantifiable. Oh
well. I guess making it work is more important to you than making it
*work better*. Hopefully you will think about this for Smarty4 when
PHP6+ is upon us.

[Dazzle CMS]

well right now for the devs they just want all bugs fixed and they
will fix speed issues and performance issues as my code been built
around smarty 3 uses all the new functions too which i slowly been
rewriting i must say compared to version 2 it is faster when
force_compile is off as if you was around when smarty 1.x or 2.x first
came out you would of seen how it improved over time to what smarty
came to be in its end. but if your not happy with smarty 3 or is not
for you you can aways go back to smarty 2 or write your own being an
open source product just like im working it when everything is done it
will be community involved too

Steve :)

[Massimiliano Torromeo]

On Fri, Nov 6, 2009 at 7:37 PM, dave B. <dave...@gmail.com> wrote:
> I agree to a certain point but adding a include path to the already
> pre-existing include paths is just that:
> adding to the environment and keeping the status quo of what is pre-
> existing.
> The same goes with modifying the autoload extensions by not
> overwriting the pre-existing extensions.

Well, this is not exactly without side-effects. If I was relying on
the autoload to include a certain file that was present in the include
path and you add another path to the list, if there is a file with the
same name it will include the wrong one. There are also other
situations in which this could become a problem. Anyway, adding paths
to the include path, or adding some extensions to the autoload
extensions is not keeping the status quo.

[Monte Ohrt]

Calm down Dave, you can implement your own autoloader that adjusts your
include_path, Smarty is just not going to do it for you. I'm sure there
will be tips and tricks instructions for this too. If you are concerned
about saving the planet, unplug your web servers and walk to work.

[dave B.]

> Well, this is not exactly without side-effects. If I was relying on
> the autoload to include a certain file that was present in the include
> path and you add another path to the list, if there is a file with the
> same name it will include the wrong one.

This is a problem with *ANY* autoloader.
The correct use of namespaces should solve this problem anyway.

[dave B.]

oh sorry no aggression intended, for those who know me I am one of the
calmest guys you will ever meet. ;-)

I just find it a shame that you would rather throw out a whole idea
(reactionary) instead of fixing a simple bug (logically), not even a
few characters in a single line that can be copy and pasted, and even
tested.

What i am concerned about is not whether I can implement a tip or
trick to make my implementation more efficient, i will do this anyway,
but I have concern for those who do *not* have the know-how to
implement such tricks or knowledge of implication and will be running
something less efficient straight out of the box. I believe this does
have an impact.

btw for what its worth, i do walk to work everyday, and shutdown
everything that is not required for 24h serving.
you can be assured, i do more than most.

[Monte Ohrt]

The issue with the spl_autoloader in Smarty is not that it's an easy
fix. The problem is that Smarty must adjust the include_path to make the
autoloader work. IMHO, that is not Smarty's job (or right) to make that
adjustment. This can have adverse affects for some, and I'd rather not
field those support questions in favor of slivering an extra microsecond
off the runtime. The Smarty autoloader is not "slow" by any means, it is
an extremely marginal difference.

I may justify a flag $smarty->use_spl_autoload. This would not be
enabled by default, but it would supply a simple way to enable it
without much knowledge behind it. This would be no different than the
developer disabling compile_check (true by default) to shave off some
runtime.

[dave B.]

IMHO the adjustment is NO different than say adding a autoload
function to the spl_autoload stack generally, or deciding to simply
write a file to disk. If others are having problems with their code
because they clear the spl_autoload_extensions stack, then it is their
code *overtaking* the stack, and they should correct it.

Maybe I need to explain a little more simply.
If you have a file in a directory called 'A' if I place another file
called 'A' in that same directory don't be surprised that the original
file 'A' no longer exists or appears modified. If you place a file in
that directory called 'B' the 'A' file still exists, not changing 'A',
and hence nothing is broken, just extended by the addition of 'B'.

The same thing occurs when you *append* a path to the include list/
extensions list. If you don't *overwrite* the entire list nothing is
broken.

I don't believe any PHP function has any more *rights* than others. In
fact code doesn't have *rights*. If you would apply your logic, "What
gives the right for Smarty to use the fwrite or unlink PHP functions,
they are potentially far more destructive functions than the include
function". That IMHO does not make sense. What makes sense is
implementing 'non destructive' coding practices. Appending the include
path *IS* non-destructive. It is a common practice in many other
programming arenas, (though in most cases they have agreed places
where to store shared libraries so the include path is not changed
that often, though to do so is not taboo if required). If someone's
code breaks because *THEY* implement destructive coding practices,
then it is the problem of the offending code *THEY* implement.

If you implement the fix, you will not have any support issues other
than you can point them back to their own code if nothing in the
Smarty code is being destructive.

Sorry for the long explanation, but I really find it hard to
understand your justifications when I can clearly point out the
problem and solution. I welcome any feedback to be proved otherwise.
If i am wrong on basis of evidence I can accept that. Just show me a
working example.

[Monte Ohrt]

The fundamental issue is with Smarty changing the PHP configuration environment. Using fwrite() and unlink() on Smarty's own files does not change the environment. However, appending to the include_path DOES affect all included php files, and adding an spl_autoloader also DOES affect how things get autoloaded, and as we have seen, it easily breaks when things are not setup optimally or correctly.

Therefore if we implement this by default, we'll constantly be fielding questions in the forums "Hey, Smarty is broke I get a file not found error..." Maybe you haven't been involved in open source projects that many thousands of developers use, but sometimes it is better to take the path of least resistance.

I'm not against Smarty supporting the php autoloader, but just not enabled by default. So we could supply a flag of use_spl_autoload for an easy way to enable it.

[U.Tews]

By define('SMARTY_SPL_AUTOLOAD',1); in your script you can now select
the buildin 'spl_autoload' instead of the default 'smartyAutoloader'.

Note that the constant must be defined before you include the
Smarty.class.php.

The corresponding update is in the SVN.

On Nov 9, 4:57 pm, Monte Ohrt <mo...@ohrt.com> wrote:
> The fundamental issue is with Smarty changing the PHP configuration
> environment. Using fwrite() and unlink() on Smarty's own files does not
> change the environment. However, appending to the include_path DOES affect
> all included php files, and adding an spl_autoloader also DOES affect how
> things get autoloaded, and as we have seen, it easily breaks when things are
> not setup optimally or correctly.
>
> Therefore if we implement this by default, we'll constantly be fielding
> questions in the forums "Hey, Smarty is broke I get a file not found
> error..." Maybe you haven't been involved in open source projects that many
> thousands of developers use, but sometimes it is better to take the path of
> least resistance.
>
> I'm not against Smarty supporting the php autoloader, but just not enabled
> by default. So we could supply a flag of use_spl_autoload for an easy way to
> enable it.
>

[dave B.]

> change the environment. However, appending to the include_path DOES affect
> all included php files,

I would really like to see a working example of that issue doing what
you say it does.
I have *never* come across that problem in the entire 10 years i have
been coding this stuff for a living.

[Monte Ohrt]

What the include_path? There could easily be similarly named classes/methods spanning separate folders, and developers organize their include paths in the order they want them traversed. Many developers use the include_path heirarchy to organize how class files get loaded. Name spaces should elude this situation, but sadly php 5.3 is far from main stream. Also depending on how the include_path is changed, this could create many new (unnecessary) stat() calls for each included file, which is probably not desirable.

Anyways, you now have the option to enable the spl_autoloader.

[Monte Ohrt]

So you may know this guy, Andrei Zmievski. He is a long-time PHP core developer, and also a collegue of mine. I threw him the question "What are your general thoughts about 3rd party libraries adjusting the php include_path?" (I didn't mention Smarty or autoloaders.)

His response:

"I'm generally not a fan of that. Users have to be conscious of what's in their include path, especially in a production env"

This supplements my thoughts about Smarty adjusting the include_path. As it stands Smarty will supply the option of the spl_autoloader, but will not be enabled by default.

Monte

[dave B.]

I see none of those problems you mentioned being caused by appending a
include path.

I only see namespace conflict between libraries, which is not a
problem of appending the include path, but conflicts in namespace and
scope. If your application stops working because you conflict with the
naming of the libraries you use, you should change your names and
don't blame the library that you want to include unless that library
obviously pollutes the global namespace.

In the general programming world (JAVA, C++,Python,..this list is
long, i code in many... damn even ashamedly Flash AS3) most of these
problems are solved by using name mangling in qualified namespaces.
Conceptually, a qualified name is represented as a namespace-URI/local-
part pair.

For Smarty this would look something like (PHP 5.3.0+ style):

namespace net\smarty\SuperCoolTemplateEngine;

Where the namespace URI comes from the domain name of smarty.net (the
makers of the library) and the local-part is the library name, here I
have used "SuperCoolTemplateEngine" as the local-part but it can be
whatever you like, the URI part is *sacred* though, you should-not/
cannot change this. You would then create a library directory
structure that matches the namespace.

For the example:
net/
smarty/
SuperCoolTemplateEngine/

BTW I am not inventing this structure, it is used by other languages
as well, even though it is not a language specification of any of them
but a programming paradigm. Just google "qualified namespaces". It is
not surprising that PHP5.3 resolves this directory structure natively
in the spl_autload function, it is the long *over-due* past.

However, even *without* the PHP5.3 namespace keyword you can *still*
use these directory names anyway to construct your library paths and
the qualified namespace system still works perfectly.

Yes it is true that PHP has not had a *implicit* namespace capability
until now, though if you are building libraries there is nothing
stopping you from adhering to a namespace methodology. You can just
see the efforts of Zend and PEAR as systems for dealing with this in
PHP, though admittedly not as nice as true fully qualified names. In
fact you even do it in Smarty, though the methodology is also ad-hoc
and inconsistent with everybody else.

I really hope that PHP developers take a leaf from the others in using
*qualified* namespaces. Why the other guys can do it so easily and the
PHP coders can't is NOT a mystery, it is caused by perpetuating myth,
not co-operating with each other and having no f*$king idea in general
(excuse my Australian, yes i am, don't worry, i have a license from
birth for using the F word. lol. :-P). The funny thing is you can
still implement qualified namespaces today *without* PHP 5.3.0+ and it
solves all those problems.

Of course you may have exceptions, all the *broken* libraries in your
include path that you have until now been coping with, and I agree you
have to be conscious of what is in your include path, by removing
anything that is trying to *overtake* your environment.

This is as easy to fix as:
1) Take the broken library out of the include path.
2) Put it in it's own folder out of harms way. (maybe name it based
upon URI for consistency)
3) Reference it directly if you need it.

There is also a not so immediate but eventual step 4.
4) Point out that it's broken and try and get it fixed for the future.

I don't believe that the Smarty project should concern itself with
support for developers who have namespace problems OR broken libraries
of other projects. That is like if somebody would ask "should I use
print or echo?" that is a problem probably more related to the PHP
project and learning programming in general, and not to any specific
PHP project. This is not Smarty's support problem in relation to it's
templating engine, and looking back through the Smarty community
forums this is generally not the case anyway.

I think it is more important that Smarty concentrates on implementing
proven standards, and building a really awesome, super fast, user
friendly template engine, in that order of priority. (The *awesome*
part you already have and for that I really tip my hat off to
everybody. Great work! Amazing effort.)

Correctly using a namespace is a programming paradigm. It has nothing
to do with if we are allowed to append include paths or whether or not
their exists a *namespace* keyword in PHP. This type of thinking is
complete and utter myth.

I think the attitude of taking the path of least resistance is also
not helping the situation, in fact, it is just making it worse. You
have the opportunity *now* to correct it for the not so distant
future, "grab it by the horns" I say.

On Nov 9, 9:56 pm, Monte Ohrt <mo...@ohrt.com> wrote:
> So you may know this guy, Andrei Zmievski. He is a long-time PHP core
> developer, and also a collegue of mine. I threw him the question "What are
> your general thoughts about 3rd party libraries adjusting the php
> include_path?" (I didn't mention Smarty or autoloaders.)
>
> His response:
>
> "I'm generally not a fan of that. Users have to be conscious of what's in
> their include path, especially in a production env"
>
> This supplements my thoughts about Smarty adjusting the include_path. As it
> stands Smarty will supply the option of the spl_autoloader, but will not be
> enabled by default.
>
> Monte
>
>
>
> On Mon, Nov 9, 2009 at 2:37 PM, Monte Ohrt <mo...@ohrt.com> wrote:
> > What the include_path? There could easily be similarly named
> > classes/methods spanning separate folders, and developers organize their
> > include paths in the order they want them traversed. Many developers use the
> > include_path heirarchy to organize how class files get loaded. Name spaces
> > should elude this situation, but sadly php 5.3 is far from main stream. Also
> > depending on how the include_path is changed, this could create many new
> > (unnecessary) stat() calls for each included file, which is probably not
> > desirable.
>
> > Anyways, you now have the option to enable the spl_autoloader.
>

[U.Tews]

I just made some benchmarking on our smarty.net Linux server comparing
the buildin spl_autoload and our smartyAutoload.

The server is running
Apache/2.2.11 (Ubuntu)
PHP/5.2.6-3ubuntu4.2 with Suhosin-Patch
APC 3.0.19

Test case was a cached page using Smarty3 (current SVN version).

spl_autoload 0,0060 msec
smartyAutoload 0,0044 msec

So the smartyAutoload was much faster. I think this is because
spl_autoload must check if the file does exist befor loading for
proper function. If we use smartyAutoload we do know that the internal
files are existing and we load them without checking.
In the above test of spl_autoload the Smarty filepath was appended to
the existing include path. Prepending it saved about 0.2 msec but on
the other hand this would slowdown loading files from other pathes and
the big difference still remains.

The conclusion is that it does not make sense to use spl_autoload for
Smarty3.

Merging the code of some commonly used classes into the
Smarty.class.php script file did reduce the total time by another 0.4
msec. So our current efforts to optimize Smarty3 will be concentrated
on optimizing the number of script files which must be loaded and
general code optimizations.

Regards Uwe

[Monte Ohrt]

Also realize, the spl_autoload will traverse through the include_path running a file stat() on each included file, where as the smartyAutoload() already knows the correct path to the file.

[dave B.]

> So the smartyAutoload was much faster. I think this is because
> spl_autoload must check if the file does exist befor loading for
> proper function. If we use smartyAutoload we do know that the internal
> files are existing and we load them without checking.

I doubt that is the reason. The include keyword must check for
existence natively.
It returns false on failure.

I commonly take advantage of that behaviour:
if ( @include() ) { // do something} else { //do something else};

[dave B.]

> Also realize, the spl_autoload will traverse through the include_path
> running a file stat() on each included file, where as the smartyAutoload()
> already knows the correct path to the file.

The include keyword must do the same as the include_path is part of
it's scope.
I guess it does probably check local existence before falling back to
the include_path, that makes sense, I really wonder though if it
performs a stat on every file before deciding which one to return,
that doesn't make sense.

[dave B.]

OK I concede but with caveat. :-)

I ran a simpler bench. You can try it out yourself.

Here are the tests:
test1.php
<?php
echo basename(__FILE__).": Straight Include<br />";
$time = microtime(true);
include('classes/testclass.php');
new testclass($time);
?>

test2.php
<?php
echo basename(__FILE__).": Using Default Autoload Function<br />";
set_include_path(realpath(dirname
(__FILE__)).DIRECTORY_SEPARATOR.'classes');
spl_autoload_register();
$time = microtime(true);
new testclass($time);
?>

test3.php
<?php
echo basename(__FILE__).": Using Custom Autoload Function Simple
Include<br />";
function autoload_by_include($classname)
{
include('classes/'.$classname.'.php');
}
spl_autoload_register('autoload_by_include');
$time = microtime(true);
new testclass($time);
?>

classes/testclass.php
<?php
class testclass
{
function __construct($time)
{
echo microtime(true) - $time;
}
}
?>


My Results:
PHP 5.2.10 -freebsd

test1.php: Straight Include
5.2213668823242E-5

test2.php: Using Default Autoload Function
0.00010395050048828

test3.php: Using Custom Autoload Function Simple Include
6.3896179199219E-5

Basically the custom autoloader is practically twice as fast as the
default, though test1 and test3 will never support qualified names,
and test3 will never support namespaces in PHP 5.3.0, further
perpetuating the problem of namespace collisions. (Though I think you
are right though Monte, it is traversing but I don't think it is
setting a stat() , it probably sets a loop on the include_path list,
tries the first item directly by looping on an extension stack, then
starts searching again the same way on every nested child directory.
So it needs at least two loops on every request and a directory
listing.)

however things start looking quite different in PHP 5.3.0

My Results:
PHP 5.3.0 - ubuntu
(times are a bit slower, this machine is in a virtual space, not that
this matters each test is run in the same comparable environment)

test1.php: Straight Include
0.00021600723266602

test2.php: Using Default Autoload Function
0.00022006034851074

test3.php: Using Custom Autoload Function Simple Include
0.00023603439331055

Interesting here is that using the default spl_autoloader (test2) is
slightly faster than the simple custom loader (test3) AND test2
supports qualified namespaces out of the box. Of course in this test
i am *overtaking* the entire include_path and setting it with just one
path, when I appended the include_path in test2 manually (i have some
PEAR/Zend/ADOdb libs in separate paths) I was getting about equal
values for test2 and test3.

test2.php: Using Default Autoload Function (appended)
0.00023818016052246

If everybody used the same autoloader function and did not add their
own to the autoload stack, this would also speed things up. To me to
use the default spl_autoload is the easiest choice for libraries as it
points down the road in the use of qualified namespaces.

However as I said i do concede so using a flag to use this default
method in smarty is OK. It is important to note PHP 5.3.0 is currently
in stable release, so i think this is a pertinent issue.

Oh the caveat? This issue has nothing to do with the *taboo* of
appending the include_path. ;-)

Dave.

[Monte Ohrt]

Your test is not realistic, you would never replace the include_path. try appending to it, you'll see how the extra stat() per folder per file affects performance (see Uwe's tests.)

[dave B.]

I mention that.

> > supports qualified namespaces out of the box. Of course in this test
> > i am *overtaking* the entire include_path and setting it with just one
> > path, when I appended the include_path in test2 manually (i have some
> > PEAR/Zend/ADOdb libs in separate paths) I was getting about equal
> > values for test2 and test3.