[PATCH 0/2] Remove unsupported u-boot-malta package

  • Done
  • quality assurance status badge
Details
4 participants
  • Josselin Poiret
  • Ludovic Courtès
  • Maxim Cournoyer
  • Simon Tournier
Owner
unassigned
Submitted by
Maxim Cournoyer
Severity
normal
M
M
Maxim Cournoyer wrote on 14 Jan 2023 04:05
(address . guix-patches@gnu.org)
20230114030542.23524-1-maxim.cournoyer@gmail.com
The package was causing issues when migrating the make-u-boot-package
procedure to use Guix's infrastructure for cross-compilation, because Guix
would now attempt to build a cross-compiler for mips64el, which is not
supported per (guix platforms).


Maxim Cournoyer (2):
platforms: Raise an exception when no suitable platform is found.
gnu: Remove u-boot-malta.

gnu/packages/bootloaders.scm | 3 ---
gnu/packages/bootstrap.scm | 2 +-
guix/platform.scm | 44 ++++++++++++++++++++++++++----------
3 files changed, 33 insertions(+), 16 deletions(-)


base-commit: 4913ac74915c4229aeb3ca26a5f9920c759fb6a3
--
2.38.1
M
M
Maxim Cournoyer wrote on 14 Jan 2023 04:08
[PATCH 1/2] platforms: Raise an exception when no suitable platform is found.
(address . 60802@debbugs.gnu.org)
20230114030817.23959-2-maxim.cournoyer@gmail.com
This was motivated by #60786, which produced a cryptic, hard to understand
backtrace.

* guix/platform.scm (&platform-not-found-error): New exception type.
(make-platform-not-found-error): New exception constructor.
(platform-not-found-error?): New predicate.
(lookup-platform-by-system): Raise an exception when no platform is found.
Update documentation.
(lookup-platform-by-target): Likewise.
(lookup-platform-by-target-or-system): Likewise.
---

gnu/packages/bootstrap.scm | 2 +-
guix/platform.scm | 44 +++++++++++++++++++++++++++-----------
2 files changed, 33 insertions(+), 13 deletions(-)

