Sorry for the 2 mails but I forgot to give you the URL :

https://github.com/php/php-src/pull/1890/files

On 27 April 2016 at 19:14, Pierrick Charron <[email protected]> wrote:

> Hi Anatol,
>
> I created a new patch from the one first published but this time this one
> target 7.0 and only expose new constants to that do not require any logic
> on the extension side.
> These constants are just exposed if they are available in the version
> installed and are bridge in the curl_setop function.
>
> If that's OK I'll commit this on 7.0 and merge it also on master. Then
> I'll work on adding new things that require logic and clean those for 7.1
> and add tests if possible.
>
> Regards
> Pierrick
>
> On 27 April 2016 at 12:55, Anatol Belski <[email protected]> wrote:
>
>>
>>
>> > -----Original Message-----
>> > From: [email protected] [mailto:[email protected]] On Behalf Of
>> Pierrick
>> > Charron
>> > Sent: Wednesday, April 27, 2016 6:20 PM
>> > To: Anatol Belski <[email protected]>
>> > Cc: Davey Shafik <[email protected]>; PHP internals <
>> [email protected]>;
>> > [email protected]
>> > Subject: Re: [PHP-DEV] Re: ext/curl update
>> >
>> > Yep I'll check if I can add some test that could be easy to implement
>> using
>> > curl_easy_getinfo or using the php local server. Otherwise not sure we
>> could
>> > easily compile PHP with all those libcurl versions...
>> >
>> >
>> > On 27 April 2016 at 11:37, Anatol Belski <[email protected]
>> > <mailto:[email protected]> > wrote:
>> >
>> >
>> >       Hi,
>> >
>> >       > -----Original Message-----
>> >       > From: [email protected] <mailto:[email protected]>
>> > [mailto:[email protected] <mailto:[email protected]> ] On Behalf
>> Of
>> > Pierrick
>> >       > Charron
>> >       > Sent: Wednesday, April 27, 2016 2:20 PM
>> >       > To: Anatol Belski <[email protected]
>> > <mailto:[email protected]> >
>> >       > Cc: Davey Shafik <[email protected] <mailto:[email protected]> >; PHP
>> > internals <[email protected] <mailto:[email protected]> >;
>> >       > [email protected] <mailto:[email protected]>
>> >       > Subject: Re: [PHP-DEV] Re: ext/curl update
>> >       >
>> >       > I agree, but I don't really now how I could test those things
>> since they
>> > almost all
>> >       > of the time only affect how libcurl will handle the
>> request/cache and
>> > we have no
>> >       > way to retrieve options like curl_easy_getopt or something
>> similar.
>> >       >
>> >       > On 27 April 2016 at 02:46, Anatol Belski <[email protected]
>> > <mailto:[email protected]>
>> >       > <mailto:[email protected] <mailto:[email protected]> >
>> >
>> > wrote:
>> >       >
>> >       >
>> >       >       Hi,
>> >       >
>> >       >       > -----Original Message-----
>> >       >       > From: [email protected] <mailto:[email protected]>
>> > <mailto:[email protected] <mailto:[email protected]> >
>> >       > [mailto:[email protected] <mailto:[email protected]>
>> > <mailto:[email protected] <mailto:[email protected]> > ] On Behalf Of
>> >       > Davey
>> >       >       > Shafik
>> >       >       > Sent: Sunday, April 24, 2016 2:25 AM
>> >       >       > To: Pierrick Charron <[email protected]
>> > <mailto:[email protected]>  <mailto:[email protected]
>> > <mailto:[email protected]> > >
>> >       >       > Cc: PHP internals <[email protected]
>> > <mailto:[email protected]>
>> >
>> >       > <mailto:[email protected] <mailto:[email protected]>
>> > >;
>> > [email protected] <mailto:[email protected]>  <mailto:[email protected]
>> > <mailto:[email protected]> >
>> >       >       > Subject: [PHP-DEV] Re: ext/curl update
>> >       >       >
>> >       >       > Hi Pierrick,
>> >       >       >
>> >       >       > This should be in master for 7.1, alongside my RFC'ed
>> patch for
>> > server
>> >       > push
>> >       >       > support.
>> >       >       >
>> >       >       > You emailed me directly about the aforementioned patch
>> so I'll
>> > just
>> >       > respond
>> >       >       > here as it's relevant:
>> >       >       >
>> >       >       > The patch should hit in 7.1 but it has been requested
>> that tests be
>> >       > added — and
>> >       >       > we can't add tests with a server push supporting HTTP/2
>> server
>> > against
>> >       > which to
>> >       >       > push.
>> >       >       >
>> >       >       As from the patch, many constants have nothing to do with
>> HTTP/2
>> >       > implementation and add just name/value without any further
>> logic. If
>> > there were
>> >       > a reduced patch with only such cases, it would be acceptable
>> for 7.0
>> > as well and
>> >       > there were probably no collisions expected. What do you think?
>> >       >
>> >
>> >       So far I understood tests are exactly about HTTP2. Not sure how
>> you
>> > would tests all the constants present in libcurl. Would need to rebuild
>> with a
>> > dozen libcurl versions, but the documentation and compile time version
>> check
>> > are already reliable things.
>> >
>> But if you can fish out only the cases with name/value which don't
>> interfere with the HTTP2 work, so it's fine to add. OFC it were absurd to
>> recompile with all libcurl versions :) especially as an excellent
>> documentation to every option is presend on the cURL side. If that's only
>> an option that say affects the curl behavior and don't require any extra
>> handling, I don't think it is critical. On the other hand, if an option
>> requires some pre/post handling an thus some extra implementation - then
>> OFC it should be urgently suggested to have a good test.
>>
>> Thanks
>>
>> Anatol
>>
>>
>

Reply via email to