Add a constant to reflect --with-curlwrappers

11 years ago by Laruence — view source

unread

Hey:

there are some issues when people run some codes in a php which is
compiled with --with-curlwrappers, like #61336, or the recently test script
for #64433 (failed when curl wrappers enabled).

I know, that the curl wrapper should act the same as php http wrapper,
but for now, we need to provide the ability to user, that they can warn if
they codes can not run with curl wrappers..

here are some really usages:
https://github.com/UnionOfRAD/lithium/issues/59 and
http://weizhifeng.net/wrong-with-curlwrappers.html

I propose to add a constant : bool CURL_WRAPPERS_ENABLE

or, any other better name...

objections?

thanks

Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Hannes Magnusson — view source

unread

Hey:

there are some issues when people run some codes in a php which is
compiled with --with-curlwrappers, like #61336, or the recently test script
for #64433 (failed when curl wrappers enabled).

I know, that the curl wrapper should act the same as php http wrapper,
but for now, we need to provide the ability to user, that they can warn if
they codes can not run with curl wrappers..

here are some really usages:
https://github.com/UnionOfRAD/lithium/issues/59 and
http://weizhifeng.net/wrong-with-curlwrappers.html

I propose to add a constant : bool CURL_WRAPPERS_ENABLE

The curl wrappers have always been a major pain, with plenty of bugs
and we tend to forget to add context options there to match the
standard wrapper on new feature.

It has been marked as experimental since forever, and I think its time
to face the failed experiment and remove it.

-Hannes

11 years ago by Madara Uchiha — view source

unread

I actually agree with that. I've reached a point when I consider using
fsockopen instead of curl to fetch information from a website. I can
hardly stand cURL.

There should be a decent OOP based wrapper for making HTTP requests to
handle it decently. Meanwhile, I agree that cURL needs to go.

On Mon, Apr 1, 2013 at 2:18 AM, Hannes Magnusson
hannes.magnusson@gmail.com wrote:

Hey:

there are some issues when people run some codes in a php which is
compiled with --with-curlwrappers, like #61336, or the recently test script
for #64433 (failed when curl wrappers enabled).

I know, that the curl wrapper should act the same as php http wrapper,
but for now, we need to provide the ability to user, that they can warn if
they codes can not run with curl wrappers..

here are some really usages:
https://github.com/UnionOfRAD/lithium/issues/59 and
http://weizhifeng.net/wrong-with-curlwrappers.html

I propose to add a constant : bool CURL_WRAPPERS_ENABLE

The curl wrappers have always been a major pain, with plenty of bugs
and we tend to forget to add context options there to match the
standard wrapper on new feature.

It has been marked as experimental since forever, and I think its time
to face the failed experiment and remove it.

-Hannes

11 years ago by Pierre Joye — view source

unread

I actually agree with that. I've reached a point when I consider using
fsockopen instead of curl to fetch information from a website. I can
hardly stand cURL.

Curl stream wrappers and ext/curl are two different things. ext/curl
APIs is quite clean and much easier to use than sockets when it comes
to deal with http or other internet related protocols (yes, it
supports ssh2, ftp(s), even imap). What is discussed here is the curl
usage to replace PHP's stream implementation.

Cheers,

Pierre

@pierrejoye

11 years ago by Jordi Boggiano — view source

unread

I propose to add a constant : bool CURL_WRAPPERS_ENABLE

+1 on a constant if it has to stay. We had to do pretty ugly stuff to
detect it in the composer installer
https://github.com/composer/getcomposer.org/blob/master/web/installer#L135-148

The curl wrappers have always been a major pain, with plenty of bugs
and we tend to forget to add context options there to match the
standard wrapper on new feature.

It has been marked as experimental since forever, and I think its time
to face the failed experiment and remove it.

+2 for just removing it. In my experience most/all people using it just
do so by accident, because they copy pasted a configure line that had it
enabled from somewhere 5 years ago and never looked back.

Cheers

--
Jordi Boggiano
@seldaek - http://nelm.io/jordi

11 years ago by Laruence — view source

unread

On Mon, Apr 1, 2013 at 7:18 AM, Hannes Magnusson <hannes.magnusson@gmail.com

wrote:

Hey:

there are some issues when people run some codes in a php which is
compiled with --with-curlwrappers, like #61336, or the recently test
script
for #64433 (failed when curl wrappers enabled).

I know, that the curl wrapper should act the same as php http wrapper,
but for now, we need to provide the ability to user, that they can warn
if
they codes can not run with curl wrappers..

here are some really usages:
https://github.com/UnionOfRAD/lithium/issues/59 and
http://weizhifeng.net/wrong-with-curlwrappers.html

I propose to add a constant : bool CURL_WRAPPERS_ENABLE

The curl wrappers have always been a major pain, with plenty of bugs
and we tend to forget to add context options there to match the
standard wrapper on new feature.

It has been marked as experimental since forever, and I think its time
to face the failed experiment and remove it.

Hey:

remove is also okey for me, but, I think it probably can only happen in
5.6

before that, I think a flag to 5.4+ is a good choice :)

thanks

-Hannes

--
Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Laruence — view source

unread

Added new constant CURL_WRAPPERS_ENABLE in (include 5.4)
https://github.com/php/php-src/commit/d7f709a032a40cb475042b43db07a4698a2488b7

thanks

On Mon, Apr 1, 2013 at 7:18 AM, Hannes Magnusson <
hannes.magnusson@gmail.com> wrote:

Hey:

there are some issues when people run some codes in a php which is
compiled with --with-curlwrappers, like #61336, or the recently test
script
for #64433 (failed when curl wrappers enabled).

I know, that the curl wrapper should act the same as php http
wrapper,
but for now, we need to provide the ability to user, that they can warn
if
they codes can not run with curl wrappers..

here are some really usages:
https://github.com/UnionOfRAD/lithium/issues/59 and
http://weizhifeng.net/wrong-with-curlwrappers.html

I propose to add a constant : bool CURL_WRAPPERS_ENABLE

The curl wrappers have always been a major pain, with plenty of bugs
and we tend to forget to add context options there to match the
standard wrapper on new feature.

It has been marked as experimental since forever, and I think its time
to face the failed experiment and remove it.

Hey:

remove is also okey for me, but, I think it probably can only happen in
5.6

before that, I think a flag to 5.4+ is a good choice :)

thanks

-Hannes

--
Laruence Xinchen Hui
http://www.laruence.com/

--
Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Hannes Magnusson — view source

unread

Wait wait wait. You are introducing a constant that is going to be
available as of 5.4.15 to 5.4.2x and then removed (as it looks like we
are agreeing)?

People have been living without this constant forever now so people
have their workarounds in place and no need to complicate their code
to for the existence of this constant which is only available for few
bugfix releases anyway?

