diff mbox series

[bug#66624,emacs-team,01/15] guix: emacs-build-system: Process package source in build tree.

Message ID c2be6951978bbb6d100caaf5d2ddfc52d3375169.1697694142.git.liliana.prikler@gmail.com
State New
Headers show
Series Start reworking emacs-build-system | expand

Commit Message

Liliana Marie Prikler Oct. 19, 2023, 4:06 a.m. UTC 
* guix/build/emacs-build-system.scm (ensure-package-description)
(patch-el-files, make-autoloads): Operate on the current working directory,
either implicitly, or through (getcwd).
(enable-autoloads-compilation): Deleted variable, logic moved into
make-autoloads.
(%standard-phases): Adjust accordingly.
---
 guix/build/emacs-build-system.scm | 85 +++++++++++++------------------
 1 file changed, 34 insertions(+), 51 deletions(-)

Comments

Andrew Tropin Oct. 23, 2023, 8:41 a.m. UTC | #1 
On 2023-10-19 06:06, Liliana Marie Prikler wrote:

> * guix/build/emacs-build-system.scm (ensure-package-description)
> (patch-el-files, make-autoloads): Operate on the current working directory,
> either implicitly, or through (getcwd).

Sounds reasonable and seems to simplify build phases.

> (enable-autoloads-compilation): Deleted variable, logic moved into
> make-autoloads.
> (%standard-phases): Adjust accordingly.
> ---
>  guix/build/emacs-build-system.scm | 85 +++++++++++++------------------
>  1 file changed, 34 insertions(+), 51 deletions(-)
>
> diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
> index 3808b60445..aa083c6409 100644
> --- a/guix/build/emacs-build-system.scm
> +++ b/guix/build/emacs-build-system.scm
> @@ -132,29 +132,25 @@ (define* (build #:key outputs inputs #:allow-other-keys)
>      (parameterize ((%emacs emacs))
>        (emacs-compile-directory (elpa-directory out)))))
>  
> -(define* (patch-el-files #:key outputs #:allow-other-keys)
> -  "Substitute the absolute \"/bin/\" directory with the right location in the
> -store in '.el' files."
> -
> -  (let* ((out (assoc-ref outputs "out"))
> -         (elpa-name-ver (store-directory->elpa-name-version out))
> -         (el-dir (string-append out %install-dir "/" elpa-name-ver))
> -         (el-files (find-files (getcwd) "\\.el$")))
> -    (define (substitute-program-names)
> -      (substitute* el-files
> -        (("\"/bin/([^.]\\S*)\"" _ cmd-name)
> -         (let ((cmd (which cmd-name)))
> -           (unless cmd
> -             (error "patch-el-files: unable to locate " cmd-name))
> -           (string-append "\"" cmd "\"")))))
> -
> -    (with-directory-excursion el-dir
> -      ;; Some old '.el' files (e.g., tex-buf.el in AUCTeX) are still
> -      ;; ISO-8859-1-encoded.
> -      (unless (false-if-exception (substitute-program-names))
> -        (with-fluids ((%default-port-encoding "ISO-8859-1"))
> -          (substitute-program-names))))
> -    #t))
> +(define* (patch-el-files #:key inputs outputs #:allow-other-keys)
> +  "Substitute the absolute \"/bin/\" and \"/sbin\" directories with the right
> +locations in the store in '.el' files."
> +
> +  (define substitute-program-names
> +    (let ((el-files (find-files (getcwd) "\\.el$")))
> +      (lambda ()
> +        (substitute* el-files
> +          (("\"/(s?bin/[^.]\\S*)\"" _ cmd)
> +           (let ((cmd (search-input-file inputs cmd)))
> +             (unless cmd
> +               (error "patch-el-files: unable to locate " (basename cmd)))
> +             (string-append "\"" cmd "\"")))))))
> +
> +  (unless (false-if-exception (substitute-program-names))
> +    ;; Some old '.el' files (e.g., tex-buf.el in AUCTeX) are still
> +    ;; ISO-8859-1-encoded.
> +    (with-fluids ((%default-port-encoding "ISO-8859-1"))
> +      (substitute-program-names))))
>  
>  (define (find-root-library-file name)
>    (let loop ((parts (string-split
> @@ -224,10 +220,8 @@ (define* (ensure-package-description #:key outputs #:allow-other-keys)
>        (emacs-batch-edit-file (string-append name ".el")
>          %write-pkg-file-form)))
>  
> -  (let* ((out (assoc-ref outputs "out"))
> -         (elpa-name-ver (store-directory->elpa-name-version out)))
> -    (with-directory-excursion (elpa-directory out)
> -      (and=> (find-root-library-file elpa-name-ver) write-pkg-file))))
> +  (let ((name (store-directory->elpa-name-version (assoc-ref outputs "out"))))
> +    (and=> (find-root-library-file name) write-pkg-file)))
>  
>  (define* (check #:key tests? (test-command '("make" "check"))
>                  (parallel-tests? #t) #:allow-other-keys)
> @@ -306,24 +300,15 @@ (define* (move-doc #:key outputs #:allow-other-keys)
>                    info-files)))
>      #t))
>  
> -(define* (make-autoloads #:key outputs inputs #:allow-other-keys)
> +(define* (make-autoloads #:key outputs #:allow-other-keys)
>    "Generate the autoloads file."
> -  (let* ((emacs (search-input-file inputs "/bin/emacs"))
> -         (out (assoc-ref outputs "out"))
> -         (elpa-name-ver (store-directory->elpa-name-version out))
> -         (elpa-name (package-name->name+version elpa-name-ver))
> -         (el-dir (elpa-directory out)))
> -    (parameterize ((%emacs emacs))
> -      (emacs-generate-autoloads elpa-name el-dir))))
> -
> -(define* (enable-autoloads-compilation #:key outputs #:allow-other-keys)
> -  "Remove the NO-BYTE-COMPILATION local variable embedded in the generated
> -autoload files."
> -  (let* ((out (assoc-ref outputs "out"))
> -         (autoloads (find-files out "-autoloads.el$")))
> -    (substitute* autoloads
> -      ((";; no-byte-compile.*") ""))
> -    #t))
> +  (emacs-generate-autoloads
> +   (package-name->name+version (store-directory->elpa-name-version
> +                                (assoc-ref outputs "out")))
> +   (getcwd))
> +  ;; Ensure that autoloads can be byte-compiled.
> +  (substitute* (find-files "." "-autoloads\\.el$")
> +    ((";; no-byte-compile.*") "")))
>  
>  (define* (validate-compiled-autoloads #:key outputs #:allow-other-keys)
>    "Verify whether the byte compiled autoloads load fine."
> @@ -358,7 +343,11 @@ (define (elpa-directory store-dir)
>  (define %standard-phases
>    (modify-phases gnu:%standard-phases
>      (replace 'unpack unpack)
> +    (add-after 'unpack 'ensure-package-description
> +      ensure-package-description)
>      (add-after 'unpack 'expand-load-path expand-load-path)
> +    (add-after 'unpack 'patch-el-files patch-el-files)
> +    (add-after 'expand-load-path 'make-autoloads make-autoloads)
>      (add-after 'expand-load-path 'add-install-to-native-load-path
>        add-install-to-native-load-path)
>      (delete 'bootstrap)
> @@ -366,14 +355,8 @@ (define %standard-phases
>      (delete 'build)
>      (replace 'check check)
>      (replace 'install install)
> -    (add-after 'install 'make-autoloads make-autoloads)
> -    (add-after 'make-autoloads 'enable-autoloads-compilation
> -      enable-autoloads-compilation)
> -    (add-after 'enable-autoloads-compilation 'patch-el-files patch-el-files)
> -    (add-after 'patch-el-files 'ensure-package-description
> -      ensure-package-description)
>      ;; The .el files are byte compiled directly in the store.
> -    (add-after 'ensure-package-description 'build build)
> +    (add-after 'install 'build build)
>      (add-after 'build 'validate-compiled-autoloads validate-compiled-autoloads)
>      (add-after 'validate-compiled-autoloads 'move-doc move-doc)))

I didn't spot any obvious issues with the patch series.  Thank you for working on it!
Liliana Marie Prikler Nov. 1, 2023, 7:54 p.m. UTC | #2 
Am Montag, dem 23.10.2023 um 12:41 +0400 schrieb Andrew Tropin:
> On 2023-10-19 06:06, Liliana Marie Prikler wrote:
> 
> > * guix/build/emacs-build-system.scm (ensure-package-description)
> > (patch-el-files, make-autoloads): Operate on the current working
> > directory either implicitly, or through (getcwd).
> 
> Sounds reasonable and seems to simplify build phases.
> 
> > [...]
> 
> I didn't spot any obvious issues with the patch series.  Thank you
> for working on it!
Thanks for checking.

Pushed to emacs-team now.

Cheers
Liliana Marie Prikler Nov. 2, 2023, 9:32 a.m. UTC | #3 
Am Mittwoch, dem 01.11.2023 um 20:54 +0100 schrieb Liliana Marie
Prikler:
> Am Montag, dem 23.10.2023 um 12:41 +0400 schrieb Andrew Tropin:
> > On 2023-10-19 06:06, Liliana Marie Prikler wrote:
> > 
> > > * guix/build/emacs-build-system.scm (ensure-package-description)
> > > (patch-el-files, make-autoloads): Operate on the current working
> > > directory either implicitly, or through (getcwd).
> > 
> > Sounds reasonable and seems to simplify build phases.
> > 
> > > [...]
> > 
> > I didn't spot any obvious issues with the patch series.  Thank you
> > for working on it!
> Thanks for checking.
> 
> Pushed to emacs-team now.
> 
> Cheers
Forgot to mark as done.
diff mbox series

Patch

diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
index 3808b60445..aa083c6409 100644
--- a/guix/build/emacs-build-system.scm
+++ b/guix/build/emacs-build-system.scm
@@ -132,29 +132,25 @@  (define* (build #:key outputs inputs #:allow-other-keys)
     (parameterize ((%emacs emacs))
       (emacs-compile-directory (elpa-directory out)))))
 
-(define* (patch-el-files #:key outputs #:allow-other-keys)
-  "Substitute the absolute \"/bin/\" directory with the right location in the
-store in '.el' files."
-
-  (let* ((out (assoc-ref outputs "out"))
-         (elpa-name-ver (store-directory->elpa-name-version out))
-         (el-dir (string-append out %install-dir "/" elpa-name-ver))
-         (el-files (find-files (getcwd) "\\.el$")))
-    (define (substitute-program-names)
-      (substitute* el-files
-        (("\"/bin/([^.]\\S*)\"" _ cmd-name)
-         (let ((cmd (which cmd-name)))
-           (unless cmd
-             (error "patch-el-files: unable to locate " cmd-name))
-           (string-append "\"" cmd "\"")))))
-
-    (with-directory-excursion el-dir
-      ;; Some old '.el' files (e.g., tex-buf.el in AUCTeX) are still
-      ;; ISO-8859-1-encoded.
-      (unless (false-if-exception (substitute-program-names))
-        (with-fluids ((%default-port-encoding "ISO-8859-1"))
-          (substitute-program-names))))
-    #t))
+(define* (patch-el-files #:key inputs outputs #:allow-other-keys)
+  "Substitute the absolute \"/bin/\" and \"/sbin\" directories with the right
+locations in the store in '.el' files."
+
+  (define substitute-program-names
+    (let ((el-files (find-files (getcwd) "\\.el$")))
+      (lambda ()
+        (substitute* el-files
+          (("\"/(s?bin/[^.]\\S*)\"" _ cmd)
+           (let ((cmd (search-input-file inputs cmd)))
+             (unless cmd
+               (error "patch-el-files: unable to locate " (basename cmd)))
+             (string-append "\"" cmd "\"")))))))
+
+  (unless (false-if-exception (substitute-program-names))
+    ;; Some old '.el' files (e.g., tex-buf.el in AUCTeX) are still
+    ;; ISO-8859-1-encoded.
+    (with-fluids ((%default-port-encoding "ISO-8859-1"))
+      (substitute-program-names))))
 
 (define (find-root-library-file name)
   (let loop ((parts (string-split
@@ -224,10 +220,8 @@  (define* (ensure-package-description #:key outputs #:allow-other-keys)
       (emacs-batch-edit-file (string-append name ".el")
         %write-pkg-file-form)))
 
-  (let* ((out (assoc-ref outputs "out"))
-         (elpa-name-ver (store-directory->elpa-name-version out)))
-    (with-directory-excursion (elpa-directory out)
-      (and=> (find-root-library-file elpa-name-ver) write-pkg-file))))
+  (let ((name (store-directory->elpa-name-version (assoc-ref outputs "out"))))
+    (and=> (find-root-library-file name) write-pkg-file)))
 
 (define* (check #:key tests? (test-command '("make" "check"))
                 (parallel-tests? #t) #:allow-other-keys)
@@ -306,24 +300,15 @@  (define* (move-doc #:key outputs #:allow-other-keys)
                   info-files)))
     #t))
 
-(define* (make-autoloads #:key outputs inputs #:allow-other-keys)
+(define* (make-autoloads #:key outputs #:allow-other-keys)
   "Generate the autoloads file."
-  (let* ((emacs (search-input-file inputs "/bin/emacs"))
-         (out (assoc-ref outputs "out"))
-         (elpa-name-ver (store-directory->elpa-name-version out))
-         (elpa-name (package-name->name+version elpa-name-ver))
-         (el-dir (elpa-directory out)))
-    (parameterize ((%emacs emacs))
-      (emacs-generate-autoloads elpa-name el-dir))))
-
-(define* (enable-autoloads-compilation #:key outputs #:allow-other-keys)
-  "Remove the NO-BYTE-COMPILATION local variable embedded in the generated
-autoload files."
-  (let* ((out (assoc-ref outputs "out"))
-         (autoloads (find-files out "-autoloads.el$")))
-    (substitute* autoloads
-      ((";; no-byte-compile.*") ""))
-    #t))
+  (emacs-generate-autoloads
+   (package-name->name+version (store-directory->elpa-name-version
+                                (assoc-ref outputs "out")))
+   (getcwd))
+  ;; Ensure that autoloads can be byte-compiled.
+  (substitute* (find-files "." "-autoloads\\.el$")
+    ((";; no-byte-compile.*") "")))
 
 (define* (validate-compiled-autoloads #:key outputs #:allow-other-keys)
   "Verify whether the byte compiled autoloads load fine."
@@ -358,7 +343,11 @@  (define (elpa-directory store-dir)
 (define %standard-phases
   (modify-phases gnu:%standard-phases
     (replace 'unpack unpack)
+    (add-after 'unpack 'ensure-package-description
+      ensure-package-description)
     (add-after 'unpack 'expand-load-path expand-load-path)
+    (add-after 'unpack 'patch-el-files patch-el-files)
+    (add-after 'expand-load-path 'make-autoloads make-autoloads)
     (add-after 'expand-load-path 'add-install-to-native-load-path
       add-install-to-native-load-path)
     (delete 'bootstrap)
@@ -366,14 +355,8 @@  (define %standard-phases
     (delete 'build)
     (replace 'check check)
     (replace 'install install)
-    (add-after 'install 'make-autoloads make-autoloads)
-    (add-after 'make-autoloads 'enable-autoloads-compilation
-      enable-autoloads-compilation)
-    (add-after 'enable-autoloads-compilation 'patch-el-files patch-el-files)
-    (add-after 'patch-el-files 'ensure-package-description
-      ensure-package-description)
     ;; The .el files are byte compiled directly in the store.
-    (add-after 'ensure-package-description 'build build)
+    (add-after 'install 'build build)
     (add-after 'build 'validate-compiled-autoloads validate-compiled-autoloads)
     (add-after 'validate-compiled-autoloads 'move-doc move-doc)))