[bug#49181] Fix missing phases in Emacs builds

[Carlo Zancanaro <carlo@zancanaro.id.au>]

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

Hi there!

I went to install emacs-protobuf-mode today and found that the 
build wasn't working. I investigated and it looks like someone 
renamed the add-source-to-load-path phase to expand-load-path, but 
they left a few references behind. The first of my patches fixes 
that.

The second of my patches is more controversial. It changes 
modify-phases to error out if the asked-for phase doesn't exist in 
add-before/add-after clauses. I think this is the right move, 
because it's hard to imagine when the default behaviour of "add to 
the end of the phases list" is helpful. In most cases the extra 
phases are setup/transformation phases that we need to run before 
the final "install" phase, so it's far more useful to fail early 
than to add these to the end of the phases list.

Carlo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Fix-references-to-emacs-build-system-s-expand-lo.patch --]
[-- Type: text/x-diff, Size: 2698 bytes --]

From d07b375d6d0a1c354587b285a2547aee8f6e9f96 Mon Sep 17 00:00:00 2001
From: Carlo Zancanaro <carlo@zancanaro.id.au>
Date: Wed, 23 Jun 2021 15:57:40 +1000
Subject: [PATCH 1/2] gnu: Fix references to emacs-build-system's
 expand-load-path phase