-Hannes

Added new constant CURL_WRAPPERS_ENABLE in (include 5.4)
https://github.com/php/php-src/commit/d7f709a032a40cb475042b43db07a4698a2488b7

thanks

On Mon, Apr 1, 2013 at 7:18 AM, Hannes Magnusson
hannes.magnusson@gmail.com wrote:

Hey:

there are some issues when people run some codes in a php which is
compiled with --with-curlwrappers, like #61336, or the recently test
script
for #64433 (failed when curl wrappers enabled).

I know, that the curl wrapper should act the same as php http
wrapper,
but for now, we need to provide the ability to user, that they can warn
if
they codes can not run with curl wrappers..

here are some really usages:
https://github.com/UnionOfRAD/lithium/issues/59 and
http://weizhifeng.net/wrong-with-curlwrappers.html

I propose to add a constant : bool CURL_WRAPPERS_ENABLE

The curl wrappers have always been a major pain, with plenty of bugs
and we tend to forget to add context options there to match the
standard wrapper on new feature.

It has been marked as experimental since forever, and I think its time
to face the failed experiment and remove it.

Hey:

remove is also okey for me, but, I think it probably can only happen in
5.6

before that, I think a flag to 5.4+ is a good choice :)

thanks

-Hannes

--
Laruence Xinchen Hui
http://www.laruence.com/

--
Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Xinchen Hui — view source

unread

发自我的 iPhone

在 2013-4-3，13:56，Hannes Magnusson hannes.magnusson@gmail.com 写道：

Wait wait wait. You are introducing a constant that is going to be
available as of 5.4.15 to 5.4.2x and then removed (as it looks like we
are agreeing)?
how could that be? This constant will in the final release of 5.5.0.

Thanks

People have been living without this constant forever now so people
have their workarounds in place and no need to complicate their code
to for the existence of this constant which is only available for few
bugfix releases anyway?

-Hannes

Added new constant CURL_WRAPPERS_ENABLE in (include 5.4)
https://github.com/php/php-src/commit/d7f709a032a40cb475042b43db07a4698a2488b7

thanks

On Mon, Apr 1, 2013 at 7:18 AM, Hannes Magnusson
hannes.magnusson@gmail.com wrote:

Hey:

there are some issues when people run some codes in a php which is
compiled with --with-curlwrappers, like #61336, or the recently test
script
for #64433 (failed when curl wrappers enabled).

I know, that the curl wrapper should act the same as php http
wrapper,
but for now, we need to provide the ability to user, that they can warn
if
they codes can not run with curl wrappers..

here are some really usages:
https://github.com/UnionOfRAD/lithium/issues/59 and
http://weizhifeng.net/wrong-with-curlwrappers.html

I propose to add a constant : bool CURL_WRAPPERS_ENABLE

The curl wrappers have always been a major pain, with plenty of bugs
and we tend to forget to add context options there to match the
standard wrapper on new feature.

It has been marked as experimental since forever, and I think its time
to face the failed experiment and remove it.

Hey:

remove is also okey for me, but, I think it probably can only happen in
5.6

before that, I think a flag to 5.4+ is a good choice :)

thanks

-Hannes

--
Laruence Xinchen Hui
http://www.laruence.com/

--
Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Ferenc Kovacs — view source

unread

发自我的 iPhone

在 2013-4-3，13:56，Hannes Magnusson hannes.magnusson@gmail.com 写道：

Wait wait wait. You are introducing a constant that is going to be
available as of 5.4.15 to 5.4.2x and then removed (as it looks like we
are agreeing)?
how could that be? This constant will in the final release of 5.5.0.

Thanks

What Hannes wanted to say that we seem to agree that this experimental
feature was a failure and it should be removed.
If we only remove this in 5.6 then I agree that adding the constant
is worthwhile, but if we would remove it with 5.5.0, then I'm not that
sure, as people already has their workarounds.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

11 years ago by Stas Malyshev — view source

unread

Hi!

Added new constant CURL_WRAPPERS_ENABLE in (include 5.4)
https://github.com/php/php-src/commit/d7f709a032a40cb475042b43db07a4698a2488b7

I must say the process of how it was done was not very good. Not only
there were no substantial discussion on adding this new thing in stable
version before the commit, the time between message announcing it and
asking if there are any objections and the commit was barely a day. It's
not enough time at all to solicit feedback, and this change is not
really something urgent that needs to be committed ASAP. And it turns
out, there are objections. Could we be a little less quick with
commits into a stable version and give it some time when we're talking
about new things and not bugfixes?

Thanks,

Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

11 years ago by Laruence — view source

unread

On Wed, Apr 3, 2013 at 4:22 PM, Stas Malyshev smalyshev@sugarcrm.comwrote:

Hi!

Added new constant CURL_WRAPPERS_ENABLE in (include 5.4)

https://github.com/php/php-src/commit/d7f709a032a40cb475042b43db07a4698a2488b7

I must say the process of how it was done was not very good. Not only
there were no substantial discussion on adding this new thing in stable
version before the commit, the time between message announcing it and
asking if there are any objections and the commit was barely a day. It's
not enough time at all to solicit feedback, and this change is not
really something urgent that needs to be committed ASAP. And it turns
out, there are objections. Could we be a little less quick with
commits into a stable version and give it some time when we're talking
about new things and not bugfixes?

Hey:
I have to say, the objections only shows up after it happens..

but yes,  I am a little rush at this commit, that is because, you can

see, the test script in ext/standard/tests/stream is depends on a ugly
trick to skip . it make me very uncomfortable.

I send the mail the day before yesterday,  and it's really not a big

change (add a constant), then no new feedback in a day, I think it's okey
to commit.

however, you are right, it's not a long time, so if objections becomes

strong, I can revert it.

thanks

Thanks,

Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

--
Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Ferenc Kovacs — view source

unread

On Wed, Apr 3, 2013 at 4:22 PM, Stas Malyshev <smalyshev@sugarcrm.com

wrote:

Hi!

Added new constant CURL_WRAPPERS_ENABLE in (include 5.4)

https://github.com/php/php-src/commit/d7f709a032a40cb475042b43db07a4698a2488b7

I must say the process of how it was done was not very good. Not only
there were no substantial discussion on adding this new thing in stable
version before the commit, the time between message announcing it and
asking if there are any objections and the commit was barely a day. It's
not enough time at all to solicit feedback, and this change is not
really something urgent that needs to be committed ASAP. And it turns
out, there are objections. Could we be a little less quick with
commits into a stable version and give it some time when we're talking
about new things and not bugfixes?

Hey:
I have to say, the objections only shows up after it happens

