[DISCUSSION] Package-Oriented Autoloader - vote against
619 views
Skip to first unread message
Drak
May 13, 2013, 2:05:40 PM5/13/13
to php...@googlegroups.com
This is a concurrent discussion thread the voting thread for a new autoloader standard https://groups.google.com/forum/?fromgroups=#!topic/php-fig/Ua46E344_Ls
Drak
I have voted against this proposal and since then, others have also begun to express some dissatisfaction with the proposal. Andre has taken issue with the fact this PSR would compete with PSR-0 when it should be amending it to make it better.
I would call on all voting members to seriously consider the implications and vote -1 sending this back to the drawing board for more discussions or into an early grave.
Myself, I would have simply voted no and been done with it, however, it's clear from the noise generated that this proposal is not ready so I feel the only responsible thing to do is to bring this to attention. If you feel the same way, please vote against the proposal.
Regards,
Drak
---------- Forwarded message ----------
From: Paul Jones <pmjo...@gmail.com>
Date: 10 May 2013 16:31
Subject: [VOTE] Package-Oriented Autoloader
To: php...@googlegroups.com
Hi everyone,
I hereby call for votes on the Package-Oriented Autoloader:
<https://github.com/php-fig/fig-standards/blob/master/proposed/package-oriented-autoloader.md>
The member count at the start of the vote is 27. The quorum requirement is 9 votes. A simple majority of +1 over -1 is required for the measure to pass. The vote started on Fri 10 May 2013 at about 1030am CDT, and closes on Fri 24 May 2013 at about the same time.
Please restrict this thread to votes only; discussion may proceed on other threads.
The text of the measure is attached below for posterity.
* * *
PSR-X: Package-Oriented Autoloader
==================================
The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD",
"SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be
interpreted as described in [RFC 2119](http://tools.ietf.org/html/rfc2119).
1. Overview
-----------
This PSR specifies the rules for an interoperable autoloader. It is intended
as a co-existing alternative to, not a replacement for,
[PSR-0](https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md).
It removes some legacy compatibility features that were built into PSR-0, in
particular the handling of underscores in class names, and it allows for
classes to map to shallower directory structures.
2. Definitions
--------------
- `class`: The term "class" refers to PHP classes, interfaces, and traits.
- `absolute class name`: A fully-qualified namespace and class name.
- `namespace`: Given an `absolute class name` of `Foo\Bar\Dib\Zim`, the
`namespace` is `Foo\Bar\Dib`.
- `namespace name`: Given an `absolute class name` of `Foo\Bar\Dib\Zim`, the
`namespace names` are `Foo`, `Bar`, and `Dib`.
- `namespace prefix`: One or more contiguous `namespace names` at the start of
the `namespace`. Given an `absolute class name` of `Foo\Bar\Dib\Zim`, the
`namespace prefix` may be `Foo`, `Foo\Bar`, or `Foo\Bar\Dib`.
- `relative class name`: The parts of the `absolute class name` that appear
after the `namespace prefix`. Given an `absolute class name` of
`Foo\Bar\Dib\Zim` and a `namespace prefix` of `Foo\Bar`, the `relative class
name` is `Dib\Zim`.
- `base directory`: The absolute directory path on disk where the files for
`relative class names` have their root.
3. Specification
----------------
- A class file MUST contain only one class definition.
- An absolute class name MUST begin with a top-level namespace name, which
MUST be followed by zero or more sub-namespace names, and MUST end in a
class name.
- The namespace prefix of an absolute class name MUST be mapped to a base
directory; that namespace prefix MAY be mapped to more than one base
directory.
- The relative class name MUST be mapped to a sub-path by replacing namespace
separators with directory separators, and the result MUST be suffixed with
`.php`.
4. Example Implementations
--------------------------
The example implementations MUST NOT be regarded as part of the specification;
they are examples only. Class loaders MAY contain additional features and MAY
differ in how they are implemented. As long as a class loader adheres to the
rules set forth in the specification they MUST be considered compatible
with this PSR.
### Example: Project-Specific Implementation
The following is one possible project-specific implementation of the
specification.
```php
<?php
// if this closure is registered in a file at /path/to/project/autoload.php ...
spl_autoload_register(function ($absoluteClass) {
$namespacePrefix = 'Foo\Bar';
$baseDirectory = __DIR__ . '/src/';
if (0 === strncmp($namespacePrefix, $absoluteClass, strlen($namespacePrefix))) {
$relativeClass = substr($absoluteClass, strlen($namespacePrefix));
$relativeFile = str_replace('\\', '/', $relativeClass) . '.php';
$path = $baseDirectory . $relativeFile;
if (is_readable($path)) {
require $path;
return true;
}
}
return false;
});
// ... then the following line would cause the autoloader to attempt to load
// the \Foo\Bar\Dib\Zim class from /path/to/project/src/Dib/Zim.php
new \Foo\Bar\Dib\Zim;
```
### Example: General-Purpose Implementation
The following is one possible general-purpose implementation of the
specification.
```php
<?php
namespace Example;
/**
*
* An example implementation for a package-oriented autoloader. Note that this
* is only an example, and is not a specification in itself.
*
*/
class PackageOrientedAutoloader
{
/**
*
* An array where the key is a namespace prefix, and the value is a
* a base directory for classes in that namespace.
*
* @var array
*
*/
protected $prefix_base = array();
/**
*
* Sets the directory for a namespace prefix.
*
* @param string $prefix The namespace prefix.
*
* @param string $dir The directory containing classes in that
* namespace.
*
* @return void
*
*/
public function setNamespacePrefixBase($prefix, $base)
{
$base = rtrim($base, DIRECTORY_SEPARATOR);
$this->prefix_base[$prefix] = $base;
}
/**
*
* Loads the class file for an absolute class name.
*
* @param string $absolute The absolute class name.
*
*/
public function load($absolute)
{
// a partial relative file name
$relative = '';
// go through the individual names in the absolute class name
$names = explode('\\', $absolute);
while ($names) {
// take the last element off the absolute class name,
// and add it to the partial relative file name
$relative .= DIRECTORY_SEPARATOR . array_pop($names);
// the remaining elements indicate the namespace prefix
$prefix = implode('\\', $names);
// is there a base for this namespace prefix?
if (isset($this->prefix_base[$prefix])) {
// yes. create a complete file name from the namespace dir
// and partial name
$file = $this->prefix_base[$prefix] . $relative . '.php';
// can we read the file from the filesystem?
if ($this->readFile($file)) {
// done!
return true;
}
}
}
// never found a file for the class
return false;
}
/**
*
* Uses `require` to read a file from the filesystem.
*
* @param string $file
*
* @return bool True if the file gets read; false if it does not.
*
*/
protected function readFile($file)
{
if (! is_readable($file)) {
return false;
}
require $file;
return true;
}
}
```
Given the example general-purpose implementation, and a `foo-bar` package of
classes on disk at the following paths ...
/path/to/packages/foo-bar/
src/
Baz.php # Foo\Bar\Baz
Dib/
Zim.php # Foo\Bar\Dib\Zim
... one would register the path to the class files for the `Foo\Bar` namespace
prefix like so:
```php
<?php
// instantiate the loader
$loader = new \Example\PackageOrientedLoader;
// register the base directory for the namespace prefix
$loader->setNamespacePrefixBase(
'Foo\Bar',
'/path/to/packages/foo-bar/src'
);
// register the autoloader
spl_autoload_register(array($loader, 'load'));
// the following line would cause the autoloader to attempt to load
// the \Foo\Bar\Dib\Zim class from /path/to/packages/foo-bar/src/Dib/Zim.php
new \Foo\Bar\Dib\Zim;
```
* * *
-- pmj
--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
From: Paul Jones <pmjo...@gmail.com>
Date: 10 May 2013 16:31
Subject: [VOTE] Package-Oriented Autoloader
To: php...@googlegroups.com
Hi everyone,
I hereby call for votes on the Package-Oriented Autoloader:
<https://github.com/php-fig/fig-standards/blob/master/proposed/package-oriented-autoloader.md>
The member count at the start of the vote is 27. The quorum requirement is 9 votes. A simple majority of +1 over -1 is required for the measure to pass. The vote started on Fri 10 May 2013 at about 1030am CDT, and closes on Fri 24 May 2013 at about the same time.
Please restrict this thread to votes only; discussion may proceed on other threads.
The text of the measure is attached below for posterity.
* * *
PSR-X: Package-Oriented Autoloader
==================================
The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD",
"SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be
interpreted as described in [RFC 2119](http://tools.ietf.org/html/rfc2119).
1. Overview
-----------
This PSR specifies the rules for an interoperable autoloader. It is intended
as a co-existing alternative to, not a replacement for,
[PSR-0](https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md).
It removes some legacy compatibility features that were built into PSR-0, in
particular the handling of underscores in class names, and it allows for
classes to map to shallower directory structures.
2. Definitions
--------------
- `class`: The term "class" refers to PHP classes, interfaces, and traits.
- `absolute class name`: A fully-qualified namespace and class name.
- `namespace`: Given an `absolute class name` of `Foo\Bar\Dib\Zim`, the
`namespace` is `Foo\Bar\Dib`.
- `namespace name`: Given an `absolute class name` of `Foo\Bar\Dib\Zim`, the
`namespace names` are `Foo`, `Bar`, and `Dib`.
- `namespace prefix`: One or more contiguous `namespace names` at the start of
the `namespace`. Given an `absolute class name` of `Foo\Bar\Dib\Zim`, the
`namespace prefix` may be `Foo`, `Foo\Bar`, or `Foo\Bar\Dib`.
- `relative class name`: The parts of the `absolute class name` that appear
after the `namespace prefix`. Given an `absolute class name` of
`Foo\Bar\Dib\Zim` and a `namespace prefix` of `Foo\Bar`, the `relative class
name` is `Dib\Zim`.
- `base directory`: The absolute directory path on disk where the files for
`relative class names` have their root.
3. Specification
----------------
- A class file MUST contain only one class definition.
- An absolute class name MUST begin with a top-level namespace name, which
MUST be followed by zero or more sub-namespace names, and MUST end in a
class name.
- The namespace prefix of an absolute class name MUST be mapped to a base
directory; that namespace prefix MAY be mapped to more than one base
directory.
- The relative class name MUST be mapped to a sub-path by replacing namespace
separators with directory separators, and the result MUST be suffixed with
`.php`.
4. Example Implementations
--------------------------
The example implementations MUST NOT be regarded as part of the specification;
they are examples only. Class loaders MAY contain additional features and MAY
differ in how they are implemented. As long as a class loader adheres to the
rules set forth in the specification they MUST be considered compatible
with this PSR.
### Example: Project-Specific Implementation
The following is one possible project-specific implementation of the
specification.
```php
<?php
// if this closure is registered in a file at /path/to/project/autoload.php ...
spl_autoload_register(function ($absoluteClass) {
$namespacePrefix = 'Foo\Bar';
$baseDirectory = __DIR__ . '/src/';
if (0 === strncmp($namespacePrefix, $absoluteClass, strlen($namespacePrefix))) {
$relativeClass = substr($absoluteClass, strlen($namespacePrefix));
$relativeFile = str_replace('\\', '/', $relativeClass) . '.php';
$path = $baseDirectory . $relativeFile;
if (is_readable($path)) {
require $path;
return true;
}
}
return false;
});
// ... then the following line would cause the autoloader to attempt to load
// the \Foo\Bar\Dib\Zim class from /path/to/project/src/Dib/Zim.php
new \Foo\Bar\Dib\Zim;
```
### Example: General-Purpose Implementation
The following is one possible general-purpose implementation of the
specification.
```php
<?php
namespace Example;
/**
*
* An example implementation for a package-oriented autoloader. Note that this
* is only an example, and is not a specification in itself.
*
*/
class PackageOrientedAutoloader
{
/**
*
* An array where the key is a namespace prefix, and the value is a
* a base directory for classes in that namespace.
*
* @var array
*
*/
protected $prefix_base = array();
/**
*
* Sets the directory for a namespace prefix.
*
* @param string $prefix The namespace prefix.
*
* @param string $dir The directory containing classes in that
* namespace.
*
* @return void
*
*/
public function setNamespacePrefixBase($prefix, $base)
{
$base = rtrim($base, DIRECTORY_SEPARATOR);
$this->prefix_base[$prefix] = $base;
}
/**
*
* Loads the class file for an absolute class name.
*
* @param string $absolute The absolute class name.
*
*/
public function load($absolute)
{
// a partial relative file name
$relative = '';
// go through the individual names in the absolute class name
$names = explode('\\', $absolute);
while ($names) {
// take the last element off the absolute class name,
// and add it to the partial relative file name
$relative .= DIRECTORY_SEPARATOR . array_pop($names);
// the remaining elements indicate the namespace prefix
$prefix = implode('\\', $names);
// is there a base for this namespace prefix?
if (isset($this->prefix_base[$prefix])) {
// yes. create a complete file name from the namespace dir
// and partial name
$file = $this->prefix_base[$prefix] . $relative . '.php';
// can we read the file from the filesystem?
if ($this->readFile($file)) {
// done!
return true;
}
}
}
// never found a file for the class
return false;
}
/**
*
* Uses `require` to read a file from the filesystem.
*
* @param string $file
*
* @return bool True if the file gets read; false if it does not.
*
*/
protected function readFile($file)
{
if (! is_readable($file)) {
return false;
}
require $file;
return true;
}
}
```
Given the example general-purpose implementation, and a `foo-bar` package of
classes on disk at the following paths ...
/path/to/packages/foo-bar/
src/
Baz.php # Foo\Bar\Baz
Dib/
Zim.php # Foo\Bar\Dib\Zim
... one would register the path to the class files for the `Foo\Bar` namespace
prefix like so:
```php
<?php
// instantiate the loader
$loader = new \Example\PackageOrientedLoader;
// register the base directory for the namespace prefix
$loader->setNamespacePrefixBase(
'Foo\Bar',
'/path/to/packages/foo-bar/src'
);
// register the autoloader
spl_autoload_register(array($loader, 'load'));
// the following line would cause the autoloader to attempt to load
// the \Foo\Bar\Dib\Zim class from /path/to/packages/foo-bar/src/Dib/Zim.php
new \Foo\Bar\Dib\Zim;
```
* * *
-- pmj
--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
André R.
May 13, 2013, 2:50:36 PM5/13/13
to php...@googlegroups.com
Hi Drake,
On Monday, May 13, 2013 8:05:40 PM UTC+2, Drak wrote:
Short: I support calling for -1 on this right now, for long version see inline.
On Monday, May 13, 2013 8:05:40 PM UTC+2, Drak wrote:
This is a concurrent discussion thread the voting thread for a new autoloader standard https://groups.google.com/forum/?fromgroups=#!topic/php-fig/Ua46E344_LsI have voted against this proposal and since then, others have also begun to express some dissatisfaction with the proposal. Andre has taken issue with the fact this PSR would compete with PSR-0 when it should be amending it to make it better.
Basically I feel we should have taken the IETF approach here, first set goals for what we want to do (could be several things), then in case of incremental update start off with the existing approved proposal with the addition of "Obsoletes: <link>PSR-0</link>" in the header.
Personally I would have done the following changes:
- Get rid of "_" compatibility
- Clarify that namespace prefixes may point to a flat/different structure or path to phar file (I don't have anything against the Package proposal, Composer already does this and IDE's index your project anyway so they probably don't care)
- Add section to strongly recommending ending namespace prefixes with "\" to avoid miss lookups
- Change to avoid any uncached disk stats, so if you say Symfony\\Bundle\\AsseticBundle\\* is in a given directory, then it better be there, otherwise we are specifying a autoloader that is slow by design in the wrong hands
I would call on all voting members to seriously consider the implications and vote -1 sending this back to the drawing board for more discussions or into an early grave.
I wouldn't personally send this to an early grave, but I agree voting members should vote -1 so we can do another round on this one.
Paul Jones
May 13, 2013, 2:55:53 PM5/13/13
to php...@googlegroups.com
On May 13, 2013, at 1:50 PM, André R. wrote:
> first set goals for what we want to do (could be several things),
> Personally I would have done the following changes:
> - Get rid of "_" compatibility
> - Clarify that namespace prefixes may point to a flat/different structure
> or path to phar file (I don't have anything against the Package proposal, Composer already does this and IDE's index your project anyway so they probably don't care)
> - Add section to strongly recommending ending namespace prefixes with "\" to avoid miss lookups
> - Change to avoid any uncached disk stats, so if you say Symfony\\Bundle\\AsseticBundle\\* is in a given directory, then it better be there, otherwise we are specifying a autoloader that is slow by design in the wrong hands
> I would call on all voting members to seriously consider the implications and vote -1 sending this back to the drawing board for more discussions or into an early grave.
-- pmj
André R.
May 14, 2013, 3:26:17 AM5/14/13
to php...@googlegroups.com
On Monday, May 13, 2013 8:55:53 PM UTC+2, pmjones wrote:
On May 13, 2013, at 1:50 PM, André R. wrote:
> first set goals for what we want to do (could be several things),
Which we did, over the course of several weeks. I am unable to resist the urge to ask "Where were you then?" but I understand that everyone here is busy and can't always pay attention.
The answer is simple, but two folded:
1. Every half year we do a release, it's quite time consuming. We are still doing it, but at the end of deep freeze so things calm down slightly.
2. The first 1-2 pages of the thread was about bylaws and amending PSR's, so off-topic; which means I personally categorized it as another time sink thread quite quickly and ignored it.
> Personally I would have done the following changes:
> - Get rid of "_" compatibility
Which the proposal does.
> - Clarify that namespace prefixes may point to a flat/different structure
Which the proposal does.
I know, both first set of points is in the proposal. It's not a bad proposal, I just think we can do better.
> or path to phar file (I don't have anything against the Package proposal, Composer already does this and IDE's index your project anyway so they probably don't care)
This is interesting, and I think it would have been good for you to have participated in the earlier discussion to mention it.
> - Add section to strongly recommending ending namespace prefixes with "\" to avoid miss lookups
Implementation issue, but duly noted.
> - Change to avoid any uncached disk stats, so if you say Symfony\\Bundle\\AsseticBundle\\* is in a given directory, then it better be there, otherwise we are specifying a autoloader that is slow by design in the wrong hands
Probably outside scope.
Imo the spec / interface should define what should happen if you get a namespace match but missing file in that namespace, otherwise we push the ambiguity down on implementers / users. And to me, a base system like a autoloader should never cause slow downs by design, so a match not existing on disk should throw a exception, either custom one or something like Logic-/Runtime-Exception.
Phil Sturgeon
May 14, 2013, 8:51:14 AM5/14/13
to php...@googlegroups.com
Would that not just be:
PHP Fatal error: Class 'Bla' not found
André R.
May 14, 2013, 10:08:03 AM5/14/13
to php...@googlegroups.com
On Tuesday, May 14, 2013 2:51:14 PM UTC+2, Phil Sturgeon wrote:
Would that not just be:PHP Fatal error: Class 'Bla' not found
+1
Basically just returning false if file is not found in matching namespace, allowing other autoloaders in the chain to try as well, something which is not possible with exceptions.
Donald Gilbert
May 14, 2013, 10:16:54 AM5/14/13
to php...@googlegroups.com
The current one does return false if the file is not found so that other autoloaders down the line can try it. It's not quite clear what you expect the behavior to be? Are you wanting your PSR-4 autoloader of choice to be THE autoloader, and not allow others to pick up the slack? Asking a genuine question there, as I'd like to understand better.
Drak
May 14, 2013, 10:55:09 AM5/14/13
to php...@googlegroups.com
I was under the impression that an autoloader just needs to exit. PHP does not process a true or false return, but rather does an internal class_exists() after each autoloader exits and only dispatches to the next if the class continues to not be found. When autoloaders run out and the include path then PHP issues a Fatal error.
Regards,
Drak
On 14 May 2013 13:51, Phil Sturgeon <em...@philsturgeon.co.uk> wrote:
Would that not just be:PHP Fatal error: Class 'Bla' not found
--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/17e33029-b386-4d39-87cc-cc3c5bc71e74%40googlegroups.com?hl=en.
Donald Gilbert
May 14, 2013, 10:59:36 AM5/14/13
to php...@googlegroups.com
I had not looked into the actual function of the loader, but if that's the case, then awesome!
Jason Judge
May 14, 2013, 12:01:55 PM5/14/13
to php...@googlegroups.com
On Monday, 13 May 2013 19:55:53 UTC+1, pmjones wrote:
...
> - Add section to strongly recommending ending namespace prefixes with "\" to avoid miss lookups
Implementation issue, but duly noted.
I think this is an implementation bug in the example autoloader given in the spec. Higher up in the spec it is very clear (to me) that when dealing with a namespace, that namespace is split up into distinct "namespace names". Matching that namespace with registered namespaces then happens against a series of namespace names. So Foo\Bar\Dib would match Foo\Bar but not FooBar, because "FooBar" is not a namespace name in "Foo\Bar".
What the example autoloader in the spec does, is very naively does a string comparison between the namespace being loaded and the registered namespaces, and in doing so, allows "Foo" to match "FooBar".
So the questions are: am I interpreting the spec correctly? Is it really saying the namespace being loaded must have "namespace names" that exactly match the "namespace names" registered with the autoloader? If that is correct, does it make sense to fix the example in a roundabout way by suggesting registered namespaces include a trailing backslash rather than fixing the autoploader?
That's the first autoloader example. The second one, so far as I can see, does not support multiple base directories for a single namespace. If you register a namespace twice, with different base directories, then the second will overwrite the first. I am wondering if a note should be added to those examples to highlight their limitations, rather than the current disclaimer saying that real autoloaders are not going to look like this. Besides, is that second example actually PSR-X if it does not allow a namespace to be registered more than once?
-- Jason
Donald Gilbert
May 14, 2013, 12:22:52 PM5/14/13
to php...@googlegroups.com
The class being loaded must have "namespace names" that exactly match the "namespace names" registered. Otherwise, it won't get loaded, and PHP will fatal error. I know you know this, but PHP will fatal error otherwise.
If I misunderstood what you were trying to say, let me know.
Your suggestion that registered namespaces should end with a '\' is a good one though. It's possible that it would help speed up the lookup times and reduce partial matches that then check for the existence of the file. But, that would be an implementation issue, IMO. The proposed autoloader loops until it finds a matching file, and files found at that location should always have the correct namespace / class structure. I can't come up with an example where it wouldn't.
André R.
May 14, 2013, 6:03:10 PM5/14/13
to php...@googlegroups.com
On Tuesday, May 14, 2013 4:16:54 PM UTC+2, Donald Gilbert wrote:
The current one does return false if the file is not found so that other autoloaders down the line can try it. It's not quite clear what you expect the behavior to be? Are you wanting your PSR-4 autoloader of choice to be THE autoloader, and not allow others to pick up the slack? Asking a genuine question there, as I'd like to understand better.
Current proposal, and similarly to how psr-0 works in regards to this specific topic:
spl_autoload_register(function ($absoluteClass) {
$namespacePrefix = 'Foo\Bar';
$baseDirectory = __DIR__ . '/src/';
if (0 === strncmp($namespacePrefix, $absoluteClass, strlen($namespacePrefix))) {
$relativeClass = substr($absoluteClass, strlen($namespacePrefix));
$relativeFile = str_replace('\\', '/', $relativeClass) . '.php';
$path = $baseDirectory . $relativeFile;
if (is_readable($path)) {
require $path;
return true;
}
}
return false;
});
$namespacePrefix = 'Foo\Bar';
$baseDirectory = __DIR__ . '/src/';
if (0 === strncmp($namespacePrefix, $absoluteClass, strlen($namespacePrefix))) {
$relativeClass = substr($absoluteClass, strlen($namespacePrefix));
$relativeFile = str_replace('\\', '/', $relativeClass) . '.php';
$path = $baseDirectory . $relativeFile;
if (is_readable($path)) {
require $path;
return true;
}
}
return false;
});
Stricter version that does not allow you to specify same namespace several times which would causes several un cached file stat lookups:
spl_autoload_register(function ($absoluteClass) {
$namespacePrefix = 'Foo\Bar';
$baseDirectory = __DIR__ . '/src/';
if (0 === strncmp($namespacePrefix, $absoluteClass, strlen($namespacePrefix))) {
$relativeClass = substr($absoluteClass, strlen($namespacePrefix));
$relativeFile = str_replace('\\', '/', $relativeClass) . '.php';
$path = $baseDirectory . $relativeFile;
if (is_readable($path)) {
require $path;
return true;
}
$namespacePrefix = 'Foo\Bar';
$baseDirectory = __DIR__ . '/src/';
if (0 === strncmp($namespacePrefix, $absoluteClass, strlen($namespacePrefix))) {
$relativeClass = substr($absoluteClass, strlen($namespacePrefix));
$relativeFile = str_replace('\\', '/', $relativeClass) . '.php';
$path = $baseDirectory . $relativeFile;
if (is_readable($path)) {
require $path;
return true;
}
else {
return false;// Originally I proposed to throw, but that would not allow next autoloader in the stack to function if user really needs a fallback system of some sort
}
}
return false;
});
return false;// Originally I proposed to throw, but that would not allow next autoloader in the stack to function if user really needs a fallback system of some sort
}
}
return false;
});
Note: As pointed out by Drak, all occurrences of "return true|false;" here could simply have been "return;" instead and would have had same mening from php's perpective. But keeping the true and false values makes the code more readable imho.
Donald Gilbert
May 14, 2013, 6:27:56 PM5/14/13
to php...@googlegroups.com
I see where you're coming from now. IMO, not being able to have duplicate registrations for the same namespace would not be desired behavior. If that were the case, it would make it difficult to impossible for users to utilize the autoloader to override classes if need be. That was a large percentage of the use case for me with Joomla. When I wanted to test out a bug fix or feature improvement, instead of hacking the core files, I could just register a new prefix lookup path that would find the files in my specified location first, and use my overridden version.
While I didn't notice a speed fluctuation by adding a single additional lookup path, I'm sure that multiple additional lookup paths for a single namespace could cause problems. I wonder if we should set up some tests to see how much it's effected. Additionally, from an implementation side of things, Composers dump-autoload works around the issue by caching the file locations, so in that regard, it's a moot point.
Jason Judge
May 15, 2013, 10:11:32 AM5/15/13
to php...@googlegroups.com
What I was trying to say, is that there seems to be a proposed "fix" to a problem that is solved by putting a "\" on the end of every namespace prefix as it is registered with the autoloader. My observation - right or wrong - was that this is a problem with the spl autoloader example in the spec, and not a problem with the spec itself. IMO the autoloader is wrong, and it is that which needs to be fixed to prevent it tying to match namespaces such as Foo onto registered namespaces such as FooBar.
Igor Wiedler
May 16, 2013, 7:04:17 AM5/16/13
to php...@googlegroups.com
Agreed, increasing the specificity by matching the namespace with a trailing backslash would be preferable to prevent incorrect matches and increase performance for those edge cases where namespaces overlap.
I would like to see this fixed.
Igor Wiedler
May 16, 2013, 7:06:51 AM5/16/13
to php...@googlegroups.com
Actually, the spec itself is not affected by this issue, because it does not define how matching ought to be done. But the reference implementation that will surely be copied a lot does have that issue.
Phil Sturgeon
May 16, 2013, 11:13:59 AM5/16/13
to php...@googlegroups.com
Actually, the spec itself is not affected by this issue, because it does not define how matching ought to be done. But the reference implementation that will surely be copied a lot does have that issue.
Could you PR and fix it? I'll almost certainly get it wrong. :)
Beau Simensen
May 16, 2013, 2:31:15 PM5/16/13
to php...@googlegroups.com
On Thursday, May 16, 2013 10:13:59 AM UTC-5, Phil Sturgeon wrote:
Actually, the spec itself is not affected by this issue, because it does not define how matching ought to be done. But the reference implementation that will surely be copied a lot does have that issue.Could you PR and fix it? I'll almost certainly get it wrong. :)
I'd be happy to tweak the reference implementations. But is that allowed? Ya'll are in the middle of a vote. Wouldn't changing any part of it be disallowed at this point?
Also, to be clear, for the smaller reference implementation, I'd think the following would work:
spl_autoload_register(function ($absoluteClass) {$namespacePrefix = 'Foo\\Bar\\';$baseDirectory = __DIR__ . '/src/';if (0 === strncmp($namespacePrefix, $absoluteClass, strlen($namespacePrefix))) {$relativeClass = substr($absoluteClass, strlen($namespacePrefix));$relativeFile = str_replace('\\', '/', $relativeClass) . '.php';$path = $baseDirectory . $relativeFile;if (is_readable($path)) {require $path;}}
});
The changes are:
- Adding a trailing \\ to the namespace
- Removing return statements as they seem to have no real impact
I'd be happy to submit this as soon as it is made clear whether or not we should change the proposal in any way at this point.
Drak
May 16, 2013, 3:14:20 PM5/16/13
to php...@googlegroups.com
That's correct.
Drak
Paul Jones
May 16, 2013, 3:59:31 PM5/16/13
to php...@googlegroups.com
Objections?
-- pmj
Beau Simensen
May 16, 2013, 4:36:00 PM5/16/13
to php...@googlegroups.com
I sent two pull requests. I'm not sure what other things people wanted done on the general-purpose one so I split them so that I can continue to work on them independently if needed.
Project-Specific https://github.com/php-fig/fig-standards/pull/122
General-Purpose https://github.com/php-fig/fig-standards/pull/123
Phil Sturgeon
May 16, 2013, 4:50:09 PM5/16/13
to php...@googlegroups.com
Merged.
Hari K T
May 16, 2013, 9:51:35 PM5/16/13
to php...@googlegroups.com
I am not a voting member. But I feel when something is taken to vote either it should be withdrawn from voting to make changes or change it after that ( how to do the changes written in bylaw ? ) .
Also Phil , I think before you merge it will be good if someone can review and approve. Else that may cause you in trouble one day. I like the Typo3 way ( if I am correct ) review is made by a minimum of 2 and then only merged ( But TYPO3 is here if they can tell :-) ).--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/488afc00-e291-4bb9-9c66-ee8ad536b089%40googlegroups.com?hl=en.
Larry Garfield
May 16, 2013, 10:09:52 PM5/16/13
to php...@googlegroups.com
I have to agree. I have nothing
against the changes to the sample code, but modifying a proposal
during the vote is simply bad news all around. It's been a
problem on php-internals too. You want to change something, you
withdraw it and resubmit. The whole point of a 2 week voting
period is to give people time to consider, be gone for a weekend,
have too much to do at work, and still be able to cast a
considered, informed vote. If we allow the proposal to change
mid-vote, that becomes impossible. Someone who voted 2 days ago
and is not reading daily (like a sane person) would get screwed by
this.
Case in point: I'm about to disappear to DrupalCon/Symfony Live for a week, as is Fabien (another of the voting representatives here), which means our time to even bother reading the list next week will be extremely limited. If this had been done 2 days from now, there's a good chance neither of us would even have noticed to be able to object.
It's absolutely inappropriate to modify a PSR in any way during the voting period.
Phil, I have to agree with Hari that a 20 min turnaround time on a merge under those circumstances is also absolutely inappropriate. Paul asked for objections at 3 PM Central Time today. You merged the changes at 3:50. That's not allowing for objections; that's middle-of-the-night stealth changes. That has no place in this organization.
Paul (Jones), "if you don't like it, change your vote" is a down right rude approach to take to your colleagues on this list. It is flippant and quite frankly unprofessional.
Am I being a process stickler? Damned straight. The single biggest problem with this group right now is the "loudest person wins" approach to consensus building being the modus operandi and process. That's a horrific process (I've seen it and been part of it before; I speak from experience), and we MUST get out of that mindset if this group is going to be a long-term Force for Good (as I think we all want it to be).
--Larry Garfield
Case in point: I'm about to disappear to DrupalCon/Symfony Live for a week, as is Fabien (another of the voting representatives here), which means our time to even bother reading the list next week will be extremely limited. If this had been done 2 days from now, there's a good chance neither of us would even have noticed to be able to object.
It's absolutely inappropriate to modify a PSR in any way during the voting period.
Phil, I have to agree with Hari that a 20 min turnaround time on a merge under those circumstances is also absolutely inappropriate. Paul asked for objections at 3 PM Central Time today. You merged the changes at 3:50. That's not allowing for objections; that's middle-of-the-night stealth changes. That has no place in this organization.
Paul (Jones), "if you don't like it, change your vote" is a down right rude approach to take to your colleagues on this list. It is flippant and quite frankly unprofessional.
Am I being a process stickler? Damned straight. The single biggest problem with this group right now is the "loudest person wins" approach to consensus building being the modus operandi and process. That's a horrific process (I've seen it and been part of it before; I speak from experience), and we MUST get out of that mindset if this group is going to be a long-term Force for Good (as I think we all want it to be).
--Larry Garfield
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAESZFtLMR840OBbomffJD%2BCEuqApcJ8_nb2NxNk5eEuH5_Q%3D7w%40mail.gmail.com?hl=en.
Paul Jones
May 17, 2013, 1:26:17 AM5/17/13
to php...@googlegroups.com
On May 16, 2013, at 9:09 PM, Larry Garfield wrote:
> It's absolutely inappropriate to modify a PSR in any way during the voting period.
>
> Phil, I have to agree with Hari that a 20 min turnaround time on a merge under those circumstances is also absolutely inappropriate. Paul asked for objections at 3 PM Central Time today. You merged the changes at 3:50. That's not allowing for objections; that's middle-of-the-night stealth changes. That has no place in this organization.
>
> Paul (Jones), "if you don't like it, change your vote" is a down right rude approach to take to your colleagues on this list. It is flippant and quite frankly unprofessional.
>
> Am I being a process stickler? Damned straight. The single biggest problem with this group right now is the "loudest person wins" approach to consensus building being the modus operandi and process. That's a horrific process (I've seen it and been part of it before; I speak from experience), and we MUST get out of that mindset if this group is going to be a long-term Force for Good (as I think we all want it to be).
In the name of good conduct all around, I will withdraw the vote on the other thread.
My many apologies.
-- pmj
André R.
May 17, 2013, 10:34:18 AM5/17/13
to php...@googlegroups.com
Removing the return inside if (is_readable($path)) will result in the code iterating always to the end despite already having found a file, there should be a return; there.
Drak
May 17, 2013, 10:44:20 AM5/17/13
to php...@googlegroups.com
Excuse me if I am missing something, but there is nothing being iterated in this code example. Also, should use include $path rather than require because is_readable already makes the include guaranteed. include is less expensive than require.
Regards,
Drak
André R.
May 17, 2013, 10:45:44 AM5/17/13
to php...@googlegroups.com
On Wednesday, May 15, 2013 12:27:56 AM UTC+2, Donald Gilbert wrote:
I see where you're coming from now. IMO, not being able to have duplicate registrations for the same namespace would not be desired behavior. If that were the case, it would make it difficult to impossible for users to utilize the autoloader to override classes if need be. That was a large percentage of the use case for me with Joomla. When I wanted to test out a bug fix or feature improvement, instead of hacking the core files, I could just register a new prefix lookup path that would find the files in my specified location first, and use my overridden version.
We allowed the same in eZ Publish 4.x if people specified a constant. It's imho still hacking, difference is that there will be no warnings if system is updated / upgraded but the external class is not patched with changes.
In 5.x we use DI instead witch allows the same in a cleaner way.
If needed, fix is simple, as proposed before on this list, add a flag for what to actually do on a false match can include the following options:
- NS_MATCH_NO_FILE_EXCEPTION
- NS_MATCH_NO_FILE_RETURN
- NS_MATCH_NO_FILE_EXCEPTION
- NS_MATCH_NO_FILE_RETURN
- NS_MATCH_NO_FILE_IGNORE / CONTINUE
André R.
May 17, 2013, 10:51:55 AM5/17/13
to php...@googlegroups.com
right, my bad, was thinking the full fledge example from PSR-0
Beau Simensen
May 17, 2013, 11:00:05 AM5/17/13
to php...@googlegroups.com
Andre, do you have remaining issues with the project-specific autoloader since it is not iterating anything? It is intended to be a simple example with no frills. The problem with potentially matching outside of the namespace has been addressed. Are there issues you see with the general-purpose autoloader example?
Drak, if include is better than require I'd be happy to change that. (or someone else can in the meantime; i'm about ready to head out for the long trek to Portland)
--
You received this message because you are subscribed to a topic in the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/php-fig/SJTL1ec46II/unsubscribe?hl=en.
To unsubscribe from this group and all its topics, send an email to php-fig+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/a62d765c-2b99-4a34-93dc-1a79733ebf14%40googlegroups.com?hl=en.
Phil Sturgeon
May 17, 2013, 11:56:36 AM5/17/13
to php...@googlegroups.com
But that big green button is so shiny!
It seemed pretty trivial, but it broke stuff. Not only was it inappropriate but it was wrong too, I definitely messed up there. Sorry all.
Reply all
Reply to author
Forward
0 new messages