[PHP-DEV] Re: cvs: php-src(PHP_5_3) /ext/phar/tests ini_set.phpt
Hannes Magnusson wrote: What about 5.2? I thought the extension was supposed to work fine there too (at least there are bunch of tests that skip with the reason needs 5.2). So you have three branches in pecl? HEAD, PHP_5_3 and PHP_5_2? (Not that fix is required in 5.2, everyone who did try this functionality ignored the obvious wrong return value and committed the tests as-is and now 5.2 is apparently in security/critical-bugfix-mode-only.) We have 1 branch in pecl. The source uses #ifdef for the 5.2-specific stuff. So, yes, this test will need to be split into two. Don't you just love arbitrary changes? :) Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: cvs: php-src(PHP_5_3) /ext/phar/tests ini_set.phpt
Hannes Magnusson wrote: bjori Tue Dec 9 13:02:40 2008 UTC Modified files: (Branch: PHP_5_3) /php-src/ext/phar/tests ini_set.phpt Log: MFH: fix test http://cvs.php.net/viewvc.cgi/php-src/ext/phar/tests/ini_set.phpt?r1=1.4.2.1r2=1.4.2.2diff_format=u Index: php-src/ext/phar/tests/ini_set.phpt diff -u php-src/ext/phar/tests/ini_set.phpt:1.4.2.1 php-src/ext/phar/tests/ini_set.phpt:1.4.2.2 --- php-src/ext/phar/tests/ini_set.phpt:1.4.2.1 Fri Aug 1 13:38:47 2008 +++ php-src/ext/phar/tests/ini_set.phpt Tue Dec 9 13:02:40 2008 @@ -25,7 +25,7 @@ string(1) 1 string(1) 1 string(1) 1 -string(1) 1 -string(1) 1 +bool(false) +bool(false) string(1) 1 string(1) 1 Hi, Heads up: there have been several commits to ext/phar in php-src by non-maintainers who did not merge to pecl/phar, and a couple to pecl/phar that were not merged to php-src. You have 2 choices. 1) merge that sucker 2) email all of the phar maintainers whenever you do a commit so we at least know about it and can do the merge. php-src/phar HEAD/PHP_5_3 and pecl/phar are intended to be 100% in sync. Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] problem with include_path and write-based file functions
Hi,
This bug:
http://bugs.php.net/bug.php?id=46680
uncovers 2 larger issues.
1) Where should a function like file_put_contents() create its file if
it doesn't already exist and FILE_USE_INCLUDE_PATH is specified? The
test for this is ext/standard/file/file_put_contents_variation4.phpt and
a few others.
file_put_contents($filename, File in include path, FILE_USE_INCLUDE_PATH);
arbitrarily creates $filename in the first portion of include_path on
PHP 5.2, and arbitrarily creates $filename in cwd in PHP 5.3. Of
course, if the file already exists in any of the include_path, both PHP
5.2 and 5.3 will overwrite it.
The same is true of fopen() in write mode.
2) Should file_get_contents()/fopen() in read mode fall back to current
directory if not found in include_path? The test for this is
ext/standard/file/fopen_variation5.phpt and others
In PHP 5.2, the functions simply fail if the file does not exist in
include_path. Thus if include_path does not contain ., the file is
not found. In PHP 5.3, there is a fallback to check cwd for the file.
For #1 I think both PHP 5.2 and 5.3 are broken. If you specify that a
file should be written to inside include_path, it should fail if no file
is found - allowing arbitrary file creation is just plain stupid and
also dangerous - include_path has no business being used to create a
file, it should only be used to find existing files. In any case, I
think allowing include_path to be used at all for file modification is
horrendous, and think it should be deprecated and removed from PHP 6.
We should simply add file_resolve_path() function which can be used to
locate a file.
?php
if ($path = file_resolve_path('somefile.txt')) {
file_put_contents($path, 'blah');
} else {
file_put_contents($someOtherPath, 'blah');
}
?
For #2 I think PHP 5.3 is broken, and the fallback to cwd should be
removed from php_resolve_path.
Comments?
Greg
P.S. the tests themselves should not create directories outside of their
tests/ subdirectory, and should use things like PATH_SEPARATOR, but
that's a minor issue
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Namespace resolution rules has been changed?
David Grudl wrote:
Try to answer the question: what is the $obj instance of?
namespace foo;
$obj = $factory-loadClass('bar\class');
bar\class
dynamic class names are always FQN.
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Namespace resolution rules has been changed?
Stan Vassilev | FM wrote:
Hi Marcin,
Stan also requested this, so it should be considered as a possibility.
Personally, I would rather not introduce this land mine. It requires
the user to do an implicit prepending of namespace name (foo) to bar
in the use statement as well as a translation of A, which could fast
lead to unreadable code.
It is probably best to simply require a fully qualified name where it is
intended. Thus
1) require leading \ in use statements
2) allow namespace\blah in use statements, as this is a valid fully
qualified name.
?php
namespace my\ns;
// this is a better way to do the suggested use bar as A;
use namespace\bar as A;
use \bar as B;
class mine extends \my\parentclass {}
?
Greg, I can't spot where does your example differ from what I and Marcin
suggested? Please explain.
I would not allow use blah\blah as bar;
And there's one more pain point which I posted earlier on the list
about, but now as I prepare my framework for 5.3 namespaces, I *really*
feel the pain in practice: the inability to pass a class/function name
without specifying it fully as a string.
My suggestion was: use foo\bar\baz as alias;$name = nameof alias;
// $name == 'foo\bar\baz'
Are you aware of __NAMESPACE__? Also, if you are using a lot of
external namespace names, you might consider simply defining constants:
namespace foo\bar\baz;
const ns = __NAMESPACE__;
then you can simply do
use foo\bar\baz; $name = baz\ns;
I don't see a huge pressing need for nameof since the above is 3 extra
lines of code - total - per NS.
If this sounds good, I will add an example to the new docs
yet-to-be-committed.
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Namespace resolution rules has been changed?
Marcin Kurzyna wrote:
?php
namespace foo;
use bar as A; /// -resolves as A == \foo\bar
use \bar as B; /// -resolves as B == \bar
?
Hi Marcin,
Stan also requested this, so it should be considered as a possibility.
Personally, I would rather not introduce this land mine. It requires
the user to do an implicit prepending of namespace name (foo) to bar
in the use statement as well as a translation of A, which could fast
lead to unreadable code.
It is probably best to simply require a fully qualified name where it is
intended. Thus
1) require leading \ in use statements
2) allow namespace\blah in use statements, as this is a valid fully
qualified name.
?php
namespace my\ns;
// this is a better way to do the suggested use bar as A;
use namespace\bar as A;
use \bar as B;
class mine extends \my\parentclass {}
?
Stas, I'm sure you have an opinion on this?
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Namespace resolution rules has been changed?
Jaroslav Hanslík wrote:
David Grudl napsal(a):
Hello!
This code leads to fatal error: Class 'Nette\Loaders\Nette\Object'. Is
it a bug in current implementation or namespace resolution rules has
been changed?
namespace Nette;
class Object
{}
namespace Nette\Loaders;
class AutoLoader extends Nette\Object
{}
$obj = new AutoLoader;
Thank you,
David Grudl
I'm maybe wrong but I think that namespace resolution rules has been
changed. This should work:
namespace Nette\Loaders;
class AutoLoader extends \Nette\Object
{}
Hi,
The resolution rules have changed in that way. The reason was an
inconsistency:
?php
// Autoload extends Sub\Object
class Autoloader extends Sub\Object {}
// Another extends Object
class Another extends Object {}
?
versus this code:
?php
namespace Nette;
// Nette\Autoload extends Sub\Object
class Autoloader extends Sub\Object {}
// Nette\Another extends Nette\Object
class Another extends Object {}
?
The problem was that it isn't immediately clear why we would consider
Sub\Object to be the same as \Sub\Object, but Object to be
different from \Object inside a namespace context, but the same
outside a namespace context.
This is an important thing for developers of namespaced code to realize.
The docs have been updated in my personal checkout of php-doc, and I am
revising them to make them clearer, and also waiting on a few other
details to make it into CVS before committing.
Thanks,
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] karma request
Hi, I'd like to request karma for ZE in order to do minor bugfixes and refactoring. I have no intention to commit any feature changes or major commits, and am perfectly happy writing patches for these to be reviewed by the big guns. Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] bracketed namespace declarations
Steph Fox wrote: Hey Greg, Remember, the patch I'm proposing would only be necessary for people using un-namespaced code combined with namespaced code (already a bad idea) *and* scattering use statements throughout the global code. If it's 'already a bad idea', why support it? Many people want this support, and it is only a bad idea if the code is designed this way. If you are simply combining several files together, it is a necessary feature. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] bracketed namespace declarations
Hi,
Stas and company decided that they wanted namespaces to have two legal
syntax choices:
1) single namespace per file:
?php
namespace foo;
...
?
2) multiple namespaces per file:
?php
namespace foo1 {
}
namespace foo2 {
}
?
I've implemented these two syntax options in a patch found at
http://pear.php.net/~greg/bracketed.patch.txt based on earlier work of
Dmitry Stogov.
It turns out there are some tricky details to work out, especially with
regard to importing via use statements. If we just allow global,
un-namespaced code, for instance, we could end up with this mess:
?php
use blah\blah;
$f = new blah;
namespace one {
use foo\bar as blah;
$a = new blah;
}
// what is this?
blah::hi();
?
Technically, you could argue that blah::hi() should resolve to
blah\blah::hi(), but it is very difficult to track and figure out what
blah means by eye. Thus, in the patch I implemented, if bracketed
namespace declarations exist, global use statements are not allowed, but
must exist within namespace ns {} brackets.
This creates a problem - how do you combine namespaced and unnamespaced
code? To solve this, I introduced the oft-suggested namespace {}
syntax for un-namespaced code. Thus, the above script would become:
?php
namespace {
use blah\blah;
$f = new blah;
}
namespace one {
use foo\bar as blah;
$a = new blah;
}
namespace {
use blah\blah;
blah::hi();
}
?
Another important point is that imports specified by the use statement
disappear when we exit a namespace block. This way, it is very easy to
combine un-namespaced code that uses namespaces, and namespaced code.
Also very important to note is that the namespace {} syntax is
optional for any code that does not import other things via the use
statement - its sole purpose is to provide a clear visual and logical
wrapper within which imports occur.
Thus, this is also legal:
?php
namespace newly\coded\stuff {
class mine { ... }
}
// old stuff here
class PEAR { ... }
?
Lastly, if global code does make use of namespaces, but does not import
anything, it can also happily co-exist with bracketed namespace code:
?php
namespace ns {
class mine { ... }
}
$a = new ns\mine;
// and so on
?
Thanks,
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] bracketed namespace declarations
Stan Vassilev | FM wrote:
?php
// global, scope 1
namespace { // global, scope 2 }
// global, scope 1
namespace { // global, scope 3 }
// global, scope 1
namespace foo\bar { // foo\bar, scope 4 }
// global, scope 1
?
I am afraid I must shed my generally congenial public nature to express
a strong opinion:
This is a horribly complex idea Stan. PHP's implementation of
namespaces do not define scope. Once we open that Pandora's box, there
is no turning back.
Big time -1 from me.
Remember, the patch I'm proposing would only be necessary for people
using un-namespaced code combined with namespaced code (already a bad
idea) *and* scattering use statements throughout the global code.
Also, the *only* supported use case behind allowing multiple namespaces
per file is to allow people to mash pre-existing separate PHP files
together, and have them still compile. Requiring brackets is designed
to make it more readable, and the use restriction furthers that goal.
I would have implemented this requiring all code to be encased in
namespace {} or namespace nsname {}, but that turned out to be virtually
impossible in the parser because of the need to also allow declare
statements outside namespace declarations
Thanks,
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Call it: allow reserved words in a class or not?
Dan wrote:
Quite.
It does appear as though, as a group, that you're struggling with the entire
concept of namespaces. As demonstrated by this discussion, the resolution
issues, the separator farce, and so on.
It may be due to weaknesses in the PHP engine as a whole, I don't know...
but it strikes me that most of these issues should be trivial ones, or ones
that could be eliminated simply by approaching language design decisions
with some common sense.
In .NET, I can stick an Array class into my own namespace, extending the
System.Array type if I want to and use it in my code without issue. Why can
I not do that here? Is it simply that you're so worried about backwards
compatibility that you feel that you can't make the necessary changes to the
language to implement something fully?
Hi Dan,
OSS = show us a working patch and we'll consider it. Do this before
saying the issues should be trivial ones.
Not too long ago I spent a significant amount of time trying to make it
possible to use a few reserved words as method names or function names,
and the result was a patch that made the parser and lexer significantly
more complex, with lots of bugs that were very difficult to fix, and
some edge cases where parsing simply failed.
As an example of why your suggestion is not logically possible to
implement, what should the following code print, 1 or 2?
?php
namespace namespace\namespace;
function test(){echo 1\n;}
namespace namespace;
function test(){echo 2\n;}
echo namespace\test();
?
No human can tell whether you mean to call the
namespace\namespace\test() function, or simply to call the test()
function from the current namespace. Do you really expect a machine to
do any better?
Please spend at least 2 minutes carefully trying to think of an edge
case before lamenting any feature in the language, otherwise your posts
kill your credibility on the list and are frankly nothing other than
annoying noise.
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: 5.3+ build broken, see compile log
Pierre Joye wrote: Hi, A commit in the last 24 hours breaks the builds: d:\php-sdk\snap_5_3\vc9\x86\snap53_vc9\zend\zend_vm_execute.h(2735) : error C2036: 'void *' : unknown size d:\php-sdk\snap_5_3\vc9\x86\snap53_vc9\zend\zend_vm_execute.h(10445) : error C2036: 'void *' : unknown size d:\php-sdk\snap_5_3\vc9\x86\snap53_vc9\zend\zend_vm_execute.h(17792) : error C2036: 'void *' : unknown size Hi, Unrelated to this windows compilation problem (thanks Felipe for the quick fix), I'm concerned to see that I missed an example of opcode modification. We can't modify opcodes directly because of opcode caches. This code needs to be refactored slightly to either create the needed stuff in compile-time and pass it in with ZEND_OP_DATA or one of the unused op1/op2 (best) or run-time estrndup finagling (not so good). Thanks for pointing this out Pierre. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: Namespace Resolution
Lukas Kahwe Smith wrote: On 04.11.2008, at 18:43, Ryan Panning wrote: Just to make my post clear, I'm in favor of this approach for non-qualified calls in a namespace. 1. global 2. autoload 3. fail A couple of us (Hannes, Stas, Derick, Matt Wilson and I) were just chatting on IRC and we all favor the following: 1) ns 2) autoload (for classes) and global (for functions/constants) 3) fail So no fallback to the global namespace for classes, but fallback for all functions/constants (regardless of internal or not) Independently, I made this patch yesterday to do what you're describing. http://pear.php.net/~greg/resolv.patch.txt It turns out that we already fallback to all global constants (not just internal ones). Only functions and classes were falling back to internal-only. Once someone reviews and commits we can focus solely on bugfixes for namespaces and finally get out alpha3 within a week (my guesstimate, based on what things look like). Regarding the patch: it is minimal changes for class name resolution. It only removes the lines that tell the engine to do fallback for classes. We may also want to remove the few lines of functionality that does fallback to internal for classes in zend_fetch_class, but I'd rather wait on that, and do a full review of the way those constants are being used in ZEND_FETCH_CONSTANT and all the other 5 places constants are resolved, as I recycled some constants, which could use some renaming. The patch also does not update any tests, some of which should fail. Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] fixing opcode modification in namespace implementation
Hi, I assigned this to helly arbitrarily, but wanted to make sure that Stas and Dmitry also see it. This should be fixed ASAP. The opcode modification was easy to fix, since we weren't using op1. I also fixed a cosmetic issue Dmitry wanted fixed, which was replacing the non-verbose 1 and 2 used to identify namespace constant vs. namespace function in zend_resolve_non_class_name() with #defines. http://bugs.php.net/bug.php?id=46500 patch at http://pear.php.net/~greg/fix_ns1.patch.txt Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] namespace separator and whining
Christian Schneider wrote: Lukas Kahwe Smith wrote: one could also do 1) ns 2) global 3) autoload I'm in favour of this (if it avoids performance problems) as I don't see a problem with giving global priority over autoload. Hi, This is the current name resolution. The problem is that: ?php namespace foo; $a = new Exception(); ? will instantiate foo::Exception only if the class already exists, but will instantiate Exception otherwise. This defeats autoload's purpose for existing. We've been over this before. It's dangerous. There is one essential difference between classes and functions/constants: autoload. The only time the question of load order and fallback becomes and issue is when code is not explicitly loaded. In other words, a developer who is relying upon an external function will do this at some point before calling the function: ?php include 'path/to/code/containing/function/source.php'; ? In other words, PHP expects you to actually load the source of functions or constants you're going to be using in advance of using it. This, I believe, is a reasonable expectation. Classes, however, do *not* share this same expectation, because autoload's explicit purpose is to allow using classes *before* their source has been loaded. In other words, it is perfectly all right to have a different name resolution for classes than we have for functions and constants because of this different expectation. It is dangerous to fallback for classes prior to autoload, but it is not at all dangerous for functions/constants because the expectation is different. For this reason, the only resolution that we should be considering is: classes: 1) try ns::class 2) autoload ns::class 3) fail functions/constants: 1) try ns::function/ns::const 2) try internal function/const 3) fail. Note that I say try internal because the only purpose behind allowing this is to make it easier to use PHP's built-in functionality. Any userspace stuff should be specifically \prefixed or imported via use. And (yes) we need import for functions. Greg P.S. my mail server has been down for almost a week, apologies to anyone who is wondering why I haven't replied to your message, I probably didn't get it. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] namespace separator and whining
Lukas Kahwe Smith wrote:
On 04.11.2008, at 17:15, Gregory Beaver wrote:
In other words, it is perfectly all right to have a different name
resolution for classes than we have for functions and constants because
of this different expectation. It is dangerous to fallback for classes
prior to autoload, but it is not at all dangerous for
functions/constants because the expectation is different.
To me the key question is do we want to let people overload global
classes easily or are we mostly talking about convinience? Class names
are typed quite seldom (and even in a few years down the road the bulk
of PHP's features will be provided via functions), so I would not think
that we really need to focus on convenience here.
OK, taking a step back. Let's distill the two problems we're trying to
solve
1) namespaces provide encapsulation - a safe way to name things without
worrying about whether it will conflict with external code
2) accessing internal functionality - the most used stuff in PHP should
still be easy to use in namespaces.
#1 means that when using our code inside namespaces, when there is
ambiguity we want it to look for namespaced stuff first and foremost.
#2 means we want to be able to access stuff like strlen() and
array_map() without any monkey business.
The largest conflict between #1 and #2 happens with classes because of
autoload. __autoload() is expensive from a performance standpoint, and
should not be called unnecessarily.
I posit that the most used stuff in PHP are internal functions (not
global userspace functions, not internal classes, not userspace classes)
and so this needs to be a priority. In addition, there is already
confusion over things like:
?php
// which of these is a function?
$a = 1;
unset($a);
$a = key(array('hi' = 1));
include('oops.php');
?
so requiring a prefix for functions will lead to parse errors in user
code like:
?php
\include('oops.php');
?
so the best resolution for functions is:
1) ns\function
2) internal function
Classes are different (TM).
if we do:
1) ns\class
2) autoload ns\class
3) internal class
then this code has hidden performance hits:
?php
namespace foo;
include 'external_autoload.php'; // __autoload must be declared
unnamespaced
$a = new ArrayObject(array('hi')); // calls autoload unnecessarily
?
if we really mean the ArrayObject class and not the foo\ArrayObject
class. Thus, the convenience of internal fallback has disadvantages
(major performance problems) that do not exist with functions or constants.
Notifying the user via E_NOTICE/E_STRICT/E_WHATEVER simply adds
annoyance, and introduces potential for behavior change. For instance,
if a user decides to start using some external library that defines an
autoload when they didn't have it before, suddenly all of their code
throws E_NOTICEs every time an internal class name is referenced. Or,
without the E_NOTICE, performance suddenly slows way down and the user
blames the external library.
If we do:
1) ns\class
2) internal class
3) autoload ns\class
then our code example fails in a more subtle way. The ambiguity of
ArrayObject resolves always in favor of internal class ArrayObject,
which means that users have to always explicitly import namespaced
classes to avoid this problem. Why is it a problem?
Let's say we're doing our own userspace implementation of PDO with a few
extensions, for a system where PDO is disabled:
?php
namespace Myproject;
class PDO {} // userspace implementation of PDO
?
now in another file:
?php
namespace Myproject;
$a = new PDO(...);
?
[note: this implementation depends on __autoload existing]
Now, if PDO extension is disabled, the above code will successfully
autoload Myproject\PDO. Imagine that on an upgrade, PDO is enabled.
Now, the object instantiated is of class PDO, not Myproject\PDO.
Suddenly, there are subtle logic errors that creep in (not a fatal
error), but the code continues to operate normally.
This resolution is very dangerous, and is a clear violation of the
primary purpose of namespaces.
If we do this:
1) ns\class
2) autoload ns\class
we get a stronger E_NOTICE (fatal E_ERROR) when an internal class is
used without explicit import. This is robust, easy to debug, and forces
good code without much pain. Our example user above has no problems
with the external library, and changes need not be made after initial
coding.
By doing the resolution I've suggested (and Stas, incidentally, was the
first to suggest this):
classes:
1) ns\class
2) autoload ns\class
3) FAILBOAT
functions/constants:
1) ns\func or ns\const
2) internal func\const
3) FAILBOAT
We get the best of #1 and the best of #2, and it makes coding easier in
the long run.
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: /endnamespacediscussion
David Grudl wrote:
I hope it is only very bad joke :-(
namespace myNamespace;
class theLoader
{
function load($class) { ... }
}
// somewhere else
spl_autoload_register(array(myNamespace\theLoader, load));
- registers myNamespacetabheLoader::load()
Fortunately, there is a simple workaround:
spl_autoload_register(array('myNamespace\theLoader', 'load'));
or slightly less simple:
spl_autoload_register(array(myNamespace\\theLoader, load));
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] UltraSimple Namespace Solution
Nathan Rixham wrote: seen. Personally though I'd love to see stas' #1 get implemented and - used for all functions in a namespace so.. one::step::two(); //always static method of class one::step-two(); //always function of namespace. But it's still ambiguous (only in a rarely though) - if an object with a method two is assigned to the static variable of a class called one then problem comes back. First of all, this is not going to happen (voted down). Second of all one::$step is how static variables are accessed, not one::step. So there is no potential for conflict. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] my last attempt at sanity with namespaces
Stanislav Malyshev wrote: Hi! Yes, but most times when there is conflict it will be between two sets of code. So importing someone else's namespace explicitly and giving it a new name is a good call IMHO. If you have two distinct sets of code, why you use same namespace for both of them? Namespaces are specifically designed so you could have different sets of code in different places. nb Stas - I asked the same question about warnings, Greg updated his proposal since then to answer it. As it is now, every call to class::method() not accompanied with use should produce E_WARNING. I do not think it is an acceptable situation - this would make code migration a nightmare, since even if you never use functions and never even have any chance for a conflict, you still have to insert hundreds of imports into your code, just to shut up the warnings. I don't think it is a good idea. Feature that you do not need, can not disable and have to work around is called bug. Hi Stas, This is not what is proposed. The E_WARNING is *only* on a name conflict. Please re-read or try the patch: http://pear.php.net/~greg/resolve_conflict.patch.txt Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] namespaces sanity: addition to RFC explaining why Stas's proposal doesn't work
Hi again, I was asked to explain why I hadn't included ClassName-Method(); in the list of ideas that solve the ambiguity problem. I added a brief section to the RFC that does so: http://wiki.php.net/rfc/namespaceissues#why_stas_s_proposed_solution_doesn_t_work Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] turn off gcov for php 4.4?
Hi all, Isn't it time to retire PHP 4.4 in gcov.php.net? It's just wasted resources for that version Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] question on how to solve major stream filter design flaw
Hi,
I'm grappling with a design flaw I just uncovered in stream filters, and
need some advice on how best to fix it. The problem exists since the
introduction of stream filters, and has 3 parts. 2 of them can probably
be fixed safely in PHP 5.2+, but I think the third may require an
internal redesign of stream filters, and so would probably have to be
PHP 5.3+, even though it is a clear bugfix (Ilia, your opinion
appreciated on this).
The first part of the bug that I encountered is best described here:
http://bugs.php.net/bug.php?id=46026. However, it is a deeper problem
than this, as the attempts to cache data is dangerous any time a stream
filter is attached to a stream. I should also note that the patch in
this bug contains feature additions that would have to wait for PHP 5.3.
I ran into this problem because I was trying to use stream filters to
read in a bz2-compressed file within a zip archive in the phar
extension. This was failing, and I first tracked the problem down to an
attempt by php_stream_filter_append to read in a bunch of data and cache
it, which caused more stuff to be passed into the bz2 decompress filter
than it could handle, making it barf. After fixing this problem, I ran
into the problem described in the bug above because of
php_stream_fill_read_buffer doing the same thing when I tried to read
the data, because I requested it return 176 decompressed bytes, and so
php_stream_read passed in 176 bytes to the decompress filter. Only 144
of those bytes were actually bz2-compressed data, and so the filter
barfed upon trying to decompress the remaining data (same as bug #46026,
found differently).
You can probably tell from my explanation that this is an
extraordinarily complex problem. There's 3 inter-related problems here:
1) bz2 (and zlib) stream filter should stop trying to decompress when it
reaches the stream end regardless of how many bytes it is told to
decompress (easy to fix)
2) it is never safe to cache read data when a read stream filter is
appended, as there is no safe way to determine in advance how much of
the stream can be safely filtered. (would be easy to fix if it weren't
for #3)
3) there is no clear way to request that a certain number of filtered
bytes be returned from a stream, versus how many unfiltered bytes should
be passed into the stream. (very hard to fix without design change)
I need some advice on #3 from the original designers of stream filters
and streams, as well as any experts who have dealt with this kind of
problem in other contexts. In this situation, should we expect stream
filters to always stop filtering if they reach the end of valid input?
Even in this situation, there is potential that less data is available
than passed in. A clear example would be if we requested only 170
bytes. 144 of those bytes would be passed in as the complete compressed
data, and bz2.decompress would decompress all of it to 176 bytes. 170
of those bytes would be returned from php_stream_read, and 6 would have
to be placed in a cache for future reads. Thus, there would need to be
some way of marking the cache as valid because of this logic path:
?php
$a = fopen('blah.zip');
fseek($a, 132); // fills read buffer with unfiltered data
stream_filter_append($a, 'bzip2.decompress'); // clears read buffer cache
$b = fread($a, 170); // fills read buffer cache with 6 bytes
fseek($a, 3, SEEK_CUR); // this should seek within the filtered data
read buffer cache
stream_filter_append($a, 'zlib.inflate');
?
The question is what should happen when we append the second filter
'zlib.inflate' to filter the filtered data? If we clear the read buffer
as we did in the first case, it will result in lost data. So, let's
assume we preserve the read buffer. Then, if we perform:
?php
$c = fread($a, 7);
?
and assume the remaining 3 bytes expand to 8 bytes, how should the read
buffer cache be handled? Should the first 3 bytes still be the filtered
bzip2 decompressed data, and the last 3 replaced with the 8 bytes of
decompressed zlib data?
Basically, I am wondering if perhaps we need to implement a read buffer
cache for each stream filter. This could solve our problem, I think.
The data would be stored like so:
stream: 170 bytes of unfiltered data, and a pointer to byte 145 as the
next byte for php_stream_read()
bzip2.decompress filter: 176 bytes of decompressed bzip2 data, and a
pointer to byte 171 as the next byte for php_stream_read()
zlib.inflate filter: 8 bytes of decompressed zlib data, and a pointer to
byte 8 as the next byte for php_stream_read()
This way, we would essentially have a stack of stream data. If the zlib
filter were then removed, we could back up to the bzip2 filter and so
on. This will allow proper read cache filling, and remove the weird
ambiguities that are apparent in a filtered stream. I don't think we
would need to worry about backwards compatibility here, as the most
common use case would be unaffected by this change, and the use case it
Re: [PHP-DEV] question on how to solve major stream filter design flaw
Marcus Boerger wrote:
The filters need an input state and an output state. And they need to
respect the the fact that number of requested bytes does not necessarily
mean reading the same amount of data on the other end. In fact the output
side does generally not know whether less, the same amount or more input is
to be read. But this can be dealt with inside the filter. However the
filters should return the number input vs the number of output filters
always independently. Regarding states we would be interested if reaching
EOD on the input state meant reaching EOD on the output side prior to the
requested amount, at the requested amount or not at all yet (more data
available).
Hi,
All of this makes sense, but how to implement it? Currently, there is
no clear way to answer questions like:
1) how much data did the filter consume, and how much is left over?
2) how much data is left in the original stream prior to filtering?
Perhaps this could be implemented by overloading ftell() so that it can
accept a filter resource as well as a stream resource? Also, a new
function could be implemented:
stream_filter_datasize(resource $filter) // I'm open to better names
that would either return the total size of filtered data available, or
false if this is indeterminate. This would allow:
?php
$startoffile = 123;
$a = fopen('blah.zip');
fseek($a, $startoffile);
$filter = stream_filter_append($a, 'bzip2.decompress');
$b = fread($a, 170);
echo ftell($a),, ,ftell($filter),,
,stream_filter_datasize($filter),\n;
// outputs 145, 170, 176
?
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] namespace issues
Vesselin Kenashkov wrote: I'm -1 for removing constants and functions from the namespaces. As I wrote already in another thread, is it possible to have a fatal error thrown when the engine detects an ambiguity situation? Are there any logical (i mean from OOP point of view) or internals (performance and so on) problems with this solution? In my opinion is better than dropping funcs consts. Hi, There are problems with the internal implementation. For instance, constants do not store the line number on which they were created, and are not actually registered as existing until runtime, so conflict checks would happen at runtime. This would mean your application could work perfectly until a rarely executed branch of code is accessed and boom, fatal error. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Disable PHAR by default
Jani Taskinen wrote: Seems like PHAR causes quite unexpected things, f.e. http://bugs.php.net/bug.php?id=46194 where PHP crashes if file does not exist. Please, can this crap be disabled by default (ALWAYS) and only those who actually need it can enable it? Hi Jani, Classic overreacting, and disrespect to years of careful work. First of all, this bug has existed for all of 10 hours, and the problem is easy to fix, and caused because I never imagined PHP would be stupid enough to pass in a null pointer to zend_compile_file (kind of hard to compile a file whose name is undefined). This is a 3-line fix, and is only caused by a bug in the CGI handler coupled with lighttpd, which is setting filename to 0x0, something none of the other sapis do when a file is non-existent. Let's think about this for a second: why pass in a null pointer for a filename to a function whose purpose is to compile a file in the first place? How about we disable the CGI sapi under lighttpd by default? Don't be such a weenie. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] true namespaces, yet another point of view
Lukas Kahwe Smith wrote:
On 24.09.2008, at 01:17, Guilherme Blanco wrote:
For those that do not understand very well the explanation of jvlad...
He's suggesting to change the class struct to be an scope struct, and
have a property that tells if it's a namespace or a class, and reuse
the implementation of class which already works very well.
The nested support is achieved afai understand through the Adjacency
List algorithm... can you confirm this to me?
or just leave the organization of things to classes (with long class
names with a nice prefix to prevent collissions) and leave it to
class_alias() (and equivalent features for functions .. also with the
option of a compile time aliasing) to handle the aliasing.
this removes the need for namespaces and use statements, while making it
possible to make class/function names short (that are long for
organizational and collision prevention reasons).
Hi,
This approach doesn't work because aliasing with class_alias() does not
allow name conflicts:
?php
class foo {
function __construct(){echo 'hi',\n;}}
class_alias('foo', 'XMLReader');
$a = new XMLReader;
?
results in:
Warning: Cannot redeclare class XMLReader in
/home/cellog/workspace/php5/test.php on line 4
whereas:
?php
class foo {
function __construct(){echo 'hi',\n;}}
use ::foo as XMLReader;
$a = new XMLReader;
?
results in:
hi
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] solving the namespace conflict issues betweenfunction/staticmethod class constant/ns constant
Daniel Convissor wrote:
On Mon, Sep 22, 2008 at 08:07:10PM +0400, Dmitry Stogov wrote:
Yes. Changing :: into any other separator solves the functions/static
methods and constants ambiguity, but it also breaks intuitive syntax.
Which is more important? Considering there have been threads upon
threads filled with myriad messages, it seems resolving the ambiguity is
significanly more important than an intuitive syntax.
Please, let's pick a different operator for namespaces. This will
eliminate the scoping/resolution issues for both the engine and the
humans trying to interpret PHP code.
[/me searching for the vote tally on which operator people preferred]
[/me alas, can't find it at the moment]
I recall there being a vote on this. I'm sure some of you have a saved
copy and/or link to it. Can't we pick the operator on that list that got
the second highest vote count?
Hi,
The second highest vote was :::, but there was strong objection to this
as well from some. The problem, I still believe, is that we are focused
on having the same:::stupid:::operator:::between:::everything.
The truth is that in source files, there is a clear boundary between
namespace definition as in:
?php
namespace foo::bar;
?
and the things in the namespace, as in:
?php
class bar {}
?
Without context, the above definition of class bar could define
literally any class name:
bar
something::bar
another::long::name::bar
depending on the namespace declaration at the top of the file. This
means that it is an arbitrary choice in the current implementation to
use :: to join the two things (namespace name and class name).
In truth, it will be a lot easier to debug code if this is a different
separator, as the boundary between namespace and element will be crystal
clear. For instance:
(current implementation)
my::framework::someFunction()-aProperty-someMethod();
is my::framework a class or a namespace?
Let's use an arbitrary separator .. to mark the boundary between
namespace and class/function
(new implementation)
my::framework..someFunction()-aProperty-someMethod();
OK, now we know that my::namespace is a namespace, and someFunction is a
function.
my..framework::someFunction()-aProperty-someMethod();
The above is also clear: my is a namespace, framework is a class name,
and someFunction is a static method.
This has the advantage that existing code based on PHP 5.3 alpha 1 and 2
won't require very much modification, it is easy to read, and easy to
understand what a line of code does without needing any context information.
I took my old patch and moved from the - separator to the .. separator
in about 45 minutes of work, so changing the separator will not be
difficult. Here's the new patch against PHP_5_3:
http://pear.php.net/~greg/ns.element.2.patch.txt
Unlike the first patch, it introduces a new token, T_NS_MEMBER, instead
of recycling T_OBJECT_OPERATOR.
This is also easy to document. Here's how I would do it:
Although namespaces can span multiple files, they are similar to static
classes.
Namespaces are used in PHP to link related classes, functions, and
constants much as static classes can be used to link related variables,
methods, and constants.
The classes, functions, and constants defined in a namespace are called
namespace elements just as the variables, methods and constants
defined in a static class are static class members
A static class example:
?php
class myclass
{
const one = 1;
static public $a;
static function show()
{
echo __METHOD__,\n;
}
}
myclass::show();
$variable = myclass::$a;
echo myclass::one;
?
Static class members can be accessed using the :: operator, as in
myclass::show().
Here is a namespace example:
file1.php:
?php
namespace mynamespace;
const one = 1;
function show()
{
echo __FUNCTION__,\n;
}
class myclass
{
function show()
{
echo __METHOD__,\n;
}
}
class mystaticclass
{
static function show()
{
echo __METHOD__,\n;
}
}
?
main.php:
?php
include 'file1.php';
mynamespace..show();
echo mynamespace..one;
$a = new mynamespace..myclass;
$a-show();
?
Namespace elements can be accessed using the .. operator, as in
mynamespace..show().
Another example showing how to access static class members within a
namespace:
?php
include 'file1.php';
mynamespace..mystaticclass::show();
?
Namespaces can be nested using the :: operator as in:
file2.php:
?php
namespace my::nested::ns;
class mystaticclass
{
static function show()
{
echo __METHOD__,\n;
}
}
?
The use statement allows importing both namespace elements and
namespace names into the current scope:
?php
include 'file1.php';
incldue 'file2.php';
use mynamespace..mystaticclass;
use my::nested::ns;
use my::nested;
use my::nested::ns..mystaticclass as myclass;
mystaticclass::show(); // mynamespace..mystaticclass::show()
myclass::show(); // my::nested::ns..mystaticclass::show()
ns..mystaticclass::show(); //
Re: [PHP-DEV] alpha3
jvlad wrote: Adding support for functions, constants and even variables is actually quite do-able with the solution I suggested (different separator between namespace name and function/constant/variable name) and can be added easily. Adding support for functions, constants and even variables is actually quite do-able with the solution I suggested (different separator between namespace name and function/constant/variable name) and can be added easily. Greg, Does it mean that the namespace separator will not be (or likely not to be) the same when you need access to namespaced classes vs namespaced functions? What's about consistency of the design? If you can't resolve the ambiguity or any other problems around namespaces without changing the separator, why not to change it for classes too? Hi, The patch I posted here: http://pear.php.net/~greg/ns.element.patch.txt does exactly what you are talking about. For some reason, some people find this too difficult to digest. I've already expressed my opinion on the matter (after all, I did spend almost a week developing the patch). Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] solving the namespace conflict issues between function/staticmethod class constant/ns constant
Dmitry Stogov wrote:
Hi Greg,
Greg Beaver wrote:
Lupus Michaelis wrote:
Larry Garfield a écrit :
I agree that #5 seems like the best solution. The problem is caused
by the double meaning of ::. All of the other solutions feel like
bandaids.
They are not a double meaning : it is a scope resolver. Like in C++.
So please all stop this war about namespaces, and look where it was
resolved in other languages. It will be wisely :)
Hi,
Amazingly enough, this work has already been done. The first thing
noticed is that C++ is a compiling language, and PHP isn't. Therefore
at compile time, it is possible to know every single file that will be
available, every single class, and so on. This enables all kinds of
clever resolution such as importing ns::* and it actually works with
full performance.
PHP, on the other hand, is a dynamic language, and it is impossible to
know in advance which files, which classes, which namespaces will be
defined. Other dynamic languages that have implemented namespaces have
solved the problems mentioned via run-time resolution, which is much slower.
One key difference from C++ is that namespaces do *not* define scope in
PHP. The only constructs that define scope different from global scope
are functions and now closures (as I understand them from reading the
mailing list).
I suppose I should have mentioned option #6: do all class, function and
constant resolution at run-time, obsoleting all opcode caches and make
PHP 5.3 an unusable release for any high traffic websites.
Please check your suggestions before publishing them.
?php
class Foo {
function bar() {
}
}
function simplecall() {
for ($i = 0; $i 100; $i++) {
Foo::bar();
Foo::bar();
Foo::bar();
Foo::bar();
Foo::bar();
Foo::bar();
Foo::bar();
Foo::bar();
Foo::bar();
Foo::bar();
}
}
simplecall();
?
snip
hi Dmitry,
Let me quote myself from right above your reply:
option #6: do all class, function and constant resolution at run-time
This would be slow and eliminate most of the benefit of opcode caches.
The script you posted does not demonstrate option #6 because that's not
how PHP works or has ever worked. Which suggestion are you trying to
debunk?
I was never saying the current implementation is slow, only that it is
broken. Without changes, it is dangerous in PHP's namespace
implementation to
1) ever use short names inside a namespace declaration for any class in
the same namespace not defined in the same file
2) mix functions and classes in the same project with hierarchical
namespace declarations, or mix namespace constants with class constants
#1 makes namespaces essentially more verbose than the existing situation
(:: is longer than _), and #2 requires users to perform more diligence
than they would have to before namespaces. In both cases, this either
introduces more chance for subtle logic failure compared to PHP prior to
namespaces, or requires longer class names. I highly doubt that is the
intention.
Returning to the original debate, if you really believe this conflict is
not an issue, then why was the first user note published last December a
note about this conflict?
http://us3.php.net/manual/en/language.namespaces.php#80035
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] solving the namespace conflict issues between function/static method class constant/ns constant
Larry Garfield wrote:
On Saturday 20 September 2008 6:43:41 pm Richard Quadling wrote:
5) a simply syntax change to namespaces, introducing a new concept:
namespace element.
A namespace element is a class, function or constant defined within a
namespace declaration:
?php
namespace foo;
class bar {} // class element bar in namespace foo
function bar(){} // function element bar in namespace foo
const bar=1; // const element bar in namespace foo
?
This is similar to class elements:
?php
class foo {
function bar(){} // method element bar in class foo
const bar=1; // constant element bar in class foo
public $bar=1; // variable element bar in class foo
}
?
Currently, this code:
?php
namespace foo::bar;
class buh{}
?
creates a class named foo::bar::buh, essentially joining the namespace
foo::bar and its class element buh with the separator ::. This
turns out to be the root of the problem with the conflicts between class
elements and namespace elements. The last patch introduces a new
namespace element operator to delineate the boundary between namespace
name and element name. For the patch, I recycled T_OBJECT_OPERATOR (-).
current way:
?php
foo::bar-test(); // namespace foo::bar, call to function element test()
foo-bar::test(); // namespace foo, call to static method element test()
in class element bar
foo-myconst; // namespace foo constant myconst
foo::myconst; // class foo constant myconst
?
The patch is at:
http://pear.php.net/~greg/ns.element.patch.txt
This is the most extensive change. The patch preserves :: as global
element accessor (::strlen() calls strlen internal function, for
instance). I'm happy to answer any other questions.
[snip]
I agree that #5 seems like the best solution. The problem is caused by the
double meaning of ::. All of the other solutions feel like bandaids.
Of course, the problem then is finding a symbol that is not already used. I
don't think reusing - is any wiser than reusing ::. # would be great if it
Let's be clear. Here is a sample of code with the new syntax:
?php
namespace name::still::has::double::colon;
const oscopy = 1;
class blow{
const byblow = 1;
static function hard(){}
}
namespace another::name;
use name::still::has::double::colon;
// fully qualified name:
// note that - is only used once
// to specify the boundary between namespace name and namespace member.
$a = new name::still::has::double::colon-blow;
$a = new colon-blow;
// static method call
colon-blow::hard();
// or
name::still::has::double::colon-blow::hard();
// ns constant
echo colon-oscopy;
namespace blow;
function hard(){}
namespace a::third::name;
// demonstrate importing a class name
// which is different from importing a namespace name
use name::still::has::double::colon-blow;
// class static method
blow::hard();
// namespace function
::blow-hard();
// class constant
echo blow::byblow;
?
Two important things to note:
- ambiguity between namespace name and class name is eliminated
?php
namespace foo;
class test{
}
namespace foo::test;
class test{}
namespace third;
use foo::test;
$a = new test; // this is namespace foo, class test
$a = new test::test; // this is namespace foo::test, class test
?
- this separator is self-documenting. One does not need to grep source
code to determine whether blah-thing is a namespaced function or a
static method of class blah. The same is true of use statements. It is
crystal clear whether we are importing a namespace or a class name.
This will make debugging someone else's code a lot easier than it
otherwise would be.
Jochem has informed me he is taking care of the wiki RFC for this stuff.
I am allergic to wikis, unfortunately.
Thanks,
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Namespaced function vs static method in global class
Ionut Gabriel Stan wrote:
snip
echo Test::foo(); // outputs namespace function
echo 'br';
echo ::Test::foo(); // outputs namespace function
snip
My questions are:
1. How should I call the static method foo in the global class Test (except
for call_user_func)
2. How should I call the static method baz of class Bar in namespace Test
with call_user_func
3. Is there no possibility to change your minds and replace the double colon
with something else, likes a simple colon? I find it very confusing to figure
out what is what and the way I should call it.
Hi Ionut,
Stas did a good job of answering how to call the static method using
call_user_func. The answer to #1 under the current implementation is
you can't which is why I posted a patch to allow differentiating
between the two earlier this week.
However, I have since discovered a much better solution, and have a
working patch, but am checking details offlist before I propose it, as
it involves a slight but important change to namespace syntax. Stay
tuned for further details. As for your 3rd question, I guarantee this
will not happen, as it is technically impossible to do. For instance:
switch ($something) {
case oops:
is:this:part:of:oops();
break;
}
The above code could resolve to either case oops: followed by
is:this:part:of:oops() or as case oops:is:this:part:of:oops() and
there is no way for a computer to tell the difference without
introducing required whitespace. Since PHP != Python, that won't happen :).
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PATCH] Re: [PHP-DEV] namespace examples (solving name resolutionorderissues)
Stanislav Malyshev wrote: Hi! In other words, there is simply no comparison. Userspace class usage outnumbers internal class usage by an order of magnitude in typical OO PHP code. I didn't claim userspace class usage outnumbers internal class usage or otherwise. What I claimed is that since we have a lot of files (framework has 1821, as you very helpfully pointed out) and those use 950 instances of internals classes, application using Framework has big chance to score hundreds of uncacheable autoloads. And that is not counting user code, which would add to the problem. Right, and since you pointed out this limitation and then found another example that has the same problem that cannot be solved (non-autoload with load order determining what classname is instantiated), that invalidates any possible approach. I was trying to say that this leaves us with only 1 choice, which is whether to use the name resolution you proposed or stick with the current one. Currently we have: 1) check current namespace 2) check internal 3) autoload if possible 4) fail I remind all of the name resolution for classes that you proposed: 1) check current namespace 2) autoload if possible 3) fail Since my little script proved that the ratio of userspace vs. internal classes heavily favors userspace classes, this load order makes sense. However, I want to point out that it doesn't take a script to prove that the ratio of userspace functions vs. internal functions is exactly the opposite (far more internal functions vs. userspace functions), so I propose we have separate name resolution for classes and functions classes: 1) check current namespace 2) autoload if possible 3) fail functions: 1) check current namespace 2) check internal 3) fail Load order would matter for functions, but the risk is worth the benefit. I believe the initial implementation of namespaces had this order for classes, so Dmitry's first patch would have the necessary changes. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PATCH] Re: [PHP-DEV] namespace examples (solving name resolutionorderissues)
Stanislav Malyshev wrote:
Hi!
The caching is at runtime. Basically, it's in executor_globals, and so
is not linked to any opcode array (I couldn't figure out any other
globally accessible storage location). It works like an associative
array. If file X.php loads XMLReader in namespace namespacename, it
creates this entry:
array(
'X.php\0namespacename::XMLReader' = {class entry for XMLReader}
);
I though a bit about that patch, and I see following problems with it:
1. If you do not use autoloading, you still can get different
resolution, e.g.:
?php
namespace foo;
$a = new XMLReader;
resolves to ::XMLReader and cached for this file. But suppose some other
file, somewhere, loaded foo::XMLReader prior to that. Then the same code
here would resolve now to foo::XMLReader as before, right?
Isn't it the same scenario you complained about? If so this patch does
not solve this for 100% of cases, unless I misunderstand how the patch
works.
2. While the caching indeed helps per-file, typical OO application
contains dozens of files, and each file may use dozens of internal
classes (my install has 100+ internal classes). Meaning, there is still
potential for hundreds of non-cacheable exhaustive autoload searches.
This is not a good scenario.
Your first point is correct, and the second is interesting, but does not
quite match reality. I whipped up a script to count the number of
internal vs. user classes in a project, it's at
http://pear.php.net/~greg/detect_classes.phps
I ran it on PEAR2, which is a pre-namespace project at the moment, and
came up with these stats:
Internal classes: 239
User classes: 768
in 171 files
I ran the same script over PEAR, and got:
Internal classes: 110
User classes: 2027
in 231 files
Now, you may argue that these are atypical OO applications, so I also
ran the script over Zend Framework 1.5, with these results:
Internal classes: 950
User classes: 16167
in 1821 files
In other words, there is simply no comparison. Userspace class usage
outnumbers internal class usage by an order of magnitude in typical OO
PHP code.
PEAR2 uses an average of 2 internal classes per file, and 7 userspace
classes. PEAR uses an average of 0.5 internal classes per file, and 8
userspace classes per file. Zend Framework uses an average of 0.5
internal classes per file and ~9 userspace classes per file. Thus, if I
had to choose between defaulting to current namespace (as you suggested)
and to internal classes, the choice is easy. I would choose current
namespace because there would be far fewer use statements, and the
behavior is deterministic.
The script is at:
http://pear.php.net/~greg/detect_classes.phps
run with php detect_classes.php /path/to/files
Oh, and the script is one of the few cases where there are 3 internal
classes used and only 1 userspace class :).
Thanks,
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PATCH] Re: [PHP-DEV] namespace examples (solving name resolutionorder issues)
Stas and I continued our discussion off list, and decided to pop it back on the list, so here is 1 message, another to follow. Greg === Stanislav Malyshev wrote: Hi! ...the message you replied to? I must be missing something here. In my last reply, I raised a number of points, including having unqualified names resolve only to namespace and relation of this to functions and constants. I do not see these points addressed or discussed anywhere in your email. Some less important points - like tradeoffs you need to take for working with internal classes (second reply part in my email) - weren't mentioned either. Hi Stas, In your last reply, you asserted that the only solution was to always resolve unqualified names to namespace::name. The patch I sent actually shows that this is unnecessary. Also, I do not see the explanation how this patch is different from the last one and how the difference in the speed is achieved - you just claim it has gone from 466x to 5.2x but I did not find any explanation about what is the change allowed you to do this. I would like to understand what this change is and to do that from reading a patch is kind of hard and I can't be sure what I understood was actually your intent. Could you explain it? You could also contact me on IM or IRC if you prefer. Basically, the patch does 2 things 1) resolution order is changed to: foo.php: $a = new foo(); a) does namespace::foo class exist? b) try to autoload namespace::foo c) try to locate internal class ::foo 2) if internal class ::foo is found, then cache this information so that if another reference to the foo class occurs in foo.php, resolution short circuits to: a) use internal class ::foo This second point is why the slowdown went from ridiculous to slight. Since your primary complaint about the resolution order is performance, and the patch removes the performance problem, this makes your suggestion of never checking internal classes unnecessary. That was the point of my previous email with the patch. Clear now? Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [PATCH] Re: [PHP-DEV] namespace examples (solving name resolutionorder issues)
Hi,
This is the 2nd of two exchanges that occurred off-list that Stas and I
would like to put back on-list.
Greg
===
Stanislav Malyshev wrote:
Hi!
Basically, the patch does 2 things
1) resolution order is changed to:
foo.php:
$a = new foo();
a) does namespace::foo class exist?
b) try to autoload namespace::foo
c) try to locate internal class ::foo
2) if internal class ::foo is found, then cache this information
so that
if another reference to the foo class occurs in foo.php, resolution
short circuits to:
a) use internal class ::foo
This second point is why the slowdown went from ridiculous to slight.
OK, now I understand better what it does, thanks. Do I understand
right that the caching is in runtime, when the class access opcode is
executed (since you couldn't autoload in compile time)? If so, how
long the cache entry is kept there? I.e. if I have execution path that
goes to file A, then B, then A again, then C, then B, etc. and then
unroll the stack back - do I store all results for second entrance
into A? Is it per-op-array (since we don't have per-file data
structure AFAIK)? What happens if include path (or autoloader)
changes, so that the class name that could not be loaded before can be
loaded now - e.g. application adds plugin definitions, etc. - will it
be ensured that autoloading is never attempted again for the same
class? What happens if somebody loads that namespace::foo class
manually (with require) - will the old resolution hold or the new one
apply?
Also I note that even if the cache stores all resolutions for all
internal classes times all namespaces, it will still require at least
one exhaustive autoload search per internal class per op-array - or
per file, if you somehow find a way to store cache per-file in
runtime, and these can not be eliminated with bytecode caching, unlike
all other filesystem accesses.
Hi,
The caching is at runtime. Basically, it's in executor_globals, and so
is not linked to any opcode array (I couldn't figure out any other
globally accessible storage location). It works like an associative
array. If file X.php loads XMLReader in namespace namespacename, it
creates this entry:
array(
'X.php\0namespacename::XMLReader' = {class entry for XMLReader}
);
So that the next time we reach X.php and the XMLReader class, it sees
this entry, and does not attempt autoload. As for your questions about
include_path/autoload, I had not added this to the patch, but the easy
solution is to clean the hash on include_path change, or autoload
change. I assume this code would break with the patch:
sneakyreader.php:
?php
namespace foo;
class XMLReader {}
?
main.php:
?php
namespace foo;
$a = new XMLReader; // loads ::XMLReader
include 'sneakyreader.php';
$a = new XMLReader; // still loads ::XMLReader
?
Whereas with the current behavior, it would instead instantiate
foo::XMLReader on the second access. However, this is definitely a wtf
situation - the exact same looking line of code would load a different
class name entirely depending on what was included.
The cache does not eliminate the first autoload per-file.
I'm sure there are other issues to work out, but this should give a
better idea of what I am trying to do.
As for your question of whether to simply error out if foo::XMLReader
doesn't exist (in the main.php example above), this also would be
acceptable for classes, but not for functions, as the ratio of
user-defined to internal functions in any source code example is 1:many,
many. However, we don't autoload functions or constants, so load ordr
is an absolute, much as it was for classes in PHP 4 before autoload was
introduced.
It is not an easy problem to solve, but we do need to solve it prior to
the next release. Should we take this back on-list?
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Scoping of use statements and a strategy for 5.3/6.0 release of namespace
Hi Stan, I realized I missed 2 of your points, response below: Stan Vassilev | FM wrote: Hi, Multiple namespaces per file were introduced to allow certain workflows in PEAR and frameworks like Symphony which can combine multiple classes and namespaces in a single package. They work like this: namespace X; ... namespace Y; ... The problem is, no one thought of scoping use statements, so now those merged files share their use imports, thus breaking all the code where collisions occur. Also we have the problems with name resolution of internal vs user classes and autoloaders, discussed here. I just posted a patch that solves this problem, and a few minutes ago forwarded two messages that further clarify how the patch works and why it is a good solution to the problem. And we also have the non-intuitive differentiation between resolving functions/classes/constant which will most likely lead people to believe functions/constants aren't supported in any way in namespaces (if they try, and it doesn't work, they won't try second time). The other 2 patches I have posted solve these issues decisively. Since these would change namespace edge cases and introduce a new way to import, I think an alpha3 is warranted. The name resolution changes should not affect any existing working code that utilizes namespaces, but should make it possible to do autoload without having to use every single namespace class. The patches do need review, and I am certain the patch introducing function::blah::here() could be better written, and needs a Sara/Dmitry type to take a quick look and say what could be done to improve it. However, I don't think these facts warrant removal of the key feature of namespaces, especially since they are relatively easy to solve - these patches did not take many hours to whip up, and do not change very much. Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: namespace RFC
Lukas Kahwe Smith wrote: Hello all, All the recent discussions about namespaces, have left many of us wondering if our implementation is rock solid or not. However the discussions were not really well organized. This is why I am thankful that Marcus and Felipe have spend the better part of this Sunday to write an RFC [1] that hopefully summarizes all the key concerns. Also they have created a patch that they feel addresses the concerns. So I ask you all to review the RFC and provide feedback. If you feel something is missing, the best approach is probably to work with Marcus and Felipe directly to get your concerns added. Only if there is a difference of opinion in this process should this be brought to internals. This way we can hopefully keep the discussion more focused on Hi, This is not a difference of opinion, but I would like to correct a misconception: phar does *not* solve the problem that prompts developers to concatenate files. The problem is that the loading/unloading of the scanner and parser can be significant overhead, and by cramming all code into a single file, can result in a 10%-30% performance improvement over code in separate files, even with an opcode cache. This has been verified independently by Zend labs (Stanislav can attest to this) on their machines. The performance difference depends on whether external code is loaded using require with absolute path, require_once with relative path, autoload, or some mix of these. The problem requires streamlining of the loading process for files, and could perhaps be addressed by attacking the bottleneck, but is not a trivial problem to solve. Just wanted to set the record straight. Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Inconsistencies in 5.3
Lukas Kahwe Smith wrote: On 13.08.2008, at 22:18, Stanislav Malyshev wrote: Simply include a script from two locations with different namespaces or one with namespace and the otherone without. I'm afraid you misunderstand how namespaces work. As I explained numerous times, namespaces are file-local, and this when including file, it does not matter a bit what was including context. I think Marcus is talking about files that are included that do not specify a namespace explicitly. In this situation the context does matter. We do not know if the developer in question is aware that the context would matter in this case. Actually like I said in a previous email it would be nice to at least not throw a warning if the file that is included specifies an explicit namespace (I assume that is possible?). Maybe adding a new include is a solution. This way developers can say explicitly what they want to do without having to suppress the warning. Then again quickly some smartass developer is going to teach people that these annoying warnings go away if you just use this new include everywhere. Then again, I am not sure if I even have my head wrapped around this entire namespace thing. Hi, Currently, if you include a file within a class method that contains function definitions, they remain functions outside the class. If you include a file that contains a class. In short, only global cpde is executed in the scope, and there is no precedent in PHP to redefine re-usable elements based on scope. Why would namespaces be any different? This whole argument makes no sense to me. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] enabling everything by default
Antony Dovgal wrote: On 01.08.2008 14:11, Antony Dovgal wrote: I can agree that disabling something that was already enabled in 5.2 might create some confusion, but why enable scarcely created extensions by default, especially if they are known to cause lost of obscure problems in the past (like Phar)? See http://bugs.php.net/bug.php?id=45613 for example. Who would have thought that there are multithreaded web-servers, eh? I'm going to disable ext/phar before alpha2. You've been warned. Hi, I've just returned home finally, and look forward to taking a look at issues. I even committed 2 new code coverage tests while on the road, in case you doubt my resolve to fix things up. Tony, please drop the posturing, it makes you look somewhat ridiculous :). I *explicitly* asked that the question of enabling phar by default be voted on prior to rc 1 when I enabled it, and again reminded about it a month later. Please see the links below to the emails in question. Let's not forget - disabling ext/phar by default involves changing a 1 to a 0 in config.m4, it's hardly difficult to do. I agree that it needs to happen prior to the final release cycle or not at all. Is alpha2 the new rc1? That makes no sense to me if so. Thanks, Greg From May 14, 2008: http://marc.info/?l=php-internalsm=121078127525090w=2 From June 23, 2008: http://marc.info/?l=php-internalsm=121419199122615w=2 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: simple solution to another namespace conundrum?
Jessie Hernandez wrote:
Hi Greg,
How is this different from my original proposal
(http://news.php.net/php.internals/34097,
http://news.php.net/php.internals/34097)?
The patch committed only affects non-namespaced code, your proposal
affected all code. In other words, this always works:
?php
namespace Foo;
include 'blah.php';
use Blah::Closure;
$a = new Closure();
?
whereas this:
?php
include 'blah.php';
use Blah::Closure;
$a = new Closure();
?
had the potential to fail with fatal error without code change on
upgrade to a newer PHP version that creates the Closure class. The
committed patch addresses only this shortcoming, and changes nothing else.
In other words, this:
?php
namespace Foo;
$a = new Exception('hi');
var_dump(get_class($a));
?
still prints Exception and not Foo::Exception.
Additionally, unlike your proposal, all existing namespace tests still
pass with the behavior change. 2 new tests were added, 1 to test the
relaxed behavior, and 1 to test that this next example is a fatal error
because definition of Foo::Closure is in the same file as the use
Blah::Closure:
?php
namespace Foo;
include 'blah.php';
class Closure{}
use Blah::Closure; // unqualified name Closure is already taken, fatal
error.
?
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] interesting update on phar performance
Hi all, I decided to run my standard phpMyAdmin test without APC enabled and got startling results from siege: Date Time, Trans, Elap Time, Data Trans, Resp Time, Trans Rate, Throughput, Concurrent,OKAY, Failed 2008-06-20 02:02:35,915, 60.01, 1, 0.98, 15.25,0.02, 14.88, 915, 0 -- phpMyAdmin on disk 2008-06-20 02:05:04,911, 60.04, 1, 0.98, 15.17,0.02, 14.86, 911, 0 -- phpMyAdmin in phar with phar.cache_list That's right - they are identical in performance. With APC, there is a performance difference (I'm not sure why, to be honest, this one is really hard to profile): Date Time, Trans, Elap Time, Data Trans, Resp Time, Trans Rate, Throughput, Concurrent,OKAY, Failed 2008-06-20 01:34:00, 2735, 59.72, 5, 0.33, 45.80,0.08, 14.95,2735, 0 -- phpMyAdmin on disk 2008-06-20 01:36:11, 2409, 60.14, 5, 0.37, 40.06,0.08, 14.95,2409, 0 -- phpMyAdmin in phar with phar.cache_list However, the difference is negligible. Thanks to Gopal for the mini-tutorial on using copy-on-write to implement phar.cache_list, and kcachegrind for finding the obvious bottlenecks. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] simple solution to another namespace conundrum?
Hi,
You probably have seen Derick's blog post
http://www.derickrethans.nl/namespaces_in_php.php
It occurred to me today that there might be a simple, elegant solution
to this problem.
First, let's imagine someone writes this code:
?php
include '/path/to/library/Closure.php';
use Fancy::Closure;
$a = new Closure('for ($a = 1;$a10;$a++) docrap();');
?
Now, in PHP 5.4, we introduce an internal class Closure and the user's
code suddenly breaks on the use statement because we check for an
internal name conflict.
Here's the kicker: why do we need to worry about a name conflict? This
user's code was designed to work exclusively with Fancy::Closure, and
doesn't care about any present or future internal classes that
conflict! Also, because this is a compile-time alias that only affects
the current script file, if we were to allow the above user's code to
essentially override the internal Closure class, there is no possible
harm because
1) the user explicitly imported Fancy::Closure
2) the user therefore never intends to use the internal ::Closure in
this script
In fact, the same thing is true for existing classnames. Why should we
care if a user does this?
?php
include '/path/to/my/date/lib.php';
use My::DateTime;
$a = new DateTime('2006/04/05');
?
The user is obviously intentionally creating a DateTime class, and
doesn't care about the internal classname in this script.
The attached patch against PHP_5_3 would fix the issue by eliminating
the check for conflict with CG(class_table) in the global namespace for
internal classes. It however preserves this check for userspace classes
so that (for instance) php-src/tests/Zend/ns_030.phpt does not fail.
The basic idea is that we do have control over the userspace classes we
include, but have no control over the internal classes.
A new test would also be needed:
066.php.inc:
?php
namespace A;
class B
{
function __construct(){echo __CLASS__;}
}
?
--TEST--
066: Name ambiguity (import name internal class name)
--FILE--
?php
include __DIR__ . '/066.php.inc';
use A::B as stdClass;
new stdClass();
--EXPECT--
A::B
Thanks,
Greg
Index: Zend/zend_compile.c
===
RCS file: /repository/ZendEngine2/zend_compile.c,v
retrieving revision 1.647.2.27.2.41.2.68
diff -u -r1.647.2.27.2.41.2.68 zend_compile.c
--- Zend/zend_compile.c 15 Jun 2008 18:27:37 - 1.647.2.27.2.41.2.68
+++ Zend/zend_compile.c 20 Jun 2008 15:47:41 -
@@ -4959,6 +4959,7 @@
char *lcname;
zval *name, *ns, tmp;
zend_bool warn = 0;
+ zend_class_entry **pce;
if (!CG(current_import)) {
CG(current_import) = emalloc(sizeof(HashTable));
@@ -5012,14 +5013,16 @@
efree(tmp);
}
efree(ns_name);
- } else if (zend_hash_exists(CG(class_table), lcname,
Z_STRLEN_P(name)+1)) {
- char *tmp = zend_str_tolower_dup(Z_STRVAL_P(ns),
Z_STRLEN_P(ns));
-
- if (Z_STRLEN_P(ns) != Z_STRLEN_P(name) ||
- memcmp(tmp, lcname, Z_STRLEN_P(ns))) {
- zend_error(E_COMPILE_ERROR, Cannot use %s as %s
because the name is already in use, Z_STRVAL_P(ns), Z_STRVAL_P(name));
+ } else if (SUCCESS == zend_hash_find(CG(class_table), lcname,
Z_STRLEN_P(name)+1, (void **)pce)) {
+ if (pce[0]-type == ZEND_USER_CLASS) {
+ char *tmp = zend_str_tolower_dup(Z_STRVAL_P(ns),
Z_STRLEN_P(ns));
+
+ if (Z_STRLEN_P(ns) != Z_STRLEN_P(name) ||
+ memcmp(tmp, lcname, Z_STRLEN_P(ns))) {
+ zend_error(E_COMPILE_ERROR, Cannot use %s as
%s because the name is already in use, Z_STRVAL_P(ns), Z_STRVAL_P(name));
+ }
+ efree(tmp);
}
- efree(tmp);
}
if (zend_hash_add(CG(current_import), lcname, Z_STRLEN_P(name)+1, ns,
sizeof(zval*), NULL) != SUCCESS) {
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: simple solution to another namespace conundrum?
Dmitry Stogov wrote:
Looks fine, but probably we should emit error only if class declared in
the same source file.
Thanks. Dmitry.
Great, didn't realize how easy this would be. Attached patch does this,
but requires another test:
067.php.inc:
?php
class B
{
function __construct(){echo __CLASS__;}
}
?
--TEST--
067: Name ambiguity (import name userspace class name in another file)
--FILE--
?php
include __DIR__ . '/066.php.inc';
include __DIR__ . '/067.php.inc';
use A::B;
new B();
--EXPECT--
A::B
I can't express how excited I am about this patch - it fixes the biggest
single problem with the use statement and negates the need for any
weird stuff like namespace __user__; as proposed. Now, once we fix
resolution rules, namespaces will be stellar.
Greg
Index: Zend/zend_compile.c
===
RCS file: /repository/ZendEngine2/zend_compile.c,v
retrieving revision 1.647.2.27.2.41.2.68
diff -u -r1.647.2.27.2.41.2.68 zend_compile.c
--- Zend/zend_compile.c 15 Jun 2008 18:27:37 - 1.647.2.27.2.41.2.68
+++ Zend/zend_compile.c 20 Jun 2008 16:37:53 -
@@ -4959,6 +4959,7 @@
char *lcname;
zval *name, *ns, tmp;
zend_bool warn = 0;
+ zend_class_entry **pce;
if (!CG(current_import)) {
CG(current_import) = emalloc(sizeof(HashTable));
@@ -5012,14 +5013,16 @@
efree(tmp);
}
efree(ns_name);
- } else if (zend_hash_exists(CG(class_table), lcname,
Z_STRLEN_P(name)+1)) {
- char *tmp = zend_str_tolower_dup(Z_STRVAL_P(ns),
Z_STRLEN_P(ns));
-
- if (Z_STRLEN_P(ns) != Z_STRLEN_P(name) ||
- memcmp(tmp, lcname, Z_STRLEN_P(ns))) {
- zend_error(E_COMPILE_ERROR, Cannot use %s as %s
because the name is already in use, Z_STRVAL_P(ns), Z_STRVAL_P(name));
+ } else if (SUCCESS == zend_hash_find(CG(class_table), lcname,
Z_STRLEN_P(name)+1, (void **)pce)) {
+ if (pce[0]-type == ZEND_USER_CLASS
!strcmp(pce[0]-filename, CG(compiled_filename))) {
+ char *tmp = zend_str_tolower_dup(Z_STRVAL_P(ns),
Z_STRLEN_P(ns));
+
+ if (Z_STRLEN_P(ns) != Z_STRLEN_P(name) ||
+ memcmp(tmp, lcname, Z_STRLEN_P(ns))) {
+ zend_error(E_COMPILE_ERROR, Cannot use %s as
%s because the name is already in use, Z_STRVAL_P(ns), Z_STRVAL_P(name));
+ }
+ efree(tmp);
}
- efree(tmp);
}
if (zend_hash_add(CG(current_import), lcname, Z_STRLEN_P(name)+1, ns,
sizeof(zval*), NULL) != SUCCESS) {
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] heads up: new people in power at PEAR
Hi, For the past year, I have been representing PEAR as its president, but decided not to seek re-election in order to pursue more heavily development on phar and pyrus. As of today, we finished our second set of elections, and I'm happy to announce that David Coallier will be taking my place as president, so if there are issues that affect PHP core that involve PEAR, you should contact David, and if decisions in the PHP core affect PEAR, expect to hear from David :). The role of president in the PEAR organization is more of an ambassadorial than dictatorial role, and the president is the primary spokesperson and spiritual leader of the organization. Also elected was a combination of new and old members for the PEAR Group, but that group is strictly empowered to decide things internal to PEAR, so you all probably don't care much :). He can be reached through the email alias [EMAIL PROTECTED] (as soon as the alias is updated) or through [EMAIL PROTECTED] (which will also go to all of the newly elected PEAR group members). For those interested, here are the official results: http://pear.php.net/election/ Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: FastArray, great addition
Pierre Joye wrote: hi all, As you noticed already PHP finally got simple C-like array, thanks to Etienne and Tony for their great work! My only wish is to actually respect the informal decision we took a while back, to do not use fast, improved, better or similar wording in function or extension names. What's about naming it CArray or something similar? To reflect what it is, it is not a fast(er) hash table like our current array, it is a C-like array. Why not call it SplVector? Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] potential shutdown order issue
Hi,
I'm getting errors of hashtable already destroyed when running
phpMyAdmin with PHP 5.3, and (of course), thinking this is a phar issue,
I've traced through and found a problem in the shutdown order.
Basically, php_request_shutdown() calls zend_deactivate() which calls
zend_destroy_rsrc_list(EG(regular_list)). On the next line,
zend_post_deactivate_modules() is called, which steps through the module
list and unloads all the dynamically loaded modules. If one of these
modules (like mysqli) declares a resource type, then in
module_destructor() zend_clean_module_rsrc_dtors() is called, which
calls zend_clean_modules_rsrc_dtors_cb() (zend_list.c:253).
This function then walks over EG(regular_list), which had been
previously destroyed.
I think this issue could be fixed by moving the
zend_destroy_rsrc_list(EG(regular_list)) call after the module
shutdown. I've attached a patch demonstrating the principle (this fixes
the hashtable destroyed message and I don't get anything bad like a
segfault).
Could someone with more brains double-check this one? I think it can be
reproduced with a debug build using mysqli loaded dynamically and any
script that uses mysqli, but I've only reproduced it with phpMyAdmin.
Thanks,
Greg
Index: Zend/zend.c
===
RCS file: /repository/ZendEngine2/zend.c,v
retrieving revision 1.308.2.12.2.35.2.18
diff -u -r1.308.2.12.2.35.2.18 zend.c
--- Zend/zend.c 29 Apr 2008 08:15:16 - 1.308.2.12.2.35.2.18
+++ Zend/zend.c 16 Jun 2008 02:53:00 -
@@ -901,7 +901,8 @@
shutdown_compiler(TSRMLS_C);
} zend_end_try();
- zend_destroy_rsrc_list(EG(regular_list) TSRMLS_CC);
+ /* shutdown order issue */
+ /* zend_destroy_rsrc_list(EG(regular_list) TSRMLS_CC); */
#ifdef ZEND_DEBUG
if (GC_G(gc_enabled) !CG(unclean_shutdown)) {
Index: main/main.c
===
RCS file: /repository/php-src/main/main.c,v
retrieving revision 1.640.2.23.2.57.2.22
diff -u -r1.640.2.23.2.57.2.22 main.c
--- main/main.c 21 May 2008 15:55:31 - 1.640.2.23.2.57.2.22
+++ main/main.c 16 Jun 2008 02:53:02 -
@@ -1527,6 +1527,8 @@
zend_post_deactivate_modules(TSRMLS_C);
} zend_end_try();
+ zend_destroy_rsrc_list(EG(regular_list) TSRMLS_CC);
+
/* 9. SAPI related shutdown (free stuff) */
zend_try {
sapi_deactivate(TSRMLS_C);
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] phar development update
Hi all, A quick note on significant improvements to phar committed recently: * tar-based archives now fully support metadata and signatures, bringing them in line with more of the features of phar-based phar archives. * true asymmetric signing support using OpenSSL private/public key pairs * phar.cache_list provides pre-parsing of phar manifests at MINIT, with performance enhancement that matches on-disk for non-APC and improves perf to 19 req/sec (in-phar) vs. 28 req/sec (on-disk) in my benching of phpMyAdmin with APC. This performance benefit only accrues with webservers that run MINIT once and then spawn other processes/threads with RINIT. Also, Scott is working on a patch to sqlite that will allow using databases mounted inside a phar archive with a phar url, which will greatly simplify pharring apps that use sqlite. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] missing pestrndup
Hi, Is there any reason pestrndup is missing from PHP 5.3's zend_alloc.h but is present in HEAD? Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] add pestrndup
Hi, I need this patch committed ASAP to PHP_5_3 zend_alloc.h, as the commit I just made for ext/phar depends on it (oops). Could someone help me out? Thanks :), Greg Index: zend_alloc.h === RCS file: /repository/ZendEngine2/zend_alloc.h,v retrieving revision 1.63.2.2.2.12.2.4 diff -u -r1.63.2.2.2.12.2.4 zend_alloc.h --- zend_alloc.h31 Dec 2007 07:17:04 - 1.63.2.2.2.12.2.4 +++ zend_alloc.h13 Jun 2008 03:59:30 - @@ -113,6 +113,7 @@ #define safe_perealloc(ptr, nmemb, size, offset, persistent) ((persistent)?_safe_realloc((ptr), (nmemb), (size), (offset)):safe_erealloc((ptr), (nmemb), (size), (offset))) #define perealloc_recoverable(ptr, size, persistent) ((persistent)?__zend_realloc((ptr), (size)):erealloc_recoverable((ptr), (size))) #define pestrdup(s, persistent) ((persistent)?strdup(s):estrdup(s)) +#define pestrndup(s, length, persistent) ((persistent)?zend_strndup((s),(length)):estrndup((s),(length))) #define pemalloc_rel(size, persistent) ((persistent)?__zend_malloc(size):emalloc_rel(size)) #define pefree_rel(ptr, persistent)((persistent)?free(ptr):efree_rel(ptr)) -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: cvs: ZendEngine2(PHP_5_3) / zend_alloc.h
Matt Wilmas wrote: mattwil Fri Jun 13 04:16:59 2008 UTC Modified files: (Branch: PHP_5_3) /ZendEngine2 zend_alloc.h Log: MFH: Add pestrndup() Thanks Matt Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] CVS Account Request: baptiste750
Hannes Magnusson wrote: On Wed, Jun 11, 2008 at 22:15, Rasmus Lerdorf [EMAIL PROTECTED] wrote: Actually, checking through my group@ archive, I don't see 2 messages from you. The only one I see from you is one from June 9 complaining that your cvs account wasn't granted. I see that someone has approved your account now. Sorry, my fault. I silently forwarded the mail to pear-group@ and his account was approved few minutes later. I have a patch adding a combobox to pick which group the request should be sent to (pear/php, and maybe add gtk/pecl/doc/... in the future?), so the pear guys don't have to subscribe to this list or manually search for outstanding requests, which I'll commit in few minutes... Hi, Sorry about this, I dropped the ball here. As a note, I have been approving PEAR account requests, and simply monitored internals@ for emails. This worked great until the past couple of weeks when I had crappy internet while on the road, so I missed a few (3 to be exact). This is not the responsibility of people outside PEAR like Rasmus, and the solution is actually a technical one (being able to filter unapproved accounts by project at master.php.net, i.e. PEAR requests only are shown), along with the patch Hannes has already committed. This way, when I am away, it is also possible for people outside PEAR to easily filter the requests, and I can also do that when I come back quite easily. No worries folks - the average turnaround for account approval has been under 24 hours for over a year now, down from an average of 1 month, so this 1 week aberration is a 1-time occurrence. In addition, when PEAR2 gets rolling (and it already is starting) we use a separate subversion repo with its own auth system, which will help as the volume of cvs account requests should decline. Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Calling original function from an overloaded function
Christoph Dorn wrote: I have been looking at xdebug and have figured out how to overload a function. As a test I have overloaded the var_dump function. Now how do I call the original var_dump function from my implementation? Borrowing from OO terminology I have subclassed the var_dump method, now I want to call parent::var_dump() from my subclass. Also how does overloading work when two extensions overload the same function? Can this be done? Is there some sort of order? Can I let the other extension overload the function and then overload the function from the other extension? Any pointers would be greatly appreciated. Hi, phar does exactly what you're describing, this is all encapsulated in func_interceptors.c: http://cvs.php.net/viewvc.cgi/php-src/ext/phar/func_interceptors.c?revision=1.20.2.1content-type=text%2Fplainpathrev=1.20.2.1 Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH] fix building openssl shared on unix in 5.3
Pierre Joye wrote: hi Greg, Please commit it, thanks for the patch! Someone with ZendEngine2 karma will need to commit. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] fix building openssl shared on unix in 5.3
Hi, I was trying to test building openssl shared, and ran into an unexported symbol issue because the arginfo constant arrays are never exported for dll/so use. Could someone check out this patch and make sure it doesn't fubar windows in particular? The same patch will apply in all branches just fine. Thanks, Greg Index: Zend/zend_arg_defs.c === RCS file: /repository/ZendEngine2/zend_arg_defs.c,v retrieving revision 1.2.2.2.2.2.2.1 diff -u -r1.2.2.2.2.2.2.1 zend_arg_defs.c --- Zend/zend_arg_defs.c31 Dec 2007 07:17:04 - 1.2.2.2.2.2.2.1 +++ Zend/zend_arg_defs.c1 Jun 2008 03:26:00 - @@ -18,29 +18,29 @@ /* $Id: zend_arg_defs.c,v 1.2.2.2.2.2.2.1 2007/12/31 07:17:04 sebastian Exp $ */ -ZEND_BEGIN_ARG_INFO(first_arg_force_ref, 0) +ZEND_API ZEND_BEGIN_ARG_INFO(first_arg_force_ref, 0) ZEND_ARG_PASS_INFO(1) ZEND_END_ARG_INFO(); -ZEND_BEGIN_ARG_INFO(second_arg_force_ref, 0) +ZEND_API ZEND_BEGIN_ARG_INFO(second_arg_force_ref, 0) ZEND_ARG_PASS_INFO(0) ZEND_ARG_PASS_INFO(1) ZEND_END_ARG_INFO(); -ZEND_BEGIN_ARG_INFO(third_arg_force_ref, 0) +ZEND_API ZEND_BEGIN_ARG_INFO(third_arg_force_ref, 0) ZEND_ARG_PASS_INFO(0) ZEND_ARG_PASS_INFO(0) ZEND_ARG_PASS_INFO(1) ZEND_END_ARG_INFO(); -ZEND_BEGIN_ARG_INFO(fourth_arg_force_ref, 0) +ZEND_API ZEND_BEGIN_ARG_INFO(fourth_arg_force_ref, 0) ZEND_ARG_PASS_INFO(0) ZEND_ARG_PASS_INFO(0) ZEND_ARG_PASS_INFO(0) ZEND_ARG_PASS_INFO(1) ZEND_END_ARG_INFO(); -ZEND_BEGIN_ARG_INFO(fifth_arg_force_ref, 0) +ZEND_API ZEND_BEGIN_ARG_INFO(fifth_arg_force_ref, 0) ZEND_ARG_PASS_INFO(0) ZEND_ARG_PASS_INFO(0) ZEND_ARG_PASS_INFO(0) @@ -48,5 +48,5 @@ ZEND_ARG_PASS_INFO(1) ZEND_END_ARG_INFO(); -ZEND_BEGIN_ARG_INFO(all_args_by_ref, 1) +ZEND_API ZEND_BEGIN_ARG_INFO(all_args_by_ref, 1) ZEND_END_ARG_INFO(); -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Short syntax for array literals [...]
I've thought about allowing [] for a while and personally have come up with my own litmus test for new features. 1) is the syntax missing from the language? 2) if so, does the syntax add missing functionality or significant maintenance benefit? 2) if not, does the new syntax add significant value? #1 no, array() is the same [-1] #2 not applicable [0] #3 [-.5] * can't google [] * makes arrays simpler to type and take up less space * adds potential for confusion between array access and creation: $a['hi']; $a;['hi']; both are now suddenly valid PHP * syncs with javascript and other languages * opens pandoras box - PHP is simpler than Perl because there are not 20 ways of doing the same thing with different punctuation shorthands So I find #1 is -1, #2 is 0, #3 is about -.5 Although the idea is somewhat attractive, I've found no drawbacks to array() syntax, and plenty of dangers with adding any new alternate syntax, and this ultimately makes my vote -1 Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] magic quotes finale
Rasmus Lerdorf wrote:
I see absolutely no reason to force people to go through and change:
if(!get_magic_quotes_gpc())
to:
if (!function_exists('get_magic_quotes_gpc') || !get_magic_quotes_gpc())
when there is no technical reason to force them to do so. It is slower,
more verbose and completely useless.
I whole-heartedly agree.
To the others: please examine this from a practical instead of a
philosophical position.
What is the problem that needs solving?
* magic_quotes_gpc escapes input, which is bad.
How to fix it?
* disable magic_quotes_gpc = on, disable set_magic_quotes_gpc(1)
Implicit in this statement is that the problem is *not*:
* Users use get_magic_quotes_gpc() check whether this faulty ini is
enabled, and set_magic_quotes_gpc(off) only if it is enabled.
If we take the step of removing the get_magic_quotes_gpc() function, or
of adding an E_DEPRECATED, we make upgrading to PHP 5.3 harder, for no
benefit.
As a side note, the silent majority (developers who do not post to this
list) were represented at php|tek, and the few I spoke to about the way
magic_quotes is being handled unequivocally agreed with my assessment
for the exact same reasons.
I strongly encourage everyone to do a realistic tradeoff analysis and
come to understand why Rasmus's solution is the only possible solution
to this problem that both solves the *actual* problem and has real
benefit to existing well-written applications.
Thanks,
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] HEADS UP: phar now enabled statically by default
Hi, I've reversed my initial patch enabling phar as shared by default, and instead now enable it statically by default. I was able to solve the dependency on zlib/bz2 I noted in my last message through some very simple logic, and testing confirms that phar can be built statically with zlib/bz2 shared with absolutely no problems whatsoever because phar never uses these extensions directly, only through the stream filter API. So, in short, problem solved, and phar can be built statically and works just great with static zlib/bz2 or shared zlib/bz2. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Please help about zend engine
Andreas K Santoso wrote: Hello sir/ma'am My name is andre from Indonesia. I am a student of a university, and i'm doing my thesis. I subscribe to this list to look for some help. Oh by the way, my thesis is about PHP file encryption. So i need to decrypt the file when it's accessed and before any further processing. I'm surely not as smart and experienced as all of you, so please be patient Mr. Malyshev told me that i have 2 options 1. Override the zend_compile function (he said that this one seems to be the easiest way) 2. Use the PHP stream system (http://php.net/streams) to create filters that decrypt data on-the-fly. So i decided to try the second option. But still i can't understand it well. The example i've read (http://php.net/streams) is not clear enough for me. I mean, how to make the filter itself, and how to use it? and can the filter automaticaly applied to every .php files? And can anyone tell me where i must insert my codes if i want to override the zend_compile? I'm afraid that i don't have much time left, so if i can't use option 2, i will use option 1 instead. Sorry if my english impolite or confusing. Thank you very much for your patience and help. Hi Andreas, For stream filters, check out the zlib and bz2 stream filters in ext/zlib/zlib_filter.c and ext/bz2/bz2_filter.c The filter can't be automatically applied without overriding zend_compile, but I still highly recommend you implement it as a stream filter. Why? You can check for encryption on file inclusion and then append the stream filter to the returned stream inside the zend_file_handle, or disable this in php.ini and decrypt on a per-file basis manually. For an example of zend_compile() interception that does something similar, look at the end of ext/phar/phar.c. In this case, phar's zend_compile override checks for filenames containing '.phar' and attempts to process them as a phar archive, creates a phar stream URL and passes that to zend_stream_open_function to return a file_handle. You could simply call zend_stream_open_function, and then read in the first few bytes of the file handle in the modified zend_file_handle * to determine if decryption is necessary, and then use code something like this to append the filter and compile the file: filter = php_stream_filter_create(my.encrypt, NULL, php_stream_is_persistent(file_handle-handle.stream.handle) TSRMLS_CC); php_stream_filter_append(file_handle-handle.stream.handle-readfilters, encrypt_filter); return encrypt_orig_compile_file(file_handle, type TSRMLS_CC); where encrypt_orig_compile_file is the saved value of zend_compile_file. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: 5.3 Namespace resolution rules suggestions
Nathan Rixham wrote:
Ryan Panning wrote:
Jessie Hernandez wrote:
Hi Stan,
I made a proposal and patch a few months ago...
The developers should really take a serious look at this issue or it
will come back to haunt them later. I'm not sure why no one seems
comment on your proposal and patch. It seemed like a well thought-out
and good proposal to me.
I'm also wondering what happened to the namespace declaration change
to brackets {}. I've downloaded the latest snap of 5.3 and that has
not changed as of yet.
is it too late to scrap all this and go with Java/AS3 style
base.package.class please?
yes
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: 5.3 Namespace resolution rules suggestions
Stan Vassilev | FM wrote:
Hi,
I'd like to nudge the discussion back to issues with the resolution
rules that we're discovering :)
The actual char(s) used can only be mildly annoying to some (at worst),
compared.
Can we please agree on those (or explain why you'd not):
1) functions, constants and classes should use the same resolution
rules. Making special cases just for some of them, or just for user or
just internal ones will lead to confusion.
I have to agree. In fact, there is a simple and elegant solution to the
problem that involves augmenting Jessie's patch to include a more
helpful error message on class/function not found, and a simple 51-line
PHP script for converting non-namespaced to namespaced code. With these
two things, I think we could safely change the name resolution in PHP.
2) can someone please explain why is it useful to override internal
function inside a namespace. Isn't it better to explicitly refer to
global functions as global, which would allow compile-time resolution in
a lot more cases than now.
The reasoning behind this decision that I was given by Dmitry and
Stanislav was 2-fold.
1) Currently, the lookup order is optimized for internal
functions/classes [more detail below]
2) migrating existing code to namespaces will require much more work if
all internal functions/classes must be prefixed with :: or used to be
avialable
Detail:
1) Currently the lookup order is optimized for internal functions/classes.
At compile-time, all internal classes/functions exist. However, it is
possible that not all userspace functions/classes exist yet. As such,
if a T_STRING is encountered, here is how the resolution works currently
in the worst case (roughly, in pseudo-code):
= is it an internal function/class? if so, use it
= does it exist in the current namespace? if so use it
= otherwise, mark it for runtime resolution
= run code
= when we reach this opcode, resolve the function
In the best case:
= is it an internal function/class? use it
However, changing this to allow resolving always to the current
namespace would change this to:
worst case:
= does it exist in the current namespace? if so use it
= otherwise, mark it for runtime resolution
= run code
= when we reach this opcode, resolve the function
best case:
= does it exist in the current namespace? if so use it
This results in a fatal error for all internal functions/classes. The
error message for a non-existing class/function should at least hint
that a use statement is needed, an element missing from Jessie's patch.
In any case, #1 proves to be false, as adding a use clause for each
internal function/class is no performance hit at all.
2) migrating existing code to namespaces will require more work if all
internal functions/classes must be prefixed with :: or used
As a test, I tried adding a namespace declaration to the top of one of
PEAR's larger file, and then ran it with PHP 5.3 and the patch. To my
(initial) surprise, it ran without error. On further investigation, I
realized why this is so - none of the internal functions/classes were
actually resolved, all of them were delayed until runtime, and none were
actually executed.
What this means is that by adding a namespace declaration, it is quite
possible that your code will execute happily until it suddenly
encounters a function/class you forgot to use or prefix with ::.
Fortunately, I just wrote a simple namespace conversion script that
solves this problem decisively. The eventual script should be checked
over for missing userspace class/functions, as only internal
class/functions will be used. Here it is:
?php
namespace NSParser;
class Parser
{
protected $tokens;
protected $i = -1;
protected $classes = array();
protected $functions = array();
protected $use = array();
protected $ret;
function __construct($path, $namespace)
{
$classes = ::get_declared_classes();
$classes = ::array_merge($classes, ::get_declared_interfaces());
$this-classes = ::array_flip($classes);
unset($this-classes['NSParser::Parser']);
$functions = ::get_defined_functions();
$this-functions = ::array_flip($functions['internal']);
if (@::is_file($path)) {
$path = ::file_get_contents($path);
}
$this-tokens = ::token_get_all($path);
foreach ($this-tokens as $token) {
if (!::is_array($token)) {
$token = array(::ord($token), $token);
}
}
$ret = ?php\nnamespace . $namespace . ;\n;
do {
if ($this-tokens[$this-i][0] == T_STRING) {
if (isset($this-classes[$this-tokens[$this-i][1]])) {
$this-use[$this-tokens[$this-i][1]] = 1;
}
if (isset($this-functions[$this-tokens[$this-i][1]])) {
$this-use[$this-tokens[$this-i][1]] = 1;
}
}
} while (++$this-i
Re: [PHP-DEV] Re: 5.3 Namespace resolution rules suggestions
Stanislav Malyshev wrote:
Hi!
$classes = ::get_declared_classes();
$classes = ::array_merge($classes, ::get_declared_interfaces());
$this-classes = ::array_flip($classes);
unset($this-classes['NSParser::Parser']);
$functions = ::get_defined_functions();
$this-functions = ::array_flip($functions['internal']);
if (@::is_file($path)) {
$path = ::file_get_contents($path);
Do you really think that's how PHP code should look like - constant
obsessive ::-ing? For my taste, it looks very bad.
Actually, no. The irony of your message is that the code you quote is
used to generate use statements so ::this ::is ::unnecessary.
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] phar enabled by default as of now
Hi, I just committed to ext/phar's config.m4/w32 to make phar enabled by default as a shared extension. This is a better default setting than building phar in statically, as phar has optional dependencies on the zlib and bz2 extensions to enable compressed phar archives. If phar is built statically, then it would require both zlib and bz2 to be built statically in order to use them, something that is obviously not going to happen, as they have been around a while and nobody has even suggested it (to my knowledge). Having phar shared means one can easily plug in zlib/bz2 support without recompiling PHP. I'm open to further input/tweaks on this decision. If you have a better idea, go ahead and commit to phar's config.m4/w32, these files are very easy to change if needed. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [HEADS UP] pecl/phar is now ext/phar
Richard Quadling wrote: 2008/5/13 Antony Dovgal [EMAIL PROTECTED]: On 13.05.2008 01:45, Gregory Beaver wrote: Thanks to all who have contributed, particularly Marcus, Steph, Lars, and the others who chimed in with ideas on the list. phar_detect_phar_fname_ext() fails if is_complete = 1 and filename contains .. For example: Breakpoint 1, phar_detect_phar_fname_ext (filename=0x124db80 /local/qa/5_3.zts/ext/phar/tests/DataArchive.phar, check_length=1, Am I being picky in saying that having a . in a folder containing a file should be allowed? And if there is an extension on the filename, then it will also have a . This was always the intention, and is already fixed. In fact, opening URLs like phar://whatever/has.dot/my.phar/internal/file.php already worked, the only thing broken was instantiating a Phar object with a path containing . What happens for relative filenames? They have worked since phar 2.0.0a1. The only requirement for phar to detect its file is that you either use a registered alias or that the filename contain a .. Executable phar archives must also contain .phar as part of the filename extension. In short, the problem with Phar objects crept in only because none of us had a path in our dev environment containing a . and didn't think to add a test case for it until the problem appeared. It's now fixed, and all tests pass in CVS. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] objection to enabling phar for testing in 5.3?
Hi, I wonder if there is any objection to this plan: 1) enable phar by default for the PHP 5.3 betas, so that it can receive full testing 2) before RC1, do the formal vote on whether it should be enabled by default in the release This way, phar can be tested for the possibility of enabling, but there is an explicit safety net in place should it need reconsidering. My assumption here is that there will be a few beta releases and possibly an alpha release or two prior to the first RC. Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [HEADS UP] pecl/phar is now ext/phar
Hi, It's time for helly's birthday present from me (and indirectly, Derick, who did the cp -r) :). As Johannes requested, pecl/phar has been copied to php-src/ext/phar, development will continue there. I suspect there may need to be some changes at gcov.php.net to keep things running smoothly, so this is a heads up message. At this point, phar is ready for the big question: enable by default or not. My benchmarking shows no detectable difference in speed between ext/phar enabled and ext/phar disabled for regular applications, but I would love some confirmation on this. In addition, I just released phar 2.0.0b1 through pecl. Full documentation is available through http://php.net/phar of the latest API. Changes from the last update I sent to internals: * code coverage is pushing 80%, up from about 63% * small performance increases * many, many small and some large bugs fixed * compression/conversion API completely re-worked * lots of other improvements detailed in the manual * 373 regression/unit tests * filename requirements completely rewritten to allow processing things like openoffice odt zip files * full API mirroring of ext/zip, so phar can be easily used by developers used to this API * phar.extract_list removed as obsolete I benchmarked phpMyAdmin in/out of phar, and got 17 req/sec with phar, and 28 req/sec without phar using APC. I also ran a compressed phar, and got 6.62 req/sec, so compression is a significant performance hit. There is obvious room for improvement of performance, and this is going to be a major focus moving forward, as correctness is nearly complete (there are still a few lines of code that are testable, mostly extreme edge cases), and documentation is nearly complete. Another area of focus will be tutorials in the manual on how to create phars, usability tips, and performance information for comparing the formats. At this point, phar's API is slushy in the sense that I would like to freeze it where it is unless we've completely missed the boat on something. I would like a full review from interested parties, and I'd also like to openly call for security review of the code. I have spent a lot of time trying to think of bad guy stuff that could be done with phar, and thus far have covered things like: * phar.readonly=1 (default) cannot be disabled except in system php.ini (ini_set fails), and means that executable phar archives cannot be modified by any PHP process. * aliases cannot contain any path separators or : or ; * tar/zip-based archives can't be magically made into phar archives simply by renaming them, as they won't contain the executable stub necessary to run them * open_basedir/safemode checks are in place every time an external path is accessed - I particularly need checking of this constraint, as I am relatively unfamiliar with either system. I've also checked for every buffer overflow/invalid read that I could find, using safe functions wherever possible, and carefully checking the few uses of strcat and company. Valgrind reports no leaks/problems (except in libz, which is an external library used by zlib compression) in 2.0.0b1. In spite of my paranoia and exhaustive attempts to kill any potential security issues, I don't claim to be any kind of security expert, so any and all scrutiny of phar on this front is greatly appreciated. Thanks to all who have contributed, particularly Marcus, Steph, Lars, and the others who chimed in with ideas on the list. Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] php_stream_display_wrapper_errors issue
Pierre Joye wrote: Hi Greg, On Mon, Apr 14, 2008 at 12:17 AM, Gregory Beaver [EMAIL PROTECTED] wrote: Hi, If a stream wrapper does not log errors, by default, we grab strerror(errno) to figure out the error message, but this is not a good idea for any wrapper but plain_wrapper for the obvious reason that errno is not used by wrappers that don't use sys calls. Is this patch against 5.3 acceptable (I'll merge to HEAD on commit if so)? I think it fixes the possible misguided error messages. To go one step further, I wonder if it would make sense to add a stream_strno and stream_strerror to _php_stream_wrapper_ops. It may be helpful to display the actual error or for debugging purposes (user land or internally). Comments? Hi, In theory, a stream wrapper should be using php_stream_wrapper_log_error() for all errors, so I think stream_strerror should be unnecessary. Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] php_stream_display_wrapper_errors issue
Hi,
If a stream wrapper does not log errors, by default, we grab
strerror(errno) to figure out the error message, but this is not a good
idea for any wrapper but plain_wrapper for the obvious reason that errno
is not used by wrappers that don't use sys calls.
Is this patch against 5.3 acceptable (I'll merge to HEAD on commit if so)?
Greg
Index: main/streams/streams.c
===
RCS file: /repository/php-src/main/streams/streams.c,v
retrieving revision 1.82.2.6.2.18.2.7
diff -u -r1.82.2.6.2.18.2.7 streams.c
--- main/streams/streams.c 27 Mar 2008 10:33:40 -
1.82.2.6.2.18.2.7
+++ main/streams/streams.c 13 Apr 2008 20:41:23 -
@@ -164,7 +164,11 @@
free_msg = 1;
} else {
- msg = strerror(errno);
+ if (wrapper == php_plain_files_wrapper) {
+ msg = strerror(errno);
+ } else {
+ msg = operation failed;
+ }
}
} else {
msg = no suitable wrapper could be found;
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] phar API update
Hi,
A quick update on Phar's API for those who are keeping score:
1) Phar-isWritable() now works properly and does what Liz was hoping:
tells you whether you can actually modify the phar archive by looking at
the archive's file perms as well as phar.readonly
2) PharFileInfo-getContent() added to retrieve file contents like
$phar['a.txt']-getContent()
3) addFile/addFromString/addEmptyDir added to match ext/zip API
4) fixed compression API. Here are the new methods:
bool Phar-isCompressed([int compression]) // takes Phar::GZ, Phar::BZ2
or no parameter to check for any whole-archive compression
Phar|PharData Phar-compress(int compression[, string ext]) // takes
Phar::GZ/Phar::BZ2 and optional file extension to override default renaming
Phar|PharData Phar-decompress([, string ext]) // takes optional file
extension to override default renaming
bool Phar-compressFiles(int compression) // takes Phar::GZ/Phar::BZ2
and compressed all files within the archive
bool Phar-decompressFiles() // decompress all files within the archive
Phar Phar-convertToExecutable([int format[, int compression[, string
ext]]])
PharData Phar-convertToData([int format[, int compression[, string ext]]])
// takes Phar::TAR, Phar::ZIP, or Phar::PHAR as format
// takes Phar::GZ, Phar::BZ2 as compression
// optional file extension to override default renaming
PharFileInfo-isCompressed([int compression]) // takes Phar::GZ,
Phar::BZ2 or no parameter to check for any compression of file within
the archive
PharFileInfo-compress(int compression) // Phar::GZ/Phar::BZ2
PharFileInfo-decompress()
Here's code example of the new API:
?php
$phar = new Phar('blah.phar');
$tar = $phar-convertToData(Phar::TAR); // creates blah.tar
$tgz = $tar-compress(Phar::GZ); // creates blah.tar.gz
$phargz = $tar-convertToExecutable(Phar::PHAR, Phar::GZ, '.gz.phar');
// creates blah.gz.phar (default would be blah.phar.gz)
var_dump($tgz-isCompressed(), $tgz-isCompressed(Phar::GZ),
$tgz-isCompressed(Phar::BZ2)); // true, true, false
$phar['a.txt'] = 'hi';
echo $phar['a.txt']-getContent(); // hi
$phar-compressFiles(Phar::GZ); // internal files compressed with zlib
compression
var_dump($phar['a.txt']-isCompressed(),
$phar['a.txt']-isCompressed(Phar::GZ),
$phar['a.txt']-isCompressed(Phar::BZ2)); // true, true, false
$phar['a.txt']-decompress(); // now this specific file is not
compressed within the archive
$phar-decompressFiles(); // internal files decompressed
$phar['a.txt']-compress(Phar::BZ2); // now this specific file is
compressed with bzip2 compression
$tar2 = $tgz-decompress('.2.tar'); // creates blah.2.tar
$phartar2 = $tar2-convertToExecutable(); // create blah.2.phar.tar
?
Thanks,
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] spl documentation
Philip Olson wrote: On 11/04/2008, Alexey Zakhlestin [EMAIL PROTECTED] wrote: I noticed, that http://www.php.net/~helly/php/ext/spl/http://www.php.net/%7Ehelly/php/ext/spl/was updated almost a year ago. Is the newer version available anywhere? Good point, there're so many new things in there. Marcus? Etienne? Anyone up to do regenerate some docs ? I've a more recent build that covers DLLists, but only Marcus is able to upload it to ~helly. I guess a new version will be online as soon as I finish documenting Heaps and priority queues. I think all effort should go into writing real documentation within the phpdoc cvs module instead of unreadable and unofficial doxygen output. Personally I feel the link to the doxygen output should be removed from php.net/spl (but won't) so anyway those are my feelings. Etienne is planning to work on both, which is great, but I encourage everyone to update the official spl documentation and not worry much about doxygen. +1 This finally makes sense with the new doc style, which is OO-friendly. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] practical phar considerations
Pierre Joye wrote: Hi, On Wed, Apr 9, 2008 at 6:48 AM, Gregory Beaver [EMAIL PROTECTED] wrote: For other examples, take a look at at ext/zip. There is no support for opendir() in the stream wrapper of ext/zip because it requires the kind of path grepping that pecl/phar does. That's easy to add and will be available at some point, that was simply not needed and nobody ever requested this feature. Extract or add using a pattern is however much more useful. It is already partially supported. Yes, I agree. Yes, this shouldn't be allowed while $phar['foo/bar/baz'] = new DirectoryIterator(); could be allowed. I will simply add makeEmptyDir() a la zip, although it is too bad ext/zip never thought to go with mkdir(). I thought about mkdir but it is a bad name as it implies a shell command. makeEmptyDir while being longer is self explaining. I also asked before choosing this name, as far as I remember nobody proposed mkdir either. I'm fine with makeEmptyDir(), I was just thinking out loud :) This wasn't intended to be a referendum on ext/zip, just to be clear. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] practical phar considerations
Steph Fox wrote:
Hi Greg,
What I would like to do, however, is to rethink offsetSet(). I think
that we should introduce a property of PharFileInfo called content
that is read/write, and can be used to perform the equivalent of
file_get_contents()/file_put_contents(). This will allow a consistency
that is much more intuitive. offsetSet() could then simply be removed,
or perhaps be refactored as a way to pass in a PharFileInfo and clone
the information.
Could you offer up some pseudo-code showing how this will be used? I
can't visualize it at all from that description.
I should have done this in the original post, because it turns out that
it would require offsetGet() to implicitly create the file, which brings
me back to my original objection to these changes. I'm adding in
getContents() right now to retrieve the file contents into a string, so
one can do:
?php
$phar = new Phar('blah.phar');
echo $phar['path/to/file.txt']-getContents();
?
The truth is, I think offsetSet() could accept either a string/fp (file
contents) or a PharFileInfo() that can be used to set a slew of things:
- metadata
- per-file compression
- file contents
This would also be relatively trivial to implement.
One problem with this suggestion is that phar renames the archive by
default when compressing. I would rather we combine compression and
format conversion into a single convert() method that accepts constants.
$phar = new Phar('blah.phar');
$phar = $phar-convert(Phar::GZ); // compress with gzip
$phar = $phar-convert(Phar::TAR | PHAR::GZ); // convert to .phar.tar.gz
$phar = $phar-convert(Phar::NONE | PHAR::DATA); // convert to
non-executable .tar, returns PharData object
$phar = $phar-convert(Phar::EXE); // convert back to .phar.tar
Didn't we just go full circle here? There was a proposed convert()
method that did absolutely everything (on e-paper) and it was far less
intuitive than when conversion was separated from compression. That's
why you ended up implementing them separately, as I recall.
It does look similar, but the key difference here is that convert() was
a method that changed the underlying object. If we return a new object,
this is a completely different ballpark, the current implementation
actually can remain the same and remove the intermediary files.
This would also solve a subtle but important problem with the current
conversion API where intermediary files are lying around.
Can't we simply delete those intermediary files on success? That would
be the normal procedure when renaming, no?
Actually, no. The current implementation has no modification to the
original file on disk, as there is no way to know in advance if the user
actually intended to erase it, and it is actually impossible to do so if
there are any open references to the archive (i.e. Phar objects or
stream fp's lying around).
Do you want to take a crack at a patch for any of these API features
once they are worked out (buildFromDirectory should be an easy one, if
you would like to start there)?
Yes, I'm willing to help with it to a degree, as I said in a previous
mail.
How soon can we expect a patch for this? Steph also expressed interest,
and we are on a rather tight time schedule to get this to beta, which
will mean a feature freeze (no new crap, just bug fixes).
I'll implement buildFromDirectory() this weekend. It would've been last
weekend, but I got caught up in trying to bring the last few non-core
PECL modules into line over versioning structure, which hasn't been
entirely plain sailing. (Still 5 + IBM to go.)
OK.
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] phar API update
Hi,
I've just implemented these features:
1) new getContents() method allows directly retrieving file contents.
Here is a full-circle example:
?php
$phar = new Phar('blah.phar');
$phar['a.txt'] = 'hi';
echo $phar['a.txt']-getContents();
?
2) addFile/addEmptyDir/addFromString. API is identical to ext/zip
?php
$phar = new Phar('blah.phar');
$phar-addEmptyDir('hello');
var_dump($phar['hello']-isDir());
$phar-addFile('/path/to/file.txt');
var_dump($phar['/path/to/file.txt']-getContents());
$phar-addFile('/path/to/file.txt', 'file.txt');
var_dump($phar['file.txt']-getContents());
$phar-addFromString('localname.txt', 'hi');
var_dump($phar['localname.txt']);
?
Still left to do:
1) fix up conversion/compression and decide how best to do this
2) buildFromDirectory (Steph)
Not going to do:
1) $phar['access']['directory']['this']['way']
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] phar['blah.php']-isWritable() works now
Hi, There were some rumblings about wanting a way to detect whether a specific file/archive was writeable. I made some small changes to the way we detect this when the ini setting is changed, and added a test. Now the existing isWritable() from SplFileInfo always works properly for Phar. In addition, is_writeable/is_writable work, as does stat and related functions, all of which were actually broken should one ini_set() phar.readonly to another value. http://news.php.net/php.pecl.cvs/10330 One step closer to satisfying the API grumblings :). We need some assistance from Mac OS X experts, changes are needed to make it compile on Tiger (and, apparently, on some Leopard systems that upgraded from Tiger). Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: CVS Account Request: mimmi
Thomas Mueller wrote: distributing a new package Which package? For what? Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] practical phar considerations
Lars Strojny wrote:
Hi Greg,
Am Samstag, den 29.03.2008, 17:58 -0500 schrieb Greg Beaver:
[...]
If one uses file_put_contents('/path/to/this/file', 'hi') and
'/path/to/this' does not exist, there is an error. The same is true of
fopen, regardless of mode. mkdir() even fails unless the recursive
parameter is explicitly specified. I do not think that this behavior
should be different in ext/phar, especially because all that is needed
is to create 1 directory (full/path/to/) and add the file.
I mostly agree. Except that the whole argument chain applies to the
current API. You mentioned mkdir(foo/bar/baz) triggering an error
without the recursive argument, but Phar['foo/bar/baz'] does not.
[... Performance concerns ...]
$phar['long/path/to/subdir/file'] = 'hi';
compared to:
$phar['long']['path']['to']['subdir']['file'] = 'hi';
is significantly slower. Each [] results in a call to offsetGet and
instantiation of a new PharFileInfo object plus the final offsetSet call.
Could you estimate how hard that performance hit would be?
We're talking about a similar difference between iterating over an array
and using an ArrayIterator, which is to say about 20 times slower. This
is due to the many extra opcodes for each offset retrieval, which then
calls an expensive method call to offsetGet(), creates the directory
inside the phar archive, writes to disk, and continues. Because we are
writing to disk, without a not-at-all-obvious startBuffering() call,
with large archives, the performance difference could be 100 times
slower or more.
In addition, although Phar does support the idea of thinking of archives
as multi-dimensional arrays, this is an abstraction not supported by the
actual archive file formats. For other examples, take a look at at
ext/zip. There is no support for opendir() in the stream wrapper of
ext/zip because it requires the kind of path grepping that pecl/phar
does. Internally, no archive format stores paths the way directories
are stored in a typical file system. In other words, typically a
directory is a file that contains a list of other files or directories.
This allows very quick navigation on a random access disk, but causes
extreme bloat inside an archive. Files are stored as full paths in what
is the equivalent of a single-dimensional array. In other words, the
truth of the matter is that phar's ArrayAccess API is a 100% accurate
reflection of the true structure of the actual archives we are
accessing, and the iteration supported is a convenience for simulating
regular file systems. The API you describe actually layers an
artificial complexity over this design, and as a result requires much
more code complexity.
Let's also remember we're talking about phar creation, which is going to
be far less common than phar usage. To use a phar, you don't even need
to know what phar *is* let alone what the API is. One just includes the
phar archive, or adds it to include_path with a phar:// stream wrapper path.
I've spent a significant amount of time actually implementing the new
read-only implementation of offsetGet() in the past day, and although I
think this might be possible I am running into a weird segfault in SPL
on shutdown caused by a double destruction of the object. In addition,
the patch is not even close to complete, as it does not support the
addition of ArrayAccess to PharFileInfo, which would be required in
order to implement what your RFC describes. The complexity of the patch
is reaching the hundreds of lines and will be in the thousands of lines,
all to add syntactic sugar that is far more inefficient than the current
syntax - is this really a necessary feature?
As Elizabeth points out, most people are going to build their iterator
by creating a directory structure and then add the entire directory all
at once. Use of ArrayAccess to write to the archive would be done for
one or two files at most.
When reading from a phar, there is no real incentive to use relative
paths outside of scanning a directory for files (to locate drivers or
plugins, for instance), and iterating over the entire archive is usually
reserved for searching for a particular file. The ability to glob
through a phar archive would of course be the most ideal solution for
that approach (perhaps globiterator already works? I haven't tried it).
What I would like to do, however, is to rethink offsetSet(). I think
that we should introduce a property of PharFileInfo called content
that is read/write, and can be used to perform the equivalent of
file_get_contents()/file_put_contents(). This will allow a consistency
that is much more intuitive. offsetSet() could then simply be removed,
or perhaps be refactored as a way to pass in a PharFileInfo and clone
the information.
Yes, this shouldn't be allowed while
$phar['foo/bar/baz'] = new DirectoryIterator(); could be allowed.
I will simply add makeEmptyDir() a la zip, although it is too bad
ext/zip never thought to
Re: [PHP-DEV] Re: [RFC] Namespace syntax decision
Marcus Boerger wrote: language out there that is in use and has anything like that. And even more scary to me, you did not solve anything by this because people still could write code prior to the namespace keyword. So no matter what we are screwed ? [EMAIL PROTECTED]:~/workspace/php5$ cat test.php ?php $a = 'before namespace'; namespace oops; [EMAIL PROTECTED]:~/workspace/php5$ sapi/cli/php -n test.php Fatal error: Namespace declaration statement has to be the very first statement in the script in /home/cellog/workspace/php5/test.php on line 3 Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] REMINDER - stream wrappers in include_path
Marcus Boerger wrote:
Hello Gregory,
+ for (p = ptr; isalnum((int)*p) || *p == '+' || *p == '-' ||
*p == '.'; p++);
+ /* p - ptr 1 allows us to skip things like C:\whatever */
+ if ((*p == ':') (p - ptr 1) (path_len - (p - path)
2) (p[1] == '/') (p[2] == '/')) {
+ /* .:// or ..:// is not a stream wrapper */
+ if (p[-1] != '.' || p[-2] != '.' || p - 2 != ptr) {
+ p += 3;
+ is_stream_wrapper = 1;
+ }
+ }
You missed one part though. C stops execution of a boolean expression on
the first one that decides on the result. So if p[1] is '\0' then p[2] will
never be accessed. So there is no access violation at all.
good point (i.e. duh on my part). attached patch removes that
unnecessary paranoia.
Analyzing the check for '..:', took a long time :-) And I like that we
check for this common case without going to the wrapper list. And we do not
need to check for the common case '.' either as you require two chars in
front of the ':', cool!
I found a few minor optimizations of this code just now, attached patch
should be even better.
However with the check below:
+ if ((*p == ':') (filename_length - (p - filename) 2) (p[1] ==
'/') (p[2] == '/')) {
+ wrapper = php_stream_locate_url_wrapper(filename,
actual_path, STREAM_OPEN_FOR_INCLUDE TSRMLS_CC);
+ if (wrapper == php_plain_files_wrapper) {
+ if (tsrm_realpath(actual_path, resolved_path
TSRMLS_CC)) {
+ return estrdup(resolved_path);
+ }
+ }
return NULL;
Don't we need to check for wrapper being NULL as in:
if (!wrapper || wrapper == php_plain_files_wrapper) {
Probably, I've added that in too.
Greg
Index: main/fopen_wrappers.c
===
RCS file: /repository/php-src/main/fopen_wrappers.c,v
retrieving revision 1.175.2.3.2.13.2.9
diff -u -r1.175.2.3.2.13.2.9 fopen_wrappers.c
--- main/fopen_wrappers.c 24 Mar 2008 09:30:41 -
1.175.2.3.2.13.2.9
+++ main/fopen_wrappers.c 26 Mar 2008 20:01:04 -
@@ -447,14 +447,23 @@
char resolved_path[MAXPATHLEN];
char trypath[MAXPATHLEN];
const char *ptr, *end, *p;
+ char *actual_path;
+ php_stream_wrapper *wrapper;
if (!filename) {
return NULL;
}
- /* Don't resolve paths which contain protocol */
+ /* Don't resolve paths which contain protocol (except of file://) */
for (p = filename; isalnum((int)*p) || *p == '+' || *p == '-' || *p ==
'.'; p++);
- if ((*p == ':') (p - filename 1) (p[1] == '/') (p[2] ==
'/')) {
+ /* checking for enough length after p to ensure we don't read past the
end of filename */
+ if ((*p == ':') (p[1] == '/') (p[2] == '/')) {
+ wrapper = php_stream_locate_url_wrapper(filename, actual_path,
STREAM_OPEN_FOR_INCLUDE TSRMLS_CC);
+ if (!wrapper || wrapper == php_plain_files_wrapper) {
+ if (tsrm_realpath(actual_path, resolved_path
TSRMLS_CC)) {
+ return estrdup(resolved_path);
+ }
+ }
return NULL;
}
@@ -473,7 +482,19 @@
ptr = path;
while (ptr *ptr) {
- end = strchr(ptr, DEFAULT_DIR_SEPARATOR);
+ /* Check for stream wrapper */
+ int is_stream_wrapper = 0;
+
+ for (p = ptr; isalnum((int)*p) || *p == '+' || *p == '-' || *p
== '.'; p++);
+ /* p - ptr 1 allows us to skip things like C:\whatever */
+ if ((*p == ':') (p - ptr 1) (p[1] == '/') (p[2] ==
'/')) {
+ /* .:// or ..:// is not a stream wrapper */
+ if (p[-1] != '.' || p[-2] != '.' || p - 2 != ptr) {
+ p += 3;
+ is_stream_wrapper = 1;
+ }
+ }
+ end = strchr(p, DEFAULT_DIR_SEPARATOR);
if (end) {
if ((end-ptr) + 1 + filename_length + 1 = MAXPATHLEN) {
ptr = end + 1;
@@ -494,7 +515,23 @@
memcpy(trypath+len+1, filename, filename_length+1);
ptr = NULL;
}
- if (tsrm_realpath(trypath, resolved_path TSRMLS_CC)) {
+ actual_path = trypath;
+ if (is_stream_wrapper) {
+ wrapper = php_stream_locate_url_wrapper(trypath,
actual_path, STREAM_OPEN_FOR_INCLUDE TSRMLS_CC);
+ if (!wrapper) {
+ continue;
Re: [PHP-DEV] REMINDER - stream wrappers in include_path
Dmitry Stogov wrote: I hope it's the last iteration, but check me anyway. The patch is based on latest Gregory's patch. - optimized out strncpy() calls - zend_resolve_path() replaced with php_resolve_path() - improved php_resolve_path() to resolve file://... - fixed possible double-free issue in _php_stream_open_wrapper_ex() Thanks. Dmitry. Greg Beaver wrote: Andi Gutmans wrote: Can we please use strlcpy() instead of strncpy()? This is a coding standard we implemented years ago. obviously an easy change. FYI - this also needs to be fixed in fopen_with_path_rel in PHP_5_2, as I copied most of the code from that function. Greg works great here - commit with all due haste :) Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] REMINDER - stream wrappers in include_path
Stanislav Malyshev wrote:
stream wrapper. Here is an example:
oops.broken://UNC/path
I wonder if .://UNC/path is treated as .+//UNC/path (and the same
for ..). It should anyway :) However I'm not too worried without
pathes like foo.bar - not likely to have path without any slashes
unless it's . or .., and if you do, you always can say ./foo.bar
That's a great question. In attempting to answer, I think I may have
unfortunately found a severe flaw in the patch, allowing reading past
the end of the filename and the include_path.
If we pass a file named hello: to php_resolve_path, this code:
if ((*p == ':') (p - filename 1) (p[1] == '/') (p[2] == '/')) {
would look past the end of the filename by 1 (p[1] = '\0', but p[2] is
an off by one read). Instead, we should be checking to see if p -
filename filename_length - 2 so we have enough room for the stream
wrapper and 1 character (i.e. blah://a, not blah://).
To get around the problem Stas raises, we need to disallow . or ..
as stream wrapper names in php_stream_locate_url_wrapper and check
for them explicitly in php_resolve_path.
The attached patch is identical to Dmitry's wrapper6.patch.txt and fixes
these two issues.
Note that the check for . and .. is only needed when scanning
include_path, and the part at the end that checks dirname(__FILE__) does
not need it because there is no way __FILE__ could be
.://path/to/something or ..:/path/to/something if . and .. are
disallowed as stream wrapper names.
Unfortunately, I don't have quite enough time to get the attached patch
working, but I'm including it for the smarties to figure out how to
handle, I've put /* XXX FIXME */ where things need work.
Thanks,
Greg
Index: main/fopen_wrappers.c
===
RCS file: /repository/php-src/main/fopen_wrappers.c,v
retrieving revision 1.175.2.3.2.13.2.9
diff -u -r1.175.2.3.2.13.2.9 fopen_wrappers.c
--- main/fopen_wrappers.c 24 Mar 2008 09:30:41 -
1.175.2.3.2.13.2.9
+++ main/fopen_wrappers.c 25 Mar 2008 19:00:36 -
@@ -447,14 +447,24 @@
char resolved_path[MAXPATHLEN];
char trypath[MAXPATHLEN];
const char *ptr, *end, *p;
+ char *actual_path;
+ php_stream_wrapper *wrapper;
+ int path_len = strlen(path);
if (!filename) {
return NULL;
}
- /* Don't resolve paths which contain protocol */
+ /* Don't resolve paths which contain protocol (except of file://) */
for (p = filename; isalnum((int)*p) || *p == '+' || *p == '-' || *p ==
'.'; p++);
- if ((*p == ':') (p - filename 1) (p[1] == '/') (p[2] ==
'/')) {
+ /* XXX FIXME checking for enough length after p to ensure we don't read
past the end of filename */
+ if ((*p == ':') (p - filename = filename_length - 3) (p[1] ==
'/') (p[2] == '/')) {
+ wrapper = php_stream_locate_url_wrapper(filename, actual_path,
STREAM_OPEN_FOR_INCLUDE TSRMLS_CC);
+ if (wrapper == php_plain_files_wrapper) {
+ if (tsrm_realpath(actual_path, resolved_path
TSRMLS_CC)) {
+ return estrdup(resolved_path);
+ }
+ }
return NULL;
}
@@ -473,7 +483,19 @@
ptr = path;
while (ptr *ptr) {
- end = strchr(ptr, DEFAULT_DIR_SEPARATOR);
+ /* Check for stream wrapper */
+ int is_stream_wrapper = 0;
+
+ for (p = ptr; isalnum((int)*p) || *p == '+' || *p == '-' || *p
== '.'; p++);
+ /* XXX FIXME checking for . or .. and also ensuring there is
enough length left in include_path to check for // */
+ if ((*p == ':') (p - ptr 1) ((p + 1) - path = path_len
- 3) (p[1] == '/') (p[2] == '/')) {
+ /* .:// or ..:// is not a stream wrapper */
+ if (!((p - ptr == 1 *(p - 1) == '.') || (p - ptr ==
2 *(p - 2) == '.' *(p - 1) == '.'))) {
+ p += 3;
+ is_stream_wrapper = 1;
+ }
+ }
+ end = strchr(p, DEFAULT_DIR_SEPARATOR);
if (end) {
if ((end-ptr) + 1 + filename_length + 1 = MAXPATHLEN) {
ptr = end + 1;
@@ -494,7 +516,23 @@
memcpy(trypath+len+1, filename, filename_length+1);
ptr = NULL;
}
- if (tsrm_realpath(trypath, resolved_path TSRMLS_CC)) {
+ actual_path = trypath;
+ if (is_stream_wrapper) {
+ wrapper = php_stream_locate_url_wrapper(trypath,
actual_path, STREAM_OPEN_FOR_INCLUDE TSRMLS_CC);
+ if (!wrapper) {
+ continue;
+ } else if (wrapper !=
Re: [PHP-DEV] REMINDER - stream wrappers in include_path
Marcus Boerger wrote:
Hello Gregory,
Tuesday, March 25, 2008, 8:01:56 PM, you wrote:
Stanislav Malyshev wrote:
stream wrapper. Here is an example:
oops.broken://UNC/path
I wonder if .://UNC/path is treated as .+//UNC/path (and the same
for ..). It should anyway :) However I'm not too worried without
pathes like foo.bar - not likely to have path without any slashes
unless it's . or .., and if you do, you always can say ./foo.bar
That's a great question. In attempting to answer, I think I may have
unfortunately found a severe flaw in the patch, allowing reading past
the end of the filename and the include_path.
If we pass a file named hello: to php_resolve_path, this code:
if ((*p == ':') (p - filename 1) (p[1] == '/') (p[2] == '/')) {
would look past the end of the filename by 1 (p[1] = '\0', but p[2] is
an off by one read). Instead, we should be checking to see if p -
filename filename_length - 2 so we have enough room for the stream
wrapper and 1 character (i.e. blah://a, not blah://).
How so?
Just reorder and put (p - filename 1 first)
snip
Hi Marcus,
It looks like I wasn't clear - the problem is not checking characters
before the :, but after. With the existing code, if the string ended
with : the code would attempt the check : +1 and : + 2, which
could result in out-of-bounds read (read after the terminating '\0'.
However, in answer to your other question, we do need stream wrappers
longer than 1 character, as we would otherwise could be unnecessarily
checking drive letters on windows thinking they're stream wrappers.
However, I took your excellent suggestions into account in the attached
patch, which fully works and prevents use of either . or .. as stream
wrapper names.
Otherwise, its identical to wrapper6.patch.txt
One last check by Dmitry before commit and we should be good to go.
Thanks for the catch on the logical problem Stas and the coding
suggestions Marcus - this is the kind of teamwork that I dream of. The
open source gods must be looking down on us kindly.
Greg
Index: main/fopen_wrappers.c
===
RCS file: /repository/php-src/main/fopen_wrappers.c,v
retrieving revision 1.175.2.3.2.13.2.9
diff -u -r1.175.2.3.2.13.2.9 fopen_wrappers.c
--- main/fopen_wrappers.c 24 Mar 2008 09:30:41 -
1.175.2.3.2.13.2.9
+++ main/fopen_wrappers.c 26 Mar 2008 03:43:28 -
@@ -447,14 +447,24 @@
char resolved_path[MAXPATHLEN];
char trypath[MAXPATHLEN];
const char *ptr, *end, *p;
+ char *actual_path;
+ php_stream_wrapper *wrapper;
+ int path_len = strlen(path);
if (!filename) {
return NULL;
}
- /* Don't resolve paths which contain protocol */
+ /* Don't resolve paths which contain protocol (except of file://) */
for (p = filename; isalnum((int)*p) || *p == '+' || *p == '-' || *p ==
'.'; p++);
- if ((*p == ':') (p - filename 1) (p[1] == '/') (p[2] ==
'/')) {
+ /* checking for enough length after p to ensure we don't read past the
end of filename */
+ if ((*p == ':') (filename_length - (p - filename) 2) (p[1] ==
'/') (p[2] == '/')) {
+ wrapper = php_stream_locate_url_wrapper(filename, actual_path,
STREAM_OPEN_FOR_INCLUDE TSRMLS_CC);
+ if (wrapper == php_plain_files_wrapper) {
+ if (tsrm_realpath(actual_path, resolved_path
TSRMLS_CC)) {
+ return estrdup(resolved_path);
+ }
+ }
return NULL;
}
@@ -473,7 +483,19 @@
ptr = path;
while (ptr *ptr) {
- end = strchr(ptr, DEFAULT_DIR_SEPARATOR);
+ /* Check for stream wrapper */
+ int is_stream_wrapper = 0;
+
+ for (p = ptr; isalnum((int)*p) || *p == '+' || *p == '-' || *p
== '.'; p++);
+ /* p - ptr 1 allows us to skip things like C:\whatever */
+ if ((*p == ':') (p - ptr 1) (path_len - (p - path) 2)
(p[1] == '/') (p[2] == '/')) {
+ /* .:// or ..:// is not a stream wrapper */
+ if (p[-1] != '.' || p[-2] != '.' || p - 2 != ptr) {
+ p += 3;
+ is_stream_wrapper = 1;
+ }
+ }
+ end = strchr(p, DEFAULT_DIR_SEPARATOR);
if (end) {
if ((end-ptr) + 1 + filename_length + 1 = MAXPATHLEN) {
ptr = end + 1;
@@ -494,7 +516,23 @@
memcpy(trypath+len+1, filename, filename_length+1);
ptr = NULL;
}
- if (tsrm_realpath(trypath, resolved_path TSRMLS_CC)) {
+ actual_path = trypath;
+ if (is_stream_wrapper) {
+ wrapper =
Re: [PHP-DEV] REMINDER - stream wrappers in include_path
Dmitry Stogov wrote:
Hi Greg,
In your second patch you forgot break, so it couldn't work.
I don't see any reason in second patch at all, because you call
php_resolve_patch() from _php_stream_open_wrapper_ex() anyway.
The bad thing with the first part that it calls realpath() function
twice for each include(relative_path.php) and it makes several
file-system accesses (look into `strace ... 21| grep lstat`). With
your patch I see 6 syscalls more.
So I would prefer my patch that is more efficient for regular files.
The same patch with fixed white-spaces is attached.
Your patch is broken, it causes an endless loop in
tests/include_path.phpt when running phar tests.
Incidentally, I was unaware of the strace command (I've seen it bandied
about on irc, but thought it was something else), so thank you for
edumacatin me there.
The attached patch has no additional syscalls over the current case for
both existing and non-existing files, and actually works for phar as
well :). It's also simpler (I removed the include stuff from
zend_vm_def.h), although you may want to add it back in and bench it to
see if it makes any difference in performance, since I found such a
dramatic reduction in operations here with it.
Greg
Index: main/fopen_wrappers.c
===
RCS file: /repository/php-src/main/fopen_wrappers.c,v
retrieving revision 1.175.2.3.2.13.2.9
diff -u -r1.175.2.3.2.13.2.9 fopen_wrappers.c
--- main/fopen_wrappers.c 24 Mar 2008 09:30:41 -
1.175.2.3.2.13.2.9
+++ main/fopen_wrappers.c 24 Mar 2008 13:24:34 -
@@ -473,7 +473,15 @@
ptr = path;
while (ptr *ptr) {
- end = strchr(ptr, DEFAULT_DIR_SEPARATOR);
+ /* Check for stream wrapper */
+ int is_stream_wrapper = 0;
+
+ for (p = ptr; isalnum((int)*p) || *p == '+' || *p == '-' || *p
== '.'; p++);
+ if ((*p == ':') (p - ptr 1) (p[1] == '/') (p[2] ==
'/')) {
+ p += 2;
+ is_stream_wrapper = 1;
+ }
+ end = strchr(p, DEFAULT_DIR_SEPARATOR);
if (end) {
if ((end-ptr) + 1 + filename_length + 1 = MAXPATHLEN) {
ptr = end + 1;
@@ -494,6 +502,25 @@
memcpy(trypath+len+1, filename, filename_length+1);
ptr = NULL;
}
+ if (is_stream_wrapper) {
+ char *actual;
+ php_stream_wrapper *wrapper =
php_stream_locate_url_wrapper(trypath, actual, STREAM_OPEN_FOR_INCLUDE
TSRMLS_CC);
+
+ if (!wrapper) {
+ continue;
+ } else if (wrapper == php_plain_files_wrapper) {
+ strncpy(trypath, actual, MAXPATHLEN);
+ } else {
+ if (wrapper-wops-url_stat) {
+ php_stream_statbuf ssb;
+
+ if (SUCCESS ==
wrapper-wops-url_stat(wrapper, trypath, 0, ssb, NULL TSRMLS_CC)) {
+ return estrdup(trypath);
+ }
+ }
+ continue;
+ }
+ }
if (tsrm_realpath(trypath, resolved_path TSRMLS_CC)) {
return estrdup(resolved_path);
}
@@ -511,6 +538,29 @@
exec_fname_length + 1 + filename_length + 1 MAXPATHLEN) {
memcpy(trypath, exec_fname, exec_fname_length + 1);
memcpy(trypath+exec_fname_length + 1, filename,
filename_length+1);
+
+ /* Check for stream wrapper */
+ for (p = trypath; isalnum((int)*p) || *p == '+' || *p
== '-' || *p == '.'; p++);
+ if ((*p == ':') (p - trypath 1) (p[1] == '/')
(p[2] == '/')) {
+ char *actual;
+ php_stream_wrapper *wrapper =
php_stream_locate_url_wrapper(trypath, actual, STREAM_OPEN_FOR_INCLUDE
TSRMLS_CC);
+
+ if (!wrapper) {
+ return NULL;
+ } else if (wrapper == php_plain_files_wrapper)
{
+ strncpy(trypath, actual, MAXPATHLEN);
+ } else {
+ if (wrapper-wops-url_stat) {
+ php_stream_statbuf ssb;
+
+ if (SUCCESS ==
wrapper-wops-url_stat(wrapper, trypath, 0, ssb, NULL TSRMLS_CC)) {
+ return
Re: [PHP-DEV] REMINDER - stream wrappers in include_path
Marcus Boerger wrote:
Hello Gregory,
Monday, March 24, 2008, 2:28:00 PM, you wrote:
Index: main/fopen_wrappers.c
===
RCS file: /repository/php-src/main/fopen_wrappers.c,v
retrieving revision 1.175.2.3.2.13.2.9
diff -u -r1.175.2.3.2.13.2.9 fopen_wrappers.c
--- main/fopen_wrappers.c 24 Mar 2008 09:30:41 -
1.175.2.3.2.13.2.9
+++ main/fopen_wrappers.c 24 Mar 2008 13:24:34 -
@@ -473,7 +473,15 @@
ptr = path;
while (ptr *ptr) {
- end = strchr(ptr, DEFAULT_DIR_SEPARATOR);
+ /* Check for stream wrapper */
+ int is_stream_wrapper = 0;
+
+ for (p = ptr; isalnum((int)*p) || *p == '+' || *p == '-' ||
*p == '.'; p++);
+ if ((*p == ':') (p - ptr 1) (p[1] == '/') (p[2]
== '/')) {
However how about:
if ((p ptr) !memcmp(p, ://, 3)) {
this would unfortunately break UNC paths on unix, which start with //.
We have to verify that the stuff between : and : in the previous thing
is a valid stream wrapper, which means it can only be
alphanumeric/+/-/. However, I imagine the memcmp could be applied as if
((*p == ':') (p - ptr 1) !memcmp(p, ://, 3))
Would that really be faster?
I think prefixinf with STREAMS_ would be good.
OK, updated and attached
Greg
Index: main/fopen_wrappers.c
===
RCS file: /repository/php-src/main/fopen_wrappers.c,v
retrieving revision 1.175.2.3.2.13.2.9
diff -u -r1.175.2.3.2.13.2.9 fopen_wrappers.c
--- main/fopen_wrappers.c 24 Mar 2008 09:30:41 -
1.175.2.3.2.13.2.9
+++ main/fopen_wrappers.c 24 Mar 2008 13:46:39 -
@@ -473,7 +473,15 @@
ptr = path;
while (ptr *ptr) {
- end = strchr(ptr, DEFAULT_DIR_SEPARATOR);
+ /* Check for stream wrapper */
+ int is_stream_wrapper = 0;
+
+ for (p = ptr; isalnum((int)*p) || *p == '+' || *p == '-' || *p
== '.'; p++);
+ if ((*p == ':') (p - ptr 1) (p[1] == '/') (p[2] ==
'/')) {
+ p += 2;
+ is_stream_wrapper = 1;
+ }
+ end = strchr(p, DEFAULT_DIR_SEPARATOR);
if (end) {
if ((end-ptr) + 1 + filename_length + 1 = MAXPATHLEN) {
ptr = end + 1;
@@ -494,6 +502,25 @@
memcpy(trypath+len+1, filename, filename_length+1);
ptr = NULL;
}
+ if (is_stream_wrapper) {
+ char *actual;
+ php_stream_wrapper *wrapper =
php_stream_locate_url_wrapper(trypath, actual, STREAM_OPEN_FOR_INCLUDE
TSRMLS_CC);
+
+ if (!wrapper) {
+ continue;
+ } else if (wrapper == php_plain_files_wrapper) {
+ strncpy(trypath, actual, MAXPATHLEN);
+ } else {
+ if (wrapper-wops-url_stat) {
+ php_stream_statbuf ssb;
+
+ if (SUCCESS ==
wrapper-wops-url_stat(wrapper, trypath, 0, ssb, NULL TSRMLS_CC)) {
+ return estrdup(trypath);
+ }
+ }
+ continue;
+ }
+ }
if (tsrm_realpath(trypath, resolved_path TSRMLS_CC)) {
return estrdup(resolved_path);
}
@@ -511,6 +538,29 @@
exec_fname_length + 1 + filename_length + 1 MAXPATHLEN) {
memcpy(trypath, exec_fname, exec_fname_length + 1);
memcpy(trypath+exec_fname_length + 1, filename,
filename_length+1);
+
+ /* Check for stream wrapper */
+ for (p = trypath; isalnum((int)*p) || *p == '+' || *p
== '-' || *p == '.'; p++);
+ if ((*p == ':') (p - trypath 1) (p[1] == '/')
(p[2] == '/')) {
+ char *actual;
+ php_stream_wrapper *wrapper =
php_stream_locate_url_wrapper(trypath, actual, STREAM_OPEN_FOR_INCLUDE
TSRMLS_CC);
+
+ if (!wrapper) {
+ return NULL;
+ } else if (wrapper == php_plain_files_wrapper)
{
+ strncpy(trypath, actual, MAXPATHLEN);
+ } else {
+ if (wrapper-wops-url_stat) {
+ php_stream_statbuf ssb;
+
+ if (SUCCESS ==
Re: [PHP-DEV] Re: [PECL-DEV] About that PECL versioning thing
Steph Fox wrote: Hello Pierre, Aside from Pierre's arguments in favour of using package.xml to set the extension version (which 3 PECL extensions - two of them Pierre's - do at present), does anyone have any objection to the proposal at http://wiki.php.net/rfc/peclversioning? I'm not in favour of using package.xml to set the version. I'm in favour of allowing package.xml usage. Nobody's attempting to prevent package.xml usage, least of all me! I actually want to extend package.xml to give more information (QA related) so that people have more of a clue about what they're dealing with. E.g. code coverage %, maintenance status, whether the package is of general/special interest (this last mostly for hosting companies) and some kind of grading system. But this is all open to discussion and probably won't happen for a long while. This kind of meta-data doesn't belong in a package.xml, but it would make perfect sense to host it through REST on pecl.php.net. Don't forget, package.xml is used by external channels as well, and they have completely different requirements. package.xml is intended to be used by the pear installer to install packages and provide essential metadata only. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] REMINDER - stream wrappers in include_path
Dmitry Stogov wrote:
REALPATH_FAILED looks like a hack :(
PHP itself doesn't need it at all, according to your patch
php_stream_open() may just return NULL immediately instead of setting
of this flag.
However I am not sure that it will work for all cases. The fact that
we cannot resolve the path doesn't mean that we cannot open the file.
(On some systems getcwd() may fail on some reasons).
I remember Solaris having issues with this, now that you mention it.
However, the place that I added this already returned NULL if realpath
fails, it doesn't change the behavior, just moves the realpath to an
earlier position and eliminates an unnecessary double call. However, if
we are comfortable having the double realpath on files not found in
include_path (this would only have a performance affect on apps with
lots of conditional loading of drivers where the majority are not
found), this is fine with me. I've attached a patch that removes
REALPATH_FAILED so there are options.
Another option would be to determine which OSes this can fail, and
simply not use REALPATH_FAILED on those OSes.
I didn't test the patch with pecl/phar.
Could you explain why php goes into the endless loop?
because the cut/paste to plain_wrapper.c:1419 needs a ptr = end, unlike
the code in fopen_wrappers.c. Once you fix this problem, then the real
logic problem rears its head, which is that we go into plain_wrapper to
open an external stream wrapper. It is far less efficient (about 4-6%
by my measurements) to do this, which is why I moved the include_path
parsing into streams.c
This bug only happens when the last item in include_path is a stream
wrapper and the file is not found in include_path.
Greg
Index: main/fopen_wrappers.c
===
RCS file: /repository/php-src/main/fopen_wrappers.c,v
retrieving revision 1.175.2.3.2.13.2.9
diff -u -r1.175.2.3.2.13.2.9 fopen_wrappers.c
--- main/fopen_wrappers.c 24 Mar 2008 09:30:41 -
1.175.2.3.2.13.2.9
+++ main/fopen_wrappers.c 24 Mar 2008 14:39:45 -
@@ -473,7 +473,15 @@
ptr = path;
while (ptr *ptr) {
- end = strchr(ptr, DEFAULT_DIR_SEPARATOR);
+ /* Check for stream wrapper */
+ int is_stream_wrapper = 0;
+
+ for (p = ptr; isalnum((int)*p) || *p == '+' || *p == '-' || *p
== '.'; p++);
+ if ((*p == ':') (p - ptr 1) (p[1] == '/') (p[2] ==
'/')) {
+ p += 2;
+ is_stream_wrapper = 1;
+ }
+ end = strchr(p, DEFAULT_DIR_SEPARATOR);
if (end) {
if ((end-ptr) + 1 + filename_length + 1 = MAXPATHLEN) {
ptr = end + 1;
@@ -494,6 +502,25 @@
memcpy(trypath+len+1, filename, filename_length+1);
ptr = NULL;
}
+ if (is_stream_wrapper) {
+ char *actual;
+ php_stream_wrapper *wrapper =
php_stream_locate_url_wrapper(trypath, actual, STREAM_OPEN_FOR_INCLUDE
TSRMLS_CC);
+
+ if (!wrapper) {
+ continue;
+ } else if (wrapper == php_plain_files_wrapper) {
+ strncpy(trypath, actual, MAXPATHLEN);
+ } else {
+ if (wrapper-wops-url_stat) {
+ php_stream_statbuf ssb;
+
+ if (SUCCESS ==
wrapper-wops-url_stat(wrapper, trypath, 0, ssb, NULL TSRMLS_CC)) {
+ return estrdup(trypath);
+ }
+ }
+ continue;
+ }
+ }
if (tsrm_realpath(trypath, resolved_path TSRMLS_CC)) {
return estrdup(resolved_path);
}
@@ -511,6 +538,29 @@
exec_fname_length + 1 + filename_length + 1 MAXPATHLEN) {
memcpy(trypath, exec_fname, exec_fname_length + 1);
memcpy(trypath+exec_fname_length + 1, filename,
filename_length+1);
+
+ /* Check for stream wrapper */
+ for (p = trypath; isalnum((int)*p) || *p == '+' || *p
== '-' || *p == '.'; p++);
+ if ((*p == ':') (p - trypath 1) (p[1] == '/')
(p[2] == '/')) {
+ char *actual;
+ php_stream_wrapper *wrapper =
php_stream_locate_url_wrapper(trypath, actual, STREAM_OPEN_FOR_INCLUDE
TSRMLS_CC);
+
+ if (!wrapper) {
+ return NULL;
+ } else if (wrapper == php_plain_files_wrapper)
{
+
Re: [PHP-DEV] REMINDER - stream wrappers in include_path
Dmitry Stogov wrote: Gregory Beaver wrote: Dmitry Stogov wrote: REALPATH_FAILED looks like a hack :( PHP itself doesn't need it at all, according to your patch php_stream_open() may just return NULL immediately instead of setting of this flag. However I am not sure that it will work for all cases. The fact that we cannot resolve the path doesn't mean that we cannot open the file. (On some systems getcwd() may fail on some reasons). I remember Solaris having issues with this, now that you mention it. However, the place that I added this already returned NULL if realpath fails, it doesn't change the behavior, just moves the realpath to an earlier position and eliminates an unnecessary double call. However, if we are comfortable having the double realpath on files not found in include_path (this would only have a performance affect on apps with lots of conditional loading of drivers where the majority are not found), this is fine with me. I've attached a patch that removes REALPATH_FAILED so there are options. Another option would be to determine which OSes this can fail, and simply not use REALPATH_FAILED on those OSes. I didn't test the patch with pecl/phar. Could you explain why php goes into the endless loop? because the cut/paste to plain_wrapper.c:1419 needs a ptr = end, unlike the code in fopen_wrappers.c. You are right. Once you fix this problem, then the real logic problem rears its head, which is that we go into plain_wrapper to open an external stream wrapper. It is far less efficient (about 4-6% by my measurements) to do this, which is why I moved the include_path parsing into streams.c Did you measure pecl/phar or regular files? I only measured regular files, my concern is that we don't affect any existing users, 100% of whom use regular files at the moment (obviously :). This bug only happens when the last item in include_path is a stream wrapper and the file is not found in include_path. In general I don't have objections except of changing zend_resolve_path() (which is a callback for ZE) into php_resolve_path(). Anyway I'll look into patch once again with fresh head. I would also ask Stas and other interested people to look into it. Thanks Dmitry, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] REMINDER - stream wrappers in include_path
Stanislav Malyshev wrote: this would unfortunately break UNC paths on unix, which start with //. Unix has UNC paths? Beats me, I'm sure posix-based systems don't, but it is called Uniform Naming Convention so it's possible somebody might implement it (http://en.wikipedia.org/wiki/Path_(computing)#Uniform_Naming_Convention). :) Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [PECL-DEV] About that PECL versioning thing
Steph Fox wrote: What a horrible name :) How about pecl_graveyard? siberia -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: [RFC] Namespace syntax decision
Marcus Boerger wrote:
Hello Internals,
we all were asked to stop discussing syntax of namespaces as we were told
that we would decide after the namespace functionality was fully implemented.
Now I think that the functionallity is pretty much settled we should
revisit the syntax. We all have been very patient so far. Anyway here goes
my take on it:
PHP is very close to Java and C++ in terms of Syntax. And many of our users
are familiar with one or even both of them. Also we have a tendency to
especially take syntax from those two or be in line with those two
languages. That said I see two ways:
1) namespace foo { }
2) package foo;
I did not notice until now that you had changed the keyword to package.
-1 on this change. Either namespace blah; or namespace blah {}.
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC] Namespace syntax decision
Felipe Pena wrote:
Em Sex, 2008-03-21 às 17:01 -0500, Gregory Beaver escreveu:
1) namespace foo { }
This is acceptable if nothing can exist outside namespace foo {} except
declare and other namespace declarations.
Indeed!
Here's my try:
http://felipe.ath.cx/diff/namespace.diff
http://felipe.ath.cx/diff/namespace.phpt
Hi Felipe,
The patch is pristine coding, but I do think the error message needs
some loving:
+ if (zend_do_namespace_check(TSRMLS_C) == FAILURE) {
+ zend_error(E_COMPILE_ERROR, Only namespace declaration
is allowed
in the script);
This implies that only namespace is allowed, and would be confusing to
99% of users. Might I suggest this error message instead:
This script contains namespaces, all other code must be contained
within namespace declarations
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] short_open_tag
Stefan Walk wrote: Johannes Schlüter schrieb: Now we have the big issue: Do we want to have short open tags forever? Well, without tooo much thinking my idea would be to drop ? but keep ?=, ?= shouldn't conflict with ?xml tags in the same file, but make it simple to do templating using PHP, on the other hand when not echo'ing stuff you already have to write more soo the four additional characters (php ) don't matter that much - especially every decent editor/ide should be able to give you a completion on that, if you want. ul ? foreach ($items as $item): ? li?=$item?/li ? endforeach ? /ul you can have short stuff without outputting stuff too. I see many good reasons to disable short open tags. However, there is a compromise that is better from all vantage points: ul ?p foreach ($items as $item): ? li?=$item ?/li ?p endforeach ? /ul ?p is a valid PI and would prevent ?xml from being parsed as PHP. Also legal is ?: as PI's can start with or contain : or _. Honestly though, this is not so important to me, my primary concern is the conflict with ?xml. I mention it out of deference for those who actually do care about writing scripts that are xml-compliant for some strange reason :). Also possible and relatively simple to implement would be to allow an = at the start of an expression to alias to T_ECHO, so that ?p =$item ? would work like ?=$item?. This is, however, very perl-ish, so I mention it only as a possible way to preserve that aspect of short tags for template usage. God forbid we start seeing regular scripts using = to mean echo :). As a note, I use exclusively ?php in my templates and also use ?xml to generate xhtml, so I am very much against per-script enabling of short tag ? for the annoyance it would introduce of forcing an ini_set() at the top of each template and the bottom as well to be a good citizen and restore the old value. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] REMINDER - stream wrappers in include_path
Dmitry Stogov wrote:
Hi Greg,
I've fixed your patch to work with all functions (fopen,
file_get_contente).
Please verify it with ext/phar and then I'll commit it.
Thanks. Dmitry.
Gregory Beaver wrote:
Hi,
Please look at the stream wrappers in include_path patch I posted
last week.
Thanks,
Greg
Hi Dmitry,
the patch has a few major problems and a few minor ones. First off,
there are spaces instead of tabs, which is not a huge deal, I've
corrected that in the attached patch.
It is, however, logically void to do stream wrapper operations in
plain_wrapper.c. I removed that code entirely. Instead, the path
resolve is now done in php_stream_open_wrapper_ex where it belongs,
before determining which stream wrapper to open. This will result in a
double call to the function when calling include, but because the 2nd
call will simply return after determining it has received a full path.
I've attached 2 patches (apparently, Zend is not included in cvs diff -u
of php5), 1 for adding path resolution to include/require (this is a
significant performance improvement over the existing code). The second
patch adds stream wrapper support to both php_resolve_path and
_php_stream_open_wrapper_ex, which automatically gives stream wrapper in
include_path support to all current and future functions that use
include_path.
Greg
Index: main/fopen_wrappers.c
===
RCS file: /repository/php-src/main/fopen_wrappers.c,v
retrieving revision 1.175.2.3.2.13.2.8
diff -u -r1.175.2.3.2.13.2.8 fopen_wrappers.c
--- main/fopen_wrappers.c 13 Mar 2008 14:09:54 -
1.175.2.3.2.13.2.8
+++ main/fopen_wrappers.c 21 Mar 2008 16:50:45 -
@@ -452,11 +452,11 @@
return NULL;
}
- /* Don't resolve patches which contain protocol */
+ /* Don't resolve paths which contain protocol */
for (p = filename; isalnum((int)*p) || *p == '+' || *p == '-' || *p ==
'.'; p++);
-if ((*p == ':') (p - filename 1) (p[1] == '/') (p[2] == '/')) {
- return NULL;
-}
+ if ((*p == ':') (p - filename 1) (p[1] == '/') (p[2] ==
'/')) {
+ return NULL;
+ }
if ((*filename == '.'
(IS_SLASH(filename[1]) ||
@@ -473,7 +473,15 @@
ptr = path;
while (ptr *ptr) {
- end = strchr(ptr, DEFAULT_DIR_SEPARATOR);
+ /* Check for stream wrapper */
+ int is_stream_wrapper = 0;
+
+ for (p = ptr; isalnum((int)*p) || *p == '+' || *p == '-' || *p
== '.'; p++);
+ if ((*p == ':') (p - ptr 1) (p[1] == '/') (p[2] ==
'/')) {
+ p += 2;
+ is_stream_wrapper = 1;
+ }
+ end = strchr(p, DEFAULT_DIR_SEPARATOR);
if (end) {
if ((end-ptr) + 1 + filename_length + 1 = MAXPATHLEN) {
ptr = end + 1;
@@ -494,6 +502,25 @@
memcpy(trypath+len+1, filename, filename_length+1);
ptr = NULL;
}
+ if (is_stream_wrapper) {
+ char *actual;
+ php_stream_wrapper *wrapper =
php_stream_locate_url_wrapper(trypath, actual, STREAM_OPEN_FOR_INCLUDE
TSRMLS_CC);
+
+ if (!wrapper) {
+ continue;
+ } else if (wrapper == php_plain_files_wrapper) {
+ strncpy(trypath, actual, MAXPATHLEN);
+ } else {
+ if (wrapper-wops-url_stat) {
+ php_stream_statbuf ssb;
+
+ if (SUCCESS ==
wrapper-wops-url_stat(wrapper, trypath, 0, ssb, NULL TSRMLS_CC)) {
+ return estrdup(trypath);
+ }
+ }
+ continue;
+ }
+ }
if (tsrm_realpath(trypath, resolved_path TSRMLS_CC)) {
return estrdup(resolved_path);
}
@@ -511,6 +538,29 @@
exec_fname_length + 1 + filename_length + 1 MAXPATHLEN) {
memcpy(trypath, exec_fname, exec_fname_length + 1);
memcpy(trypath+exec_fname_length + 1, filename,
filename_length+1);
+
+ /* Check for stream wrapper */
+ for (p = trypath; isalnum((int)*p) || *p == '+' || *p
== '-' || *p == '.'; p++);
+ if ((*p == ':') (p - trypath 1) (p[1] == '/')
(p[2] == '/')) {
+ char *actual;
+ php_stream_wrapper *wrapper =
php_stream_locate_url_wrapper(trypath, actual, STREAM_OPEN_FOR_INCLUDE
TSRMLS_CC
[PHP-DEV] Re: [RFC] Namespace syntax decision
Marcus Boerger wrote:
Hello Internals,
we all were asked to stop discussing syntax of namespaces as we were told
that we would decide after the namespace functionality was fully implemented.
Now I think that the functionallity is pretty much settled we should
revisit the syntax. We all have been very patient so far. Anyway here goes
my take on it:
PHP is very close to Java and C++ in terms of Syntax. And many of our users
are familiar with one or even both of them. Also we have a tendency to
especially take syntax from those two or be in line with those two
languages. That said I see two ways:
1) namespace foo { }
This is acceptable if nothing can exist outside namespace foo {} except
declare and other namespace declarations.
2) package foo;
I favor 1) if we allow namespace nesting and 2) if not. The current way of
nesting is very confusing:
namespace foo; namespace bar;
I prefer this syntax because it discourages multiple namespaces, but
still allows them (nesting is an inaccurate term in this case, I
strongly encourage not using it)
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] REMINDER - stream wrappers in include_path
Hi, Please look at the stream wrappers in include_path patch I posted last week. Thanks, Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] profiling followup on addition of streams to include_path - it's *faster*
Alexey Zakhlestin wrote: what about including(_once) by absolute path? Hi, There is no difference from the existing code for include_once, the first line of php_resolve_path has a check for IS_ABSOLUTE_PATH, all of the code I wrote follows this line. Calls to include (no _once) would add 1 function call to php_resolve_path, an insignificant difference. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] fast streams support in include_path
Hi,
Here's the last attempt at streams support in include_path.
After siccing callgrind on the patch, I'm confident it meets performance
requirements. Even in the worst case, it is significantly faster than
PHP 5.2, and insignificantly slower (1%) than the current CVS.
This patch is optimized so that it is:
1% faster than the current implementation for include/require
1% slower than current include_once/require_once implementation
It also fixes a compile warning caused by missing cast from const char *
to char *
Greg
Index: Zend/zend_vm_def.h
===
RCS file: /repository/ZendEngine2/zend_vm_def.h,v
retrieving revision 1.59.2.29.2.48.2.41
diff -u -r1.59.2.29.2.48.2.41 zend_vm_def.h
--- Zend/zend_vm_def.h 5 Mar 2008 13:34:12 - 1.59.2.29.2.48.2.41
+++ Zend/zend_vm_def.h 8 Mar 2008 18:35:06 -
@@ -3091,8 +3091,19 @@
}
break;
case ZEND_INCLUDE:
- case ZEND_REQUIRE:
- new_op_array =
compile_filename(Z_LVAL(opline-op2.u.constant), inc_filename TSRMLS_CC);
+ case ZEND_REQUIRE: {
+ char *resolved_path;
+
+ resolved_path =
zend_resolve_path(Z_STRVAL_P(inc_filename), Z_STRLEN_P(inc_filename) TSRMLS_CC);
+ if (resolved_path) {
+ zval rz;
+ ZVAL_STRINGL(rz, resolved_path,
strlen(resolved_path), 0);
+ new_op_array =
compile_filename(Z_LVAL(opline-op2.u.constant), rz TSRMLS_CC);
+ efree(resolved_path);
+ } else {
+ new_op_array =
compile_filename(Z_LVAL(opline-op2.u.constant), inc_filename TSRMLS_CC);
+ }
+ }
break;
case ZEND_EVAL: {
char *eval_desc =
zend_make_compiled_string_description(eval()'d code TSRMLS_CC);
Index: Zend/zend_vm_execute.h
===
RCS file: /repository/ZendEngine2/zend_vm_execute.h,v
retrieving revision 1.62.2.30.2.49.2.40
diff -u -r1.62.2.30.2.49.2.40 zend_vm_execute.h
--- Zend/zend_vm_execute.h 5 Mar 2008 13:34:12 -
1.62.2.30.2.49.2.40
+++ Zend/zend_vm_execute.h 8 Mar 2008 18:35:22 -
@@ -1700,8 +1700,19 @@
}
break;
case ZEND_INCLUDE:
- case ZEND_REQUIRE:
- new_op_array =
compile_filename(Z_LVAL(opline-op2.u.constant), inc_filename TSRMLS_CC);
+ case ZEND_REQUIRE: {
+ char *resolved_path;
+
+ resolved_path =
zend_resolve_path(Z_STRVAL_P(inc_filename), Z_STRLEN_P(inc_filename) TSRMLS_CC);
+ if (resolved_path) {
+ zval rz;
+ ZVAL_STRINGL(rz, resolved_path,
strlen(resolved_path), 0);
+ new_op_array =
compile_filename(Z_LVAL(opline-op2.u.constant), rz TSRMLS_CC);
+ efree(resolved_path);
+ } else {
+ new_op_array =
compile_filename(Z_LVAL(opline-op2.u.constant), inc_filename TSRMLS_CC);
+ }
+ }
break;
case ZEND_EVAL: {
char *eval_desc =
zend_make_compiled_string_description(eval()'d code TSRMLS_CC);
@@ -4885,8 +4896,19 @@
}
break;
case ZEND_INCLUDE:
- case ZEND_REQUIRE:
- new_op_array =
compile_filename(Z_LVAL(opline-op2.u.constant), inc_filename TSRMLS_CC);
+ case ZEND_REQUIRE: {
+ char *resolved_path;
+
+ resolved_path =
zend_resolve_path(Z_STRVAL_P(inc_filename), Z_STRLEN_P(inc_filename) TSRMLS_CC);
+ if (resolved_path) {
+ zval rz;
+ ZVAL_STRINGL(rz, resolved_path,
strlen(resolved_path), 0);
+ new_op_array =
compile_filename(Z_LVAL(opline-op2.u.constant), rz TSRMLS_CC);
+ efree(resolved_path);
+ } else {
+ new_op_array =
compile_filename(Z_LVAL(opline-op2.u.constant), inc_filename TSRMLS_CC);
+ }
+ }
break;
case ZEND_EVAL: {
Re: [PHP-DEV] Re: 5.3 Release Planning
Antony Dovgal wrote: On 07.03.2008 05:43, Gregory Beaver wrote: Just a quick note: I'd like to consider another possible approach, having pecl/phar synced from stable pecl release. I'm not sure it's good idea. IMO it should go trough much more thorough testing to be included into the core. Hi, I wholeheartedly agree that phar needs more testing. I also think some other areas of php could have used more testing, such as the serious issues I found in zlib stream filters, but they were useful for years even with edge case flaws. Areas that I know phar needs work: * big-endian like PPC tar/zip support is incomplete, I need to write the simple byte order reversal that will complete this, and add the same compiler flags to the tar struct that we use on zip structs. This is an easy fix. * finishing up include_path support for streams. Other threads address this issue. This will remove the need for most of the proposed magic in pecl/phar/TODO. * ensuring full code coverage of tests, we're currently at about 68-70% * finishing up Steph's work on data-only tar/zip support (non-executable tar/zip read/write) * 2 open bugs at pecl.php.net * more hammering should be done on the web front controller to ensure edge cases have been considered Areas of phar that are rock-solid: * phar stream read/write * file format support for the 3 file formats read/write * anything else not yet mentioned. I expect to be able to wrap up the issues listed very quickly. Tony has been fantastic at finding memleaks/segfaults as new features are added, and we welcome the minutest scrutiny. And I'm still not convinced we should include any PECL extensions in the core, I believe it should go the other way round. This is secondary, but I'm proposing using phar to test the mechanism that will allow ext/ to migrate to pecl/ and still work as it did in ext/ Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] profiling followup on addition of streams to include_path - it's *faster*
Hi,
When I posted yesterday's patch to add stream support to include_path
(http://news.php.net/php.internals/36031) I mentioned that I suspected
benchmarking would reveal it to be slow. My primary goal is to provide
no impact on current users who are using a traditional include_path,
with a secondary goal of improving performance of those who use the new
syntax. Today I ran callgrind on the thing, with some surprising results.
With the patch, include is *faster* for our traditional users than it is
now.
With the patch, include_once with 1000 unique files is about 3% slower
- not the whole execution, just include_once
With the patch, include_once with 1 unique file included 1 times is
insignificantly slower (about 0.4%)
For these reasons, I'm really encouraged :). The next step is to
absolutely ensure correctness and then see if the streams part of
include_path can be optimized at all (or if it needs it).
Details
==
I just ran callgrind on this script:
?php
set_include_path('.:/usr/local/lib/php:/home/cellog/workspace/php5/ext/phar');
for ($i = 0; $i 10; $i++) {
include 'extra.php';
}
The empty file extra.php (zero byte) is in
/home/cellog/workspace/php5/ext/phar/extra.php ensuring that we
traverse include_path to find it.
To my great shock, the script runs *faster* with my patch, because it
executes significantly more instruction cycles in
php_stream_open_for_zend_ex without the patch.
Note that this does not measure the cost of *_once. *_once is a lot
harder to measure, so I created 10,000 files (yikes) via this script:
?php
for ($i = 1; $i = 1; $i++) file_put_contents('test' . $i, '');
and then ran this test script:
?php
set_include_path('.:/usr/local/lib/php:/home/cellog/workspace/php5/poop');
for ($i = 1; $i = 1; $i++) {
include_once 'test' . $i;
}
callgrind reported that php_resolve_path was about twice as slow as the
other version, resulting in a 3% degradation of include_once performance
over the current version (which is much faster than 5.2.x, incidentally).
Finally, to test the _once aspect of include_once, I ran this script:
?php
set_include_path('.:/usr/local/lib/php:/home/cellog/workspace/php5/ext/phar');
for ($i = 0; $i 10; $i++) {
include_once 'extra.php';
}
With this script, it really highlights the most common use case of
include/require_once: attempting to include the same file multiple
times. The difference in performance was insignificant, with callgrind
reporting a total execution portion of 75.12% for CVS, and 75.57% with
my patch.
So, it looks like the biggest performance hit would be for users
including more than 1000 different files, and would result in
approximately 3% slower performance *of include_once*. I'm curious how
many of our readers have a PHP setup that includes close to this many
files, because it seems rather unlikely to me that anyone would include
more than a few hundred in a single process.
The surprising news is that users who are using include would see a
performance improvement from my patch, so I recommend that portion be
committed regardless of other actions. This improvement proabbly
results from removing an include_path search in plain_wrapper.
Thanks,
Greg
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH] php_resolve_path correctness update (broken in CVS as well)
Hi,
php_resolve_path does not exit if passed a stream wrapper, but instead
tries to find it in include_path. This can cause really weird error
messages and breaks pecl/phar completely (and probably other stream
wrappers), so I added stream wrapper detection to the exit shunt in the
start of php_resolve_path.
The patch is otherwise the same as yesterday's.
Greg
Index: main/fopen_wrappers.c
===
RCS file: /repository/php-src/main/fopen_wrappers.c,v
retrieving revision 1.175.2.3.2.13.2.7
diff -u -r1.175.2.3.2.13.2.7 fopen_wrappers.c
--- main/fopen_wrappers.c 5 Mar 2008 13:34:12 -
1.175.2.3.2.13.2.7
+++ main/fopen_wrappers.c 8 Mar 2008 06:11:31 -
@@ -447,6 +447,9 @@
char resolved_path[MAXPATHLEN];
char trypath[MAXPATHLEN];
char *ptr, *end;
+ php_stream_wrapper *wrapper;
+ const char *p;
+ int n = 0;
if (!filename) {
return NULL;
@@ -462,21 +465,52 @@
return NULL;
}
}
+ /* test for stream wrappers and return */
+ for (p = filename; p - filename filename_length (isalnum((int)*p)
|| *p == '+' || *p == '-' || *p == '.'); p++) {
+ n++;
+ }
+
+ if (n filename_length - 3 (*p == ':') (!strncmp(//, p+1, 2)
|| ( filename_length 4 !memcmp(data, filename, 4 {
+ /* found stream wrapper, this is an absolute path until stream
wrappers implement realpath */
+ return estrndup(filename, filename_length);
+ }
- ptr = path;
+ ptr = (char *) path;
while (ptr *ptr) {
+ int len, is_stream_wrapper = 0, maybe_stream = 1;
+
end = strchr(ptr, DEFAULT_DIR_SEPARATOR);
+#ifndef PHP_WIN32
+ /* search for stream wrapper */
+ if (end - ptr = 1) {
+ maybe_stream = 0;
+ goto not_stream;
+ }
+ for (p = ptr, n = 0; p end (isalnum((int)*p) || *p == '+'
|| *p == '-' || *p == '.'); p++) {
+ n++;
+ }
+
+ if (n == end - ptr *p !strncmp(//, p+1, 2)) {
+ is_stream_wrapper = 1;
+ /* seek to real end of include_path portion */
+ end = strchr(end + 1, DEFAULT_DIR_SEPARATOR);
+ } else {
+ maybe_stream = 0;
+ }
+not_stream:
+#endif
if (end) {
if ((end-ptr) + 1 + filename_length + 1 = MAXPATHLEN) {
ptr = end + 1;
continue;
}
memcpy(trypath, ptr, end-ptr);
+ len = end-ptr;
trypath[end-ptr] = '/';
memcpy(trypath+(end-ptr)+1, filename,
filename_length+1);
ptr = end+1;
} else {
- int len = strlen(ptr);
+ len = strlen(ptr);
if (len + 1 + filename_length + 1 = MAXPATHLEN) {
break;
@@ -486,6 +520,35 @@
memcpy(trypath+len+1, filename, filename_length+1);
ptr = NULL;
}
+
+ if (!is_stream_wrapper maybe_stream) {
+ /* search for stream wrapper */
+ for (p = trypath; isalnum((int)*p) || *p == '+' || *p
== '-' || *p == '.'; p++) {
+ n++;
+ }
+ }
+
+ if (is_stream_wrapper || (n len - 3 (*p == ':') (n 1)
(!strncmp(//, p+1, 2) || !memcmp(data, trypath, 4 {
+ char *actual;
+
+ wrapper = php_stream_locate_url_wrapper(trypath,
actual, STREAM_OPEN_FOR_INCLUDE TSRMLS_CC);
+ if (wrapper == php_plain_files_wrapper) {
+ strncpy(trypath, actual, MAXPATHLEN);
+ } else if (!wrapper) {
+ /* if wrapper is NULL, there was a mal-formed
include_path stream wrapper, so skip this ptr */
+ continue;
+ } else {
+ if (wrapper-wops-url_stat) {
+ php_stream_statbuf ssb;
+
+ if (SUCCESS ==
wrapper-wops-url_stat(wrapper, trypath, 0, ssb, NULL TSRMLS_CC)) {
+ return estrdup(trypath);
+ }
+ }
+ continue;
+ }
+ }
+
if (tsrm_realpath(trypath, resolved_path TSRMLS_CC)) {
return estrdup(resolved_path);
[PHP-DEV] Re: 5.3 Release Planning
Johannes Schlüter wrote: Hi all, recently there were quite a few proposals about stuff for 5.3. If we implement them all we won't finish in a soonish time and we get new ideas postponing the 5.3 release therefore the following: - Scanner based on re2c: Going to re2c promises to make maintenance simpler and increase the parsers performance. Currently the prototype does not provide support for Zend multibyte mode. This seems doable within April. Hence I ask for finishing by end of April the latest. Once that is done we will go over to a freeze and switch to bug fixing only mode. If this is done sooner than we'll freeze sooner, too. If there are delays we'll freeze anyways and use the scanner for a later release. - bundling pecl/intl According to Stas it's ready for being bundled and was voted in. The only question I see there is the how to bundle it. There we should definitely find a better thing than symlinking around on the CVS server, an option might be to to switch to subversion and use svn:externals or keeping stuff in pecl and then having some scripts (maybe part of buildconf/the release scripts) fetching them into a checkout. Just ideas ... For the moment symlinking might be the best. - bundling pecl/phar Lots of work has been done there, I'd like to have it in, general PECL discussion: See above. Hi, Just a quick note: I'd like to consider another possible approach, having pecl/phar synced from stable pecl release. I think this can be done via a simple configure script that wgets the latest stable and uses a clever little PHP_Archive-based phar to extract the source files. There are of course other ways to do this that may be better than my quick-and-dirty description, but it is certainly possible. As the primary maintainer of the PEAR installer, this would make it easier for me to fix issues found in this process. Because phar is new, this will give more wiggle room for experimenting, and what we figure out could then be used to allow the current vaporware proposal of moving all exts to pecl. I am also fine with symlinking until this can be perfected, as it is just an idea to be implemented right now. phar is in a state of slight flux: if stream wrappers get supported inside include_path, it would change a lot of the implementation stuff I've needed to do, and greatly simplify phar, so it will continue to improve with this addition. Greg -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
