[bug#28954] [PATCH] Update Scribus to 1.5.3. Re-vamp package.

[Eric Bavier <ericbavier@centurylink.net>]

On Mon, 23 Oct 2017 12:46:51 +0200
Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
> To: 28954@debbugs.gnu.org
> Subject: [bug#28954] [PATCH] Update Scribus to 1.5.3.  Re-vamp package.
> Date: Mon, 23 Oct 2017 12:46:51 +0200
> Sender: "Guix-patches"
>  <guix-patches-bounces+ericbavier=centurylink.net@gnu.org>
> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux)
> 
> Hello,
> 
> The following patch updates Scribus to 1.5.3.  It requires changes to
> the package definition itself (in particular patches and additional
> inputs).  
> 
[...]
> From e42a5b62476e63184c3a4164d73410decbbb2a75 Mon Sep 17 00:00:00 2001
> From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
> Date: Mon, 23 Oct 2017 12:25:13 +0200
> Subject: [PATCH] gnu: scribus: Update to 1.5.3.  Re-vamp package.
> 
> * gnu/packages/patches/scribus-fix-poppler-0.58-breakage.patch:
> * gnu/packages/patches/scribus-fix-poppler-packaging.patch: New files.
> * gnu/local.mk (dist_patch_DATA): Register new files.
> 
> * gnu/packages/scribus.scm (scribus): Update to 1.5.3.
> [input]: Add fontconfig, harfbuzz, hunspell, icu4c, libcdr, libfreehand,
> libmspub, librevenge, libvisio, openssl, podofo, poppler.
> [native-input]: Move qttools from inputs.  Add boost.
> [home-page]: Fix URL.

Remove the blank line between entries.

> ---
>  gnu/local.mk                                       |   2 +
>  .../scribus-fix-poppler-0.58-breakage.patch        | 526 +++++++++++++++++++++
>  .../patches/scribus-fix-poppler-packaging.patch    |  95 ++++

Could you include a url where these patches came from in the patches
themselves?  You mention in the package definition that they came from
upstream's trunk.

Or, for large patches, some have experimented with using an origin that
fetches the patch from upstream rather than pulling the patch into
Guix.

>  gnu/packages/scribus.scm                           |  57 ++-
>  4 files changed, 669 insertions(+), 11 deletions(-)
>  create mode 100644 gnu/packages/patches/scribus-fix-poppler-0.58-breakage.patch
>  create mode 100644 gnu/packages/patches/scribus-fix-poppler-packaging.patch
> 
[...] 
>  (define-public scribus
>    (package
>      (name "scribus")
> -    (version "1.5.2")
> +    (version "1.5.3")
>      (source (origin
>                (method url-fetch)
>                (uri (string-append "mirror://sourceforge/scribus/scribus-devel/"
>                                    version "/scribus-" version ".tar.xz"))
>                (sha256
>                 (base32
> -                "1xbl7h4x32y2nfhn57ivjziyvlnwadcbizqwfqx5srfsmqiyqppc"))))
> +                "0kyp45vidxa3v35ic9592db4zk3m8al26vck38q5v7z14x3hp8vk"))
> +              (patches
> +               ;; These patches are necessary to build package with Poppler >=
> +               ;; 0.58.  They are extracted from trunk and will be included in
> +               ;; Scribus 1.5.4.
> +               (search-patches "scribus-fix-poppler-0.58-breakage.patch"
> +                               "scribus-fix-poppler-packaging.patch"))))
>      (build-system cmake-build-system)
>      (arguments
> -     `(#:tests? #f ; no test target
> +     `(#:tests? #f                      ;no test target

I see no need for this whitespace-only change.

>         #:configure-flags
> -       '("-DCMAKE_CXX_FLAGS=-std=gnu++11")))
> +       '("-DWANT_GRAPHICSMAGICK=1")
> +       #:phases
> +       (modify-phases %standard-phases
> +         ;; Fix typo.  Patch included in trunk.  See
> +         ;; <https://bugs.scribus.net/view.php?id=14850>.
> +         (add-before 'configure 'fix-podofo
> +           (lambda _
> +             (substitute* "cmake/modules/FindLIBPODOFO.cmake"
> +               (("find_package\\(OPENSSL\\)") "find_package(OpenSSL)"))
> +             #t)))))

Could you move this into an origin snippet instead?  We'd want a fix
like this included in the source when someone uses 'guix build -S
scribus'

>      (inputs
>       `(("cairo" ,cairo)
>         ("cups" ,cups)
> +       ("fontconfig" ,fontconfig)
> +       ("freetype" ,freetype)
>         ("graphicsmagick" ,graphicsmagick)
> +       ("harfbuzz" ,harfbuzz)
> +       ("hunspell" ,hunspell)
> +       ("icu4c" ,icu4c)
>         ("lcms" ,lcms)
> +       ("libcdr" ,libcdr)
> +       ("libfreehand" ,libfreehand)
>         ("libjpeg" ,libjpeg)
> +       ("libmspub" ,libmspub)
> +       ("libpagemaker" ,libpagemaker)
> +       ("librevenge" ,librevenge)
>         ("libtiff" ,libtiff)
> +       ("libvisio" ,libvisio)
>         ("libxml2" ,libxml2)
> -       ("python" ,python-2)
> -       ("freetype" ,freetype)
> +       ("openssl" ,openssl)
> +       ("podofo" ,podofo)
> +       ("poppler" ,poppler)
> +       ("python" ,python-2)             ;need Python library
>         ("qtbase" ,qtbase)
>         ("qtdeclarative" ,qtdeclarative)
> -       ("qttools" ,qttools)
>         ("zlib" ,zlib)))
>      (native-inputs
> -     `(("util-linux" ,util-linux)
> -       ("pkg-config" ,pkg-config)))
> -    (home-page "http://scribus.net")
> +     `(("boost" ,boost)

Is boost really a native input?  I'm surprised.  Is it used only for
build-related things?


Otherwise LGTM.  Could you send an updated patch?  Then I'll try
building it.

Thanks,
`~Eric