diff mbox series

[bug#70985,v2,5/6] gnu: rust: Guard against cross-libc returning #f.

Message ID 0c15a853521c4a9cfa8f4c3558e3aee9fb0a9d91.1720195563.git.mail@cbaines.net
State New
Headers show
Series [bug#70985,v2,1/6] guix: packages: Add new &package-unsupported-target-error. | expand

Commit Message

Christopher Baines July 5, 2024, 4:06 p.m. UTC
* gnu/packages/rust.scm (make-rust-sysroot/implementation): Guard against
cross-libc returning #f.

Change-Id: Ia0d5c889c6f5cd3478ad985c79feb9ba1c472c29
---
 gnu/packages/rust.scm | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Efraim Flashner July 7, 2024, 4:26 p.m. UTC | #1
On Fri, Jul 05, 2024 at 06:06:02PM +0200, Christopher Baines wrote:
> * gnu/packages/rust.scm (make-rust-sysroot/implementation): Guard against
> cross-libc returning #f.
> 
> Change-Id: Ia0d5c889c6f5cd3478ad985c79feb9ba1c472c29
> ---
>  gnu/packages/rust.scm | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/gnu/packages/rust.scm b/gnu/packages/rust.scm
> index a385344473..f1de34b277 100644
> --- a/gnu/packages/rust.scm
> +++ b/gnu/packages/rust.scm
> @@ -73,7 +73,9 @@ (define-module (gnu packages rust)
>    #:use-module (ice-9 match)
>    #:use-module (ice-9 optargs)
>    #:use-module (srfi srfi-1)
> -  #:use-module (srfi srfi-26))
> +  #:use-module (srfi srfi-26)
> +  #:use-module (srfi srfi-34)
> +  #:use-module (srfi srfi-35))
>  
>  ;; This is the hash for the empty file, and the reason it's relevant is not
>  ;; the most obvious.
> @@ -1464,7 +1466,11 @@ (define make-rust-sysroot/implementation
>           (modify-inputs (package-native-inputs base-rust)
>                          (prepend (cross-gcc target
>                                              #:libc (cross-libc target))
> -                                 (cross-libc target)
> +                                 (or (cross-libc target) ; could be #f
> +                                     (raise (condition
> +                                             (&package-unsupported-target-error
> +                                              (package (libc-for-target target))
> +                                              (target target)))))
>                                   (cross-binutils target)))))
>        (properties
>         `((hidden? . #t)
> -- 
> 2.45.2

rust does support architectures without a libc, for example the mingw
targets a few lines above this snippet. I think it would make more sense
to only include (cross-libc target) and not if not.

I just tried building zoxide for powerpc-linux-gnu without (cross-libc
target) and it tried to link with the x86_64 libc, so it looks like it
is necessary where it is available.

How about we make this section even worse with:
(if (false-if-exception (cross-libc target))
    (modify-inputs ...
                   (cross-libc target)
                   ...)
    (modify-inputs ...))    ; no (cross-libc target)

In the meantime, I'll try to figure something out for actually putting
everything inside one modify-inputs so we don't have an ever expanding
number of them.
Efraim Flashner July 7, 2024, 4:57 p.m. UTC | #2
On Fri, Jul 05, 2024 at 06:06:02PM +0200, Christopher Baines wrote:
> * gnu/packages/rust.scm (make-rust-sysroot/implementation): Guard against
> cross-libc returning #f.
> 
> Change-Id: Ia0d5c889c6f5cd3478ad985c79feb9ba1c472c29
> ---
>  gnu/packages/rust.scm | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/gnu/packages/rust.scm b/gnu/packages/rust.scm
> index a385344473..f1de34b277 100644
> --- a/gnu/packages/rust.scm
> +++ b/gnu/packages/rust.scm
> @@ -73,7 +73,9 @@ (define-module (gnu packages rust)
>    #:use-module (ice-9 match)
>    #:use-module (ice-9 optargs)
>    #:use-module (srfi srfi-1)
> -  #:use-module (srfi srfi-26))
> +  #:use-module (srfi srfi-26)
> +  #:use-module (srfi srfi-34)
> +  #:use-module (srfi srfi-35))
>  
>  ;; This is the hash for the empty file, and the reason it's relevant is not
>  ;; the most obvious.
> @@ -1464,7 +1466,11 @@ (define make-rust-sysroot/implementation
>           (modify-inputs (package-native-inputs base-rust)
>                          (prepend (cross-gcc target
>                                              #:libc (cross-libc target))
> -                                 (cross-libc target)
> +                                 (or (cross-libc target) ; could be #f
> +                                     (raise (condition
> +                                             (&package-unsupported-target-error
> +                                              (package (libc-for-target target))
> +                                              (target target)))))
>                                   (cross-binutils target)))))
>        (properties
>         `((hidden? . #t)
> -- 
> 2.45.2

This will probably work:

(native-inputs
 `((,(string-append "gcc-cross-" target) ,(cross-gcc target
                                                     #:libc (cross-libc target)))
   ,(when (false-if-exception (cross-libc target))
          `(,(string-append "glibc-cross-" target) ,(cross-libc target)))
   (,(string-append "binutils-cross-" target) ,(cross-binutils target))
   ,(when (target-mingw? target)
      (if (string=? "i686-w64-mingw32" target)
          `("mingw-w64-i686-winpthreads" ,mingw-w64-i686-winpthreads)
          `("mingw-w64-x86_64-winpthreads" ,mingw-w64-x86_64-winpthreads)))
   ,@(package-native-inputs base-rust)))
Christopher Baines July 9, 2024, 9:25 a.m. UTC | #3
Efraim Flashner <efraim@flashner.co.il> writes:

> On Fri, Jul 05, 2024 at 06:06:02PM +0200, Christopher Baines wrote:
>> * gnu/packages/rust.scm (make-rust-sysroot/implementation): Guard against
>> cross-libc returning #f.
>> 
>> Change-Id: Ia0d5c889c6f5cd3478ad985c79feb9ba1c472c29
>> ---
>>  gnu/packages/rust.scm | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>> 
>> diff --git a/gnu/packages/rust.scm b/gnu/packages/rust.scm
>> index a385344473..f1de34b277 100644
>> --- a/gnu/packages/rust.scm
>> +++ b/gnu/packages/rust.scm
>> @@ -73,7 +73,9 @@ (define-module (gnu packages rust)
>>    #:use-module (ice-9 match)
>>    #:use-module (ice-9 optargs)
>>    #:use-module (srfi srfi-1)
>> -  #:use-module (srfi srfi-26))
>> +  #:use-module (srfi srfi-26)
>> +  #:use-module (srfi srfi-34)
>> +  #:use-module (srfi srfi-35))
>>  
>>  ;; This is the hash for the empty file, and the reason it's relevant is not
>>  ;; the most obvious.
>> @@ -1464,7 +1466,11 @@ (define make-rust-sysroot/implementation
>>           (modify-inputs (package-native-inputs base-rust)
>>                          (prepend (cross-gcc target
>>                                              #:libc (cross-libc target))
>> -                                 (cross-libc target)
>> +                                 (or (cross-libc target) ; could be #f
>> +                                     (raise (condition
>> +                                             (&package-unsupported-target-error
>> +                                              (package (libc-for-target target))
>> +                                              (target target)))))
>>                                   (cross-binutils target)))))
>>        (properties
>>         `((hidden? . #t)
>> -- 
>> 2.45.2
>
> This will probably work:
>
> (native-inputs
>  `((,(string-append "gcc-cross-" target) ,(cross-gcc target
>                                                      #:libc (cross-libc target)))
>    ,(when (false-if-exception (cross-libc target))
>           `(,(string-append "glibc-cross-" target) ,(cross-libc target)))
>    (,(string-append "binutils-cross-" target) ,(cross-binutils target))
>    ,(when (target-mingw? target)
>       (if (string=? "i686-w64-mingw32" target)
>           `("mingw-w64-i686-winpthreads" ,mingw-w64-i686-winpthreads)
>           `("mingw-w64-x86_64-winpthreads" ,mingw-w64-x86_64-winpthreads)))
>    ,@(package-native-inputs base-rust)))

Thanks for taking a look. In the latest patches cross-libc isn't
changing to raise an exception (as that ended up being too complicated),
so I'm not sure the false-if-exception is going to work.

I'd also maybe stick with modify-inputs, as at least that avoids the
older inputs style.

If I've followed your first email correctly, are you thinking of
something like this?

      (native-inputs
       (if (target-mingw? target)
         (modify-inputs (package-native-inputs base-rust)
                        (prepend (cross-gcc target
                                            #:libc (cross-libc target))
                                 (cross-binutils target)
                                 (if (string=? "i686-w64-mingw32" target)
                                     mingw-w64-i686-winpthreads
                                     mingw-w64-x86_64-winpthreads)))
         (modify-inputs (or (and=> (cross-libc target)
                                   (lambda (x-libc)
                                     (modify-inputs
                                         (package-native-inputs base-rust)
                                       (prepend x-libc))))
                            (package-native-inputs base-rust))
                        (prepend (cross-gcc target
                                            #:libc (cross-libc target))
                                 (cross-binutils target)))))
Efraim Flashner July 9, 2024, 3:21 p.m. UTC | #4
On Tue, Jul 09, 2024 at 11:25:04AM +0200, Christopher Baines wrote:
> Efraim Flashner <efraim@flashner.co.il> writes:
> 
> > On Fri, Jul 05, 2024 at 06:06:02PM +0200, Christopher Baines wrote:
> >> * gnu/packages/rust.scm (make-rust-sysroot/implementation): Guard against
> >> cross-libc returning #f.
> >> 
> >> Change-Id: Ia0d5c889c6f5cd3478ad985c79feb9ba1c472c29
> >> ---
> >>  gnu/packages/rust.scm | 10 ++++++++--
> >>  1 file changed, 8 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/gnu/packages/rust.scm b/gnu/packages/rust.scm
> >> index a385344473..f1de34b277 100644
> >> --- a/gnu/packages/rust.scm
> >> +++ b/gnu/packages/rust.scm
> >> @@ -73,7 +73,9 @@ (define-module (gnu packages rust)
> >>    #:use-module (ice-9 match)
> >>    #:use-module (ice-9 optargs)
> >>    #:use-module (srfi srfi-1)
> >> -  #:use-module (srfi srfi-26))
> >> +  #:use-module (srfi srfi-26)
> >> +  #:use-module (srfi srfi-34)
> >> +  #:use-module (srfi srfi-35))
> >>  
> >>  ;; This is the hash for the empty file, and the reason it's relevant is not
> >>  ;; the most obvious.
> >> @@ -1464,7 +1466,11 @@ (define make-rust-sysroot/implementation
> >>           (modify-inputs (package-native-inputs base-rust)
> >>                          (prepend (cross-gcc target
> >>                                              #:libc (cross-libc target))
> >> -                                 (cross-libc target)
> >> +                                 (or (cross-libc target) ; could be #f
> >> +                                     (raise (condition
> >> +                                             (&package-unsupported-target-error
> >> +                                              (package (libc-for-target target))
> >> +                                              (target target)))))
> >>                                   (cross-binutils target)))))
> >>        (properties
> >>         `((hidden? . #t)
> >> -- 
> >> 2.45.2
> >
> > This will probably work:
> >
> > (native-inputs
> >  `((,(string-append "gcc-cross-" target) ,(cross-gcc target
> >                                                      #:libc (cross-libc target)))
> >    ,(when (false-if-exception (cross-libc target))
> >           `(,(string-append "glibc-cross-" target) ,(cross-libc target)))
> >    (,(string-append "binutils-cross-" target) ,(cross-binutils target))
> >    ,(when (target-mingw? target)
> >       (if (string=? "i686-w64-mingw32" target)
> >           `("mingw-w64-i686-winpthreads" ,mingw-w64-i686-winpthreads)
> >           `("mingw-w64-x86_64-winpthreads" ,mingw-w64-x86_64-winpthreads)))
> >    ,@(package-native-inputs base-rust)))
> 
> Thanks for taking a look. In the latest patches cross-libc isn't
> changing to raise an exception (as that ended up being too complicated),
> so I'm not sure the false-if-exception is going to work.

false-if-exception should work. We use it a couple of times in golang
packaging to skip the tests if we're building with gccgo.

(unless
  ;; The tests fail when run with gccgo.
  (false-if-exception (search-input-file inputs "/bin/gccgo"))
  (apply (assoc-ref %standard-phases 'check) args)))))))

> I'd also maybe stick with modify-inputs, as at least that avoids the
> older inputs style.
> 
> If I've followed your first email correctly, are you thinking of
> something like this?
> 
>       (native-inputs
>        (if (target-mingw? target)
>          (modify-inputs (package-native-inputs base-rust)
>                         (prepend (cross-gcc target
>                                             #:libc (cross-libc target))
>                                  (cross-binutils target)
>                                  (if (string=? "i686-w64-mingw32" target)
>                                      mingw-w64-i686-winpthreads
>                                      mingw-w64-x86_64-winpthreads)))
>          (modify-inputs (or (and=> (cross-libc target)
>                                    (lambda (x-libc)
>                                      (modify-inputs
>                                          (package-native-inputs base-rust)
>                                        (prepend x-libc))))
>                             (package-native-inputs base-rust))
>                         (prepend (cross-gcc target
>                                             #:libc (cross-libc target))
>                                  (cross-binutils target)))))

Thanks, I hate it :) That said, if it works then it's fine. (I lose my
confidence from (cross-libc target) getting renamed to x-libc)

I played around with it a bit more. The problem is that we can only
logic our way around before modify-inputs or inside
prepend/append/delete, so options are a bit limited as to what we can
do. I came up with the following, which I think should also work:

      (native-inputs
       (modify-inputs (package-native-inputs base-rust)
         (prepend (cross-binutils target))
         (prepend
           (cond ((and (target-mingw? target)
                       (target-x86-32? target))
                  mingw-w64-i686-winpthreads)
                 ((and (target-mingw? target)
                       (target-x86-64? target))
                  mingw-w64-x86_64-winpthreads)
                 ((or (target-linux? target)
                      (target-hurd? target))
                  (cross-libc target))
                 ;; We need something, and duplicating cross-binutils
                 ;; doesn't cause any problems.
                 (#t (cross-binutils target))))
         (prepend (cross-gcc target
                             #:libc (cross-libc target)))))

I don't like the '#t' branch of the cond, but it doesn't seem to break
anything. And we're explicit about who gets cross-libc.

I would like something like the following to work, but some of the
inputs get lost
      (native-inputs
       (modify-inputs (package-native-inputs base-rust)
         (prepend
           (cond ((and (target-mingw? target)
                       (target-x86-32? target))
                  (cross-gcc target
                             #:libc (cross-libc target))
                  mingw-w64-i686-winpthreads
                  (cross-binutils target))
                 ((and (target-mingw? target)
                       (target-x86-64? target))
                  (cross-gcc target
                             #:libc (cross-libc target))
                  mingw-w64-x86_64-winpthreads
                  (cross-binutils target))
                 ((or (target-linux? target)
                      (target-hurd? target))
                  (cross-gcc target
                             #:libc (cross-libc target))
                  (cross-libc target)
                  (cross-binutils target))
                 (else
                  (cross-gcc target
                             #:libc (cross-libc target))
                  (cross-binutils target))))))
Christopher Baines July 12, 2024, 1:56 p.m. UTC | #5
Efraim Flashner <efraim@flashner.co.il> writes:

>> I'd also maybe stick with modify-inputs, as at least that avoids the
>> older inputs style.
>> 
>> If I've followed your first email correctly, are you thinking of
>> something like this?
>> 
>>       (native-inputs
>>        (if (target-mingw? target)
>>          (modify-inputs (package-native-inputs base-rust)
>>                         (prepend (cross-gcc target
>>                                             #:libc (cross-libc target))
>>                                  (cross-binutils target)
>>                                  (if (string=? "i686-w64-mingw32" target)
>>                                      mingw-w64-i686-winpthreads
>>                                      mingw-w64-x86_64-winpthreads)))
>>          (modify-inputs (or (and=> (cross-libc target)
>>                                    (lambda (x-libc)
>>                                      (modify-inputs
>>                                          (package-native-inputs base-rust)
>>                                        (prepend x-libc))))
>>                             (package-native-inputs base-rust))
>>                         (prepend (cross-gcc target
>>                                             #:libc (cross-libc target))
>>                                  (cross-binutils target)))))
>
> Thanks, I hate it :) That said, if it works then it's fine. (I lose my
> confidence from (cross-libc target) getting renamed to x-libc)
>
> I played around with it a bit more. The problem is that we can only
> logic our way around before modify-inputs or inside
> prepend/append/delete, so options are a bit limited as to what we can
> do. I came up with the following, which I think should also work:
>
>       (native-inputs
>        (modify-inputs (package-native-inputs base-rust)
>          (prepend (cross-binutils target))
>          (prepend
>            (cond ((and (target-mingw? target)
>                        (target-x86-32? target))
>                   mingw-w64-i686-winpthreads)
>                  ((and (target-mingw? target)
>                        (target-x86-64? target))
>                   mingw-w64-x86_64-winpthreads)
>                  ((or (target-linux? target)
>                       (target-hurd? target))
>                   (cross-libc target))
>                  ;; We need something, and duplicating cross-binutils
>                  ;; doesn't cause any problems.
>                  (#t (cross-binutils target))))
>          (prepend (cross-gcc target
>                              #:libc (cross-libc target)))))

I looked at this further and actually tried building the derivations for
different targets, and realised that there needs to be a
platform-rust-target set for it to work.

So maybe we don't need to bother with the inputs here and can just add a
guard at the top, e.g.

 (define make-rust-sysroot/implementation
   (mlambda (target base-rust)
+    (unless (platform-rust-target (lookup-platform-by-target target))
+      (raise
+       (condition
+        (&package-unsupported-target-error
+         (package base-rust)
+         (target target)))))
+


I've sent a new patch series to this effect.
diff mbox series

Patch

diff --git a/gnu/packages/rust.scm b/gnu/packages/rust.scm
index a385344473..f1de34b277 100644
--- a/gnu/packages/rust.scm
+++ b/gnu/packages/rust.scm
@@ -73,7 +73,9 @@  (define-module (gnu packages rust)
   #:use-module (ice-9 match)
   #:use-module (ice-9 optargs)
   #:use-module (srfi srfi-1)
-  #:use-module (srfi srfi-26))
+  #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-35))
 
 ;; This is the hash for the empty file, and the reason it's relevant is not
 ;; the most obvious.
@@ -1464,7 +1466,11 @@  (define make-rust-sysroot/implementation
          (modify-inputs (package-native-inputs base-rust)
                         (prepend (cross-gcc target
                                             #:libc (cross-libc target))
-                                 (cross-libc target)
+                                 (or (cross-libc target) ; could be #f
+                                     (raise (condition
+                                             (&package-unsupported-target-error
+                                              (package (libc-for-target target))
+                                              (target target)))))
                                  (cross-binutils target)))))
       (properties
        `((hidden? . #t)