diff mbox series

[bug#56354] gnu: engineering: Add candle.

Message ID 87o7y74zic.fsf@gmail.com
State Accepted
Headers show
Series [bug#56354] gnu: engineering: Add candle. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Artyom V. Poptsov July 2, 2022, 9:47 a.m. UTC 
Hello,

this patch adds Candle[1] CNC controller.
Thanks,

- Artyom

References:
1. https://github.com/Denvi/Candle

Comments

Jean Pierre De Jesus DIAZ July 5, 2022, 12:16 p.m. UTC | #1 
>+    (arguments
>+     `(#:tests? #f                      ; no tests.
>+       #:phases (modify-phases %standard-phases

Can be changed to use G-Expressions:

(arguments
  (list #:tests? #f                       ;; no tests.
        #:phases
        #~(modify-phases %standard-phases
            ...)))

See, https://guix.gnu.org/manual/en/html_node/G_002dExpressions.html

>+                  (add-after 'fix-sources 'fix-application-settings-path

Doesn't depend on 'fix-sources, so it's fine to add after 'unpack.

>+                  (replace 'configure
>+                    (lambda* (#:key outputs #:allow-other-keys)
>+                      (let ((out (assoc-ref outputs "out")))
>+                        (chdir "src")
>+                        (invoke "qmake"))))

It may also be a good idea to set `QMAKE_CC' variable for cross-compilation,
like:

`(invoke "qmake" (string-append "QMAKE_CC=" #$(cc-for-target)))'

And also test cross-compilation:

./pre-inst-env guix build candle \
                          --keep-failed \
                          --target=aarch64-linux-gnu

>+                  (add-after 'configure 'fix-makefile
>+                    (lambda _
>+                      (substitute* "Makefile"
>+                        (("-pipe -Z7") "-pipe")
>+                        (("LFLAGS.*=.*DEBUG .*OPT:REF -Wl,-O1")
>+                         "LFLAGS        = -Wl,-O1"))))

Could this instead be replaced on the `candle.pro' file?

—
Jean-Pierre De Jesus DIAZ
Jean Pierre De Jesus DIAZ July 11, 2022, 11:55 a.m. UTC | #2 
Hello Artyom,

>It seems to me from the message that the issue is not in the candle
>package itself.

Probably one of the dependencies can't (currently) be cross-compiled,
anyway, thanks for setting the QMAKE_CC variable in the qmake invocation,
once perl build system gains cross-compilation (if possible) then it
would be easier to cross-compile later without modifications to the
package.

>+                            (lambda* (#:key outputs #:allow-other-keys)
>+                              (let ((out (assoc-ref outputs "out")))
>+                                (chdir "src")
>+                                (invoke "qmake"
>+                                        (string-append "QMAKE_CC="
>+                                                       #$(cc-for-target))))))

The `out' variable is not used, so the let can be safely removed and the lambda
simplified, also instead of `chdir', `with-directory-excursion' could be used,
but's a matter of preference (don't know if one or other style is preferred
inside GNU Guix).

For example:

(lambda _
  (with-directory-excursion "src"
    (invoke "qmake"
      (string-append "QMAKE_CC="
                     #$(cc-for-target)))))

>+                          (replace 'install
>+                            (lambda* (#:key outputs #:allow-other-keys)
>+                              (let ((out (assoc-ref outputs "out")))
>+                                (install-file "Candle"
>+                                              (string-append out "/bin"))))))))

The `out' binding can be also replaced by `#$output', e.g.:


(lambda* _
  (install-file "Candle"
                (string-append #$output "/bin")))

Other than that the package definition looks good to me, and did a quick pass
over the Candle source code to check that it doesn't contain any malware.

Only a bundled font is present (src/fonts/Ubuntu-Regular.tff), but that one is
not provided by GNU Guix (don't know the specific reasons, but got added then
removed) so no need to replace it with provided ones.

—
Jean-Pierre De Jesus DIAZ
diff mbox series

Patch

From a3c57bbd8cb9f87a92e063d02e69aa2f29f6a895 Mon Sep 17 00:00:00 2001
From: "Artyom V. Poptsov" <poptsov.artyom@gmail.com>
Date: Sat, 2 Jul 2022 12:37:13 +0300
Subject: [PATCH] gnu: engineering: Add candle.

* gnu/packages/engineering.scm (candle): New variable.
---
 gnu/packages/engineering.scm | 37 ++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/gnu/packages/engineering.scm b/gnu/packages/engineering.scm
index 661078594e..3bae7d100e 100644
--- a/gnu/packages/engineering.scm
+++ b/gnu/packages/engineering.scm
@@ -3702,19 +3702,32 @@  (define-public candle
               (sha256
                (base32
                 "1rinblzqg8xbi4zcyx6v3k7g2kdrgmwm7xwb6fryb8s0bd21jppv"))))
-    (build-system qt-build-system)
-    ;; (native-inputs
-    ;;  ())
-    ;; (inputs)
+    (build-system gnu-build-system)
+    (native-inputs (list qttools))
+    (inputs (list qtbase-5 qtserialport))
     (arguments
-     (list #:phases
-           #~(modify-phases %standard-phases
-               (replace 'configure
-                 (lambda* (#:key outputs #:allow-other-keys)
-                   (chdir "src")
-                   (invoke "qmake"
-                           (string-append "PREFIX=" (assoc-ref outputs "out")))
-                   #t)))))
+     `(#:phases (modify-phases %standard-phases
+                  (add-after 'unpack 'fix-sources
+                    (lambda _
+                      (substitute* (find-files "." ".*\\.h")
+                        (("const char\\* what\\(\\) const override")
+                         "const char* what() const noexcept override"))))
+                  (replace 'configure
+                    (lambda* (#:key outputs #:allow-other-keys)
+                      (let ((out (assoc-ref outputs "out")))
+                        (chdir "src")
+                        (invoke "qmake"))))
+                  (add-after 'configure 'fix-makefile
+                    (lambda _
+                      (substitute* "Makefile"
+                        (("-pipe -Z7") "-pipe")
+                        (("LFLAGS.*=.*DEBUG .*OPT:REF -Wl,-O1")
+                         "LFLAGS        = -Wl,-O1"))))
+                  (replace 'install
+                    (lambda* (#:key outputs #:allow-other-keys)
+                      (let ((out (assoc-ref outputs "out")))
+                        (install-file "Candle"
+                                      (string-append out "/bin"))))))))
     (home-page "https://github.com/Denvi/Candle")
     (synopsis "GRBL controller with G-Code visualizer")
     (description
-- 
2.34.1