all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
To: "Ludovic Courtès" <ludo@gnu.org>
Cc: Tobias Geerinckx-Rice <me@tobias.gr>,
	Josselin Poiret <dev@jpoiret.xyz>,
	60802@debbugs.gnu.org, Simon Tournier <zimon.toutoune@gmail.com>,
	Mathieu Othacehe <othacehe@gnu.org>,
	Christopher Baines <mail@cbaines.net>,
	Ricardo Wurmus <rekado@elephly.net>
Subject: [bug#60802] [PATCH v2 1/2] platforms: Raise an exception when no suitable platform is found.
Date: Mon, 16 Jan 2023 12:46:29 -0500	[thread overview]
Message-ID: <87mt6i5q4q.fsf@gmail.com> (raw)
In-Reply-To: <87o7r19ocn.fsf@gnu.org> ("Ludovic Courtès"'s message of "Sat, 14 Jan 2023 15:34:32 +0100")

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




  reply	other threads:[~2023-01-16 17:48 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-14  3:05 [bug#60802] [PATCH 0/2] Remove unsupported u-boot-malta package Maxim Cournoyer
2023-01-14  3:08 ` [bug#60802] [PATCH 1/2] platforms: Raise an exception when no suitable platform is found Maxim Cournoyer
2023-01-14  3:08   ` [bug#60802] [PATCH 2/2] gnu: Remove u-boot-malta Maxim Cournoyer
2023-01-14  4:19 ` [bug#60802] [PATCH v2 1/2] platforms: Raise an exception when no suitable platform is found Maxim Cournoyer
2023-01-14  4:19   ` [bug#60802] [PATCH v2 2/2] gnu: Remove u-boot-malta Maxim Cournoyer
2023-01-14 14:34   ` [bug#60802] [PATCH v2 1/2] platforms: Raise an exception when no suitable platform is found Ludovic Courtès
2023-01-16 17:46     ` Maxim Cournoyer [this message]
2023-01-16 20:13       ` Exception: srfi-35 vs (ice-9 exceptions (was Re: [bug#60802] [PATCH v2 1/2] platforms: Raise an exception when no suitable platform is found.) zimoun
2023-01-16 21:59         ` Maxim Cournoyer
2023-01-17 16:35           ` Ludovic Courtès
2023-01-16 22:35         ` Ricardo Wurmus
2023-01-17 19:58           ` Josselin Poiret
2023-01-19 14:38             ` Ludovic Courtès
2023-01-19 15:59               ` Katherine Cox-Buday
2023-01-17  9:22       ` [bug#60802] [PATCH v2 1/2] platforms: Raise an exception when no suitable platform is found Ludovic Courtès
2023-01-14 14:34   ` Ludovic Courtès
2023-01-19  1:55     ` bug#60802: [PATCH 0/2] Remove unsupported u-boot-malta package Maxim Cournoyer
2023-01-14 15:14 ` [bug#60802] [PATCH v3 1/2] platforms: Raise an exception when no suitable platform is found Maxim Cournoyer
2023-01-14 15:14   ` [bug#60802] [PATCH v3 2/2] gnu: Remove u-boot-malta Maxim Cournoyer
2023-01-15 13:57   ` [bug#60802] [PATCH v3 1/2] platforms: Raise an exception when no suitable platform is found Josselin Poiret via Guix-patches via
2023-01-15 22:11     ` Maxim Cournoyer
2023-01-16 11:00     ` Simon Tournier
2023-01-17  8:59     ` Ludovic Courtès
2023-01-17 12:35       ` Simon Tournier
2023-01-17 14:38         ` Maxim Cournoyer
2023-01-17 15:34 ` [bug#60802] [PATCH v4 0/2] Remove unsupported u-boot-malta package Maxim Cournoyer
2023-01-17 15:34   ` [bug#60802] [PATCH v4 1/2] platforms: Raise an exception when no suitable platform is found Maxim Cournoyer
2023-01-17 15:34   ` [bug#60802] [PATCH v4 2/2] gnu: Remove u-boot-malta Maxim Cournoyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87mt6i5q4q.fsf@gmail.com \
    --to=maxim.cournoyer@gmail.com \
    --cc=60802@debbugs.gnu.org \
    --cc=dev@jpoiret.xyz \
    --cc=ludo@gnu.org \
    --cc=mail@cbaines.net \
    --cc=me@tobias.gr \
    --cc=othacehe@gnu.org \
    --cc=rekado@elephly.net \
    --cc=zimon.toutoune@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.