Re: [PATCH] gnu: pdf: Fix installing desktop files of zathura packages.

[Mark H Weaver]

Alex Kost <address@hidden> writes:

> Mark H Weaver (2015-07-07 18:07 +0300) wrote:
>
> [...]
>>>      (arguments
>>>       `(#:make-flags
>>>         `(,(string-append "DESTDIR=" (assoc-ref %outputs "out"))
>>> -          "PLUGINDIR=/lib/zathura" "CC=gcc")
>>> +          "PREFIX=" "PLUGINDIR=/lib/zathura" "CC=gcc")
>>
>> It would be better to leave DESTDIR empty and set PREFIX=<%output>, so:
>
> It wouldn't work for these packages.

I made this change and the packages seem to build properly.  See below
for my proposed patch.

>>> -       `(,(string-append "DESTDIR=" (assoc-ref %outputs "out"))
>>> +       `(,(string-append "PREFIX=" (assoc-ref %outputs "out"))
>>
>> There is a conceptual difference between PREFIX and DESTDIR: at install
>> time, files are copied to ${DESTDIR}${PREFIX}, and then at run time
>> files are expected to be at ${PREFIX}.  So in general, we don't want to
>> use DESTDIR in Guix, and we want to set PREFIX to the output directory.
>
> I know, but these zathura plugins do not provide configure stages, and
> PREFIX is not even used in the manually written "Makefile"s.  PREFIX is
> used in "config.mk" (which is included in a Makefile) to define LIBDIR
> and DESKTOPPREFIX.
>
> And due to Makefile things are installed in ${DESTDIR}${PLUGINDIR} and
> ${DESTDIR}${DESKTOPPREFIX}.

Okay, but the default values of PLUGINDIR and DESKTOPPREFIX are defined
in terms of PREFIX.  Those variables should contain the absolute
pathnames of where these files will be located at run time.

As is, these zathura-* packages are setting DESTDIR, PREFIX, and
PLUGINDIR incorrectly.  Now, it may be that in this case, it'll work
properly anyway (for now), but I'd still prefer to set them correctly
for two reasons:

* It is very common for packagers to look at existing packages for
  examples of good practices, and I don't want to spread this confusion
  about DESTDIR and PREFIX.

* Even if these build systems don't currently depend on proper settings
  of PREFIX, DESTDIR, and PLUGINDIR today, there's no guarantee that
  this will remain the case tomorrow.

So, how about the following patch instead?  What do you think?

      Mark


>From 62c2cc48e6a34c596e1e5a0bd0f873cd3d3f9218 Mon Sep 17 00:00:00 2001
From: Mark H Weaver <address@hidden>
Date: Tue, 7 Jul 2015 14:20:54 -0400
Subject: [PATCH] gnu: zathura-{cb,ps,djvu,pdf-poppler}: Fix installation of
 desktop files.

Based on a patch by Alex Kost <address@hidden>.

* gnu/packages/pdf.scm (zathura-cb, zathura-ps, zathura-djvu)
  (zathura-pdf-poppler)[arguments]: In make-flags, set PREFIX instead of
  DESTDIR and adjust PLUGINDIR accordingly.
---
 gnu/packages/pdf.scm | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/gnu/packages/pdf.scm b/gnu/packages/pdf.scm
index 82e8c88..6cdc846 100644
--- a/gnu/packages/pdf.scm
+++ b/gnu/packages/pdf.scm
@@ -172,8 +172,9 @@
     (build-system gnu-build-system)
     (arguments
      `(#:make-flags
-       `(,(string-append "DESTDIR=" (assoc-ref %outputs "out"))
-          "PLUGINDIR=/lib/zathura" "CC=gcc")
+       `(,(string-append "PREFIX=" %output)
+         ,(string-append "PLUGINDIR=" %output "/lib/zathura")
+         "CC=gcc")
        #:tests? #f ; Package does not contain tests.
        #:phases
        (alist-delete 'configure %standard-phases)))
@@ -203,8 +204,9 @@ using libarchive.")
     (build-system gnu-build-system)
     (arguments
      `(#:make-flags
-       `(,(string-append "DESTDIR=" (assoc-ref %outputs "out"))
-          "PLUGINDIR=/lib/zathura" "CC=gcc")
+       `(,(string-append "PREFIX=" %output)
+         ,(string-append "PLUGINDIR=" %output "/lib/zathura")
+         "CC=gcc")
        #:tests? #f ; Package does not contain tests.
        #:phases
        (alist-delete 'configure %standard-phases)))
@@ -235,8 +237,9 @@ using libspectre.")
     (build-system gnu-build-system)
     (arguments
      `(#:make-flags
-       `(,(string-append "DESTDIR=" (assoc-ref %outputs "out"))
-          "PLUGINDIR=/lib/zathura" "CC=gcc")
+       `(,(string-append "PREFIX=" %output)
+         ,(string-append "PLUGINDIR=" %output "/lib/zathura")
+         "CC=gcc")
        #:tests? #f ; Package does not contain tests.
        #:phases
        (alist-delete 'configure %standard-phases)))
@@ -268,8 +271,9 @@ using the DjVuLibre library.")
     (build-system gnu-build-system)
     (arguments
      `(#:make-flags
-       `(,(string-append "DESTDIR=" (assoc-ref %outputs "out"))
-          "PLUGINDIR=/lib/zathura" "CC=gcc")
+       `(,(string-append "PREFIX=" %output)
+         ,(string-append "PLUGINDIR=" %output "/lib/zathura")
+         "CC=gcc")
        #:tests? #f ; Package does not include tests.
        #:phases
        (alist-delete 'configure %standard-phases)))
-- 
2.4.3