Subject: Re: [bug#69377] [PATCH] gnu: Add sioyek.

From: Hilton Chain <[email-masked]>
Date: Sun, 25 Feb 2024 23:53:33 +0800

Hi dan,

On Sun, 25 Feb 2024 18:12:32 +0800,
dan wrote:
>
> * gnu/packages/pdf.scm (sioyek): New variable.
>
> Change-Id: Id2708745bd7ef672b620b8e9c907126bdc5a1f55
> ---
>  gnu/packages/pdf.scm | 67 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
>
> diff --git a/gnu/packages/pdf.scm b/gnu/packages/pdf.scm
> index 0d796f65bb..587c952e37 100644
> --- a/gnu/packages/pdf.scm
> +++ b/gnu/packages/pdf.scm
> @@ -25,6 +25,7 @@
>  ;;; Copyright © 2022 Paul A. Patience <[email-masked]>
>  ;;; Copyright © 2022 Petr Hodina <[email-masked]>
>  ;;; Copyright © 2023 Felix Gruber <[email-masked]>
> +;;; Copyright © 2024 dan <[email-masked]>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -1710,3 +1711,69 @@ (define-public weasyprint
>
>  Keywords: html2pdf, htmltopdf")
>      (license license:bsd-3)))
> +
> +(define-public sioyek
> +  (package
> +    (name "sioyek")
> +    (version "2.0.0")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/ahrm/sioyek.git")

Please remove the ‘.git’ suffix, in GitHub it's used for redirection.

> +             (commit (string-append "v" version))))
> +       (sha256
> +        (base32 "1vmmp2s032ygh1byz77pg9aljmp8hx745fr7mmz11831f96mlmhq"))
> +       ;; XXX: Fix for mupdf-0.23.0+ <https://github.com/ahrm/sioyek/issues/804>.
> +       (patches (list (origin
> +                        (method url-fetch)
> +                        (uri (string-append
> +                              "https://git.alpinelinux.org/aports/plain/community"
> +                              "/sioyek/mupdf-0.23.0.patch?id="
> +                              "86e913eccf19b97a16f25d9b6cdf0f50232f1226"))
> +                        (sha256 (base32
> +                                 "0wddl7qznr7x9kmqjb05f9pag8m4w29bnnw8ghzsi1zz9ska2jmh")))))))

Please add the patch (with attribution) under gnu/packages/patches instead.

‘file-name’ field is missing, by the way.

> +    (build-system gnu-build-system)
> +    (arguments
> +     (list
> +      #:phases #~(modify-phases %standard-phases
> +                   (add-after 'unpack 'patch-lib-and-path
> +                     (lambda _
> +                       (substitute* "pdf_viewer_build_config.pro"
> +                         (("-lmupdf-third")
> +                          "")

Removing ‘-lmupdf-third’ can be added as a source snippet.

> +                         (("/share/sioyek")
> +                          "/share")
> +                         (("/etc/sioyek")
> +                          "/etc"))
> +                       (substitute* "pdf_viewer/main.cpp"
> +                         (("/usr/share/sioyek")
> +                          (string-append #$output "/share"))
> +                         (("/etc/sioyek")
> +                          (string-append #$output "/etc")))))

Please keep /etc/sioyek and /share/sioyek, think where files will be when
installed into a profile.

> +                   (replace 'configure
> +                     (lambda _
> +                       (invoke "qmake"
> +                               (string-append "PREFIX="
> +                                              #$output))))

This ‘PREFIX’ can be added to ‘#:configure-flags’.

> +                   (add-after 'install 'instal-man-page
> +                     (lambda _
> +                       (install-file "resources/sioyek.1"
> +                                     (string-append #$output "/share/man/man1")))))))
> +    (inputs (list freetype
> +                  gumbo-parser
> +                  harfbuzz
> +                  jbig2dec
> +                  libjpeg-turbo
> +                  mujs
> +                  mupdf
> +                  openjpeg
> +                  qt3d-5
> +                  qtbase-5
> +                  zlib))
> +    (home-page "https://sioyek.info")
> +    (synopsis
> +     "A PDF viewer with a focus on technical books and research papers")

Don't use an article at the beginning of synopsis.

> +    (description
> +     "Sioyek is a PDF viewer with a focus on textbooks and research papers.")
> +    (license license:gpl3+)))
>
> base-commit: 1306beaf3f6c8ddded2a956f5863bc65aad78882
> --
> 2.41.0

Otherwise LGTM, please send a revised patch.

Thanks