this happens more frequently if you don't give a chance to the people to
explain their problems before pushing the proposed changes.
this weekend and the monday was a national holiday for many
countries(Easter) so that was really bad timing.

but yes,  I am a little rush at this commit, that is because, you can
see, the test script in ext/standard/tests/stream is depends on a ugly
trick to skip . it make me very uncomfortable.

yeah, and that is what everybody else out there has to live with, so
nothing really changed apart the fact that you also bumped to the issue.
I'm not saying that the situation is ideal, but pushing changes to stable
(5.4) or feature frozen (5.5) branches without using the RFC system and
waiting for the feedback from the RMs is a bad thing to do.
We also have a couple of tests with even more uglier checks (see
http://git.php.net/?p=php-src.git;a=blob_plain;f=Zend/tests/bug55509.phpt;hb=HEAD
for
example), so I don't find that reason that convincing to throw our
processes out of the window.

I send the mail the day before yesterday,  and it's really not a big
change (add a constant), then no new feedback in a day, I think it's okey
to commit.

there were feedback from multiple people that it would be better to remove
the whole thing.
which you replied that it would be ok with you, then pushed your change.

however, you are right, it's not a long time, so if objections becomes

strong, I can revert it.

this is exactly the kind of behavior why we changed from the commit first
then revert when people complaining to the current approach, where we try
to reach a concensus (via discussion on the mailing list or voting through
RFCs) before introducing a change.
the difference between the old and the current approach is that it switches
the burden of proof: instead of forcing others to prove that one's already
commited change is bad and should be reverted, the author has the burden
that his/her change is good enough to get in.
another recent example of the problem with the old approach was the
discussion about DateTimeImmutable: the majority agreed that it was a bad
decision to ship that as-is, but nobody dared to revert it.
hopefully Derick defused that situation, props for that.

ps: if we end up keeping the constant, please rename it to
CURL_WRAPPERS_ENABLED,
Kalle summed up pretty well the reasons.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

11 years ago by Xinchen Hui — view source

unread

发自我的 iPhone

在 2013-4-3，20:42，Ferenc Kovacs tyra3l@gmail.com 写道：

On Wed, Apr 3, 2013 at 4:22 PM, Stas Malyshev <smalyshev@sugarcrm.com

wrote:

Hi!

Added new constant CURL_WRAPPERS_ENABLE in (include 5.4)

https://github.com/php/php-src/commit/d7f709a032a40cb475042b43db07a4698a2488b7

I must say the process of how it was done was not very good. Not only
there were no substantial discussion on adding this new thing in stable
version before the commit, the time between message announcing it and
asking if there are any objections and the commit was barely a day. It's
not enough time at all to solicit feedback, and this change is not
really something urgent that needs to be committed ASAP. And it turns
out, there are objections. Could we be a little less quick with
commits into a stable version and give it some time when we're talking
about new things and not bugfixes?

Hey:
I have to say, the objections only shows up after it happens

but yes,  I am a little rush at this commit, that is because, you can
see, the test script in ext/standard/tests/stream is depends on a ugly
trick to skip . it make me very uncomfortable.

I send the mail the day before yesterday,  and it's really not a big
change (add a constant), then no new feedback in a day, I think it's okey
to commit.

there were feedback from multiple people that it would be better to remove
the whole thing.
which you replied that it would be ok with you, then pushed your change.

however, you are right, it's not a long time, so if objections becomes

strong, I can revert it.

are we talking about add a constant only?

I always very respect the dev process flow,

In this case, one of important reason is : I proposed add a constant , a
tiny change, and IMO doenst worth of a RFC.

Thanks

ps: if we end up keeping the constant, please rename it to
CURL_WRAPPERS_ENABLED,
Kalle summed up pretty well the reasons.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

11 years ago by Hannes Magnusson — view source

unread

however, you are right, it's not a long time, so if objections becomes
strong, I can revert it.
this is exactly the kind of behavior why we changed from the commit first
then revert when people complaining to the current approach, where we try
to reach a concensus (via discussion on the mailing list or voting through
RFCs) before introducing a change.

I've gotta say, even though I disagree with the commit, writing an RFC
for a new constant is a total WTF.
There is absolutely no need for a RFC for it.
Heck, even that initial curtesy mail was more then I would have expected.

-Hannes

11 years ago by Stas Malyshev — view source

unread

Hi!

There is absolutely no need for a RFC for it.
Heck, even that initial curtesy mail was more then I would have expected.

Agree, no need for full scale RFC for one constant. However, sending an
email to the list and actually waiting for feedback is exactly what I
would expect, especially dealing with stable version and feature that it
is not exactly clear what's going on with it. We're not talking about
writing RFCs for every minor change, we're talking about teamwork and
have members of the team be aware of the change and have time to discuss
it if needed. Nothing bad would happen if the same commit would land a
week later, after everybody is behind it and every detail is hashed out
(or not if turns out it is out of consensus). The point here is not to
impede work but to support teamwork.

Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

11 years ago by Hannes Magnusson — view source

unread

Hi!

There is absolutely no need for a RFC for it.
Heck, even that initial curtesy mail was more then I would have expected.

Agree, no need for full scale RFC for one constant. However, sending an
email to the list and actually waiting for feedback is exactly what I
would expect, especially dealing with stable version and feature that it
is not exactly clear what's going on with it. We're not talking about
writing RFCs for every minor change, we're talking about teamwork and
have members of the team be aware of the change and have time to discuss
it if needed. Nothing bad would happen if the same commit would land a
week later, after everybody is behind it and every detail is hashed out
(or not if turns out it is out of consensus). The point here is not to
impede work but to support teamwork.

There is a thin line between impeding work and team work for such a
trivial change.
This constant is actually really useful.
The entire feature is however unfortunately broken, but had it been in
a working shape then common. Really? Send an email and wait a week
before being able to write a testcase?

Anyway. Lets move on.
I suspect removing an experimental feature in an extension that is
disabled by default and requires external library still requires an
RFC?
And according to the current rules of the game it cannot be removed in
5.5.1, but has to be removed in 5.6.0?

-Hannes

11 years ago by Laruence — view source

unread

On Thu, Apr 4, 2013 at 5:37 AM, Hannes Magnusson <hannes.magnusson@gmail.com

wrote:

On Wed, Apr 3, 2013 at 2:00 PM, Stas Malyshev smalyshev@sugarcrm.com
wrote:

Hi!

There is absolutely no need for a RFC for it.
Heck, even that initial curtesy mail was more then I would have
expected.

Agree, no need for full scale RFC for one constant. However, sending an
email to the list and actually waiting for feedback is exactly what I
would expect, especially dealing with stable version and feature that it
is not exactly clear what's going on with it. We're not talking about
writing RFCs for every minor change, we're talking about teamwork and
have members of the team be aware of the change and have time to discuss
it if needed. Nothing bad would happen if the same commit would land a
week later, after everybody is behind it and every detail is hashed out
(or not if turns out it is out of consensus). The point here is not to
impede work but to support teamwork.

