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.
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
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 run-tests. php), but still not consistent.
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`/
> > 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, autopkgtest. ubuntu. com/packages/ p/php-imagick/ xenial/ armhf autopkgtest. ubuntu. com/packages/ p/php-imagick/ xenial/ i386 autopkgtest. ubuntu. com/packages/ p/php-imagick/ xenial/ s390x
> but shows failures on i386, armhf and s390x
>
> http://
> http://
> http://
>
> 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. color.phpt) tests for different behavior based upon
This tests (025-get-
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['versionNum ber'] >= 0x692) {
//this is the new correct behaviour
return (intval( round($ someValue, 0, PHP_ROUND_ HALF_UP) ));
//old behaviour had wrong rounding.
return (intval( round($ someValue, 0, PHP_ROUND_ HALF_DOWN) ));
}
else {
}
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