Re: PHP on core-updates

[Marius Bakke <mbakke@fastmail.com>]

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

julien lepiller <julien@lepiller.eu> writes:

> I've "fixed" the glib failure by disabling a test. Attached are patches 
> I would like to see in core-updates. If it's too late to add the patch 
> for gd, then I would like to keep gd-for-php. This time I've tested them 
> and php works.

I did not get a substitute for gd on core-updates, so I don't think it
(or dependent packages) have been built yet. If so, patching it should
be okay.

[...]

> From 300af3fb262a31958f26aeac5eef33e376eda604 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien@lepiller.eu>
> Date: Wed, 1 Mar 2017 10:24:32 +0100
> Subject: [PATCH 1/3] gnu: glib: Fix test hang.
>
> * gnu/packages/glib.scm (glib)[arguments]: Fix test hang.
> ---
>  gnu/packages/glib.scm | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/gnu/packages/glib.scm b/gnu/packages/glib.scm
> index 1a794db..dab0524 100644
> --- a/gnu/packages/glib.scm
> +++ b/gnu/packages/glib.scm
> @@ -204,6 +204,9 @@ shared NFS home directories.")
>                         ;; recognize it.
>                         "/thread/thread4"))
>  
> +                     ("glib/tests/642026.c"
> +                      ("/glib/642026"))

This needs an explaining comment. Under which circumstances does this
test hang? I cannot reproduce it on 'core-updates'. 