There is a thin line between impeding work and team work for such a
trivial change.
This constant is actually really useful.
The entire feature is however unfortunately broken, but had it been in
a working shape then common. Really? Send an email and wait a week
before being able to write a testcase?

Anyway. Lets move on.
I suspect removing an experimental feature in an extension that is
disabled by default and requires external library still requires an
RFC?
And according to the current rules of the game it cannot be removed in
5.5.1, but has to be removed in 5.6.0?

Hey:

I am afraid yes, we can only remove it in 5.6.

now, since I already commit it (I am sorry for rushing then).

and you all agree that the constant is useful, so I think it's okey to
change the constant's name from curl_wrappers_enable

to curl_wrappers_enabled, and only defined when curl is built with
--with-curlwrappers.

then user can simply use if (defined(CURL_WRAPPERS_ENABLED) {}

after this, we can move on to write a RFC about remove the experiment
feature in 5.6, okey?

thanks

-Hannes

--
Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Levi Morrison — view source

unread

Did I just read that even though we have reasons to believe this is not a
good addition that we are still going to launch 5.5 with it?

11 years ago by Laruence — view source

unread

On Thu, Apr 4, 2013 at 1:39 PM, Levi Morrison morrison.levi@gmail.comwrote:

Did I just read that even though we have reasons to believe this is not a
good addition that we are still going to launch 5.5 with it?

where did you read that(not a good addition)?

thanks

--
Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Leigh — view source

unread

I find this somewhat conflicting.

If it's experimental, and will likely be removed because nobody will
maintain it, why is anything being added to it at all?

This simply makes it easier for people to build a dependance on a feature
that will ultimately cease to exist in the next major version.

It's black and white in my opinion. Support it or don't support it.

On Thu, Apr 4, 2013 at 5:37 AM, Hannes Magnusson <
hannes.magnusson@gmail.com

wrote:

On Wed, Apr 3, 2013 at 2:00 PM, Stas Malyshev smalyshev@sugarcrm.com
wrote:

Hi!

There is absolutely no need for a RFC for it.
Heck, even that initial curtesy mail was more then I would have
expected.

Agree, no need for full scale RFC for one constant. However, sending an
email to the list and actually waiting for feedback is exactly what I
would expect, especially dealing with stable version and feature that
it
is not exactly clear what's going on with it. We're not talking about
writing RFCs for every minor change, we're talking about teamwork and
have members of the team be aware of the change and have time to
discuss
it if needed. Nothing bad would happen if the same commit would land a
week later, after everybody is behind it and every detail is hashed out
(or not if turns out it is out of consensus). The point here is not to
impede work but to support teamwork.

There is a thin line between impeding work and team work for such a
trivial change.
This constant is actually really useful.
The entire feature is however unfortunately broken, but had it been in
a working shape then common. Really? Send an email and wait a week
before being able to write a testcase?

Anyway. Lets move on.
I suspect removing an experimental feature in an extension that is
disabled by default and requires external library still requires an
RFC?
And according to the current rules of the game it cannot be removed in
5.5.1, but has to be removed in 5.6.0?

Hey:

I am afraid yes, we can only remove it in 5.6.

now, since I already commit it (I am sorry for rushing then).

and you all agree that the constant is useful, so I think it's okey to
change the constant's name from curl_wrappers_enable

to curl_wrappers_enabled, and only defined when curl is built with
--with-curlwrappers.

then user can simply use if (defined(CURL_WRAPPERS_ENABLED) {}

after this, we can move on to write a RFC about remove the experiment
feature in 5.6, okey?

thanks

-Hannes

--
Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Xinchen Hui — view source

unread

发自我的 iPhone

在 2013-4-4，16:36，Leigh leight@gmail.com 写道：

I find this somewhat conflicting.

If it's experimental, and will likely be removed because nobody will
maintain it, why is anything being added to it at all?

Maintaining it.

This simply makes it easier for people to build a dependance on a feature
that will ultimately cease to exist in the next major version.

Next after next.

It's black and white in my opinion. Support it or don't support it.

Experiment doesn't means never imperove it , right?

Thanks

On Thu, Apr 4, 2013 at 5:37 AM, Hannes Magnusson <
hannes.magnusson@gmail.com

wrote:

On Wed, Apr 3, 2013 at 2:00 PM, Stas Malyshev smalyshev@sugarcrm.com
wrote:

Hi!

There is absolutely no need for a RFC for it.
Heck, even that initial curtesy mail was more then I would have
expected.

Agree, no need for full scale RFC for one constant. However, sending an
email to the list and actually waiting for feedback is exactly what I
would expect, especially dealing with stable version and feature that
it
is not exactly clear what's going on with it. We're not talking about
writing RFCs for every minor change, we're talking about teamwork and
have members of the team be aware of the change and have time to
discuss
it if needed. Nothing bad would happen if the same commit would land a
week later, after everybody is behind it and every detail is hashed out
(or not if turns out it is out of consensus). The point here is not to
impede work but to support teamwork.

There is a thin line between impeding work and team work for such a
trivial change.
This constant is actually really useful.
The entire feature is however unfortunately broken, but had it been in
a working shape then common. Really? Send an email and wait a week
before being able to write a testcase?

Anyway. Lets move on.
I suspect removing an experimental feature in an extension that is
disabled by default and requires external library still requires an
RFC?
And according to the current rules of the game it cannot be removed in
5.5.1, but has to be removed in 5.6.0?

Hey:

I am afraid yes, we can only remove it in 5.6.

now, since I already commit it (I am sorry for rushing then).

and you all agree that the constant is useful, so I think it's okey to
change the constant's name from curl_wrappers_enable

to curl_wrappers_enabled, and only defined when curl is built with
--with-curlwrappers.

then user can simply use if (defined(CURL_WRAPPERS_ENABLED) {}

after this, we can move on to write a RFC about remove the experiment
feature in 5.6, okey?

thanks

-Hannes

--
Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Julien Pauli — view source

unread

IMO , we should remove that feature from 5.5.

If Laruence is OK to maintain it, then we keep it, but Laruence, please,
improve it so.
We keep it if Laruence can make it stable for 5.5 final. If he cant , or
doesn't want to work on it any more while in betas ; then we should remove
curlwrapper from 5.5.

Laruence: If you (or someone else) are motivated to work on it and feel all
right to turn it to a stable feature without breaking streams or other
stuff , then we will discuss a way to detect it.

Why not parse the configure options by adding a new way to do it ?
I'm writing a mail about that now.

Thanks.

Julien.P

11 years ago by Pierrick Charron — view source

unread

I don't think we should remove curlwrappers from 5.5. I do agree that this
is not yet stable and ready to push as non experimental, but since we plan
to release 5.5 soon I don't think removing it right now is worth it.

I started some time ago to maintain the curl extension. I focused mainly on
adding to ext/curl all options from the libcurl api that were not available
in PHP userspace. I also fixed some bugs on curlwrappers and will be please
to fix (or at least try to fix) all bugs that we may have with curl and
curl wrappers in the hope that it will be stable enough to be release with
php nexté

If you have a bug with curlwrapper (or anything related to ext/curl) please
assign me the bug on the tracker and I will try to look at them ASAP.

Thanks

Pierrick

IMO , we should remove that feature from 5.5.

If Laruence is OK to maintain it, then we keep it, but Laruence, please,
improve it so.
We keep it if Laruence can make it stable for 5.5 final. If he cant , or
doesn't want to work on it any more while in betas ; then we should remove
curlwrapper from 5.5.

Laruence: If you (or someone else) are motivated to work on it and feel all
right to turn it to a stable feature without breaking streams or other
stuff , then we will discuss a way to detect it.

Why not parse the configure options by adding a new way to do it ?
I'm writing a mail about that now.

Thanks.

Julien.P

11 years ago by Hannes Magnusson — view source

unread

Hi

I don't think we should remove curlwrappers from 5.5. I do agree that this
is not yet stable and ready to push as non experimental, but since we plan
to release 5.5 soon I don't think removing it right now is worth it.

I started some time ago to maintain the curl extension. I focused mainly on
adding to ext/curl all options from the libcurl api that were not available
in PHP userspace. I also fixed some bugs on curlwrappers and will be please
to fix (or at least try to fix) all bugs that we may have with curl and curl
wrappers in the hope that it will be stable enough to be release with php
nexté

If you have a bug with curlwrapper (or anything related to ext/curl) please
assign me the bug on the tracker and I will try to look at them ASAP.

Its not only about maintaining it.
This experiment failed a long time ago. Overwriting the core streams
has proven itself to be the wrong way.

If there was a way for userspace to say "overload with curl" then thats fine.
We already have a procedure for this:

stream_wrapper_unregister("http");
stream_wrapper_register("http", "CurlStreamWrapper");
and then stream_wrapper_restore("http") to go back to the core streams.

I would definitely see the benefits in something like that, but as
things are now are simply not working and should be removed.
The "real way" can be re-implemented later.

-Hannes

11 years ago by Kalle Sommer Nielsen — view source

unread

2013/4/4 Hannes Magnusson hannes.magnusson@gmail.com:

stream_wrapper_unregister("http");
stream_wrapper_register("http", "CurlStreamWrapper");
and then stream_wrapper_restore("http") to go back to the core streams.

If we alter curlwrappers to work like that, then it could stay, and
since this is an experimental feature we can break BC just as much as
we like for it.

Allowing overrides to core features is indeed a very bad idea and can
cause headaches like this one, I won't go too much into this now as we
also have the phpconfopts() thread going where theres probably gonna
be some more discussions regarding this subject in general.

--
regards,

Kalle Sommer Nielsen
kalle@php.net

11 years ago by Pierre Joye — view source

unread

On Thu, Apr 4, 2013 at 9:52 PM, Hannes Magnusson
hannes.magnusson@gmail.com wrote:

Its not only about maintaining it.
This experiment failed a long time ago. Overwriting the core streams
has proven itself to be the wrong way.

I cannot agree more.

If there was a way for userspace to say "overload with curl" then thats fine.
We already have a procedure for this:

stream_wrapper_unregister("http");
stream_wrapper_register("http", "CurlStreamWrapper");
and then stream_wrapper_restore("http") to go back to the core streams.

I wonder what one will do with open streams during the switches. That
can't go well.

cheers,

Pierre

@pierrejoye

11 years ago by Julien Pauli — view source

unread

On Thu, Apr 4, 2013 at 9:52 PM, Hannes Magnusson
hannes.magnusson@gmail.com wrote:

Its not only about maintaining it.
This experiment failed a long time ago. Overwriting the core streams
has proven itself to be the wrong way.

I cannot agree more.

If there was a way for userspace to say "overload with curl" then thats
fine.
We already have a procedure for this:

stream_wrapper_unregister("http");
stream_wrapper_register("http", "CurlStreamWrapper");
and then stream_wrapper_restore("http") to go back to the core streams.

I wonder what one will do with open streams during the switches. That
can't go well.

Hey, that's true, we may meet strange behavior.

I would say, why not create a new wrapper protocol, aka "curl://".
I know it is more ugly than "http://"; , but "curl://" solves the problem
doesn't it ?

I wonder how we could use curl other protocols than http using curl://
though :p

Julien.P

11 years ago by johannes@schlueters.de — view source

unread

stream_wrapper_unregister("http");
stream_wrapper_register("http", "CurlStreamWrapper");
and then stream_wrapper_restore("http") to go back to the core
streams.

I wonder what one will do with open streams during the switches. That
can't go well.

For open streams there should be no issue - they hold the pointer to
their respective implementation.

The issue I see is that libraries might change that for whatever reasons
and not fix it up before passing control to some other library, thus
creating a hardly debugable mess.

johannes

11 years ago by Julien Pauli — view source

unread

On Fri, Apr 5, 2013 at 12:51 PM, Johannes Schlüter
johannes@schlueters.dewrote:

stream_wrapper_unregister("http");
stream_wrapper_register("http", "CurlStreamWrapper");
and then stream_wrapper_restore("http") to go back to the core
streams.

I wonder what one will do with open streams during the switches. That
can't go well.

For open streams there should be no issue - they hold the pointer to
their respective implementation.

The issue I see is that libraries might change that for whatever reasons
and not fix it up before passing control to some other library, thus
creating a hardly debugable mess.

I'm feeling like we wont be able to make it stable for 5.5 final.

Why not extract the code to pecl, make it stable if possible, then use it
as a pecl extension ?
It could also be merged to ext/curl in some way , that could be another
option.

Julien.Pauli

11 years ago by Hannes Magnusson — view source

unread

On Fri, Apr 5, 2013 at 12:51 PM, Johannes Schlüter johannes@schlueters.de
wrote:

stream_wrapper_unregister("http");
stream_wrapper_register("http", "CurlStreamWrapper");
and then stream_wrapper_restore("http") to go back to the core
streams.

I wonder what one will do with open streams during the switches. That
can't go well.

For open streams there should be no issue - they hold the pointer to
their respective implementation.

The issue I see is that libraries might change that for whatever reasons
and not fix it up before passing control to some other library, thus
creating a hardly debugable mess.

I'm feeling like we wont be able to make it stable for 5.5 final.

Right, I don't think its worth actually fixing this for 5.5, the
current experiment should be removed by 5.5 and then the possibility
to register the curl stream wrapper from userland could be introduced
in 5.5.1 for example.

For now, simply removing the config switch would be the quickest way
to achieve progress with minimal changes back and forth if anyone
wants to make that class.

-Hannes

11 years ago by Julien Pauli — view source

unread

On Fri, Apr 5, 2013 at 8:54 PM, Hannes Magnusson <hannes.magnusson@gmail.com

wrote:

On Fri, Apr 5, 2013 at 12:51 PM, Johannes Schlüter <
johannes@schlueters.de>
wrote:

stream_wrapper_unregister("http");
stream_wrapper_register("http", "CurlStreamWrapper");
and then stream_wrapper_restore("http") to go back to the core
streams.

I wonder what one will do with open streams during the switches. That
can't go well.

For open streams there should be no issue - they hold the pointer to
their respective implementation.

The issue I see is that libraries might change that for whatever reasons
and not fix it up before passing control to some other library, thus
creating a hardly debugable mess.

I'm feeling like we wont be able to make it stable for 5.5 final.

Right, I don't think its worth actually fixing this for 5.5, the
current experiment should be removed by 5.5 and then the possibility
to register the curl stream wrapper from userland could be introduced
in 5.5.1 for example.

For now, simply removing the config switch would be the quickest way
to achieve progress with minimal changes back and forth if anyone
wants to make that class.

Beta3 has been taggued with curl wrappers, and with the
new CURL_WRAPPERS_ENABLED constant :-p

Do we all agree to remove that feature (meaning moving it to a branch, or
somewhere for the interested developers to keep on making it stable) for
5.5 stable or no ?

Julien.Pauli

-Hannes

11 years ago by Pierre Joye — view source

unread

Beta3 has been taggued with curl wrappers, and with the new
CURL_WRAPPERS_ENABLED constant :-p

Do we all agree to remove that feature (meaning moving it to a branch, or
somewhere for the interested developers to keep on making it stable) for 5.5
stable or no ?

I see no chance to get it stable for 5.5, the PECL way is the right
and when it is stable we can re consider it.

Cheers,

Pierre

@pierrejoye

11 years ago by Julien Pauli — view source

unread

Beta3 has been taggued with curl wrappers, and with the new
CURL_WRAPPERS_ENABLED constant :-p

Do we all agree to remove that feature (meaning moving it to a branch, or
somewhere for the interested developers to keep on making it stable) for
5.5
stable or no ?

I see no chance to get it stable for 5.5, the PECL way is the right
and when it is stable we can re consider it.

Ok, too bad we taggued beta3 with it. I suggest we remove this feature when
going RC so.

Thx all,

Julien.Pauli

11 years ago by Xinchen Hui — view source

unread

Beta3 has been taggued with curl wrappers, and with the new
CURL_WRAPPERS_ENABLED constant :-p

Do we all agree to remove that feature (meaning moving it to a branch,
or
somewhere for the interested developers to keep on making it stable)
for 5.5
stable or no ?

I see no chance to get it stable for 5.5, the PECL way is the right
and when it is stable we can re consider it.

Ok, too bad we taggued beta3 with it. I suggest we remove this feature
when going RC so.

Hey:
only if the rfc is accepted before 5.5 final release,,, I don't think
remove this in an minor version is acceptable , isn't it?

thanks

Thx all,

Julien.Pauli

--
惠新宸 laruence
Senior PHP Engineer
http://www.laruence.com

11 years ago by Kalle Sommer Nielsen — view source

unread

Hi Julien

2013/4/11 Julien Pauli jpauli@php.net:

Ok, too bad we taggued beta3 with it. I suggest we remove this feature when
going RC so.

Huge +1

--
regards,

Kalle Sommer Nielsen
kalle@php.net

11 years ago by Pierrick Charron — view source

unread

If you decide to remove it for 5.5 RC, tell me and I'll merge this branch
https://github.com/adoy/php-src/tree/remove-curl-wrappers

Thanks
Pierrick

Beta3 has been taggued with curl wrappers, and with the new
CURL_WRAPPERS_ENABLED constant :-p

Do we all agree to remove that feature (meaning moving it to a branch,
or
somewhere for the interested developers to keep on making it stable)
for 5.5
stable or no ?

I see no chance to get it stable for 5.5, the PECL way is the right
and when it is stable we can re consider it.

Ok, too bad we taggued beta3 with it. I suggest we remove this feature
when going RC so.

Thx all,

Julien.Pauli

11 years ago by Pierre Joye — view source

unread

If you decide to remove it for 5.5 RC, tell me and I'll merge this branch
https://github.com/adoy/php-src/tree/remove-curl-wrappers

s,5.5,all branches,

Cheers,

Pierre

@pierrejoye

11 years ago by Pierrick Charron — view source

unread

Including 5.3 and 5.4 ??

Pierrick

On Thu, Apr 11, 2013 at 4:54 PM, Pierrick Charron pierrick@adoy.net
wrote:

If you decide to remove it for 5.5 RC, tell me and I'll merge this branch
https://github.com/adoy/php-src/tree/remove-curl-wrappers

s,5.5,all branches,

Cheers,

Pierre

@pierrejoye

11 years ago by Pierre Joye — view source

unread

Including 5.3 and 5.4 ??

Yes, why should we have it there? Either it is stable and ready or it
is not (as it is now).

--
Pierre

@pierrejoye

11 years ago by Kalle Sommer Nielsen — view source

unread

2013/4/12 Pierre Joye pierre.php@gmail.com:

Including 5.3 and 5.4 ??

If removed in 5.3 and 5.4, theres no need for the constant anymore.

--
regards,

Kalle Sommer Nielsen
kalle@php.net

11 years ago by Julien Pauli — view source

unread

Hi

2013/4/12 Pierre Joye pierre.php@gmail.com:

On Thu, Apr 11, 2013 at 11:17 PM, Pierrick Charron pierrick@adoy.net
wrote:

Including 5.3 and 5.4 ??

If removed in 5.3 and 5.4, theres no need for the constant anymore.

Right :-)

I agree with Pierre as well, we know this feature leads to bugs, is
experimental, and is so not very very used.

Julien.Pauli

11 years ago by Xinchen Hui — view source

unread

On Fri, Apr 12, 2013 at 1:34 AM, Kalle Sommer Nielsen kalle@php.netwrote:

Hi

2013/4/12 Pierre Joye pierre.php@gmail.com:

On Thu, Apr 11, 2013 at 11:17 PM, Pierrick Charron pierrick@adoy.net
wrote:

Including 5.3 and 5.4 ??

If removed in 5.3 and 5.4, theres no need for the constant anymore.

Right :-)

I agree with Pierre as well, we know this feature leads to bugs, is
experimental, and is so not very very used.

or most usage is by accident... :)

