diff mbox series

[bug#40767] gnu: Add maradns.

Message ID cu7tv17v3wy.fsf@systemreboot.net
State Accepted
Headers show
Series [bug#40767] gnu: Add maradns. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job

Commit Message

Arun Isaac April 25, 2020, 6:03 p.m. UTC 
>> Yes, I checked before pushing. guix lint reported no warnings.
>
> Then we may have to fix guix lint because it seems to me that this
> snippet is above limit:

guix lint checks for lines longer than 90 characters. Perhaps that's the
new policy? See the function report-long-line in guix/lint.scm. It even
has a comment noting that we don't warn at 80 characters.

> You don't need this bit, the native gcc is also present when
> cross-compiling. Otherwise, LGTM!

Removed. Please find attached the updated patch.

Comments

Mathieu Othacehe April 26, 2020, 9:05 a.m. UTC | #1 
> guix lint checks for lines longer than 90 characters. Perhaps that's the
> new policy? See the function report-long-line in guix/lint.scm. It even
> has a comment noting that we don't warn at 80 characters.

Yes it's to avoid false positives with hashes and URLs. On the other
hand, in .dir-locals.el, fill-column variable is set to 78. Maybe we
would need to be more explicit in the "Coding style" section of the
manual.

> -           (lambda _
> +           (lambda* (#:key native-inputs #:allow-other-keys)
> +             ;; make_32bit_tables generates a header file that is used during
> +             ;; compilation. Hence, during cross compilation, it should be
> +             ;; built for the host system.
> +             (when ,(%current-target-system)
> +               (substitute* "rng/Makefile"
> +                 (("\\$\\(CC\\) -o make_32bit_tables")
> +                  (string-append (assoc-ref native-inputs "gcc")
> +                                 "/bin/gcc -o make_32bit_tables"))))

Nitpicking, you can use the target argument instead of
%current-target-system, this way:

--8<---------------cut here---------------start------------->8---
(lambda* (#:key native-inputs target #:allow-other-keys)
         ;; make_32bit_tables generates a header file that is used during
         ;; compilation. Hence, during cross compilation, it should be
         ;; built for the host system.
         (when target
           (substitute* "rng/Makefile"
                        (("\\$\\(CC\\) -o make_32bit_tables")
                         (string-append (assoc-ref native-inputs "gcc")
                                        "/bin/gcc -o make_32bit_tables")))))
--8<---------------cut here---------------end--------------->8---

Feel free to go ahead with this one,

Thanks,

Mathieu
Arun Isaac April 26, 2020, 9:51 a.m. UTC | #2 
>> guix lint checks for lines longer than 90 characters. Perhaps that's the
>> new policy? See the function report-long-line in guix/lint.scm. It even
>> has a comment noting that we don't warn at 80 characters.
>
> Yes it's to avoid false positives with hashes and URLs. On the other
> hand, in .dir-locals.el, fill-column variable is set to 78. Maybe we
> would need to be more explicit in the "Coding style" section of the
> manual.

Do please raise this in guix-devel and we could get some consensus on
this.

> Nitpicking, you can use the target argument instead of
> %current-target-system, this way:
>
> --8<---------------cut here---------------start------------->8---
> (lambda* (#:key native-inputs target #:allow-other-keys)
>          ;; make_32bit_tables generates a header file that is used during
>          ;; compilation. Hence, during cross compilation, it should be
>          ;; built for the host system.
>          (when target
>            (substitute* "rng/Makefile"
>                         (("\\$\\(CC\\) -o make_32bit_tables")
>                          (string-append (assoc-ref native-inputs "gcc")
>                                         "/bin/gcc -o make_32bit_tables")))))
> --8<---------------cut here---------------end--------------->8---
>
> Feel free to go ahead with this one,

Pushed with suggested change. Thanks! :-)
diff mbox series

Patch

From c155043bd6f78add7bbf660b6ad4a592be89b694 Mon Sep 17 00:00:00 2001
From: Arun Isaac <arunisaac@systemreboot.net>
Date: Sat, 25 Apr 2020 15:53:52 +0530
Subject: [PATCH v2] gnu: maradns: Fix cross compilation.

* gnu/packages/dns.scm (maradns)[arguments]: Build make_32bit_tables for the
host.
---
 gnu/packages/dns.scm | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/dns.scm b/gnu/packages/dns.scm
index 80ed1f0b49..7364a1e885 100644
--- a/gnu/packages/dns.scm
+++ b/gnu/packages/dns.scm
@@ -43,6 +43,7 @@ 
   #:use-module (gnu packages crypto)
   #:use-module (gnu packages datastructures)
   #:use-module (gnu packages flex)
+  #:use-module (gnu packages gcc)
   #:use-module (gnu packages glib)
   #:use-module (gnu packages groff)
   #:use-module (gnu packages groff)
@@ -979,7 +980,15 @@  known public suffixes.")
        #:phases
        (modify-phases %standard-phases
          (replace 'configure
-           (lambda _
+           (lambda* (#:key native-inputs #:allow-other-keys)
+             ;; make_32bit_tables generates a header file that is used during
+             ;; compilation. Hence, during cross compilation, it should be
+             ;; built for the host system.
+             (when ,(%current-target-system)
+               (substitute* "rng/Makefile"
+                 (("\\$\\(CC\\) -o make_32bit_tables")
+                  (string-append (assoc-ref native-inputs "gcc")
+                                 "/bin/gcc -o make_32bit_tables"))))
              (invoke "./configure")))
          (add-before 'install 'create-install-directories
            (lambda* (#:key outputs #:allow-other-keys)
-- 
2.26.1