Zend_Loader_StandardAutoloader constructor & auto registering prefix|namespace

[BullfrogBlues]

Hi guys,

Took a quick peek at Zend_Loader_StandardAutoloader in zf1 trunk. Really looking forward to this release. I have one concern.

Please don't auto register the Zend prefix|namespace. Please don't allow this:

public function __construct($options = null)
{
    $this->registerPrefix('Zend', dirname(dirname(__FILE__)));

    $zfDir = dirname(dirname(dirname(__FILE__))) . '/Zend';
    if (file_exists($zfDir)) {
         $this->registerPrefix('Zend', $zfDir);
    }

    if (null !== $options) {
        $this->setOptions($options);
    }
}

Rather, Keep it simple. Agnostic.

public function __construct($options = null)
{
    if (null !== $options) {
        $this->setOptions($options);
    }
}
 An autoloader factory is the correct place for auto registering the zend.
   
This applies to zf2 and the next zf1 release.

zf1 - http://framework.zend.com/code/filedetails.php?repname=Zend+Framework&path=%2Ftrunk%2Flibrary%2FZend%2FLoader%2FStandardAutoloader.php

zf2 - https://github.com/zendframework/zf2/blob/master/library/Zend/Loader/StandardAutoloader.php


Please don't auto register the Zend prefix, let alone this:

$zfDir = dirname(dirname(dirname(__FILE__))) . '/Zend';

    if (file_exists($zfDir)) {

         $this->registerPrefix('Zend', $zfDir);

    }

Lordie me.

BB
     

[BullfrogBlues]

*bump*

Can someone fill me in on why the zend (prefix|namepsace) is (auto|magically) registered in the standard autoloader? What if I want to change the path? Why magically set it? There is no way to even override this behaviour. Yes, just set the path again, but then there is redundant overhead i.e. registering a zend (prefix|namespace) twice, once on a path that is never used.

It doesn't matter how trivial the overhead is. All things are trivial in isolation.

--
BB

---

From:
To: [hidden email]
Date: Sun, 10 Jun 2012 15:50:19 +0100
Subject: [zf-contributors] Zf 1.12

Hi guys,

Took a quick peek at Zend_Loader_StandardAutoloader in zf1 trunk. Really looking forward to this release. I have one concern.

Please don't auto register the Zend prefix|namespace. Please don't allow this:

public function __construct($options = null)
{
    $this->registerPrefix('Zend', dirname(dirname(__FILE__)));

    $zfDir = dirname(dirname(dirname(__FILE__))) . '/Zend';
    if (file_exists($zfDir)) {
         $this->registerPrefix('Zend', $zfDir);
    }

    if (null !== $options) {
        $this->setOptions($options);
    }
}

Rather, Keep it simple. Agnostic.

public function __construct($options = null)
{
    if (null !== $options) {
        $this->setOptions($options);
    }
}
 An autoloader factory is the correct place for auto registering the zend.
  
This applies to zf2 and the next zf1 release.

zf1 - http://framework.zend.com/code/filedetails.php?repname=Zend+Framework&path=%2Ftrunk%2Flibrary%2FZend%2FLoader%2FStandardAutoloader.php

zf2 - https://github.com/zendframework/zf2/blob/master/library/Zend/Loader/StandardAutoloader.php


Please don't auto register the Zend prefix, let alone this:

$zfDir = dirname(dirname(dirname(__FILE__))) . '/Zend';

    if (file_exists($zfDir)) {

         $this->registerPrefix('Zend', $zfDir);

    }

Lordie me.

BB

[Axel]

Shouldn't the check for a directory be is_dir instead of file_exists?

Gerard - <[hidden email]> schrieb:

*bump*

Can someone fill me in on why the zend (prefix|namepsace) is (auto|magically) registered in the standard autoloader? What if I want to change the path? Why magically set it? There is no way to even override this behaviour. Yes, just set the path again, but then there is redundant overhead i.e. registering a zend (prefix|namespace) twice, once on a path that is never used.

It doesn't matter how trivial the overhead is. All things are trivial in isolation.

--
BB

---

From:
To: [hidden email]
Date: Sun, 10 Jun 2012 15:50:19 +0100
Subject: [zf-contributors] Zf 1.12

Hi guys,

Took a quick peek at Zend_Loader_StandardAutoloader in zf1 trunk. Really looking forward to this release. I have one concern.

Please don't auto register the Zend prefix|namespace. Please don't allow this:

public function __construct($options = null)
{
    $this->registerPrefix('Zend', dirname(dirname(__FILE__)));

    $zfDir = dirname(dirname(dirname(__FILE__))) . '/Zend';
    if (file_exists($zfDir)) {
         $this->registerPrefix('Zend', $zfDir);
    }

    if (null !== $options) {
        $this->setOptions($options);
    }
}