> +
>                       ("glib/tests/timer.c"
>                        (;; fails if compiler optimizations are enabled, which they
>                         ;; are by default.
> -- 
> 2.7.4
>
> From 0aa21c67381203d7dfec325fdc85303f5d3db76e Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien@lepiller.eu>
> Date: Mon, 27 Feb 2017 11:09:11 +0100
> Subject: [PATCH 2/3] gnu: gd: Fix an issue with XBM decoding.
>
> * gnu/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch: New file.
> * gnu/local.scm (dist_patch_DATA): Add it.
> * gnu/packages/gd.scm (gd)[source]: Use it.
> ---
>  gnu/local.mk                                       |   1 +
>  gnu/packages/gd.scm                                |   3 +-
>  .../patches/gd-php-73968-Fix-109-XBM-reading.patch | 114 +++++++++++++++++++++
>  3 files changed, 117 insertions(+), 1 deletion(-)
>  create mode 100644 gnu/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch
>
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 3d9ad70..271d2c4 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -582,6 +582,7 @@ dist_patch_DATA =						\
>    %D%/packages/patches/gd-fix-tests-on-i686.patch		\
>    %D%/packages/patches/gd-fix-truecolor-format-correction.patch	\
>    %D%/packages/patches/gd-freetype-test-failure.patch		\
> +  %D%/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch		\
>    %D%/packages/patches/gegl-CVE-2012-4433.patch			\
>    %D%/packages/patches/geoclue-config.patch			\
>    %D%/packages/patches/ghc-dont-pass-linker-flags-via-response-files.patch	\
> diff --git a/gnu/packages/gd.scm b/gnu/packages/gd.scm
> index 8cac242..664e653 100644
> --- a/gnu/packages/gd.scm
> +++ b/gnu/packages/gd.scm
> @@ -52,7 +52,8 @@
>                 "1rp4v7n1dq38b92kl7gkvpvqqkw7nvdfnz6d5kip5klkxfki6zqk"))
>               (patches (search-patches "gd-fix-gd2-read-test.patch"
>                                        "gd-fix-tests-on-i686.patch"
> -                                      "gd-freetype-test-failure.patch"))))
> +                                      "gd-freetype-test-failure.patch"
> +                                      "gd-php-73968-Fix-109-XBM-reading.patch"))))
>      (build-system gnu-build-system)
>      (arguments
>       `(#:phases
> diff --git a/gnu/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch b/gnu/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch
> new file mode 100644
> index 0000000..bfaa4ff
> --- /dev/null
> +++ b/gnu/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch
> @@ -0,0 +1,114 @@
> +From 082c5444838ea0d84f9fb6441aefdb44d78d9bba Mon Sep 17 00:00:00 2001
> +From: "Christoph M. Becker" <cmbecker69@gmx.de>
> +Date: Fri, 20 Jan 2017 22:48:20 +0100
> +Subject: [PATCH] Fix #109: XBM reading fails with printed error

Please add a few words at the top of this file (before patch headers)
describing what it does, why it's needed, and where to find the
original. Look into other patches for examples.

> +
> +When calculating the number of required bytes of an XBM image, we have
> +to take the line padding into account.
> +
> +This patch has been taken from the gd repository and binary files have been
> +removed from the patch because our patch procedure doesn't support that format.

Is this not the original commit message? The next patch leads me to
think so. I would prefer not messing with commit messages of git-format
patches; either remove them or keep it intact.

> +---
> + src/gd_xbm.c                     |   2 +-
> + tests/xbm/CMakeLists.txt         |   1 +
> + tests/xbm/Makemodule.am          |   5 ++++-
> + tests/xbm/github_bug_109.c       |  35 +++++++++++++++++++++++++++++++++++
> + tests/xbm/github_bug_109.xbm     |   5 +++++
> + 5 files changed, 47 insertions(+), 2 deletions(-)
> + create mode 100644 tests/xbm/github_bug_109.c
> + create mode 100644 tests/xbm/github_bug_109.xbm
> + create mode 100644 tests/xbm/github_bug_109_exp.png
> +
> +diff --git a/src/gd_xbm.c b/src/gd_xbm.c
> +index 5f09b56..c2ba2ad 100644
> +--- a/src/gd_xbm.c
> ++++ b/src/gd_xbm.c
> +@@ -108,7 +108,7 @@ BGD_DECLARE(gdImagePtr) gdImageCreateFromXbm(FILE * fd)
> + 				max_bit = 32768;
> + 			}
> + 			if (max_bit) {
> +-				bytes = (width * height / 8) + 1;
> ++                bytes = (width + 7) / 8 * height;

This looks funky, but matches what's in the current HEAD of libgd:

https://github.com/libgd/libgd/blob/master/src/gd_xbm.c#L114

> + 				if (!bytes) {
> + 					return 0;
> + 				}
> +diff --git a/tests/xbm/CMakeLists.txt b/tests/xbm/CMakeLists.txt
> +index 183cf5e..08576e0 100644
> +--- a/tests/xbm/CMakeLists.txt
> ++++ b/tests/xbm/CMakeLists.txt
> +@@ -1,4 +1,5 @@
> + LIST(APPEND TESTS_FILES
> ++	github_bug_109
> + 	github_bug_170
> + )
> +
> +diff --git a/tests/xbm/Makemodule.am b/tests/xbm/Makemodule.am
> +index ba1eabd..0f5beb6 100644
> +--- a/tests/xbm/Makemodule.am
> ++++ b/tests/xbm/Makemodule.am
> +@@ -1,5 +1,8 @@
> + libgd_test_programs += \
> ++	xbm/github_bug_109 \
> + 	xbm/github_bug_170
> +
> + EXTRA_DIST += \
> +-	xbm/CMakeLists.txt
> ++	xbm/CMakeLists.txt \
> ++	xbm/github_bug_109.xbm \
> ++	xbm/github_bug_109_exp.png
> +diff --git a/tests/xbm/github_bug_109.c b/tests/xbm/github_bug_109.c
> +new file mode 100644
> +index 0000000..1a020c6
> +--- /dev/null
> ++++ b/tests/xbm/github_bug_109.c
> +@@ -0,0 +1,35 @@
> ++/**
> ++ * Test reading of XBM images with a width that is not a multiple of 8
> ++ *
> ++ * We're reading such an XBM image, and check that we got what we've expected,
> ++ * instead of an error message.
> ++ *
> ++ * See also <https://github.com/libgd/libgd/issues/109>.
> ++ */
> ++
> ++
> ++#include "gd.h"
> ++#include "gdtest.h"
> ++
> ++
> ++int main()
> ++{
> ++    gdImagePtr im;
> ++    FILE *fp;
> ++    char *path;
> ++
> ++    fp = gdTestFileOpen2("xbm", "github_bug_109.xbm");
> ++    im = gdImageCreateFromXbm(fp);
> ++    fclose(fp);
> ++    gdTestAssert(im != NULL);
> ++    gdTestAssert(gdImageGetTrueColorPixel(im, 0, 0) == 0);
> ++    gdTestAssert(gdImageGetTrueColorPixel(im, 0, 1) == 0xffffff);
> ++
> ++    path = gdTestFilePath2("xbm", "github_bug_109_exp.png");
> ++    gdAssertImageEqualsToFile(path, im);
> ++    gdFree(path);
> ++
> ++    gdImageDestroy(im);
> ++
> ++    return gdNumFailures();
> ++}
> +diff --git a/tests/xbm/github_bug_109.xbm b/tests/xbm/github_bug_109.xbm
> +new file mode 100644
> +index 0000000..f427d86
> +--- /dev/null
> ++++ b/tests/xbm/github_bug_109.xbm
> +@@ -0,0 +1,5 @@
> ++#define test_width 10
> ++#define test_height 10
> ++static unsigned char test_bits[] = {
> ++  0xFF, 0x03, 0x00, 0x00, 0xFF, 0x03, 0x00, 0x00, 0xFF, 0x03, 0x00, 0x00,
> ++  0xFF, 0x03, 0x00, 0x00, 0xFF, 0x03, 0x00, 0x00};
> +
> +--
> +2.7.4
> +
> --
> 2.7.4
>
> From 128fb06c4477792b4a4f58f21c66a50e5ab4fd0e Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien@lepiller.eu>
> Date: Mon, 27 Feb 2017 11:15:29 +0100
> Subject: [PATCH 3/3] gnu: php: Update to 7.1.2.
>
> * gnu/packages/php.scm (php): Update to 7.1.2.
> ---
>  .../patches/gd-php-73968-Fix-109-XBM-reading.patch |  7 ++++--
>  gnu/packages/php.scm                               | 27 +++++++++++-----------
>  2 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/gnu/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch b/gnu/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch
> index bfaa4ff..3fedba3 100644
> --- a/gnu/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch
> +++ b/gnu/packages/patches/gd-php-73968-Fix-109-XBM-reading.patch
> @@ -6,8 +6,11 @@ Subject: [PATCH] Fix #109: XBM reading fails with printed error
>  When calculating the number of required bytes of an XBM image, we have
>  to take the line padding into account.
>  
> -This patch has been taken from the gd repository and binary files have been
> -removed from the patch because our patch procedure doesn't support that format.
> +This bug was first reported to php on https://bugs.php.net/bug.php?id=73968.
> +php then reported it to gd in https://github.com/libgd/libgd/issues/109. This
> +patch is taken from commit 082c5444838ea0d84f9fb6441aefdb44d78d9bba of gd.
> +Binary files have been removed from the patch because our patch procedure
> +doesn't support that format.

Please add this information to the previous patch. Messing with
unrelated files in commits makes review, revert, bisecting etc
difficult. Reverting this commit as-is would cause a rebuild of
everything using gd.

>  ---
>   src/gd_xbm.c                     |   2 +-
>   tests/xbm/CMakeLists.txt         |   1 +
> diff --git a/gnu/packages/php.scm b/gnu/packages/php.scm
> index 16b0985..0dfee36 100644
> --- a/gnu/packages/php.scm
> +++ b/gnu/packages/php.scm
> @@ -50,21 +50,10 @@
>    #:use-module (guix build-system gnu)
>    #:use-module ((guix licenses) #:prefix license:))
>  
> -;; This fixes PHP bugs 73155 and 73159. Remove when gd
> -;; is updated to > 2.2.3.
> -(define gd-for-php
> -  (package (inherit gd)
> -           (source
> -            (origin
> -              (inherit (package-source gd))
> -              (patches (search-patches
> -                        "gd-fix-truecolor-format-correction.patch"
> -                        "gd-fix-chunk-size-on-boundaries.patch"))))))

Please also delete these two patches with this commit, and remove them
from local.mk.

> -
>  (define-public php
>    (package
>      (name "php")
> -    (version "7.0.14")
> +    (version "7.1.2")
>      (home-page "https://secure.php.net/")
>      (source (origin
>                (method url-fetch)
> @@ -72,7 +61,7 @@
>                                    name "-" version ".tar.xz"))
>                (sha256
>                 (base32
> -                "12ccgbrfchgvmcfb88rcknq7xmrf19c5ysdr4v8jxk51j9izy78g"))
> +                "0wg9ng230w724rpwsrhcg4pw41xm1xhz0zx76haanyymkz1s05fq"))
>                (modules '((guix build utils)))
>                (snippet
>                 '(with-directory-excursion "ext"
> @@ -179,6 +168,13 @@
>                              "ext/standard/tests/general_functions/bug44667.phpt"
>                              "ext/standard/tests/general_functions/proc_open.phpt")
>                 (("/bin/cat") (which "cat")))
> +
> +             ;; These tests fail because they include a file whose modification
> +             ;; time is 0. Touch them to make the test pass. The issue is reported
> +             ;; upstream as #74137.
> +             (utime "sapi/phpdbg/tests/include.inc" 1 1)
> +             (utime "sapi/phpdbg/tests/phpdbg_get_executable_stream_wrapper.inc" 1 1)
> +
>               ;; The encoding of this file is not recognized, so we simply drop it.
>               (delete-file "ext/mbstring/tests/mb_send_mail07.phpt")
>  
> @@ -257,8 +253,10 @@
>                           ;; The test expects an Array, but instead get the contents(?).
>                           "ext/gd/tests/bug43073.phpt"
>                           ;; imagettftext() returns wrong coordinates.
> +                         "ext/gd/tests/bug48732-mb.phpt"
>                           "ext/gd/tests/bug48732.phpt"
>                           ;; Similarly for imageftbbox().
> +                         "ext/gd/tests/bug48801-mb.phpt"
>                           "ext/gd/tests/bug48801.phpt"
>                           ;; Different expected output from imagecolorallocate().
>                           "ext/gd/tests/bug53504.phpt"
> @@ -291,10 +289,11 @@
>         ("curl" ,curl)
>         ("cyrus-sasl" ,cyrus-sasl)
>         ("freetype" ,freetype)
> -       ("gd" ,gd-for-php)
> +       ("gd" ,gd)
>         ("gdbm" ,gdbm)
>         ("glibc" ,glibc)
>         ("gmp" ,gmp)
> +       ("gnutls" ,gnutls)

Is this a recent dependency? If not, I would prefer to have it as a separate commit.

Apart from the glib test thingy, these patches LGTM for 'core-updates'
with the mentioned changes.

>         ("libgcrypt" ,libgcrypt)
>         ("libjpeg" ,libjpeg)
>         ("libpng" ,libpng)
> --
> 2.7.4

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