thanks

Julien.Pauli

--
惠新宸 laruence
Senior PHP Engineer
http://www.laruence.com

11 years ago by johannes@schlueters.de — view source

unread

Hi

2013/4/12 Pierre Joye pierre.php@gmail.com:

On Thu, Apr 11, 2013 at 11:17 PM, Pierrick Charron pierrick@adoy.net
wrote:

Including 5.3 and 5.4 ??

If removed in 5.3 and 5.4, theres no need for the constant anymore.

Right :-)

I agree with Pierre as well, we know this feature leads to bugs, is
experimental, and is so not very very used.

5.3 users might depend on some part of the behavior and have learned to
live with bugs. We shouldn't kick "features" at this stage.

johannes

11 years ago by Pierre Joye — view source

unread

On Fri, Apr 12, 2013 at 10:07 AM, Johannes Schlüter
johannes@schlueters.de wrote:

Hi

2013/4/12 Pierre Joye pierre.php@gmail.com:

On Thu, Apr 11, 2013 at 11:17 PM, Pierrick Charron pierrick@adoy.net
wrote:

Including 5.3 and 5.4 ??

If removed in 5.3 and 5.4, theres no need for the constant anymore.

Right :-)

I agree with Pierre as well, we know this feature leads to bugs, is
experimental, and is so not very very used.

