[bug#60802] [PATCH v2 1/2] platforms: Raise an exception when no suitable platform is found.

[Maxim Cournoyer]

Hi Ludovic,

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

> 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!

>> +++ 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:

--8<---------------cut here---------------start------------->8---
  (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))))
--8<---------------cut here---------------end--------------->8---

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

>> +++ 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:
https://vijaymarupudi.com/blog/2022-02-13-error-handling-in-guile.html).

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!

>> +            &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