* gnu/packages/emacs-xyz.scm (emacs-pdf-tools): Change reference from
emacs-add-source-to-load-path to emacs-expand-load-path
* gnu/packages/erlang.scm (emacs-erlang): Change reference from
add-source-to-load-path to expand-load-path
* gnu/packages/protobuf.scm (emacs-protobuf-mode): Change reference from
add-source-to-load-path to expand-load-path
---
 gnu/packages/emacs-xyz.scm | 2 +-
 gnu/packages/erlang.scm    | 2 +-
 gnu/packages/protobuf.scm  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index eef33fd437..28b87d49dd 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -3310,7 +3310,7 @@ during idle time, while Emacs is doing nothing else.")
                  ("pdf-tools-handle-upgrades" '()))))
            (add-after 'emacs-patch-variables 'emacs-expand-load-path
              (assoc-ref emacs:%standard-phases 'expand-load-path))
-           (add-after 'emacs-add-source-to-load-path 'emacs-install
+           (add-after 'emacs-expand-load-path 'emacs-install
              (assoc-ref emacs:%standard-phases 'install))
            (add-after 'emacs-install 'emacs-build
              (assoc-ref emacs:%standard-phases 'build))
diff --git a/gnu/packages/erlang.scm b/gnu/packages/erlang.scm
index 7b5dc70b5d..d52d5c3983 100644
--- a/gnu/packages/erlang.scm
+++ b/gnu/packages/erlang.scm
@@ -232,7 +232,7 @@ built-in support for concurrency, distribution and fault tolerance.")
     (arguments
      `(#:phases
        (modify-phases %standard-phases
-         (add-before 'add-source-to-load-path 'change-working-directory
+         (add-before 'expand-load-path 'change-working-directory
            (lambda _ (chdir "lib/tools/emacs") #t)))))
     (home-page "https://www.erlang.org/")
     (synopsis "Erlang major mode for Emacs")
diff --git a/gnu/packages/protobuf.scm b/gnu/packages/protobuf.scm
index 857adf1703..9c6dcb51cc 100644
--- a/gnu/packages/protobuf.scm
+++ b/gnu/packages/protobuf.scm
@@ -311,7 +311,7 @@ structured data.")
     (arguments
      `(#:phases
        (modify-phases %standard-phases
-         (add-before 'add-source-to-load-path 'change-working-directory
+         (add-before 'expand-load-path 'change-working-directory
            (lambda _ (chdir "editors") #t)))))
     (home-page "https://github.com/protocolbuffers/protobuf")
     (synopsis "Protocol buffers major mode for Emacs")
-- 
2.32.0


[-- Attachment #3: 0002-guix-Make-modify-phases-error-when-adding-before-aft.patch --]
[-- Type: text/x-diff, Size: 3948 bytes --]

From b272a65b12175a3d0354f18e19247f9ed72f5f8e Mon Sep 17 00:00:00 2001
From: Carlo Zancanaro <carlo@zancanaro.id.au>
Date: Wed, 23 Jun 2021 15:59:37 +1000
Subject: [PATCH 2/2] guix: Make modify-phases error when adding before/after a
 missing phase

* guix/build/utils.scm (alist-cons-before, alist-cons-after): Cause a match failure if the
reference is not found, rather than appending to the alist.
* tests/build-utils.scm: Update tests to match.
---
 guix/build/utils.scm  | 15 +++++++++------
 tests/build-utils.scm | 12 ++++++------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index 419c10195b..6e052a7ea1 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2015, 2018 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2018 Arun Isaac <arunisaac@systemreboot.net>
 ;;; Copyright © 2018, 2019 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2021 Carlo Zancanaro <carlo@zancanaro.id.au>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -571,18 +572,22 @@ effects, such as displaying warnings or error messages."
 (define* (alist-cons-before reference key value alist
                             #:optional (key=? equal?))
   "Insert the KEY/VALUE pair before the first occurrence of a pair whose key
-is REFERENCE in ALIST.  Use KEY=? to compare keys."
+is REFERENCE in ALIST.  Use KEY=? to compare keys.  An error is raised when no
+such pair exists."
   (let-values (((before after)
                 (break (match-lambda
                         ((k . _)
                          (key=? k reference)))
                        alist)))
-    (append before (alist-cons key value after))))
+    (match after
+      ((reference after ...)
+       (append before (alist-cons key value (cons reference after)))))))
 
 (define* (alist-cons-after reference key value alist
                            #:optional (key=? equal?))
   "Insert the KEY/VALUE pair after the first occurrence of a pair whose key
-is REFERENCE in ALIST.  Use KEY=? to compare keys."
+is REFERENCE in ALIST.  Use KEY=? to compare keys.  An error is raised when
+no such pair exists."
   (let-values (((before after)
                 (break (match-lambda
                         ((k . _)
@@ -590,9 +595,7 @@ is REFERENCE in ALIST.  Use KEY=? to compare keys."
                        alist)))
     (match after
       ((reference after ...)
-       (append before (cons* reference `(,key . ,value) after)))
-      (()
-       (append before `((,key . ,value)))))))
+       (append before (cons* reference `(,key . ,value) after))))))
 
 (define* (alist-replace key value alist #:optional (key=? equal?))
   "Replace the first pair in ALIST whose car is KEY with the KEY/VALUE pair.
diff --git a/tests/build-utils.scm b/tests/build-utils.scm
index 654b480ed9..c694b479b3 100644
--- a/tests/build-utils.scm
+++ b/tests/build-utils.scm
@@ -38,17 +38,17 @@
   '((a . 1) (x . 42) (b . 2) (c . 3))
   (alist-cons-before 'b 'x 42 '((a . 1) (b . 2) (c . 3))))
 
-(test-equal "alist-cons-before, reference not found"
-  '((a . 1) (b . 2) (c . 3) (x . 42))
-  (alist-cons-before 'z 'x 42 '((a . 1) (b . 2) (c . 3))))
+(test-assert "alist-cons-before, reference not found"
+  (not (false-if-exception
+        (alist-cons-before 'z 'x 42 '((a . 1) (b . 2) (c . 3))))))
 
 (test-equal "alist-cons-after"
   '((a . 1) (b . 2) (x . 42) (c . 3))
   (alist-cons-after 'b 'x 42 '((a . 1) (b . 2) (c . 3))))
 
-(test-equal "alist-cons-after, reference not found"
-  '((a . 1) (b . 2) (c . 3) (x . 42))
-  (alist-cons-after 'z 'x 42 '((a . 1) (b . 2) (c . 3))))
+(test-assert "alist-cons-after, reference not found"
+  (not (false-if-exception
+        (alist-cons-after 'z 'x 42 '((a . 1) (b . 2) (c . 3))))))
 
 (test-equal "alist-replace"
   '((a . 1) (b . 77) (c . 3))
-- 
2.32.0