5.3 users might depend on some part of the behavior and have learned to
live with bugs. We shouldn't kick "features" at this stage.

Not talking about removing curl stream wrappers but not making it look
stable or add constants that could give this feeling. If there is
really an interest in a curl stream wrapper (for new readers, we are
talking about using curl as a replacement for the php's stream
implemenation, for fopen and other file_get_contents, not the curl_*
functions), then it can be done in PECL and see if it stands more than
a couple of months :)

Cheers,

Pierre

@pierrejoye

11 years ago by Pierrick Charron — view source

unread

On Fri, Apr 12, 2013 at 1:34 AM, Kalle Sommer Nielsen kalle@php.net
wrote:

Hi

2013/4/12 Pierre Joye pierre.php@gmail.com:

On Thu, Apr 11, 2013 at 11:17 PM, Pierrick Charron <
pierrick@adoy.net>
wrote:

Including 5.3 and 5.4 ??

If removed in 5.3 and 5.4, theres no need for the constant anymore.

Right :-)

I agree with Pierre as well, we know this feature leads to bugs, is
experimental, and is so not very very used.

5.3 users might depend on some part of the behavior and have learned to
live with bugs. We shouldn't kick "features" at this stage.

johannes

11 years ago by Ferenc Kovacs — view source

unread

On Wed, Apr 3, 2013 at 10:42 PM, Hannes Magnusson <
hannes.magnusson@gmail.com> wrote:

