Re: [PATCH] Add php

[Marius Bakke <mbakke@fastmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4796 bytes --]

Julien Lepiller <julien@lepiller.eu> writes:

> On Thu, 17 Nov 2016 21:43:39 +0100
> Marius Bakke <mbakke@fastmail.com> wrote:
>
>> Julien Lepiller <julien@lepiller.eu> writes:
>> 
>> >> >> Unfortunately that only fixed a handful of tests, the remaining
>> >> >> 50-something had to be disabled for a variety of reasons.
>> >> >> 
>> >> >> I've added a commentary to each disabled test. If you recognize
>> >> >> any of these errors/think you know what's going on, please
>> >> >> update the patch. It would be nice to know if the iconv and gd
>> >> >> stuff is expected, and if the two sqlite tests can really be
>> >> >> ignored. The curl one is strange too.  
>> >> >
>> >> > Just as I wanted to send a similar patch ;)
>> >> >
>> >> > I've been looking at some of them. The failing sqlite test is a
>> >> > bug in sqlite that has been fixed last august 
>> >> > (https://sqlite.org/src/info/ef360601). We currently have
>> >> > version 3.14.1, when the latest upstream version is 3.15.1.
>> >> > Updating should fix the problem.
>> >> >
>> >> > 73159 has been fixed in gd:
>> >> > https://github.com/libgd/libgd/issues/289 (more recent than
>> >> > latest gd release unfortunately)
>> >> >
>> >> > 73155 has also been fixed in gd: 
>> >> > https://github.com/libgd/libgd/issues/309 (even more recent)
>> >> >
>> >> > 72482 is fixed here: 
>> >> > https://gist.github.com/anonymous/873314feb4f89bd8336711333299f748
>> >> > (a patch to the bundled libgd)
>> >> >
>> >> > 73213 is fixed here: 
>> >> > https://git.php.net/?p=php-src.git;a=blobdiff;f=ext/gd/libgd/gd.c;h=033d4fa5f0e9740e8b8c397a9038a115c617c419;hp=0b4b42fa27558fa32cc54e14dc297d9d0ba10832;hb=9acfb1a3a5268febb123b7e5fbd4eaf072c83537;hpb=c0219b323e0048440acbdd9ad74624c4bc33c335 
>> >> > (a patch to the bundled libgd)
>> >> >
>> >> > 72339 has a CVE id: 2016-5766, but it should be fixed in libgd
>> >> > 2.2.3 that we have according to the CVE description, and the
>> >> > failure is different from what the report says.
>> >> >
>> >> > 39780 has the unexpected output described in the bug report, so
>> >> > it really fails. I don't think we can fix our libgd though,
>> >> > because the bundled one has some php_* functions that are used
>> >> > to get a warning instead of an error.
>> >> >
>> >> > we could include patches to our libgd to fix two (maybe four)
>> >> > issues. We should also upgrade our sqlite version, but many
>> >> > packages will then have to be rebuilt, or we could create a
>> >> > separate package for the newer version. What do you suggest?  
>> >> 
>> >> Wow, thanks for this list! Including the two upstream gd fixes in a
>> >> "gd-for-php" package should be fine, until a new release of gd is
>> >> out. I'm more vary about including the PHP-specific ones though.
>> >> 
>> >> If there are serious problems with using an external (vanilla) gd,
>> >> I think we either need to maintain a "gd-for-php" package
>> >> indefinitely, or bite the bullet and use the bundled one.
>> >> 
>> >> Do you think it's safe to use our gd? And if not, would you be
>> >> willing to keep up with PHP development and maintain the
>> >> externalized gd component with it?
>> >
>> > Failures in tests caused by external gd are not too serious to
>> > require us to switch to the bundled one I think. We may not even
>> > need to patch our libgd with php specific patches, since the
>> > failures are only slight deviation from the spec on corner cases.
>> > If you prefer that we apply these patches too, then we could, and I
>> > would still try to keep that up to date.
>> 
>> OK. Let's use external gd for now barring any serious issues.
>> 
>> >
>> > What I am more worried about are the iconv crashes. That may be due
>> > to lacking locales though.
>> 
>> You could try commenting them out and adding "glibc-locales" to
>> native-inputs. Not sure if they will get picked up by that however.
>> 
>> A better test may be to try out that particular functionality using
>> the installed version of php. If that works, we can be reasonably
>> sure that dropping the tests is fine.
>> 
>> Attached is the final product, after adding a "gd-for-php" variable
>> with the two upstream patches, as well as sqlite-3.15.1 (separate
>> patch).
>> 
>> I'll push this tomorrow if there are no further comments. Thanks for
>> your perseverance :)
> Just one question: why defining gd-for-php with define, and not
> define-public?

It's to prevent it from showing up when people are searching for 'gd',
and also to prevent it from being included by other files.

It's only a temporary measure until the next version is released, so I
saw no reason to export it.

I'm on my way out, but will commit this tomorrow most likely :-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 454 bytes --]