On 07.03.2016 [06:38:45 -0000], Steve Langasek wrote:
> On Mon, Mar 07, 2016 at 05:09:06AM -0000, Nish Aravamudan wrote:
> > Note that if I hold php-imagick constant and test against the two version
> > of imagemagick (with and without my fixes), I get different results (while
> > upstream php-imagick against upstream imagemagick does not see any issues)
> > -- that to me indicates the bug is missed fixes in imagemagick.
> 
> Not knowing the details of upstream's testing, it could also be that the
> upstream imagemagick build they're testing against doesn't have openmp
> enabled?

I think the upstream maintainer implied it is on, but I'm not 100%. I'll
see if I can see that setting in their CI.

Also, I forgot to mention this earlier, but the segmentation faults I
see are somehow racy -- that is, they don't happen on every execution.
And the upstream maintainer implied to me that this is actually quite
common, it seems like, in the shutdown routines in imagemagick -- and
that a common google-able workaround for Ubuntu is to recompile
imagemagick from source w/o openmp support :( They *are* more
reproducible for me under phpunit (as opposed to `make
tests`/run-tests.php), but still not consistent.

> > Again, I know that seems to be the case, but Debian's results imply
> > something else. Are the launchpad sbuilds multi-cpu?
> 
> Yes, they are.

Ok, I wonder why the results are so different in Debian (and for me on
my laptop).

> > Another big difference between the version of php-imagick, ignore
> > functionality, is a huge increase in coverage tests. I belive
> > php-imagick 3.3.0~rc2 only runs 27 tests, while 3.4.0~rc6 runs 251. More
> > on this below.
> 
> Right, fair enough!

Yeah, it's a (good, but) large difference in coverage.

> So the new php-imagick upload now passes its testsuite on amd64 and ppc64el,
> but shows failures on i386, armhf and s390x
> 
>   http://autopkgtest.ubuntu.com/packages/p/php-imagick/xenial/armhf
>   http://autopkgtest.ubuntu.com/packages/p/php-imagick/xenial/i386
>   http://autopkgtest.ubuntu.com/packages/p/php-imagick/xenial/s390x
> 
> Same test failure on i386 and s390x, and only 1 test failure instead of 20+. 
> Different failure on armhf (a test timeout - possibly a problem of the
> architecture being slow rather than a bug in the code).

Yeah, I'd think the armhf failure is just an issue with the running time
as opposed to a bug. That time limit is hardcoded in the test -- would
you want me to propose upstream changing that value based upon the
platform? Or just increasing it across the board?

> Is this a new test?  Do you believe this test would fail with the previous
> version of php-imagick as well?  Should we bypass this test failure for now
> to get php7 migrated?

It is a new test, and is one I fixed in the updated version of
imagemagick. It was an architectural bug in imagemagick, actually --
where i386 had the correct value for rounding, but all other
architectures didn't.

Ah, this actually goes to the heart of the issue I mentioned earlier.
This tests (025-get-color.phpt) tests for different behavior based upon
the imagemagick version. In our case, we are running 0x689 (base
imagemagick version), but actually it's not upstream 0x689, it's 0x689 +
backports, in particular some from 6.9.2 that fix the code being tested
here.

So the test does this check:

        if ($v['versionNumber'] >= 0x692) {
                //this is the new correct behaviour
                return (intval(round($someValue, 0, PHP_ROUND_HALF_UP)));
        }
        else {
                //old behaviour had wrong rounding.
                return (intval(round($someValue, 0, PHP_ROUND_HALF_DOWN)));
        }

And executes the else-case, but we have the fixes that are in the
if-case.

I'm not sure what to do about this. Technically, we know in this test on
Ubuntu, we should now (on all architectures) be testing the if-case, so
we could carry a patch for the test that removes the conditional?

-- 
Nishanth Aravamudan
Ubuntu Server
Canonical Ltd