however, you are right, it's not a long time, so if objections
becomes

strong, I can revert it.

this is exactly the kind of behavior why we changed from the commit first
then revert when people complaining to the current approach, where we try
to reach a concensus (via discussion on the mailing list or voting
through
RFCs) before introducing a change.

I've gotta say, even though I disagree with the commit, writing an RFC
for a new constant is a total WTF.
There is absolutely no need for a RFC for it.
Heck, even that initial curtesy mail was more then I would have expected.

-Hannes

I agree that not every small change requires an RFC (albeit a small change
for one can seem a huge change for another) and I think that this is what I
wrote originally.
I'm glad that Laruence sent the original mail, I just wish that he would
have waited another day or two before the commit.
My reply was meant to address why it is a bad idea to shrug off the
concerns and say that we can revert it later if the complains doesn't stop.

--
Ferenc Kovács
@Tyr43l - http://tyrael.hu

11 years ago by Kalle Sommer Nielsen — view source

unread

2013/4/3 Laruence laruence@php.net:

On Wed, Apr 3, 2013 at 4:22 PM, Stas Malyshev smalyshev@sugarcrm.comwrote:

Hi!

Added new constant CURL_WRAPPERS_ENABLE in (include 5.4)

https://github.com/php/php-src/commit/d7f709a032a40cb475042b43db07a4698a2488b7

I must say the process of how it was done was not very good. Not only
there were no substantial discussion on adding this new thing in stable
version before the commit, the time between message announcing it and
asking if there are any objections and the commit was barely a day. It's
not enough time at all to solicit feedback, and this change is not
really something urgent that needs to be committed ASAP. And it turns
out, there are objections. Could we be a little less quick with
commits into a stable version and give it some time when we're talking
about new things and not bugfixes?

Hey:
I have to say, the objections only shows up after it happens..
but yes,  I am a little rush at this commit, that is because, you can
see, the test script in ext/standard/tests/stream is depends on a ugly
trick to skip . it make me very uncomfortable.
I send the mail the day before yesterday,  and it's really not a big
change (add a constant), then no new feedback in a day, I think it's okey
to commit.
however, you are right, it's not a long time, so if objections becomes
strong, I can revert it.

I think my argument against adding the constant is at least just as
strong as adding it. I agree we shouldn't go RFC/vote etc, over one
single constant but I think we can all agree on the fact that we
shouldn't try to add hacks to fix usage of an experimental feature
(which is essentially what this constant is).

We need more consistency, than inconsistencies.

--
regards,

Kalle Sommer Nielsen
kalle@php.net

11 years ago by Laruence — view source

unread

2013/4/3 Laruence laruence@php.net:

On Wed, Apr 3, 2013 at 4:22 PM, Stas Malyshev <smalyshev@sugarcrm.com
wrote:

Hi!

Added new constant CURL_WRAPPERS_ENABLE in (include 5.4)

https://github.com/php/php-src/commit/d7f709a032a40cb475042b43db07a4698a2488b7
I must say the process of how it was done was not very good. Not only
there were no substantial discussion on adding this new thing in stable
version before the commit, the time between message announcing it and
asking if there are any objections and the commit was barely a day. It's
not enough time at all to solicit feedback, and this change is not
really something urgent that needs to be committed ASAP. And it turns
out, there are objections. Could we be a little less quick with
commits into a stable version and give it some time when we're talking
about new things and not bugfixes?

Hey:
I have to say, the objections only shows up after it happens..
but yes,  I am a little rush at this commit, that is because, you can
see, the test script in ext/standard/tests/stream is depends on a ugly
trick to skip . it make me very uncomfortable.
I send the mail the day before yesterday,  and it's really not a big
change (add a constant), then no new feedback in a day, I think it's
okey
to commit.
however, you are right, it's not a long time, so if objections
becomes

