Re: [PHP-DEV] [RFC] Factory for Stream Wrappers
hi! I like the idea to improve this area. However I'm with Stas here. I do not feel comfortable enough with this RFC to agree to get into 5.4 at this stage. It is a sensible area and we are already late (because of the tests) with the 5.4 release plan. Next will begin in spring next year, not too far away from now :) Cheers, On Tue, Sep 13, 2011 at 12:55 AM, Stas Malyshev wrote: > Hi! > > On 9/12/11 3:45 PM, Gustavo Lopes wrote: >> >> It's a change in behavior so it makes sense. Those operations are wrapper >> operations and by their nature they are static operations, meaning a >> stream instance is not required. See the difference between php_stream_ops >> and php_stream_wrapper_ops. It would be a minor change. > > I must say I am feeling really uneasy about "minor changes" days before > beta. For later though it may make sense... > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > > -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Factory for Stream Wrappers
Hi! On 9/12/11 3:45 PM, Gustavo Lopes wrote: It's a change in behavior so it makes sense. Those operations are wrapper operations and by their nature they are static operations, meaning a stream instance is not required. See the difference between php_stream_ops and php_stream_wrapper_ops. It would be a minor change. I must say I am feeling really uneasy about "minor changes" days before beta. For later though it may make sense... -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Factory for Stream Wrappers
On Mon, 12 Sep 2011 18:00:13 +0100, Hannes Magnusson wrote: On Mon, Sep 12, 2011 at 16:56, Gustavo Lopes wrote: Ah, right. stat() and unlink() should have been static in the first place. Given the circumstances, it might be a good idea, to document url_stat, unlink, rename, mkdir and rmdir to be static and change the implementation to: * not instantiate an object if the method is static * keep the current behavior if it's not static (but emit E_STRICT) That seems like a change in behavior just to change it. It's a change in behavior so it makes sense. Those operations are wrapper operations and by their nature they are static operations, meaning a stream instance is not required. See the difference between php_stream_ops and php_stream_wrapper_ops. It would be a minor change. Continuing on the off-topic notes.. I would rather go with an alternative implementation based upon interfaces (i.e. dirwrapper, filesystemwrapper, streamwrapper...) and spec it out before adding random magical things that took over 7 years for people to figure out and document. And lets not mention the stream_filter_register() and stream_bucket_new() voodoo. The stream filtering API is a failed feature, IMO. It has the complexity of APR buckets and brigades without their performance benefits (memory is still copied all the time) -- and of course it's not properly documented. -- Gustavo Lopes -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_4/ext/session/tests/rfc1867_invalid_settings.phpt branches/PHP_5_4/ext/session/tests/rfc1867_invalid_settings_2.phpt trunk/ext/session/tes
I forget to reply-all to the list :/ On Mon, Sep 12, 2011 at 7:46 PM, Stas Malyshev wrote: > Hi! > > On 9/12/11 3:14 AM, Ferenc Kovacs wrote: >> >> you should pass both -n and -c as the make test pass those to the >> run-tests.php AFAIK, see: > > That's not what I see happening. > >> as I mentioned above, you should have the -n -c arguments, if you >> don't then this can cause the differencies. > > That's not what I see run-tests is doing. > >> what do you mean by "my php.ini"? for the test runs you shouldn't use >> any external php.ini, as it can cause differences between the test >> results. >> > > Maybe it shouldn't, but that's what happens. It doesn't use neither -c nor > -n option. before we continue further with debugging, could someone else verify my reasoning, or repeat the those test steps? I'm pretty sure that the make test/run-test.php should run the tests without any external php.inis except the tmp-php.ini (passing -n -c tmp-php.ini). it is a possibility that Stas somehow borked up his environment, but as the gcov machine also generated the same output (at least for those session tests) I would like to know that what causes this different behavior for Stas and me, and which is the correct. thanks. -- Ferenc Kovács @Tyr43l - http://tyrael.hu -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] APC in 5.4
On Mon, Sep 12, 2011 at 7:42 PM, Stas Malyshev wrote: > Hi! > > On 9/12/11 5:09 AM, Ilia Alshanetsky wrote: >> >> The agreement to include apc in 5.4 is an old one, unfortunately the >> action of doing was just missed. Also, inclusion of the extension >> won't break any code since it is self contained... > > If it's an "old one", how comes nobody ever mentioned it during all the > extensive process of naming features, voting, etc. and didn't bother to > propose it? There was a discussion and the conclusion was that APC was not ready for inclusion. See my other reply. > I don't feel good about first agreeing to a set of rules and then > immediately starting to throw them away "just this time, just for this > occasion". Why have rules and release cycles then? -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] APC in 5.4
Hi! On 9/12/11 5:09 AM, Ilia Alshanetsky wrote: The agreement to include apc in 5.4 is an old one, unfortunately the action of doing was just missed. Also, inclusion of the extension won't break any code since it is self contained... If it's an "old one", how comes nobody ever mentioned it during all the extensive process of naming features, voting, etc. and didn't bother to propose it? I don't feel good about first agreeing to a set of rules and then immediately starting to throw them away "just this time, just for this occasion". Why have rules and release cycles then? -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Factory for Stream Wrappers
On Mon, Sep 12, 2011 at 16:56, Gustavo Lopes wrote: > Em Mon, 12 Sep 2011 15:40:20 +0100, Christian Kaps > escreveu: > >> On Mon, 12 Sep 2011 14:43:33 +0100, Gustavo Lopes wrote: >>> >>> Em Mon, 12 Sep 2011 12:53:13 +0100, Sebastian Bergmann >>> escreveu: >>> Regarding state it is important to notice that PHP does *not* execute the constructor on all low level calls when instantiating the wrapper class - for whatever reason that is the case. Changing that behaviour would cause quite some side effects, with possible quite some BC breaks. >>> >>> This is a bit off-topic, but could you elaborate on this (possibly >>> submitting a bug report)? I see user_wrapper_opener trying to call the >>> constructor: >>> >>> http://lxr.php.net/opengrok/xref/PHP_5_3/main/streams/userspace.c#298 >>> >>> So any failure to do this would be a bug. >>> >> >> Not sure if it's actual, but this behaviour is documented: >> http://de3.php.net/manual/en/streamwrapper.construct.php#94731 >> > > Ah, right. stat() and unlink() should have been static in the first place. > > Given the circumstances, it might be a good idea, to document url_stat, > unlink, rename, mkdir and rmdir to be static and change the implementation > to: > > * not instantiate an object if the method is static > * keep the current behavior if it's not static (but emit E_STRICT) That seems like a change in behavior just to change it. Continuing on the off-topic notes.. I would rather go with an alternative implementation based upon interfaces (i.e. dirwrapper, filesystemwrapper, streamwrapper...) and spec it out before adding random magical things that took over 7 years for people to figure out and document. And lets not mention the stream_filter_register() and stream_bucket_new() voodoo. -Hannes -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Factory for Stream Wrappers
Em Mon, 12 Sep 2011 15:40:20 +0100, Christian Kaps escreveu: On Mon, 12 Sep 2011 14:43:33 +0100, Gustavo Lopes wrote: Em Mon, 12 Sep 2011 12:53:13 +0100, Sebastian Bergmann escreveu: Regarding state it is important to notice that PHP does *not* execute the constructor on all low level calls when instantiating the wrapper class - for whatever reason that is the case. Changing that behaviour would cause quite some side effects, with possible quite some BC breaks. This is a bit off-topic, but could you elaborate on this (possibly submitting a bug report)? I see user_wrapper_opener trying to call the constructor: http://lxr.php.net/opengrok/xref/PHP_5_3/main/streams/userspace.c#298 So any failure to do this would be a bug. Not sure if it's actual, but this behaviour is documented: http://de3.php.net/manual/en/streamwrapper.construct.php#94731 Ah, right. stat() and unlink() should have been static in the first place. Given the circumstances, it might be a good idea, to document url_stat, unlink, rename, mkdir and rmdir to be static and change the implementation to: * not instantiate an object if the method is static * keep the current behavior if it's not static (but emit E_STRICT) -- Gustavo Lopes -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Factory for Stream Wrappers
On Mon, 12 Sep 2011 14:43:33 +0100, Gustavo Lopes wrote: Em Mon, 12 Sep 2011 12:53:13 +0100, Sebastian Bergmann escreveu: Regarding state it is important to notice that PHP does *not* execute the constructor on all low level calls when instantiating the wrapper class - for whatever reason that is the case. Changing that behaviour would cause quite some side effects, with possible quite some BC breaks. This is a bit off-topic, but could you elaborate on this (possibly submitting a bug report)? I see user_wrapper_opener trying to call the constructor: http://lxr.php.net/opengrok/xref/PHP_5_3/main/streams/userspace.c#298 So any failure to do this would be a bug. Not sure if it's actual, but this behaviour is documented: http://de3.php.net/manual/en/streamwrapper.construct.php#94731 Christian -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Factory for Stream Wrappers
Em Mon, 12 Sep 2011 12:53:13 +0100, Sebastian Bergmann escreveu: Regarding state it is important to notice that PHP does *not* execute the constructor on all low level calls when instantiating the wrapper class - for whatever reason that is the case. Changing that behaviour would cause quite some side effects, with possible quite some BC breaks. This is a bit off-topic, but could you elaborate on this (possibly submitting a bug report)? I see user_wrapper_opener trying to call the constructor: http://lxr.php.net/opengrok/xref/PHP_5_3/main/streams/userspace.c#298 So any failure to do this would be a bug. BTW, another advantage of the proposed change is that you can have stream wrappers with constructors that take arguments. For the record, I'm in favor of this, even though I don't buy the testability claim. My point yesterday was mainly the RFC did not explain the advantages very clearly. -- Gustavo Lopes -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] ZE2 broken by newer gcc
On 09/12/2011 08:45 AM, Dmitry Stogov wrote:
Hi Flavius,
Unfortunately, the proposed fix is wrong.
It changes the operators precedence and it's definitely wrong.
I suppose the crash caused by side effect of some other bug.
Try to run the same script with valgrind.
Thanks. Dmitry.
Please test the following POC with gcc 4.6.1 on x64 first (perhaps
slightly earlier versions behave the same, but this one is for sure),
the dump_container() function simulates what the ZE does in
zend_call_function() and many other places. You can also look at what
valgrind says too and compare. I haven't dwelt on it at the ASM level,
but certainly something has changed. My bet is that GCC has had wrong
the operator precedence, and now they've fixed it.
I'm not speaking from my imagination, I'm speaking from what I've seen
and what I've tested, in practice. No theory. In the "meta" extension I
am calling zend_call_function(), which is how I've discovered this. The
POC resembles what is happenning as close as possible.
#include
#include
#define NUMS 10
/* a "zval" */
typedef struct _foo {
char *t;
int i;
} foo;
/* a "zend_fcall_info" */
typedef struct _foo_container {
foo ***foos;
} container;
foo** init_foos(void) {
foo** f, *f2;
int i;
f = malloc(sizeof(foo*)*NUMS);
for(i=0; i < NUMS; i++) {
f2 = malloc(sizeof(foo));
printf("allocated %p ", f2);
f[i] = f2;
f[i]->i = i*11+1;
printf("with value %d\n", i*11+1);
}
return f;
}
void dump_container(container *c) {
int i;
for(i=0; i < NUMS; i++) {
//working:
printf("foo[%d]=%d at %p\n", i, (*c->foos)[i]->i, (*c->foos)[i]);
//similar to ZE, crashing (not at i=0, but on my machine at i=1
it already delivers garbage, at i=2 it finnaly crashes
//if you're lucky, increase NUMS at the top
printf("foo[%d]=%d at %p\n", i, (*c->foos)[i]->i, *c->foos[i]);
}
}
int main(void) {
container c;
foo **foos;
foos = init_foos();
c.foos = &foos;
int i;
for(i=0; i < NUMS; i++) {
printf("%d %d at %p\n", i, foos[i]->i, foos[i]);
}
printf("--\n");
dump_container(&c);
free(foos);
return EXIT_SUCCESS;
}
--
What I cannot create, I do not understand. -- Feynman
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] APC in 5.4
The agreement to include apc in 5.4 is an old one, unfortunately the action of doing was just missed. Also, inclusion of the extension won't break any code since it is self contained... On Thu, Sep 8, 2011 at 9:07 PM, Stas Malyshev wrote: > Hi! > > On 9/8/11 6:04 PM, Kalle Sommer Nielsen wrote: >> >> This have been a while since we had the discussions about APC in 5.4 >> (or in general extensions to move in and out of the Core, but more >> about that in another thread). > > Not saying anything specifically about APC, I think it is way too late to > propose major features for 5.4 when we're days before beta. So if it's 5.4, > it's not 5.4.0 for sure. > > -- > Stanislav Malyshev, Software Architect > SugarCRM: http://www.sugarcrm.com/ > (408)454-6900 ext. 227 > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] Factory for Stream Wrappers
Am 12.09.2011 00:26, schrieb Gustavo Lopes: > A patch against trunk (or 5.4) would have been nicer. Other than that: > * This patch has a huge BC: Johannes already said that he is updating his patch. It is not our intention to break BC, we only want to add additional/optional behaviour. > Could you add some examples substantiating these claims? I have some doubts. No matter how you implement the actual instantiation code in user land it should be obvious that you always have more control over it than having PHP do it for you in a "static" way. The usual case for a factory of course would be having an object instance where a method would be called - passing in array($obj, 'method') as the callback. But even a classic function has more control already, since it can decide which class to actually instantiate and what parameters it might need to set. Regarding state it is important to notice that PHP does *not* execute the constructor on all low level calls when instantiating the wrapper class - for whatever reason that is the case. Changing that behaviour would cause quite some side effects, with possible quite some BC breaks. > If, however, the factory is an object method, I see that you could change > state in the object and the factory method would have access to this new > state (and could inject in the wrapper object). Do you still need a use case for this? The main use case for Stefan and Arne was that we want to use a wrapper to map paths into protocols (e.g. have foo:// me mapped to $basedir . '/foo/') or define a memcached:// streamwrapper that obviously needs a server to talk to. This is usually a configuration that would be injected into the factory and from there be passed on to the instance. I do not have an example at hand that shows how this feature will help with testability, for that I am sorry. Maybe Benjamin can elaborate on what his use case is. -- Sebastian BergmannCo-Founder and Principal Consultant http://sebastian-bergmann.de/ http://thePHP.cc/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [PHP-CVS] svn: /php/php-src/ branches/PHP_5_4/ext/session/tests/rfc1867_invalid_settings.phpt branches/PHP_5_4/ext/session/tests/rfc1867_invalid_settings_2.phpt trunk/ext/session/tes
Hello, these tests fail for me too, ubuntu 11.04 x86, both trunk and 5.4 branches. 2011/9/12 Ferenc Kovacs : > On Sun, Sep 11, 2011 at 11:04 PM, Stas Malyshev > wrote: >> Hi! >> >> On 9/11/11 8:44 AM, Ferenc Kovacs wrote: >>> >>> AFAIK you shoud get it(as I did on my debian machines), as both >>> display_startup_errors and error_reporting is force enabled for the >>> tests: >>> http://svn.php.net/viewvc/php/php-src/trunk/run-tests.php?view=markup#l232 >>> http://svn.php.net/viewvc/php/php-src/trunk/run-tests.php?view=markup#l230 >> >> Well, maybe I should, but I am not. If you think it's a bug and have a fix >> you're welcome to propose it. >> > > I did, and you just reverted it, this is why I'm curious. :) > could you please run the following (basically running those 2 tests > with verbose output): > TEST_PHP_EXECUTABLE=auto TEST_PHP_CGI_EXECUTABLE=auto ./sapi/cli/php > run-tests.php -g 'SKIP,FAIL,XFAIL' -n -c tmp-php.ini -v > ext/session/tests/rfc1867_invalid_settings*.phpt > I would like to know how does your COMMAND look like. > mine is something like: > /home/tyrael/checkouts/php-src/trunk/sapi/cli/php -n -c 'tmp-php.ini' > -d "output_handler=" -d "open_basedir=" -d "safe_mode=0" -d > "disable_functions=" -d "output_buffering=Off" -d > "error_reporting=32767" -d "display_errors=1" -d > "display_startup_errors=1" -d "log_errors=0" -d "html_errors=0" -d > "track_errors=1" -d "report_memleaks=1" -d "report_zend_debug=0" -d > "docref_root=" -d "docref_ext=.html" -d "error_prepend_string=" -d > "error_append_string=" -d "auto_prepend_file=" -d "auto_append_file=" > -d "magic_quotes_runtime=0" -d "ignore_repeated_errors=0" -d > "precision=14" -d "unicode.runtime_encoding=ISO-8859-1" -d > "unicode.script_encoding=UTF-8" -d "unicode.output_encoding=UTF-8" -d > "unicode.from_error_mode=U_INVALID_SUBSTITUTE" -d > "session.auto_start=0" -d "tidy.clean_output=0" -d > "zlib.output_compression=Off" -d "mbstring.func_overload=0" -d > "session.upload_progress.freq=-1" -f > "/home/tyrael/checkouts/php-src/trunk/ext/session/tests/rfc1867_invalid_settings.php" > 2>&1 > > and the second thing: > could you run the following: > ./sapi/cli/php -n -d session.upload_progress.freq=200% -d > error_reporting=-1 -d display_errors=1 -d display_startup_errors=1 -r > '' > this outputs for me: > PHP Warning: PHP Startup: session.upload_progress.freq cannot be over > 100% in Unknown on line 0 > > Warning: PHP Startup: session.upload_progress.freq cannot be over 100% > in Unknown on line 0 > > I verified this with multiple php version on multiple machines, if you > have both display_errors and display_startup_errors (and appropriate > error_reporting level), you should see those two lines. > btw: you can see from the reports that others also seeing this behaviour: > http://qa.php.net/reports/viewreports.php?version=5.4.0beta1-dev&test=%2Fext%2Fsession%2Ftests%2Frfc1867_invalid_settings.phpt > http://qa.php.net/reports/viewreports.php?version=5.4.0beta1-dev&test=%2Fext%2Fsession%2Ftests%2Frfc1867_invalid_settings_2.phpt > > -- > Ferenc Kovács > @Tyr43l - http://tyrael.hu > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php > > -- Regards, Shein Alexey -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH]#55651
hi!
On Mon, Sep 12, 2011 at 2:29 AM, Avi Brender wrote:
> Hi,
>
> Please see if the attached patch better addresses your concerns.
>
> Regarding the variable name, the PHP_FTP_OPT_USEPASVADDRESS is only internal
> and is modeled after the other variables PHP_FTP_TIMEOUT_SEC and
> PHP_FTP_OPT_AUTOSEEK. The variable actually passed to the ftp_set_option()
> function by users is FTP_USEPASVADDRESS as defined in php_ftp.c which is
> inline with the other variables FTP_AUTORESUME, FTP_TIMEOUT_SEC,
> FTP_AUTOSEEK etc.
+ REGISTER_LONG_CONSTANT("FTP_USEPASVADDRESS",
PHP_FTP_OPT_USEPASVADDRESS, CONST_PERSISTENT | CONST_CS);
It is a userland constant too.
Everyone understands FTP_AUTORESUME or FTP_TIMEOUT_SEC,
_FTP_OPT_USEPASVADDRESS is cryptic compared to the other :)
> In terms of tests, what type of tests were you thinking of? We can't ensure
> that ftp->pasvaddr is set properly in response to a PASV command unless
> there's a way to expose those internal variables to the test - I'm not
> familiar enough with the internal PHP code to know if that's possible. If
> you're referring to tests that ensure that 1/0 true/false values passed to
> ftp_set_option() work properly then I've attached a test file for that.
>
> I don't want to clutter the bugzilla ticket with many attachments so once
> the patch is approved I will post the final version in the ticket if that's
> okay.
That's fine too. You can the tests with the patch too, just do svn add
ext/ftp/tests/... before you call svn diff.
Thanks for your work so far!
Cheers,
--
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
