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