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 <pierr...@adoy.net> 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 <anatol....@belski.net> wrote:
>
>>
>>
>> > -----Original Message-----
>> > From: pierr...@webstart.fr [mailto:pierr...@webstart.fr] On Behalf Of
>> Pierrick
>> > Charron
>> > Sent: Wednesday, April 27, 2016 6:20 PM
>> > To: Anatol Belski <anatol....@belski.net>
>> > Cc: Davey Shafik <da...@php.net>; PHP internals <
>> internals@lists.php.net>;
>> > paj...@php.net
>> > 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 <anatol....@belski.net
>> > <mailto:anatol....@belski.net> > wrote:
>> >
>> >
>> >       Hi,
>> >
>> >       > -----Original Message-----
>> >       > From: pierr...@webstart.fr <mailto:pierr...@webstart.fr>
>> > [mailto:pierr...@webstart.fr <mailto:pierr...@webstart.fr> ] On Behalf
>> Of
>> > Pierrick
>> >       > Charron
>> >       > Sent: Wednesday, April 27, 2016 2:20 PM
>> >       > To: Anatol Belski <anatol....@belski.net
>> > <mailto:anatol....@belski.net> >
>> >       > Cc: Davey Shafik <da...@php.net <mailto:da...@php.net> >; PHP
>> > internals <internals@lists.php.net <mailto:internals@lists.php.net> >;
>> >       > paj...@php.net <mailto:paj...@php.net>
>> >       > 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 <anatol....@belski.net
>> > <mailto:anatol....@belski.net>
>> >       > <mailto:anatol....@belski.net <mailto:anatol....@belski.net> >
>> >
>> > wrote:
>> >       >
>> >       >
>> >       >       Hi,
>> >       >
>> >       >       > -----Original Message-----
>> >       >       > From: m...@daveyshafik.com <mailto:m...@daveyshafik.com>
>> > <mailto:m...@daveyshafik.com <mailto:m...@daveyshafik.com> >
>> >       > [mailto:m...@daveyshafik.com <mailto:m...@daveyshafik.com>
>> > <mailto:m...@daveyshafik.com <mailto:m...@daveyshafik.com> > ] On Behalf Of
>> >       > Davey
>> >       >       > Shafik
>> >       >       > Sent: Sunday, April 24, 2016 2:25 AM
>> >       >       > To: Pierrick Charron <pierr...@adoy.net
>> > <mailto:pierr...@adoy.net>  <mailto:pierr...@adoy.net
>> > <mailto:pierr...@adoy.net> > >
>> >       >       > Cc: PHP internals <internals@lists.php.net
>> > <mailto:internals@lists.php.net>
>> >
>> >       > <mailto:internals@lists.php.net <mailto:internals@lists.php.net>
>> > >;
>> > paj...@php.net <mailto:paj...@php.net>  <mailto:paj...@php.net
>> > <mailto:paj...@php.net> >
>> >       >       > 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