strong, I can revert it.

I think my argument against adding the constant is at least just as
strong as adding it. I agree we shouldn't go RFC/vote etc, over one
single constant but I think we can all agree on the fact that we
shouldn't try to add hacks to fix usage of an experimental feature
(which is essentially what this constant is).

Hey:
but as you see, many people was entrapped by the experimental feature,
and whether the experimental is there is out of their hands.

I, from a user aspect, I need to know whether my project can run in
the host environment. if not, I can warn the user to re-configure it. this
is very important.

now, the hack way , as you can see, to check if the
--with-curlwrappers exists in phpinfo(), doesn't work very well, if the
curl extension is built as shared and separated.

I don't think adding this flag is a hack way, it's just to reflect some
features, like if (class_exists(PDO)), or like (if PHP_VERSION == 5.4)...

thanks

We need more consistency, than inconsistencies.

--
regards,

Kalle Sommer Nielsen
kalle@php.net

--
Laruence Xinchen Hui
http://www.laruence.com/

11 years ago by Kalle Sommer Nielsen — view source

unread

2013/4/3 Laruence laruence@php.net:

Hey:
but as you see, many people was entrapped by the experimental feature,
and whether the experimental is there is out of their hands.

I, from a user aspect, I need to know whether my project can run in the
host environment. if not, I can warn the user to re-configure it. this is
very important.

now, the hack way , as you can see, to check if the --with-curlwrappers
exists in phpinfo(), doesn't work very well, if the curl extension is
built as shared and separated.

I don't think adding this flag is a hack way, it's just to reflect some
features, like if (class_exists(PDO)), or like (if PHP_VERSION == 5.4)...

I see your point, but I think if we start adding "features" to an
experimental feature is a bad idea, it becomes a concern when people
start to rely on experimental features.

I don't think it will be a huge concern for the RMs (cc'd) of 5.5 to
consider removing curlwrappers, then we are done with that once and
for all and we don't even need to add the constant as it would be odd
that just a few versions of 5.4.x would have it.

--
regards,

Kalle Sommer Nielsen
kalle@php.net

11 years ago by Pierre Joye — view source

unread

Hi!

Added new constant CURL_WRAPPERS_ENABLE in (include 5.4)
https://github.com/php/php-src/commit/d7f709a032a40cb475042b43db07a4698a2488b7

I must say the process of how it was done was not very good. Not only
there were no substantial discussion on adding this new thing in stable
version before the commit, the time between message announcing it and
asking if there are any objections and the commit was barely a day. It's
not enough time at all to solicit feedback, and this change is not
really something urgent that needs to be committed ASAP. And it turns
out, there are objections. Could we be a little less quick with
commits into a stable version and give it some time when we're talking
about new things and not bugfixes?

I'd to agree. I was surprised to see a commit this morning.

Cheers,

Pierre

@pierrejoye

11 years ago by Kalle Sommer Nielsen — view source

unread

Hi Laruence

2013/3/31 Laruence laruence@php.net

I propose to add a constant : bool CURL_WRAPPERS_ENABLE

or, any other better name...

objections?

I'm a -1 on this, because as we sort of agreed on (like Hannes
implied), this experimental feature did not turn out as we wanted, its
buggy and nobody maintains it. Currently to figure out if PHP was
built with curlwrappers, theres this dirty hack by printing the
phpinfo() page into an output buffer, to parse it for the build
string, this sucks.

However, since this is an experimental feature, we should either:

Make it work (most unlikely)
Remove it

Instead of adding tiny hacks to make it easier (I know a constant wont
hurt much). Cross version code is also gonna end up with even more
clutter, imagine this (if you want to utilize this new constant, which
is the idea right?):

$curlwrappers = false;

if(defined('CURL_WRAPPERS_ENABLED')) {
$curlwrappers = true;
} else {
ob_start();
phpinfo(INFO_GENERAL);

if(strpos('with-curlwrappers', ob_get_clean()) !== false) {
$curlwrappers = true;
}
}

What I'm saying is (like Hannes), adding a constant thats only gonna
be available for one minor version, is not worth it, we lived years
without it and we can live a little longer until it finally dies.

Also, seeing you already added the constant, it should be named
CURL_WRAPPERS_ENABLED, not ENABLE because it implies that its an
option used in a write context, where this constant is purely meant to
be used in a read context.

--
regards,

Kalle Sommer Nielsen
kalle@php.net

11 years ago by Hannes Magnusson — view source

unread

Hi Laruence

2013/3/31 Laruence laruence@php.net

I propose to add a constant : bool CURL_WRAPPERS_ENABLE

or, any other better name...

objections?

I'm a -1 on this, because as we sort of agreed on (like Hannes
implied), this experimental feature did not turn out as we wanted, its
buggy and nobody maintains it. Currently to figure out if PHP was
built with curlwrappers, theres this dirty hack by printing the
phpinfo() page into an output buffer, to parse it for the build
string, this sucks.

However, since this is an experimental feature, we should either:

Make it work (most unlikely)

Remove it

Instead of adding tiny hacks to make it easier (I know a constant wont
hurt much). Cross version code is also gonna end up with even more
clutter, imagine this (if you want to utilize this new constant, which
is the idea right?):

$curlwrappers = false;

if(defined('CURL_WRAPPERS_ENABLED')) {
$curlwrappers = true;

You'll actually have to assign the value of CURL_WRAPPERS_ENABLED to
$curlwrappers, as defined() only checks if the constant exist.. not if
its set to true :)

-Hannes

11 years ago by Kalle Sommer Nielsen — view source

unread

2013/4/3 Hannes Magnusson hannes.magnusson@gmail.com:

You'll actually have to assign the value of CURL_WRAPPERS_ENABLED to
$curlwrappers, as defined() only checks if the constant exist.. not if
its set to true :)

well, s/defined('CURL_WRAPPERS_ENABLED')/defined('CURL_WRAPPERS_ENABLED')
&& CURL_WRAPPERS_ENABLED/

Doesn't make it prettier (either way) =)

--
regards,

Kalle Sommer Nielsen
kalle@php.net

Add a constant to reflect --with-curlwrappers

thanks

Cheers,

Thanks,

Thanks,

cheers,

Cheers,

Cheers,

Cheers,

Cheers,

Cheers,

OK, I agree with that. I think origination of this was Pierre's comment to apply removal patch to 5.4 and 5.3 but maybe it was just misunderstood and he meant removing the constant. Anyway, removing it from 5.5 would be fine IMO.

OK, I agree with that. I think origination of this was Pierre's comment
to apply removal patch to 5.4 and 5.3 but maybe it was just
misunderstood and he meant removing the constant. Anyway, removing it
from 5.5 would be fine IMO.