Toggle diff (102 lines)
diff --git a/gnu/packages/bootstrap.scm b/gnu/packages/bootstrap.scm
index d2914fb5a7..772be7e5e4 100644
--- a/gnu/packages/bootstrap.scm
+++ b/gnu/packages/bootstrap.scm
@@ -315,7 +315,7 @@ (define* (glibc-dynamic-linker
(%current-system))))
"Return the name of Glibc's dynamic linker for SYSTEM."
;; See the 'SYSDEP_KNOWN_INTERPRETER_NAMES' cpp macro in libc.
- (let ((platform (lookup-platform-by-system system)))
+ (let ((platform (false-if-exception (lookup-platform-by-system system))))
(cond
((platform? platform)
(platform-glibc-dynamic-linker platform))
diff --git a/guix/platform.scm b/guix/platform.scm
index f873913fe0..c2be66e227 100644
--- a/guix/platform.scm
+++ b/guix/platform.scm
@@ -1,5 +1,6 @@
;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2021 Mathieu Othacehe <othacehe@gnu.org>
+;;; Copyright © 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -21,6 +22,7 @@ (define-module (guix platform)
#:use-module (guix memoization)
#:use-module (guix records)
#:use-module (guix ui)
+ #:use-module (ice-9 exceptions)
#:use-module (srfi srfi-1)
#:export (platform
platform?
@@ -29,6 +31,10 @@ (define-module (guix platform)
platform-linux-architecture
platform-glibc-dynamic-linker
+ &platform-not-found-error
+ make-platform-not-found-error
+ platform-not-found-error?
+
platform-modules
platforms
lookup-platform-by-system
@@ -70,6 +76,14 @@ (define-record-type* <platform> platform make-platform
(default #false))
(glibc-dynamic-linker platform-glibc-dynamic-linker))
+
+;;;
+;;; Exception types.
+;;;
+(define-exception-type &platform-not-found-error &error
+ make-platform-not-found-error platform-not-found-error?)
+
+
;;;
;;; Platforms.
@@ -94,23 +108,29 @@ (define platforms
(platform-modules)))))
(define (lookup-platform-by-system system)
- "Return the platform corresponding to the given SYSTEM."
- (find (lambda (platform)
- (let ((s (platform-system platform)))
- (and (string? s) (string=? s system))))
- (platforms)))
+ "Return the platform corresponding to the given SYSTEM. Raise
+&PLATFORM-NOT-FOUND-ERROR when no platform could be found."
+ (or (find (lambda (platform)
+ (let ((s (platform-system platform)))
+ (and (string? s) (string=? s system))))
+ (platforms))
+ (raise-exception (make-platform-not-found-error))))
(define (lookup-platform-by-target target)
- "Return the platform corresponding to the given TARGET."
- (find (lambda (platform)
- (let ((t (platform-target platform)))
- (and (string? t) (string=? t target))))
- (platforms)))
+ "Return the platform corresponding to the given TARGET. Raise
+&PLATFORM-NOT-FOUND-ERROR when no platform could be found."
+ (or (find (lambda (platform)
+ (let ((t (platform-target platform)))
+ (and (string? t) (string=? t target))))
+ (platforms))
+ (raise-exception (make-platform-not-found-error))))
(define (lookup-platform-by-target-or-system target-or-system)
- "Return the platform corresponding to the given TARGET or SYSTEM."
+ "Return the platform corresponding to the given TARGET or SYSTEM. Raise
+&PLATFORM-NOT-FOUND-ERROR when no platform could be found."
(or (lookup-platform-by-target target-or-system)
- (lookup-platform-by-system target-or-system)))
+ (lookup-platform-by-system target-or-system)
+ (raise-exception (make-platform-not-found-error))))
(define (platform-system->target system)
"Return the target matching the given SYSTEM if it exists or false
--
2.38.1
M
M
Maxim Cournoyer wrote on 14 Jan 2023 04:08
[PATCH 2/2] gnu: Remove u-boot-malta.
(address . 60802@debbugs.gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20230114030817.23959-3-maxim.cournoyer@gmail.com

The mips64el architecture is not currently supported, causing (guix platform)
to raise an exception when attempting to cross-build the package.

* gnu/packages/bootloaders.scm (u-boot-malta): Delete variable.

---

gnu/packages/bootloaders.scm | 3 ---
1 file changed, 3 deletions(-)

Toggle diff (16 lines)
diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 6e6bdb4c08..8dc6ff698d 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -965,9 +965,6 @@ (define*-public (make-u-boot-package board triplet
uboot-files)
#t)))))))))
-(define-public u-boot-malta
- (make-u-boot-package "malta" "mips64el-linux-gnuabi64"))
-
(define-public u-boot-am335x-boneblack
(let ((base (make-u-boot-package
"am335x_evm" "arm-linux-gnueabihf"
--
2.38.1
M
M
Maxim Cournoyer wrote on 14 Jan 2023 05:19
[PATCH v2 1/2] platforms: Raise an exception when no suitable platform is found.
(address . 60802@debbugs.gnu.org)
20230114041903.7121-2-maxim.cournoyer@gmail.com
This was motivated by #60786, which produced a cryptic, hard to understand
backtrace.

* guix/platform.scm (&platform-not-found-error): New exception type.
(make-platform-not-found-error): New exception constructor.
(platform-not-found-error?): New predicate.
(false-if-platform-not-found): New syntax.
(lookup-platform-by-system): Raise an exception when no platform is found.
Update documentation.
(lookup-platform-by-target): Likewise.
(lookup-platform-by-target-or-system): Likewise, and guard lookup calls with
false-if-platform-not-found.

---

Changes in v2:
- Add false-if-platform-not-found syntax
- Use it while evaluating lookup-platform-by-target-or-system

gnu/packages/bootstrap.scm | 2 +-
guix/platform.scm | 51 ++++++++++++++++++++++++++++----------
2 files changed, 39 insertions(+), 14 deletions(-)

Toggle diff (102 lines)
diff --git a/gnu/packages/bootstrap.scm b/gnu/packages/bootstrap.scm
index d2914fb5a7..772be7e5e4 100644
--- a/gnu/packages/bootstrap.scm
+++ b/gnu/packages/bootstrap.scm
@@ -315,7 +315,7 @@ (define* (glibc-dynamic-linker
(%current-system))))
"Return the name of Glibc's dynamic linker for SYSTEM."
;; See the 'SYSDEP_KNOWN_INTERPRETER_NAMES' cpp macro in libc.
- (let ((platform (lookup-platform-by-system system)))
+ (let ((platform (false-if-exception (lookup-platform-by-system system))))
(cond
((platform? platform)
(platform-glibc-dynamic-linker platform))
diff --git a/guix/platform.scm b/guix/platform.scm
index f873913fe0..2e61c6827a 100644
--- a/guix/platform.scm
+++ b/guix/platform.scm
@@ -21,6 +21,7 @@ (define-module (guix platform)
#:use-module (guix memoization)
#:use-module (guix records)
#:use-module (guix ui)
+ #:use-module (ice-9 exceptions)
#:use-module (srfi srfi-1)
#:export (platform
platform?
@@ -29,6 +30,11 @@ (define-module (guix platform)
platform-linux-architecture
platform-glibc-dynamic-linker
+ &platform-not-found-error
+ make-platform-not-found-error
+ platform-not-found-error?
+ false-if-platform-not-found
+
platform-modules
platforms
lookup-platform-by-system
@@ -70,6 +76,19 @@ (define-record-type* <platform> platform make-platform
(default #false))
(glibc-dynamic-linker platform-glibc-dynamic-linker))
+
+;;;
+;;; Exceptions.
+;;;
+(define-exception-type &platform-not-found-error &error
+ make-platform-not-found-error platform-not-found-error?)
+
+(define-syntax-rule (false-if-platform-not-found exp)
+ "Evaluate EXP but return #f if it raises a platform-not-found-error?
+exception."
+ (guard (ex ((platform-not-found-error? ex) #f))
+ exp))
+
;;;
;;; Platforms.
@@ -94,23 +113,29 @@ (define platforms
(platform-modules)))))
(define (lookup-platform-by-system system)
- "Return the platform corresponding to the given SYSTEM."
- (find (lambda (platform)
- (let ((s (platform-system platform)))
- (and (string? s) (string=? s system))))
- (platforms)))
+ "Return the platform corresponding to the given SYSTEM. Raise
+&PLATFORM-NOT-FOUND-ERROR when no platform could be found."
+ (or (find (lambda (platform)
+ (let ((s (platform-system platform)))
+ (and (string? s) (string=? s system))))
+ (platforms))
+ (raise-exception (make-platform-not-found-error))))
(define (lookup-platform-by-target target)
- "Return the platform corresponding to the given TARGET."
- (find (lambda (platform)
- (let ((t (platform-target platform)))
- (and (string? t) (string=? t target))))
- (platforms)))
+ "Return the platform corresponding to the given TARGET. Raise
+&PLATFORM-NOT-FOUND-ERROR when no platform could be found."
+ (or (find (lambda (platform)
+ (let ((t (platform-target platform)))
+ (and (string? t) (string=? t target))))
+ (platforms))
+ (raise-exception (make-platform-not-found-error))))
(define (lookup-platform-by-target-or-system target-or-system)
- "Return the platform corresponding to the given TARGET or SYSTEM."
- (or (lookup-platform-by-target target-or-system)
- (lookup-platform-by-system target-or-system)))
+ "Return the platform corresponding to the given TARGET or SYSTEM. Raise
+&PLATFORM-NOT-FOUND-ERROR when no platform could be found."
+ (or (false-if-platform-not-found (lookup-platform-by-target target-or-system))
+ (false-if-platform-not-found (lookup-platform-by-system target-or-system))
+ (raise-exception (make-platform-not-found-error))))
(define (platform-system->target system)
"Return the target matching the given SYSTEM if it exists or false
--
2.38.1
M
M
Maxim Cournoyer wrote on 14 Jan 2023 05:19
[PATCH v2 2/2] gnu: Remove u-boot-malta.
(address . 60802@debbugs.gnu.org)(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
20230114041903.7121-3-maxim.cournoyer@gmail.com

The mips64el architecture is not currently supported, causing (guix platform)
to raise an exception when attempting to cross-build the package.

* gnu/packages/bootloaders.scm (u-boot-malta): Delete variable.

---

(no changes since v1)

gnu/packages/bootloaders.scm | 3 ---
1 file changed, 3 deletions(-)

Toggle diff (16 lines)
diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 6e6bdb4c08..8dc6ff698d 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -965,9 +965,6 @@ (define*-public (make-u-boot-package board triplet
uboot-files)
#t)))))))))
-(define-public u-boot-malta
- (make-u-boot-package "malta" "mips64el-linux-gnuabi64"))
-
(define-public u-boot-am335x-boneblack
(let ((base (make-u-boot-package
"am335x_evm" "arm-linux-gnueabihf"
--
2.38.1
L
L
Ludovic Courtès wrote on 14 Jan 2023 15:34
Re: [PATCH v2 1/2] platforms: Raise an exception when no suitable platform is found.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87o7r19ocn.fsf@gnu.org
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

Toggle quote (13 lines)
> This was motivated by #60786, which produced a cryptic, hard to understand
> backtrace.
>
> * guix/platform.scm (&platform-not-found-error): New exception type.
> (make-platform-not-found-error): New exception constructor.
> (platform-not-found-error?): New predicate.
> (false-if-platform-not-found): New syntax.
> (lookup-platform-by-system): Raise an exception when no platform is found.
> Update documentation.
> (lookup-platform-by-target): Likewise.
> (lookup-platform-by-target-or-system): Likewise, and guard lookup calls with
> false-if-platform-not-found.

Sounds like a good idea!

Toggle quote (8 lines)
> +++ b/gnu/packages/bootstrap.scm
> @@ -315,7 +315,7 @@ (define* (glibc-dynamic-linker
> (%current-system))))
> "Return the name of Glibc's dynamic linker for SYSTEM."
> ;; See the 'SYSDEP_KNOWN_INTERPRETER_NAMES' cpp macro in libc.
> - (let ((platform (lookup-platform-by-system system)))
> + (let ((platform (false-if-exception (lookup-platform-by-system system))))

Maybe we don’t need to protect here, because it’s a

Toggle quote (7 lines)
> +++ b/guix/platform.scm
> @@ -21,6 +21,7 @@ (define-module (guix platform)
> #:use-module (guix memoization)
> #:use-module (guix records)
> #:use-module (guix ui)
> + #:use-module (ice-9 exceptions)

So far the we use (srfi srfi-35) exclusively to define condition types;
I think we should do the same here, for consistency.

Toggle quote (3 lines)
> + &platform-not-found-error
> + make-platform-not-found-error

The constructor doesn’t need to be exposed.

Toggle quote (6 lines)
> +;;;
> +;;; Exceptions.
> +;;;
> +(define-exception-type &platform-not-found-error &error
> + make-platform-not-found-error platform-not-found-error?)

So this would become (define-condition-type …).

Otherwise LGTM, thanks!

Ludo’.
L
L
Ludovic Courtès wrote on 14 Jan 2023 15:34
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87mt6l9oc6.fsf@gnu.org
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

Toggle quote (13 lines)
> This was motivated by #60786, which produced a cryptic, hard to understand
> backtrace.
>
> * guix/platform.scm (&platform-not-found-error): New exception type.
> (make-platform-not-found-error): New exception constructor.
> (platform-not-found-error?): New predicate.
> (false-if-platform-not-found): New syntax.
> (lookup-platform-by-system): Raise an exception when no platform is found.
> Update documentation.
> (lookup-platform-by-target): Likewise.
> (lookup-platform-by-target-or-system): Likewise, and guard lookup calls with
> false-if-platform-not-found.

Sounds like a good idea!

Toggle quote (7 lines)
> +++ b/guix/platform.scm
> @@ -21,6 +21,7 @@ (define-module (guix platform)
> #:use-module (guix memoization)
> #:use-module (guix records)
> #:use-module (guix ui)
> + #:use-module (ice-9 exceptions)

So far the we use (srfi srfi-35) exclusively to define condition types;
I think we should do the same here, for consistency.

Toggle quote (3 lines)
> + &platform-not-found-error
> + make-platform-not-found-error

The constructor doesn’t need to be exposed.

Toggle quote (6 lines)
> +;;;
> +;;; Exceptions.
> +;;;
> +(define-exception-type &platform-not-found-error &error
> + make-platform-not-found-error platform-not-found-error?)

So this would become (define-condition-type …).

Otherwise LGTM, thanks!

Ludo’.
M
M
Maxim Cournoyer wrote on 14 Jan 2023 16:14
[PATCH v3 2/2] gnu: Remove u-boot-malta.
(address . 60802@debbugs.gnu.org)
20230114151457.1903-3-maxim.cournoyer@gmail.com

The mips64el architecture is not currently supported, causing (guix platform)
to raise an exception when attempting to cross-build the package.

* gnu/packages/bootloaders.scm (u-boot-malta): Delete variable.

---

(no changes since v1)

gnu/packages/bootloaders.scm | 3 ---
1 file changed, 3 deletions(-)

Toggle diff (16 lines)
diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 6e6bdb4c08..8dc6ff698d 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -965,9 +965,6 @@ (define*-public (make-u-boot-package board triplet
uboot-files)
#t)))))))))
-(define-public u-boot-malta
- (make-u-boot-package "malta" "mips64el-linux-gnuabi64"))
-
(define-public u-boot-am335x-boneblack
(let ((base (make-u-boot-package
"am335x_evm" "arm-linux-gnueabihf"
--
2.38.1
M
M
Maxim Cournoyer wrote on 14 Jan 2023 16:14
[PATCH v3 1/2] platforms: Raise an exception when no suitable platform is found.
(address . 60802@debbugs.gnu.org)
20230114151457.1903-2-maxim.cournoyer@gmail.com
This was motivated by #60786, which produced a cryptic, hard to understand
backtrace.

* guix/platform.scm (&platform-not-found-error): New exception type.
(make-platform-not-found-error): New exception constructor.
(platform-not-found-error?): New predicate.
(false-if-platform-not-found): New syntax.
(lookup-platform-by-system): Raise an exception when no platform is found.
Update documentation.
(lookup-platform-by-target): Likewise.
(lookup-platform-by-target-or-system): Likewise, and guard lookup calls with
false-if-platform-not-found.
* gnu/packages/bootstrap.scm (glibc-dynamic-linker): Handle
lookup-platform-by-system call to preserve existing behavior.

---

Changes in v3:
- Use false-if-platform-not-found instead of false-if-exception in (gnu
packages bootstrap)
- Do not export make-platform-not-found-error constructor

Changes in v2:
- Add false-if-platform-not-found syntax
- Use it while evaluating lookup-platform-by-target-or-system

gnu/packages/bootstrap.scm | 3 ++-
guix/platform.scm | 50 ++++++++++++++++++++++++++++----------
2 files changed, 39 insertions(+), 14 deletions(-)

Toggle diff (102 lines)
diff --git a/gnu/packages/bootstrap.scm b/gnu/packages/bootstrap.scm
index d2914fb5a7..9ea1a3e4d1 100644
--- a/gnu/packages/bootstrap.scm
+++ b/gnu/packages/bootstrap.scm
@@ -315,7 +315,8 @@ (define* (glibc-dynamic-linker
(%current-system))))
"Return the name of Glibc's dynamic linker for SYSTEM."
;; See the 'SYSDEP_KNOWN_INTERPRETER_NAMES' cpp macro in libc.
- (let ((platform (lookup-platform-by-system system)))
+ (let ((platform (false-if-platform-not-found
+ (lookup-platform-by-system system))))
(cond
((platform? platform)
(platform-glibc-dynamic-linker platform))
diff --git a/guix/platform.scm b/guix/platform.scm
index f873913fe0..4f4da002f7 100644
--- a/guix/platform.scm
+++ b/guix/platform.scm
@@ -21,6 +21,7 @@ (define-module (guix platform)
#:use-module (guix memoization)
#:use-module (guix records)
#:use-module (guix ui)
+ #:use-module (ice-9 exceptions)
#:use-module (srfi srfi-1)
#:export (platform
platform?
@@ -29,6 +30,10 @@ (define-module (guix platform)
platform-linux-architecture
platform-glibc-dynamic-linker
+ &platform-not-found-error
+ platform-not-found-error?
+ false-if-platform-not-found
+
platform-modules
platforms
lookup-platform-by-system
@@ -70,6 +75,19 @@ (define-record-type* <platform> platform make-platform
(default #false))
(glibc-dynamic-linker platform-glibc-dynamic-linker))
+
+;;;
+;;; Exceptions.
+;;;
+(define-exception-type &platform-not-found-error &error
+ make-platform-not-found-error platform-not-found-error?)
+
+(define-syntax-rule (false-if-platform-not-found exp)
+ "Evaluate EXP but return #f if it raises a platform-not-found-error?
+exception."
+ (guard (ex ((platform-not-found-error? ex) #f))
+ exp))
+
;;;
;;; Platforms.
@@ -94,23 +112,29 @@ (define platforms
(platform-modules)))))
(define (lookup-platform-by-system system)
- "Return the platform corresponding to the given SYSTEM."
- (find (lambda (platform)
- (let ((s (platform-system platform)))
- (and (string? s) (string=? s system))))
- (platforms)))
+ "Return the platform corresponding to the given SYSTEM. Raise
+&PLATFORM-NOT-FOUND-ERROR when no platform could be found."
+ (or (find (lambda (platform)
+ (let ((s (platform-system platform)))
+ (and (string? s) (string=? s system))))
+ (platforms))
+ (raise-exception (make-platform-not-found-error))))
(define (lookup-platform-by-target target)
- "Return the platform corresponding to the given TARGET."
- (find (lambda (platform)
- (let ((t (platform-target platform)))
- (and (string? t) (string=? t target))))
- (platforms)))
+ "Return the platform corresponding to the given TARGET. Raise
+&PLATFORM-NOT-FOUND-ERROR when no platform could be found."
+ (or (find (lambda (platform)
+ (let ((t (platform-target platform)))
+ (and (string? t) (string=? t target))))
+ (platforms))
+ (raise-exception (make-platform-not-found-error))))
(define (lookup-platform-by-target-or-system target-or-system)
- "Return the platform corresponding to the given TARGET or SYSTEM."
- (or (lookup-platform-by-target target-or-system)
- (lookup-platform-by-system target-or-system)))
+ "Return the platform corresponding to the given TARGET or SYSTEM. Raise
+&PLATFORM-NOT-FOUND-ERROR when no platform could be found."
+ (or (false-if-platform-not-found (lookup-platform-by-target target-or-system))
+ (false-if-platform-not-found (lookup-platform-by-system target-or-system))
+ (raise-exception (make-platform-not-found-error))))
(define (platform-system->target system)
"Return the target matching the given SYSTEM if it exists or false
--
2.38.1
J
J
Josselin Poiret wrote on 15 Jan 2023 14:57
87zgajkiiy.fsf@jpoiret.xyz
Hi Maxim,

This looks good to me, although in the grand scheme of things I wonder
if that change is a step forward: for those kinds of procedures, we
could expect consumers to instead always properly handle the #f case
themselves, rather than baby-sitting them and systematically relying on
exceptions in the parent procedure, no? As a caricatural example: the
SRFI-1 `find` could raise an exception instead of returning #f, but I
don't think anyone would consider that proper behaviour.

I don't have a particularly strong opinion towards either option, which
probably means that there's some discussion to be had here.

Best,
--
Josselin Poiret
M
M
Maxim Cournoyer wrote on 15 Jan 2023 23:11
(name . Josselin Poiret)(address . dev@jpoiret.xyz)
87y1q3cusj.fsf@gmail.com
Hi Josselin,

Josselin Poiret <dev@jpoiret.xyz> writes:

Toggle quote (8 lines)
> Hi Maxim,
>
> This looks good to me, although in the grand scheme of things I wonder
> if that change is a step forward: for those kinds of procedures, we
> could expect consumers to instead always properly handle the #f case
> themselves, rather than baby-sitting them and systematically relying on
> exceptions in the parent procedure, no?

I like exceptions; I think they are an improvement to C-style ad-hoc
checking of each call return; when things fail they fail early and in a
clear fashion, rather than indirectly, which is easier to debug and
removes the burden to duplicate the same checks at every call site.

Toggle quote (4 lines)
> As a caricatural example: the SRFI-1 `find` could raise an exception
> instead of returning #f, but I don't think anyone would consider that
> proper behaviour.

I think the find interface is well established;
lookup-platform-by-system and friends is not really intended to return
#f, as shown in the few usages in our code base (there's only one place
where it needed to be handled in previous code, and that's a TODO
waiting to move more things to (guix platform)).

Toggle quote (3 lines)
> I don't have a particularly strong opinion towards either option, which
> probably means that there's some discussion to be had here.

Good, that's what reviews are for :-).

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 16 Jan 2023 04:26
control message for bug #60224
(address . control@debbugs.gnu.org)
87bkmzcg72.fsf@gmail.com
block 60224 by 60802
quit
S
S
Simon Tournier wrote on 16 Jan 2023 12:00
Re: [bug#60802] [PATCH v3 1/2] platforms: Raise an exception when no suitable platform is found.
87a62ivj5n.fsf@gmail.com
Hi Josselin,

On dim., 15 janv. 2023 at 14:57, Josselin Poiret via Guix-patches via <guix-patches@gnu.org> wrote:

Toggle quote (8 lines)
> This looks good to me, although in the grand scheme of things I wonder
> if that change is a step forward: for those kinds of procedures, we
> could expect consumers to instead always properly handle the #f case
> themselves, rather than baby-sitting them and systematically relying on
> exceptions in the parent procedure, no? As a caricatural example: the
> SRFI-1 `find` could raise an exception instead of returning #f, but I
> don't think anyone would consider that proper behaviour.

How to handle, is it not somehow a convention? Using the example
’find’, we could imagine it returns an exception ’not-found’ instead of
the current plain #f – this #f appears to me a convention, no? Or
’find’ could return some Maybe monad (’Just the-result’ or None) –
another convention.

Well, how to handle appears to me a convention when composing. :-)


Cheers,
simon
M
M
Maxim Cournoyer wrote on 16 Jan 2023 18:46
Re: [PATCH v2 1/2] platforms: Raise an exception when no suitable platform is found.
(name . Ludovic Courtès)(address . ludo@gnu.org)
87mt6i5q4q.fsf@gmail.com
Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (19 lines)
> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> This was motivated by #60786, which produced a cryptic, hard to understand
>> backtrace.
>>
>> * guix/platform.scm (&platform-not-found-error): New exception type.
>> (make-platform-not-found-error): New exception constructor.
>> (platform-not-found-error?): New predicate.
>> (false-if-platform-not-found): New syntax.
>> (lookup-platform-by-system): Raise an exception when no platform is found.
>> Update documentation.
>> (lookup-platform-by-target): Likewise.
>> (lookup-platform-by-target-or-system): Likewise, and guard lookup calls with
>> false-if-platform-not-found.
>
> Sounds like a good idea!

Good!

Toggle quote (10 lines)
>> +++ b/gnu/packages/bootstrap.scm
>> @@ -315,7 +315,7 @@ (define* (glibc-dynamic-linker
>> (%current-system))))
>> "Return the name of Glibc's dynamic linker for SYSTEM."
>> ;; See the 'SYSDEP_KNOWN_INTERPRETER_NAMES' cpp macro in libc.
>> - (let ((platform (lookup-platform-by-system system)))
>> + (let ((platform (false-if-exception (lookup-platform-by-system system))))
>
> Maybe we don’t need to protect here, because it’s a

We cannot do this, otherwise the other cond clauses would never been
evaluated:

Toggle snippet (27 lines)
(let ((platform (false-if-exception (lookup-platform-by-system system))))
(cond
((platform? platform)
(platform-glibc-dynamic-linker platform))

--> Clauses below here are evaluated when platform was not found.

;; TODO: Define those as platforms.
((string=? system "i686-gnu") "/lib/ld.so.1")
((string=? system "powerpc64-linux") "/lib/ld64.so.1")
((string=? system "alpha-linux") "/lib/ld-linux.so.2")

;; XXX: This one is used bare-bones, without a libc, so add a case
;; here just so we can keep going.
((string=? system "arm-elf") "no-ld.so")
((string=? system "arm-eabi") "no-ld.so")
((string=? system "xtensa-elf") "no-ld.so")
((string=? system "avr") "no-ld.so")
((string=? system "propeller-elf") "no-ld.so")
((string=? system "i686-mingw") "no-ld.so")
((string=? system "x86_64-mingw") "no-ld.so")
((string=? system "vc4-elf") "no-ld.so")

(else (error "dynamic linker name not known for this system"
system))))

I'll change it to use false-if-platform-not-found though.

Toggle quote (10 lines)
>> +++ b/guix/platform.scm
>> @@ -21,6 +21,7 @@ (define-module (guix platform)
>> #:use-module (guix memoization)
>> #:use-module (guix records)
>> #:use-module (guix ui)
>> + #:use-module (ice-9 exceptions)
>
> So far the we use (srfi srfi-35) exclusively to define condition types;
> I think we should do the same here, for consistency.

Could we instead start migrating away from srfi-35 to (ice-9
exceptions), which is the new native way to use exceptions in Guile? I
think it'd be nicer to use it in the future, to avoid newcomers being
confusing about the 3 or 4 ways to manage exceptions in Guile
(recommended read on the topic:

Migrating the whole code at once doesn't seem a good idea, so gradually
transitioning (such as using (ice-9 exceptions) for new code) appears a
good idea to me, if we agree on the direction!

Toggle quote (5 lines)
>> + &platform-not-found-error
>> + make-platform-not-found-error
>
> The constructor doesn’t need to be exposed.

Good point. Fixed in the latest revision.

--
Thanks,
Maxim
L
L
Ludovic Courtès wrote on 17 Jan 2023 09:59
Re: [PATCH v3 1/2] platforms: Raise an exception when no suitable platform is found.
(name . Josselin Poiret)(address . dev@jpoiret.xyz)
87r0vtk03h.fsf@gnu.org
Hi Josselin,

Josselin Poiret <dev@jpoiret.xyz> skribis:

Toggle quote (8 lines)
> This looks good to me, although in the grand scheme of things I wonder
> if that change is a step forward: for those kinds of procedures, we
> could expect consumers to instead always properly handle the #f case
> themselves, rather than baby-sitting them and systematically relying on
> exceptions in the parent procedure, no? As a caricatural example: the
> SRFI-1 `find` could raise an exception instead of returning #f, but I
> don't think anyone would consider that proper behaviour.

I share this sentiment in general (plus the fact that we should keep UI
aspects, such as error reports, separate from core logic). Here there’s
a precedent with other lookup procedures though
(‘lookup-bootloader-by-name’, ‘lookup-compressor’,
‘lookup-image-type-by-name’, etc.), so I think it’s okay to keep it that
way.

Ludo’.
L
L
Ludovic Courtès wrote on 17 Jan 2023 10:22
Re: [PATCH v2 1/2] platforms: Raise an exception when no suitable platform is found.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
878ri1jz0w.fsf@gnu.org
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

Toggle quote (13 lines)
>>> +++ b/gnu/packages/bootstrap.scm
>>> @@ -315,7 +315,7 @@ (define* (glibc-dynamic-linker
>>> (%current-system))))
>>> "Return the name of Glibc's dynamic linker for SYSTEM."
>>> ;; See the 'SYSDEP_KNOWN_INTERPRETER_NAMES' cpp macro in libc.
>>> - (let ((platform (lookup-platform-by-system system)))
>>> + (let ((platform (false-if-exception (lookup-platform-by-system system))))
>>
>> Maybe we don’t need to protect here, because it’s a
>
> We cannot do this, otherwise the other cond clauses would never been
> evaluated:

Yes sorry, I realized later but hit “Send” before deleting the sentence
prototype. :-)

Toggle quote (6 lines)
>> So far the we use (srfi srfi-35) exclusively to define condition types;
>> I think we should do the same here, for consistency.
>
> Could we instead start migrating away from srfi-35 to (ice-9
> exceptions), which is the new native way to use exceptions in Guile?

Maybe, but I’m not convinced it’s worth it.

Toggle quote (3 lines)
> I think it'd be nicer to use it in the future, to avoid newcomers
> being confusing about the 3 or 4 ways to manage exceptions in Guile

That’s why I suggest sticking to one way in Guix, which is currently
SRFI-34/35.

Toggle quote (4 lines)
> Migrating the whole code at once doesn't seem a good idea, so gradually
> transitioning (such as using (ice-9 exceptions) for new code) appears a
> good idea to me, if we agree on the direction!

I’m not convinced it’s worth transitioning, and I’m even less convinced
about a gradual transition, as that would make it harder for newcomers
to join, people wouldn’t be sure which style to use, etc.

It’s a discussion worth having though, but I think we can have it
separately.

Thanks,
Ludo’.
S
S
Simon Tournier wrote on 17 Jan 2023 13:35
Re: [bug#60802] [PATCH v3 1/2] platforms: Raise an exception when no suitable platform is found.
87pmbds5jf.fsf@gmail.com
Hi,

On mar., 17 janv. 2023 at 09:59, Ludovic Courtès <ludo@gnu.org> wrote:
Toggle quote (17 lines)
> Josselin Poiret <dev@jpoiret.xyz> skribis:
>
>> This looks good to me, although in the grand scheme of things I wonder
>> if that change is a step forward: for those kinds of procedures, we
>> could expect consumers to instead always properly handle the #f case
>> themselves, rather than baby-sitting them and systematically relying on
>> exceptions in the parent procedure, no? As a caricatural example: the
>> SRFI-1 `find` could raise an exception instead of returning #f, but I
>> don't think anyone would consider that proper behaviour.
>
> I share this sentiment in general (plus the fact that we should keep UI
> aspects, such as error reports, separate from core logic). Here there’s
> a precedent with other lookup procedures though
> (‘lookup-bootloader-by-name’, ‘lookup-compressor’,
> ‘lookup-image-type-by-name’, etc.), so I think it’s okay to keep it that
> way.

Well, from my small experience with other programming language, they
barely do return a boolean when they fail. I think this way using a
boolean is because some historical reasons when exceptions was not
implemented in Scheme (or other languages).

Exception allows the motto: «ask for forgiveness, not permission» while
keeping under control the side effects. Well, IMHO exception is often a
good practise for dynamically typed programming language; as Scheme (or
Python).

From my point of view, exception is not related to “should keep UI
aspects, such as error reports, separated from core logic”. This is how
the exception is handled. It is often easier to propagate exception
until an handler than propagate a boolean (as with ’find’).

And if the exception is not handled, then it just returns a backtrace,
which is more informative, IMHO.


For instance, Python returns an exception:

Toggle snippet (20 lines)
$ guix shell python python-ipython -- ipython
Python 3.9.9 (main, Jan 1 1970, 00:00:01)
Type 'copyright', 'credits' or 'license' for more information
IPython 8.2.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: lst = [1,2,3]
lst = [1,2,3]

In [2]: lst.index(2)
Out[2]: 1

In [3]: lst.index(10)
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
Input In [3], in <cell line: 1>()
----> 1 lst.index(10)

ValueError: 10 is not in list

For other instances, OCaml implements two ’find’ [1], one using an
exception and another using a Maybe monad (named ’option’).

Toggle snippet (6 lines)
val find : ('a -> bool) -> 'a list -> 'a
Raises Not_found if there is no value that satisfies f in the list l.

val find_opt : ('a -> bool) -> 'a list -> 'a option

Haskell returns a Maybe monad [2]:

Toggle snippet (3 lines)
find :: Foldable t => (a -> Bool) -> t a -> Maybe a

Well, from a signature point of view, ’find’ from SRFI-1 returns an
union type (value or boolean) which can lead to hard to detect bugs:
when composing functions, the error can be incorrectly pointed by the
compiler to a totally unrelated place.

Instead, the exception allows to keep an expected signature (say one
value as with ’find’) but raises the error at the correct place. If
there is no handler, it just raises the backtrace.

From my point of view, code using exception cannot be worse* than
without. That’s my general sentiment. :-)

*worse: for sure, we could discuss some performance penalty depending on
the context.



Cheers,
simon
M
M
Maxim Cournoyer wrote on 17 Jan 2023 15:38
(name . Simon Tournier)(address . zimon.toutoune@gmail.com)
87ilh52pl7.fsf@gmail.com
Hi Simon,

Simon Tournier <zimon.toutoune@gmail.com> writes:

Toggle quote (38 lines)
> Hi,
>
> On mar., 17 janv. 2023 at 09:59, Ludovic Courtès <ludo@gnu.org> wrote:
>> Josselin Poiret <dev@jpoiret.xyz> skribis:
>>
>>> This looks good to me, although in the grand scheme of things I wonder
>>> if that change is a step forward: for those kinds of procedures, we
>>> could expect consumers to instead always properly handle the #f case
>>> themselves, rather than baby-sitting them and systematically relying on
>>> exceptions in the parent procedure, no? As a caricatural example: the
>>> SRFI-1 `find` could raise an exception instead of returning #f, but I
>>> don't think anyone would consider that proper behaviour.
>>
>> I share this sentiment in general (plus the fact that we should keep UI
>> aspects, such as error reports, separate from core logic). Here there’s
>> a precedent with other lookup procedures though
>> (‘lookup-bootloader-by-name’, ‘lookup-compressor’,
>> ‘lookup-image-type-by-name’, etc.), so I think it’s okay to keep it that
>> way.
>
> Well, from my small experience with other programming language, they
> barely do return a boolean when they fail. I think this way using a
> boolean is because some historical reasons when exceptions was not
> implemented in Scheme (or other languages).
>
> Exception allows the motto: «ask for forgiveness, not permission» while
> keeping under control the side effects. Well, IMHO exception is often a
> good practise for dynamically typed programming language; as Scheme (or
> Python).
>
> From my point of view, exception is not related to “should keep UI
> aspects, such as error reports, separated from core logic”. This is how
> the exception is handled. It is often easier to propagate exception
> until an handler than propagate a boolean (as with ’find’).
>
> And if the exception is not handled, then it just returns a backtrace,
> which is more informative, IMHO.

You've summarized well the reasons I think using an exception here makes
sense (and why using exceptions rather than C-style booleans to
propagate error conditions is preferable in general for languages where
it's possible to do so -- luckily Scheme is one).

I'll send a v4 reworking it to use srfi-34/35, so that the discussion to
migrate to (ice-9 exceptions) can be kept separate.

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote on 17 Jan 2023 16:34
[PATCH v4 0/2] Remove unsupported u-boot-malta package
(address . 60802@debbugs.gnu.org)
20230117153432.23719-1-maxim.cournoyer@gmail.com
The package was causing issues when migrating the make-u-boot-package
procedure to use Guix's infrastructure for cross-compilation, because Guix
would now attempt to build a cross-compiler for mips64el, which is not
supported per (guix platforms).

Changes in v4:
- Use (srfi srfi-35) condition instead of (ice-9 exceptions)
- Add a target-or-system field to the condition type for extra information

Changes in v3:
- Use false-if-platform-not-found instead of false-if-exception in (gnu
packages bootstrap)
- Do not export make-platform-not-found-error constructor

Changes in v2:
- Add false-if-platform-not-found syntax
- Use it while evaluating lookup-platform-by-target-or-system

Maxim Cournoyer (2):
platforms: Raise an exception when no suitable platform is found.
gnu: Remove u-boot-malta.

gnu/packages/bootloaders.scm | 3 --
gnu/packages/bootstrap.scm | 3 +-
guix/platform.scm | 55 +++++++++++++++++++++++++++---------
3 files changed, 44 insertions(+), 17 deletions(-)


base-commit: ecda67a577570f412b103e5dd8ed1a44193a9c11
--
2.38.1
M
M
Maxim Cournoyer wrote on 17 Jan 2023 16:34
[PATCH v4 1/2] platforms: Raise an exception when no suitable platform is found.
(address . 60802@debbugs.gnu.org)
20230117153432.23719-2-maxim.cournoyer@gmail.com
This was motivated by #60786, which produced a cryptic, hard to understand
backtrace.

Given the following reproducer:
(use-modules (guix packages)
(gnu packages cross-base))

(define linux-libre-headers-cross-mips64el-linux-gnuabi64
(cross-kernel-headers "mips64el-linux-gnuabi64"))

(package-arguments linux-libre-headers-cross-mips64el-linux-gnuabi64)

Before this change:
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
In procedure struct-vtable: Wrong type argument in position 1 (expecting struct): #f

After this change:
ice-9/boot-9.scm:1685:16: In procedure raise-exception:
ERROR:
1. &platform-not-found-error: "mips64el-linux-gnuabi64"

* guix/platform.scm (&platform-not-found-error): New condition.
(platform-not-found-error?): New predicate.
(false-if-platform-not-found): New syntax.
(lookup-platform-by-system): Raise an exception when no platform is found.
Update documentation.
(lookup-platform-by-target): Likewise.
(lookup-platform-by-target-or-system): Likewise, and guard lookup calls with
false-if-platform-not-found.
* gnu/packages/bootstrap.scm (glibc-dynamic-linker): Handle
lookup-platform-by-system call to preserve existing behavior.

---

Changes in v4:
- Use (srfi srfi-35) condition instead of (ice-9 exceptions)
- Add a target-or-system field to the condition type for extra information

Changes in v3:
- Use false-if-platform-not-found instead of false-if-exception in (gnu
packages bootstrap)
- Do not export make-platform-not-found-error constructor

Changes in v2:
- Add false-if-platform-not-found syntax
- Use it while evaluating lookup-platform-by-target-or-system

gnu/packages/bootstrap.scm | 3 ++-
guix/platform.scm | 55 +++++++++++++++++++++++++++++---------
2 files changed, 44 insertions(+), 14 deletions(-)

Toggle diff (107 lines)
diff --git a/gnu/packages/bootstrap.scm b/gnu/packages/bootstrap.scm
index d2914fb5a7..9ea1a3e4d1 100644
--- a/gnu/packages/bootstrap.scm
+++ b/gnu/packages/bootstrap.scm
@@ -315,7 +315,8 @@ (define* (glibc-dynamic-linker
(%current-system))))
"Return the name of Glibc's dynamic linker for SYSTEM."
;; See the 'SYSDEP_KNOWN_INTERPRETER_NAMES' cpp macro in libc.
- (let ((platform (lookup-platform-by-system system)))
+ (let ((platform (false-if-platform-not-found
+ (lookup-platform-by-system system))))
(cond
((platform? platform)
(platform-glibc-dynamic-linker platform))
diff --git a/guix/platform.scm b/guix/platform.scm
index f873913fe0..a2d95ab507 100644
--- a/guix/platform.scm
+++ b/guix/platform.scm
@@ -22,6 +22,8 @@ (define-module (guix platform)
#:use-module (guix records)
#:use-module (guix ui)
#:use-module (srfi srfi-1)
+ #:use-module (srfi srfi-34)
+ #:use-module (srfi srfi-35)
#:export (platform
platform?
platform-target
@@ -29,6 +31,10 @@ (define-module (guix platform)
platform-linux-architecture
platform-glibc-dynamic-linker
+ &platform-not-found-error
+ platform-not-found-error?
+ false-if-platform-not-found
+
platform-modules
platforms
lookup-platform-by-system
@@ -70,6 +76,20 @@ (define-record-type* <platform> platform make-platform
(default #false))
(glibc-dynamic-linker platform-glibc-dynamic-linker))
+
+;;;
+;;; Exceptions.
+;;;
+(define-condition-type &platform-not-found-error &error
+ platform-not-found-error?
+ (target-or-system platform-not-found-error-target-or-system))
+
+(define-syntax-rule (false-if-platform-not-found exp)
+ "Evaluate EXP but return #f if it raises a platform-not-found-error?
+exception."
+ (guard (ex ((platform-not-found-error? ex) #f))
+ exp))
+
;;;
;;; Platforms.
@@ -94,23 +114,32 @@ (define platforms
(platform-modules)))))
(define (lookup-platform-by-system system)
- "Return the platform corresponding to the given SYSTEM."
- (find (lambda (platform)
- (let ((s (platform-system platform)))
- (and (string? s) (string=? s system))))
- (platforms)))
+ "Return the platform corresponding to the given SYSTEM. Raise
+&PLATFORM-NOT-FOUND-ERROR when no platform could be found."
+ (or (find (lambda (platform)
+ (let ((s (platform-system platform)))
+ (and (string? s) (string=? s system))))
+ (platforms))
+ (raise-exception (condition (&platform-not-found-error
+ (target-or-system system))))))
(define (lookup-platform-by-target target)
- "Return the platform corresponding to the given TARGET."
- (find (lambda (platform)
- (let ((t (platform-target platform)))
- (and (string? t) (string=? t target))))
- (platforms)))
+ "Return the platform corresponding to the given TARGET. Raise
+&PLATFORM-NOT-FOUND-ERROR when no platform could be found."
+ (or (find (lambda (platform)
+ (let ((t (platform-target platform)))
+ (and (string? t) (string=? t target))))
+ (platforms))
+ (raise-exception (condition (&platform-not-found-error
+ (target-or-system target))))))
(define (lookup-platform-by-target-or-system target-or-system)
- "Return the platform corresponding to the given TARGET or SYSTEM."
- (or (lookup-platform-by-target target-or-system)
- (lookup-platform-by-system target-or-system)))
+ "Return the platform corresponding to the given TARGET or SYSTEM. Raise
+&PLATFORM-NOT-FOUND-ERROR when no platform could be found."
+ (or (false-if-platform-not-found (lookup-platform-by-target target-or-system))
+ (false-if-platform-not-found (lookup-platform-by-system target-or-system))
+ (raise-exception (condition (&platform-not-found-error
+ (target-or-system target-or-system))))))
(define (platform-system->target system)
"Return the target matching the given SYSTEM if it exists or false
--
2.38.1
M
M
Maxim Cournoyer wrote on 17 Jan 2023 16:34
[PATCH v4 2/2] gnu: Remove u-boot-malta.
(address . 60802@debbugs.gnu.org)
20230117153432.23719-3-maxim.cournoyer@gmail.com

The mips64el architecture is not currently supported, causing (guix platform)
to raise an exception when attempting to cross-build the package.

* gnu/packages/bootloaders.scm (u-boot-malta): Delete variable.

---

(no changes since v1)

gnu/packages/bootloaders.scm | 3 ---
1 file changed, 3 deletions(-)

Toggle diff (16 lines)
diff --git a/gnu/packages/bootloaders.scm b/gnu/packages/bootloaders.scm
index 6e6bdb4c08..8dc6ff698d 100644
--- a/gnu/packages/bootloaders.scm
+++ b/gnu/packages/bootloaders.scm
@@ -965,9 +965,6 @@ (define*-public (make-u-boot-package board triplet
uboot-files)
#t)))))))))
-(define-public u-boot-malta
- (make-u-boot-package "malta" "mips64el-linux-gnuabi64"))
-
(define-public u-boot-am335x-boneblack
(let ((base (make-u-boot-package
"am335x_evm" "arm-linux-gnueabihf"
--
2.38.1
M
M
Maxim Cournoyer wrote on 19 Jan 2023 02:55
Re: bug#60802: [PATCH 0/2] Remove unsupported u-boot-malta package
(name . Ludovic Courtès)(address . ludo@gnu.org)
87h6wne18w.fsf_-_@gmail.com
Hello,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (44 lines)
> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> This was motivated by #60786, which produced a cryptic, hard to understand
>> backtrace.
>>
>> * guix/platform.scm (&platform-not-found-error): New exception type.
>> (make-platform-not-found-error): New exception constructor.
>> (platform-not-found-error?): New predicate.
>> (false-if-platform-not-found): New syntax.
>> (lookup-platform-by-system): Raise an exception when no platform is found.
>> Update documentation.
>> (lookup-platform-by-target): Likewise.
>> (lookup-platform-by-target-or-system): Likewise, and guard lookup calls with
>> false-if-platform-not-found.
>
> Sounds like a good idea!
>
>> +++ b/guix/platform.scm
>> @@ -21,6 +21,7 @@ (define-module (guix platform)
>> #:use-module (guix memoization)
>> #:use-module (guix records)
>> #:use-module (guix ui)
>> + #:use-module (ice-9 exceptions)
>
> So far the we use (srfi srfi-35) exclusively to define condition types;
> I think we should do the same here, for consistency.
>
>> + &platform-not-found-error
>> + make-platform-not-found-error
>
> The constructor doesn’t need to be exposed.
>
>> +;;;
>> +;;; Exceptions.
>> +;;;
>> +(define-exception-type &platform-not-found-error &error
>> + make-platform-not-found-error platform-not-found-error?)
>
> So this would become (define-condition-type …).
>
> Otherwise LGTM, thanks!

The latest version incorporate these changes and use srfi-34/35 as is
currently custom in Guix.

I've now applied it.

--
Thanks,
Maxim
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 60802@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 60802
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch