[PATCH] gnu: emacs-pdf-tools: Add missing input.

[PATCH] gnu: emacs-pdf-tools: Add missing input.

[Alex Kost]

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

The bottom line of the following text is the question: Is it possible to
combine 2 (or more) build systems for building a package?

I've noticed that the latest 'emacs-pdf-tools' depends on 'let-alist'
library (my bad that I didn't notice this during update to 0.70), so I
added it to the propagated-inputs (as we do with emacs dependencies).

However, adding a new input is not enough here, because a directory with
"let-alist.el" is not seen during elisp compilation, so it should be
added explicitly to 'emacs-byte-compile-directory' procedure.

The root problem here is this: pdf-tools package consists of 2 parts:

1. Some C code (placed in "server" subdir) used to make an auxiliary
   binary for working with pdf.  This server part provides a usual GNU
   build system, which is gladly used in our "emacs-pdf-tools" package.

2. Elisp code: to handle this part (to compile and install it) we use
   'install-lisp' phase.

For the elisp side, we actually duplicate (more or less) what is already
done by emacs-build-system.  So it would be a perfect solution for this
package if there was a way to perform a build twice: at first using
emacs-build-system to handle elisp part, and then using gnu-build-system
for the C part.

I did a little experiment: I thought maybe it could be possible just to
pick some build phases from (guix build emacs-build-system), so I added
this module to #:modules and #:imported-modules and added phases like
this:

  (modify-phases %standard-phases
    ;; ...
    (add-after 'build 'emacs-build
      (@@ (guix build emacs-build-system) build))
    (add-after 'install 'emacs-install
      (@@ (guix build emacs-build-system) install)))

The package was built successfully, but the result was not good, because
both gnu-build-system and emacs-build-system import %standard-phases :-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-emacs-pdf-tools-Add-missing-input.patch --]
[-- Type: text/x-patch, Size: 2118 bytes --]

From 5692ba83371fea16a34ff20d1f5d743f05173348 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Fri, 11 Mar 2016 11:34:20 +0300
Subject: [PATCH] gnu: emacs-pdf-tools: Add missing input.

This is a followup to commit eccd0b57a1f05b3caca28604f4d2c06556e2fe05.

* gnu/packages/emacs.scm (emacs-pdf-tools)[propagated-inputs]: Add
'let-alist'.
[arguments]: Adjust 'install-lisp' phase to compile pdf-tools using
let-alist library.
---
 gnu/packages/emacs.scm | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index c9fbfcf..ecc6817 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -1044,18 +1044,24 @@ single buffer.")
                 ("pdf-tools-handle-upgrades" '())))))
          (add-after
           'install 'install-lisp
-          (lambda* (#:key outputs #:allow-other-keys)
+          (lambda* (#:key inputs outputs #:allow-other-keys)
             (let ((target (string-append (assoc-ref outputs "out")
-                                         "/share/emacs/site-lisp/")))
+                                         "/share/emacs/site-lisp/"))
+                  (let-alist (string-append
+                              (assoc-ref inputs "let-alist")
+                              "/share/emacs/site-lisp/guix.d/let-alist-"
+                              ,(package-version let-alist))))
               (for-each (lambda (file)
                           (install-file file target))
                         (find-files "../lisp" "^(pdf|tab).*\\.elc?"))
-              (emacs-byte-compile-directory target)
+              (emacs-byte-compile-directory target (list let-alist))
               (emacs-generate-autoloads "pdf-tools" target)))))))
     (native-inputs `(("autoconf" ,autoconf)
                      ("automake" ,automake)
                      ("pkg-config" ,pkg-config)
                      ("emacs" ,emacs-no-x)))
+    (propagated-inputs
+     `(("let-alist" ,let-alist)))
     (inputs `(("poppler" ,poppler)
               ("cairo" ,cairo)
               ("glib" ,glib)
-- 
2.6.3

Re: [PATCH] gnu: emacs-pdf-tools: Add missing input.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> The bottom line of the following text is the question: Is it possible to
> combine 2 (or more) build systems for building a package?

Sure!

> I did a little experiment: I thought maybe it could be possible just to
> pick some build phases from (guix build emacs-build-system), so I added
> this module to #:modules and #:imported-modules and added phases like
> this:
>
>   (modify-phases %standard-phases
>     ;; ...
>     (add-after 'build 'emacs-build
>       (@@ (guix build emacs-build-system) build))
>     (add-after 'install 'emacs-install
>       (@@ (guix build emacs-build-system) install)))
>
> The package was built successfully, but the result was not good, because
> both gnu-build-system and emacs-build-system import %standard-phases :-)

You mean “export”?

I think you can always add a renamer upon import, using something like:

  (package
    ;; …
    (arguments
     `(#:imported-modules ((guix build emacs-build-system)
                           ,%gnu-build-system-modules)
       #:modules (((guix build emacs-build-system) #:prefix emacs:)
                  ,%gnu-build-system-modules)

       #:phases (modify-phases %standard-phases   ;from gnu-build-system
                  (add-before 'build 'emacs-build
                     (assoc-ref emacs:%standard-phases 'build))
                  ;; …
                  ))))

Would it work for you?

> From 5692ba83371fea16a34ff20d1f5d743f05173348 Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Fri, 11 Mar 2016 11:34:20 +0300
> Subject: [PATCH] gnu: emacs-pdf-tools: Add missing input.
>
> This is a followup to commit eccd0b57a1f05b3caca28604f4d2c06556e2fe05.
>
> * gnu/packages/emacs.scm (emacs-pdf-tools)[propagated-inputs]: Add
> 'let-alist'.
> [arguments]: Adjust 'install-lisp' phase to compile pdf-tools using
> let-alist library.

Otherwise the patch LGTM.

Ludo’.

Re: [PATCH] gnu: emacs-pdf-tools: Add missing input.

[Alex Kost]

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

Ludovic Courtès (2016-03-15 17:27 +0300) wrote:

> Alex Kost <alezost@gmail.com> skribis:
>
>> The bottom line of the following text is the question: Is it possible to
>> combine 2 (or more) build systems for building a package?
>
> Sure!

Great!

>> I did a little experiment: I thought maybe it could be possible just to
>> pick some build phases from (guix build emacs-build-system), so I added
>> this module to #:modules and #:imported-modules and added phases like
>> this:
>>
>>   (modify-phases %standard-phases
>>     ;; ...
>>     (add-after 'build 'emacs-build
>>       (@@ (guix build emacs-build-system) build))
>>     (add-after 'install 'emacs-install
>>       (@@ (guix build emacs-build-system) install)))
>>
>> The package was built successfully, but the result was not good, because
>> both gnu-build-system and emacs-build-system import %standard-phases :-)
>
> You mean “export”?

Yes, sometimes I mix up these words :-)

> I think you can always add a renamer upon import, using something like:

Ahahah, thanks!

