Re: Re: ext/curl update

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 <pierrick@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.php@belski.net> wrote: > >> >> >> > -----Original Message----- >> > From: pierrick@webstart.fr [mailto:pierrick@webstart.fr] On Behalf Of >> Pierrick >> > Charron >> > Sent: Wednesday, April 27, 2016 6:20 PM >> > To: Anatol Belski <anatol.php@belski.net> >> > Cc: Davey Shafik <davey@php.net>; PHP internals < >> internals@lists.php.net>; >> > pajoye@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.php@belski.net >> > <mailto:anatol.php@belski.net> > wrote: >> > >> > >> > Hi, >> > >> > > -----Original Message----- >> > > From: pierrick@webstart.fr >> > > <mailto:pierrick@webstart.fr> >> > [mailto:pierrick@webstart.fr >> > <mailto:pierrick@webstart.fr> ] On Behalf >> Of >> > Pierrick >> > > Charron >> > > Sent: Wednesday, April 27, 2016 2:20 PM >> > > To: Anatol Belski <anatol.php@belski.net >> > <mailto:anatol.php@belski.net> > >> > > Cc: Davey Shafik <davey@php.net >> > > <mailto:davey@php.net> >; PHP >> > internals <internals@lists.php.net >> > <mailto:internals@lists.php.net> >; >> > > pajoye@php.net <mailto:pajoye@php.netmF >> > > �~l$��+��{r.y> >> > > 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.php@belski.net >> > <mailto:anatol.php@belski.net> >> > > <mailto:anatol.php@belski.net >> > > <mailto:anatol.php@belski.net> > >> > >> > wrote: >> > > >> > > >> > > Hi, >> > > >> > > > -----Original Message----- >> > > > From: me@daveyshafik.com >> > > > <mailto:me@daveyshafik.com> >> > <mailto:me@daveyshafik.com >> > <mailto:me@daveyshafik.com> > >> > > [mailto:me@daveyshafik.com >> > > <mailto:me@daveyshafik.com> >> > <mailto:me@daveyshafik.com >> > <mailto:me@daveyshafik.com> > ] On Behalf Of >> > > Davey >> > > > Shafik >> > > > Sent: Sunday, April 24, 2016 2:25 AM >> > > > To: Pierrick Charron <pierrick@adoy.net >> > <mailto:pierrick@adoy.net> >> > <mailto:pierrick@adoy.net >> > <mailto:pierrick@adoy.net> > > >> > > > Cc: PHP internals <internals@lists.php.net >> > <mailto:internals@lists.php.net> >> > >> > > <mailto:internals@lists.php.net >> > > <mailto:internals@lists.php.net> >> > >; >> > pajoye@php.net <mailto:pajoye@php.net> >> > <mailto:pajoye@php.net >> > <mailto:pajoye@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 >> >> >