Rather, Keep it simple. Agnostic.

public function __construct($options = null)
{
    if (null !== $options) {
        $this->setOptions($options);
    }
}
 An autoloader factory is the correct place for auto registering the zend.
  
This applies to zf2 and the next zf1 release.

zf1 - http://framework.zend.com/code/filedetails.php?repname=Zend+Framework&path=%2Ftrunk%2Flibrary%2FZend%2FLoader%2FStandardAutoloader.php

zf2 - https://github.com/zendframework/zf2/blob/master/library/Zend/Loader/StandardAutoloader.php


Please don't auto register the Zend prefix, let alone this:

$zfDir = dirname(dirname(dirname(__FILE__))) . '/Zend';

    if (file_exists($zfDir)) {

         $this->registerPrefix('Zend', $zfDir);

    }

Lordie me.

BB

[Tomáš Fejfar]

No idea :( Anyone on this topic? I remember reading it a while back and the double prefix registration still makes no sense. 

On Tue, Jun 19, 2012 at 12:43 AM, Gerard - <[hidden email]> wrote:

*bump*

Can someone fill me in on why the zend (prefix|namepsace) is (auto|magically) registered in the standard autoloader? What if I want to change the path? Why magically set it? There is no way to even override this behaviour. Yes, just set the path again, but then there is redundant overhead i.e. registering a zend (prefix|namespace) twice, once on a path that is never used.

It doesn't matter how trivial the overhead is. All things are trivial in isolation.

--
BB

---

From:
To: [hidden email]
Date: Sun, 10 Jun 2012 15:50:19 +0100
Subject: [zf-contributors] Zf 1.12

Hi guys,

Took a quick peek at Zend_Loader_StandardAutoloader in zf1 trunk. Really looking forward to this release. I have one concern.

Please don't auto register the Zend prefix|namespace. Please don't allow this:

public function __construct($options = null)
{
    $this->registerPrefix('Zend', dirname(dirname(__FILE__)));

    $zfDir = dirname(dirname(dirname(__FILE__))) . '/Zend';
    if (file_exists($zfDir)) {
         $this->registerPrefix('Zend', $zfDir);
    }

    if (null !== $options) {
        $this->setOptions($options);
    }
}

Rather, Keep it simple. Agnostic.

public function __construct($options = null)
{
    if (null !== $options) {
        $this->setOptions($options);
    }
}
 An autoloader factory is the correct place for auto registering the zend.
  
This applies to zf2 and the next zf1 release.

zf1 - http://framework.zend.com/code/filedetails.php?repname=Zend+Framework&path=%2Ftrunk%2Flibrary%2FZend%2FLoader%2FStandardAutoloader.php

zf2 - https://github.com/zendframework/zf2/blob/master/library/Zend/Loader/StandardAutoloader.php


Please don't auto register the Zend prefix, let alone this:

$zfDir = dirname(dirname(dirname(__FILE__))) . '/Zend';

    if (file_exists($zfDir)) {

         $this->registerPrefix('Zend', $zfDir);

    }

Lordie me.

BB

[Tomáš Fejfar]

is_dir may be false for symlinked dir? (not sure though)

On Tue, Jun 19, 2012 at 8:39 AM, Axel <[hidden email]> wrote:

Shouldn't the check for a directory be is_dir instead of file_exists?

    if (file_exists($zfDir)) {

[weierophinney]

-- Gerard - <[hidden email]> wrote
(on Monday, 18 June 2012, 11:43 PM +0100):
> *bump*
>
> Can someone fill me in on why the zend (prefix|namepsace) is (auto|magically)
> registered in the standard autoloader? What if I want to change the path? Why
> magically set it? There is no way to even override this behaviour. Yes, just
> set the path again, but then there is redundant overhead i.e. registering a
> zend (prefix|namespace) twice, once on a path that is never used.

The reason it's registered is so that the autoloader can autoload ZF
classes, plain and simple. Developers shouldn't need to manually tell
the autoloader how to autoload ZF itself.

That said, lines 73-76 are redundant, and a result of the fact that it
was originally in a different vendor prefix (ZendX). I'll remove those
lines for RC2.

> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> From:
> To: [hidden email]
> Date: Sun, 10 Jun 2012 15:50:19 +0100
> Subject: [zf-contributors] Zf 1.12
>
>
> Hi guys,
>
> Took a quick peek at Zend_Loader_StandardAutoloader in zf1 trunk. Really
> looking forward to this release. I have one concern.
>
> Please don't auto register the Zend prefix|namespace. Please don't allow this:
>
> public function __construct($options = null)
> {
>     $this->registerPrefix('Zend', dirname(dirname(__FILE__)));
>
>     $zfDir = dirname(dirname(dirname(__FILE__))) . '/Zend';
>     if (file_exists($zfDir)) {
>          $this->registerPrefix('Zend', $zfDir);
>     }
>
>     if (null !== $options) {
>         $this->setOptions($options);
>     }
> }
>
> Rather, Keep it simple. Agnostic.
>
> public function __construct($options = null)
> {
>     if (null !== $options) {
>         $this->setOptions($options);
>     }
> }
>  An autoloader factory is the correct place for auto registering the zend.
>  
> This applies to zf2 and the next zf1 release.
>
> zf1 - http://framework.zend.com/code/filedetails.php?repname=Zend+Framework&
> path=%2Ftrunk%2Flibrary%2FZend%2FLoader%2FStandardAutoloader.php
>
> zf2 - https://github.com/zendframework/zf2/blob/master/library/Zend/Loader/
> StandardAutoloader.php
>
>
> Please don't auto register the Zend prefix, let alone this:
>
> $zfDir = dirname(dirname(dirname(__FILE__))) . '/Zend';
>
>     if (file_exists($zfDir)) {
>
>          $this->registerPrefix('Zend', $zfDir);
>
>     }
>
> Lordie me.
>
> BB

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

[akrabat]

On 19 Jun 2012, at 16:15, Matthew Weier O'Phinney wrote:

> -- Gerard - <[hidden email]> wrote
> (on Monday, 18 June 2012, 11:43 PM +0100):
>> *bump*
>>
>> Can someone fill me in on why the zend (prefix|namepsace) is (auto|magically)
>> registered in the standard autoloader? What if I want to change the path? Why
>> magically set it? There is no way to even override this behaviour. Yes, just
>> set the path again, but then there is redundant overhead i.e. registering a
>> zend (prefix|namespace) twice, once on a path that is never used.
>
> The reason it's registered is so that the autoloader can autoload ZF
> classes, plain and simple. Developers shouldn't need to manually tell
> the autoloader how to autoload ZF itself.
>
> That said, lines 73-76 are redundant, and a result of the fact that it
> was originally in a different vendor prefix (ZendX). I'll remove those
> lines for RC2.

I wish you could turn off the auto-registering. Is this possible to add?


Regards,

Rob...

[weierophinney]

-- Rob Allen <[hidden email]> wrote
(on Tuesday, 19 June 2012, 05:08 PM +0100):

>
> On 19 Jun 2012, at 16:15, Matthew Weier O'Phinney wrote:
>
> > -- Gerard - <[hidden email]> wrote
> > (on Monday, 18 June 2012, 11:43 PM +0100):
> > > *bump*
> > >
> > > Can someone fill me in on why the zend (prefix|namepsace) is (auto|magically)
> > > registered in the standard autoloader? What if I want to change the path? Why
> > > magically set it? There is no way to even override this behaviour. Yes, just
> > > set the path again, but then there is redundant overhead i.e. registering a
> > > zend (prefix|namespace) twice, once on a path that is never used.
> >
> > The reason it's registered is so that the autoloader can autoload ZF
> > classes, plain and simple. Developers shouldn't need to manually tell
> > the autoloader how to autoload ZF itself.
> >
> > That said, lines 73-76 are redundant, and a result of the fact that it
> > was originally in a different vendor prefix (ZendX). I'll remove those
> > lines for RC2.
>
>
> I wish you could turn off the auto-registering. Is this possible to add?

It could be a constructor option. Suggestions for names?
"autoregister_zf"?

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

[Andreas Möller]

It could be a constructor option.

Sounds like a solution to me.

Suggestions for names?
"autoregister_zf"?

Or simply "register_zf"?

Regards,

Andreas

[Anthony Shireman]

Could you have an 'autoregisterNamespace' entry in the options array (and associated methods) which could contain an array of strings to autoload? I think this would allow for the three scenarios presented (load none if option not present, load one [Zend], load many [Zend, MyFoo...]). Would that work for everyone?

On Tue, Jun 19, 2012 at 9:59 AM, Andreas Möller <[hidden email]> wrote:

It could be a constructor option.

Sounds like a solution to me.

Suggestions for names?
"autoregister_zf"?

Or simply "register_zf"?

Regards,

Andreas

[Andreas Möller]

> Could you have an 'autoregisterNamespace' entry in the options array (and associated methods) which could contain an array of strings to autoload? I think this would allow for the three scenarios presented (load none if option not present, load one [Zend], load many [Zend, MyFoo...]). Would that work for everyone?

Obviously the constructor would then do a bit too much work, I think,
as this is what registerNamespaces() is already for.

I believe the intention is to prevent it from doing too much, i.e.
register the Zend namespace by default.


Andreas

[weierophinney]

-- Anthony Shireman <[hidden email]> wrote
(on Tuesday, 19 June 2012, 10:13 AM -0700):
> Could you have an 'autoregisterNamespace' entry in the options array (and
> associated methods) which could contain an array of strings to autoload? I
> think this would allow for the three scenarios presented (load none if option
> not present, load one [Zend], load many [Zend, MyFoo...]). Would that work for
> everyone?

That much is already covered -- you can pass an array of options to the
constructor already, and provide a list of namespaces and vendor
prefixes that way. What we're covering here is whether or not to
auto-register the Zend_ prefix by default (this is currently done in the
constructor so no configuration is required).

> On Tue, Jun 19, 2012 at 9:59 AM, Andreas Möller <[hidden email]> wrote:
>
>         It could be a constructor option.
>
>
>     Sounds like a solution to me.
>
>
>         Suggestions for names?
>         "autoregister_zf"?
>
>
>     Or simply "register_zf"?
>
>
>     Regards,
>
>     Andreas
>
>

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

[akrabat]

On 19 Jun 2012, at 17:59, Andreas Möller wrote:

It could be a constructor option.

Sounds like a solution to me.

Agreed.

Suggestions for names?
"autoregister_zf"?

Or simply "register_zf"?

Either works for me.

Regards,

Rob...

[weierophinney]

-- Rob Allen <[hidden email]> wrote
(on Tuesday, 19 June 2012, 06:46 PM +0100):

> On 19 Jun 2012, at 17:59, Andreas M ller wrote:
>         It could be a constructor option.
>
>     Sounds like a solution to me.
>
> Agreed.
>
>         Suggestions for names?
>         "autoregister_zf"?
>
>     Or simply "register_zf"?
>
> Either works for me.

Implemented with "autoregister_zf":

    $loader = new Zend_Loader_StandardAutoloader(array(
        'autoregister_zf' => true
    ));

This is now on trunk and the 1.12 release branch.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

[Tomáš Fejfar]

Isn't this a BC break? Shouldn't the option be otherwise - disable autoregister - to keep it consistent? 

On Tue, Jun 19, 2012 at 9:45 PM, Matthew Weier O'Phinney <[hidden email]> wrote:

-- Rob Allen <[hidden email]> wrote
(on Tuesday, 19 June 2012, 06:46 PM +0100):
> On 19 Jun 2012, at 17:59, Andreas M ller wrote:
>         It could be a constructor option.
>
>     Sounds like a solution to me.
>
> Agreed.
>
>         Suggestions for names?
>         "autoregister_zf"?
>
>     Or simply "register_zf"?
>
> Either works for me.

Implemented with "autoregister_zf":

   $loader = new Zend_Loader_StandardAutoloader(array(
       'autoregister_zf' => true
   ));

This is now on trunk and the 1.12 release branch.

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc

[Andreas Möller]

> Isn't this a BC break? Shouldn't the option be otherwise - disable autoregister - to keep it consistent?

Sure is. Default should be true.


Andreas

[akrabat]

How can it be a BC break on a brand new component?

Regards,

Rob..

On 19 Jun 2012, at 21:48, Andreas Möller wrote:

>> Isn't this a BC break? Shouldn't the option be otherwise - disable autoregister - to keep it consistent?
>
> Sure is. Default should be true.
>
>
> Andreas

[Tomáš Fejfar]

Ouch :D LOL :)

On Tue, Jun 19, 2012 at 10:53 PM, Rob Allen <[hidden email]> wrote:

How can it be a BC break on a brand new component?

[Andreas Möller]

> How can it be a BC break on a brand new component?

Probably depends on what kind of workflow has been chosen and whether

http://framework.zend.com/svn/framework/standard/tags/release-1.12.0rc1/library/Zend/Loader/StandardAutoloader.php

can be considered a release, even if it's only an RC. Also, it's still auto-registering in ZF2.


Andreas

[weierophinney]

-- Andreas Möller <[hidden email]> wrote
(on Tuesday, 19 June 2012, 11:12 PM +0200):
> > How can it be a BC break on a brand new component?
>
> Probably depends on what kind of workflow has been chosen and whether
>
> http://framework.zend.com/svn/framework/standard/tags/release-1.12.0rc1/library/Zend/Loader/StandardAutoloader.php
>
> can be considered a release, even if it's only an RC. Also, it's still
> auto-registering in ZF2.

... which we're considering changing.

Nobody commented on the functionality in the 4+ months since it hit
trunk (and I announced it here and a few other places). The fact is, the
RC is the first we've had folks really testing what will become the
1.12.0 release. I'm fine with the change -- there are some good
arguments in place for it (one of which is supporting multiple ZF
installs).

--
Matthew Weier O'Phinney
Project Lead            | [hidden email]
Zend Framework          | http://framework.zend.com/
PGP key: http://framework.zend.com/zf-matthew-pgp-key.asc