Subject: Re: [PATCH v2] gnu: enblend-enfuse: Build documentation.

From: Maxim Cournoyer <[email-masked]>
Date: Wed, 06 Sep 2023 17:01:51 -0400

Hi,

Bruno Victal <[email-masked]> writes:

> Notes:
> * transfig is not needed according to the NEWS since 4.2.
> * help2man is not optional.
>
> * gnu/packages/photo.scm (enblend-enfuse)[native-inputs]: Remove libxml2,
> tidy-html and transfig. Replace imagemagick with imagemagick/stable. Add
> graphviz-minimal, font-ghostscript, librsvg, m4, perl-readonly,
> texlive-texloganalyser, texlive-bold-extra, texlive-cm-mf-extra-bold,
> texlive-comment, texlive-float, texlive-enumitem, texlive-mdwtools,
> texlive-hyphenat, texlive-index, texlive-listings, texlive-microtype,
> texlive-etoolbox, texlive-nag, texlive-ragged2e, texlive-shorttoc,
> texlive-bigfoot, texlive-xstring and hevea.
> [arguments]<#:phases>: Restyle. Add 'fontconfig-cache, 'exclude-doc-from-check
> and 'install-doc. Replace 'build and 'install.
> [outputs]: Add 'doc.

[...]

>      (inputs
>       (list boost
>             gsl
> @@ -395,16 +419,61 @@ (define-public enblend-enfuse
>             vigra
>             zlib))
>      (arguments
> -     (list #:configure-flags
> -           #~(list "--enable-openmp")
> -           #:phases
> -           #~(modify-phases %standard-phases
> -               (add-after 'unpack 'add-missing-include
> -                 (lambda _
> -                   (substitute* "src/minimizer.h"
> -                     ;; Fix error: ‘numeric_limits’ is not a member of ‘std’.
> -                     (("#include <vector>" line)
> -                      (string-append line "\n#include <limits>"))))))))
> +     (list
> +      #:configure-flags
> +      #~(list "--enable-openmp")
> +      #:phases
> +      #~(modify-phases %standard-phases
> +          (add-after 'unpack 'add-missing-include
> +            (lambda _
> +              (substitute* "src/minimizer.h"
> +                ;; Fix error: ‘numeric_limits’ is not a member of ‘std’.
> +                (("#include <vector>" line)
> +                 (string-append line "\n#include <limits>")))))
> +          (add-before 'build 'fontconfig-cache
> +            (lambda _
> +              (setenv "XDG_CACHE_HOME" (mkdtemp "/tmp/cache-XXXXXX"))))
> +          ;; XXX: There's some extreme sillyness when building the
> +          ;; documentation. It gets rebuilt thrice, during build, check and
> +          ;; install, possibly due to the effects of the invocation of
> +          ;; UPDATED_ON in doc/Makefile.
> +          ;; I suspect the package might also have reproducibility issues
> +          ;; with the manual and src/DefaultSig.pm. (not an exhaustive list)
> +          ;; It's worth asking upstream for help with adding support
> +          ;; for SOURCE_DATE_EPOCH.

Stylistic nitpick: we don't voice the comments using the first person
(e.g. 'I') in the code typically; we try to stick to being descriptive
and factual.

If it's worth asking upstream, we should ask them now, and link to the
upstream issue in the comment to track its resolution :-).

> +          (add-after 'configure 'exclude-doc-from-check
> +            (lambda _
> +              (substitute* "doc/Makefile"
> +                (("^(check:).+$" _ rule)
> +                 (string-append rule "\n")))))
> +          ;; XXX: Skip building the docs since they're rebuilt again
> +          ;; during install.
> +          (replace 'build
> +            (lambda args
> +              (with-directory-excursion "src"
> +                (apply (assoc-ref %standard-phases 'build) args))))
> +          ;; XXX: Save another doc rebuild when installing.
> +          (replace 'install
> +            ;; Intercept and insert a make-flag for this phase only.
> +            (lambda* (#:key make-flags #:allow-other-keys)
> +              (apply invoke "make" "install"
> +                     (cons "MAYBE_DOC=" make-flags))))
> +          ;; XXX: 'make install' doesn't install the docs.
> +          (add-after 'install 'install-doc
> +            (lambda* (#:key make-flags #:allow-other-keys)
> +              ;; Install examples first, for which the 'install' rule works.
> +              (with-directory-excursion "doc/examples"
> +                (apply invoke "make" "install" make-flags))
> +              ;; The docs have to be installed with specific rules.
> +              (with-directory-excursion "doc"
> +                (apply invoke "make"
> +                       "install-ps-local"
> +                       "install-html-local"
> +                       "install-dvi-local"
> +                       ;; Do not overwhelm the console by printing the source
> +                       ;; to stdout.
> +                       (cons "V=0" make-flags))))))))
> +    (outputs '("out" "doc"))

nitpick: it's more conventional to have the 'outputs' field before the
'build-system' one.

Otherwise, I built it locally and it LGTM.

-- 
Thanks,
Maxim