>   (package
>     ;; …
>     (arguments
>      `(#:imported-modules ((guix build emacs-build-system)
>                            ,%gnu-build-system-modules)
>        #:modules (((guix build emacs-build-system) #:prefix emacs:)
>                   ,%gnu-build-system-modules)
>
>        #:phases (modify-phases %standard-phases   ;from gnu-build-system
>                   (add-before 'build 'emacs-build
>                      (assoc-ref emacs:%standard-phases 'build))
>                   ;; …
>                   ))))
>
> Would it work for you?

Yes, brilliant!  Now I know how to combine phases from different build
systems, thank you!

So I think it's better to use 2 commits here: one to update phases and
another to add let-alist dependency (both patches attached).


[-- Attachment #2: 0001-gnu-emacs-pdf-tools-Use-emacs-build-system-for-elisp.patch --]
[-- Type: text/x-patch, Size: 4330 bytes --]

From b83adf2e1e0d13a5a09026bab40b0e5198e2f295 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Fri, 11 Mar 2016 11:34:20 +0300
Subject: [PATCH 1/2] gnu: emacs-pdf-tools: Use emacs-build-system for elisp
 side.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* gnu/packages/emacs.scm (emacs-pdf-tools)[arguments]: Add phases from
emacs-build-system to build elisp side.

Co-authored-by: Ludovic Courtès <ludo@gnu.org>
---
 gnu/packages/emacs.scm | 57 +++++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 28 deletions(-)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index c9fbfcf..38210e5 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -1018,40 +1018,41 @@ single buffer.")
     (arguments
      `(#:tests? #f ; there are no tests
        #:modules ((guix build gnu-build-system)
+                  ((guix build emacs-build-system) #:prefix emacs:)
                   (guix build utils)
                   (guix build emacs-utils))
        #:imported-modules (,@%gnu-build-system-modules
+                           (guix build emacs-build-system)
                            (guix build emacs-utils))
        #:phases
        (modify-phases %standard-phases
-         (add-after 'unpack 'enter-dir (lambda _ (chdir "server") #t))
-         (add-before
-          'configure 'autogen
-          (lambda _
-            (zero? (system* "bash" "autogen.sh"))))
-         (add-before
-          'build 'patch-variables
-          (lambda* (#:key outputs #:allow-other-keys)
-            (with-directory-excursion "../lisp"
-              ;; Set path to epdfinfo program.
-              (emacs-substitute-variables "pdf-info.el"
-                ("pdf-info-epdfinfo-program"
-                 (string-append (assoc-ref outputs "out")
-                                "/bin/epdfinfo")))
-              ;; Set 'pdf-tools-handle-upgrades' to nil to avoid "auto
-              ;; upgrading" that pdf-tools tries to perform.
-              (emacs-substitute-variables "pdf-tools.el"
-                ("pdf-tools-handle-upgrades" '())))))
-         (add-after
-          'install 'install-lisp
-          (lambda* (#:key outputs #:allow-other-keys)
-            (let ((target (string-append (assoc-ref outputs "out")
-                                         "/share/emacs/site-lisp/")))
-              (for-each (lambda (file)
-                          (install-file file target))
-                        (find-files "../lisp" "^(pdf|tab).*\\.elc?"))
-              (emacs-byte-compile-directory target)
-              (emacs-generate-autoloads "pdf-tools" target)))))))
+         ;; Build server side using 'gnu-build-system'.
+         (add-after 'unpack 'enter-server-dir
+           (lambda _ (chdir "server") #t))
+         (add-before 'configure 'autogen
+           (lambda _
+             (zero? (system* "bash" "autogen.sh"))))
+
+         ;; Build emacs side using 'emacs-build-system'.
+         (add-after 'compress-documentation 'enter-lisp-dir
+           (lambda _ (chdir "../lisp") #t))
+         (add-after 'enter-lisp-dir 'emacs-patch-variables
+           (lambda* (#:key outputs #:allow-other-keys)
+             ;; Set path to epdfinfo program.
+             (emacs-substitute-variables "pdf-info.el"
+               ("pdf-info-epdfinfo-program"
+                (string-append (assoc-ref outputs "out")
+                               "/bin/epdfinfo")))
+             ;; Set 'pdf-tools-handle-upgrades' to nil to avoid "auto
+             ;; upgrading" that pdf-tools tries to perform.
+             (emacs-substitute-variables "pdf-tools.el"
+               ("pdf-tools-handle-upgrades" '()))))
+         (add-after 'emacs-patch-variables 'emacs-install
+           (assoc-ref emacs:%standard-phases 'install))
+         (add-after 'emacs-install 'emacs-build
+           (assoc-ref emacs:%standard-phases 'build))
+         (add-after 'emacs-install 'emacs-make-autoloads
+           (assoc-ref emacs:%standard-phases 'make-autoloads)))))
     (native-inputs `(("autoconf" ,autoconf)
                      ("automake" ,automake)
                      ("pkg-config" ,pkg-config)
-- 
2.6.3


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-gnu-emacs-pdf-tools-Add-missing-input.patch --]
[-- Type: text/x-patch, Size: 925 bytes --]

From 8d64845aa9109359e32c4d08ec498bfb3b4ac870 Mon Sep 17 00:00:00 2001
From: Alex Kost <alezost@gmail.com>
Date: Tue, 15 Mar 2016 23:02:04 +0300
Subject: [PATCH 2/2] gnu: emacs-pdf-tools: Add missing input.

This is a followup to commit eccd0b57a1f05b3caca28604f4d2c06556e2fe05.

* gnu/packages/emacs.scm (emacs-pdf-tools)[propagated-inputs]: Add
'let-alist'.
---
 gnu/packages/emacs.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index 38210e5..f53f73b 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -1057,6 +1057,8 @@ single buffer.")
                      ("automake" ,automake)
                      ("pkg-config" ,pkg-config)
                      ("emacs" ,emacs-no-x)))
+    (propagated-inputs
+     `(("let-alist" ,let-alist)))
     (inputs `(("poppler" ,poppler)
               ("cairo" ,cairo)
               ("glib" ,glib)
-- 
2.6.3

Re: [PATCH] gnu: emacs-pdf-tools: Add missing input.

[Ludovic Courtès]

Alex Kost <alezost@gmail.com> skribis:

> From b83adf2e1e0d13a5a09026bab40b0e5198e2f295 Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Fri, 11 Mar 2016 11:34:20 +0300
> Subject: [PATCH 1/2] gnu: emacs-pdf-tools: Use emacs-build-system for elisp
>  side.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> * gnu/packages/emacs.scm (emacs-pdf-tools)[arguments]: Add phases from
> emacs-build-system to build elisp side.
>
> Co-authored-by: Ludovic Courtès <ludo@gnu.org>

OK.  Glad it worked as expected.  :-)

> From 8d64845aa9109359e32c4d08ec498bfb3b4ac870 Mon Sep 17 00:00:00 2001
> From: Alex Kost <alezost@gmail.com>
> Date: Tue, 15 Mar 2016 23:02:04 +0300
> Subject: [PATCH 2/2] gnu: emacs-pdf-tools: Add missing input.
>
> This is a followup to commit eccd0b57a1f05b3caca28604f4d2c06556e2fe05.
>
> * gnu/packages/emacs.scm (emacs-pdf-tools)[propagated-inputs]: Add
> 'let-alist'.

OK.

Thank you!

Ludo’.