Package: guix-patches;
Reported by: phodina <phodina <at> protonmail.com>
Date: Fri, 2 Dec 2022 05:31:02 UTC
Severity: normal
To reply to this bug, email your comments to 59762 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
guix-patches <at> gnu.org:bug#59762; Package guix-patches.
(Fri, 02 Dec 2022 05:31:02 GMT) Full text and rfc822 format available.phodina <phodina <at> protonmail.com>:guix-patches <at> gnu.org.
(Fri, 02 Dec 2022 05:31:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: phodina <phodina <at> protonmail.com> To: Guix Patches <guix-patches <at> gnu.org> Subject: Add serial-studio Date: Fri, 02 Dec 2022 05:29:47 +0000
[Message part 1 (text/plain, inline)]
Hi, this adds a nice GUI program to monitor communication on serial interfaces (e.g. UART). ---- Petr
[Message part 2 (text/html, inline)]
[0004-gnu-Add-serial-studio.patch (text/x-patch, attachment)]
[0001-gnu-Add-qmqtt.patch (text/x-patch, attachment)]
[0003-gnu-qwt-Update-to-6.2.0.patch (text/x-patch, attachment)]
[0002-gnu-Add-qtcsv.patch (text/x-patch, attachment)]
guix-patches <at> gnu.org:bug#59762; Package guix-patches.
(Mon, 16 Jan 2023 14:26:01 GMT) Full text and rfc822 format available.Message #8 received at 59762 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: phodina <phodina <at> protonmail.com> Cc: 59762 <at> debbugs.gnu.org Subject: Re: bug#59762: Add serial-studio Date: Mon, 16 Jan 2023 09:25:43 -0500
Hi Petr,
phodina <phodina <at> protonmail.com> writes:
> Hi,
>
> this adds a nice GUI program to monitor communication on serial interfaces (e.g. UART).
>
> ----
> Petr
> From b134cf1d200c867c6b6c8a729a4ad212687a101d Mon Sep 17 00:00:00 2001
> From: Petr Hodina <phodina <at> protonmail.com>
> Date: Fri, 21 Jan 2022 06:30:28 +0100
> Subject: [PATCH 4/4] gnu: Add serial-studio.
>
> * gnu/packages/qt.scm (serial-studio): New variable.
> * gnu/local.mk: Add patch.
> * gnu/packages/patches/serial-studio-remove-qsimple-updater.patch: New file.
>
> diff --git a/gnu/packages/patches/serial-studio-remove-qsimple-updater.patch b/gnu/packages/patches/serial-studio-remove-qsimple-updater.patch
> new file mode 100644
> index 0000000000..f8f93364d6
> --- /dev/null
> +++ b/gnu/packages/patches/serial-studio-remove-qsimple-updater.patch
> @@ -0,0 +1,80 @@
> +From 22b0b5014be7911406b262aeb0864d36f1fbbab5 Mon Sep 17 00:00:00 2001
> +From: Petr Hodina <phodina <at> protonmail.com>
> +Date: Wed, 19 Jan 2022 14:52:08 +0100
> +Subject: [PATCH] Remove code dependent on QSimpleUpdater. Don't include
> + qwt.pri, qtcsv.pri and qmqtt.pri.
Ideally, the disabling of QSimpleUpdater would be configurable at build
time and upstreamed. The patch is not registered in gnu/local.mk in
this commit, although the commit log says it should be.
[...]
> diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
> index 4621de0dad..d0521b4894 100644
> --- a/gnu/packages/qt.scm
> +++ b/gnu/packages/qt.scm
> @@ -3017,6 +3017,78 @@ (define (delete-unwanted-files child stat flag base level)
> system libraries.")
> (license license:lgpl2.1+)))
>
> +(define-public serial-studio
> + (package
> + (name "serial-studio")
> + (version "1.1.4")
> + (source (origin
> + (method git-fetch)
> + (uri (git-reference
> + (url "https://github.com/Serial-Studio/Serial-Studio")
> + (commit (string-append "v" version))))
> + (file-name (git-file-name name version))
> + (patches (search-patches
> + "serial-studio-remove-qsimple-updater.patch"))
> + (modules '((guix build utils) (ice-9 ftw)
> + (srfi srfi-1)))
I think the only module you require in your snippet is (guix build
utils).
> + (snippet '(begin
> + (with-directory-excursion "libs"
> + (for-each
> + delete-file-recursively
> + '("qwt" "qtcsv" "qmqtt"
> + "QSimpleUpdater")))))
> + (sha256
> + (base32
> + "0qww2pmbvmq3cjx3xk8rxq0zf54y0b0j8bykn4aq3d2dii44m85y"))))
> + (build-system qt-build-system)
> + (arguments
> + `(#:tests? #f ;no test suite
Please use (list #:tests? #f #:phases #~(modify-phases %standard-phases
[...] for new packages definitions.
> + #:phases
> + (modify-phases %standard-phases
> + (replace 'configure
> + (lambda* (#:key outputs inputs #:allow-other-keys)
> + (substitute* "Serial-Studio.pro"
> + (("/usr") (assoc-ref outputs "out"))
> + (("INCLUDEPATH \\+= src") (string-append "INCLUDEPATH += src
> +INCLUDEPATH += "
> + (assoc-ref inputs
> + "qwt")
> + "/include/qwt
> +"
> + "LIBS += -L"
> + (assoc-ref inputs
> + "qwt")
> + "/lib"
> + " -lqwt
> +"
> + "LIBS += -L"
> + (assoc-ref inputs
> + "qtcsv")
> + "/lib"
> + " -lqtcsv
> +"
> + "LIBS += -L"
> + (assoc-ref inputs
> + "qmqtt")
> + "/lib"
> + " -lQt5Qmqtt")))
The above indentation appears squashed right :-). I'd bring the
(string-append "INCLUDEPATH += src [...] on a new line, which will help
the rest fit more nicely.
> + (invoke "qmake"))))))
> + (native-inputs (list qwt qttools pkg-config))
Please sort inputs.
> + (inputs (list qtcsv
> + qmqtt
> + qwt
> + qtbase-5
> + qtserialport
> + qtsvg-5
> + qtdeclarative-5
> + qtquickcontrols2-5))
Ditto.
> + (home-page "https://github.com/Serial-Studio/Serial-Studio")
> + (synopsis "Multi-purpose serial data visualization & processing
> program")
Replace "&" with "and".
> + (description
> + "This package provides multi-purpose serial data visualization
> +and processing GUI program.")
This leaves me wanting to know a bit more, but upstream doesn't provide
much help, so OK.
> + (license license:expat)))
> +
> (define-public single-application-qt5
> ;; Change in function signature, nheko requires at least this commit
> (let ((commit "dc8042b5db58f36e06ba54f16f38b16c5eea9053"))
--
Thanks,
Maxim
guix-patches <at> gnu.org:bug#59762; Package guix-patches.
(Mon, 16 Jan 2023 14:34:02 GMT) Full text and rfc822 format available.Message #11 received at 59762 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: phodina <phodina <at> protonmail.com> Cc: 59762 <at> debbugs.gnu.org Subject: Re: bug#59762: Add serial-studio Date: Mon, 16 Jan 2023 09:33:48 -0500
Hello!
Reviewing patch 2/4.
phodina <phodina <at> protonmail.com> writes:
> Hi,
>
> this adds a nice GUI program to monitor communication on serial interfaces (e.g. UART).
>
> ----
> Petr
>
> From 367d7da4a143949d11fbc76495f935dff7a3f25c Mon Sep 17 00:00:00 2001
> From: Petr Hodina <phodina <at> protonmail.com>
> Date: Sun, 16 Jan 2022 17:59:22 +0100
> Subject: [PATCH 1/4] gnu: Add qmqtt.
>
> * gnu/packages/qt.scm (qmqtt): New variable.
>
> diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
> index 30bf429897..6aaa9c47c2 100644
> --- a/gnu/packages/qt.scm
> +++ b/gnu/packages/qt.scm
> @@ -3363,6 +3363,68 @@ (define-public python-qt.py
> bindings (PySide, PySide2, PyQt4 and PyQt5).")
> (license license:expat)))
>
> +(define-public qmqtt
> + (package
> + (name "qmqtt")
> + (version "1.0.0")
> + (source (origin
> + (method git-fetch)
> + (uri (git-reference
> + (url "https://github.com/alex-spataru/qmqtt")
> + (commit (string-append "v" version))))
> + (file-name (git-file-name name version))
> + (modules '((guix build utils) (ice-9 ftw)
> + (srfi srfi-1)))
Only (guix build utils) is needed.
> + (snippet `(begin
> + (delete-file-recursively "tests/gtest/gtest")))
Extraneous 'begin' directive.
> + (sha256
> + (base32
> + "1l96cssa9hjmabds9k9nzisxnzr3ndi3r82m3lgwq9ip94kz05nw"))))
> + (build-system cmake-build-system)
> + (arguments
> + `(#:phases (modify-phases %standard-phases
Please use plain list + gexps for new packages.
> + (replace 'configure
> + (lambda* (#:key outputs #:allow-other-keys)
> + (invoke "qmake")))
Extraneous OUTPUTS argument.
> + (replace 'install
> + (lambda* (#:key outputs source #:allow-other-keys)
> + (let* ((out (assoc-ref outputs "out")) (lib (string-append
> + out "/lib"))
> + (include (string-append out "/include")))
After switching to gexps, use gexp variables such as #$output.
Extraneous SOURCE argument.
> + (mkdir-p lib)
> + (mkdir-p include)
> + (chdir "..") ;we were in dir tests
Prefer active tense (;navigate up from test directore), avoid 'we' as
much as possible.
> + (copy-recursively "lib" lib)
> + (chdir "src/mqtt")
> + (for-each (lambda (file)
> + (install-file (string-append file) include))
> + '("qmqtt_client.h" "qmqtt_frame.h"
> + "qmqtt_global.h"
> + "qmqtt.h"
> + "qmqtt_message.h"
> + "qmqtt_networkinterface.h"
> + "qmqtt_routedmessage.h"
> + "qmqtt_router.h"
> + "qmqtt_routesubscription.h"
> + "qmqtt_socketinterface.h"
> + "qmqtt_timerinterface.h"
> + "qmqtt_message.h")))))
> + (replace 'check
> + (lambda* (#:key tests? test-options parallel- +
> + #:allow-other-keys)
Remove extraneous arguments (test-options, parallel-).
> + (when tests?
> + (chdir "tests")
> + (substitute* "gtest/gtest.pro"
> + (("gtest") ""))
> + (invoke "qmake")))))))
> + (native-inputs (list googletest perl python))
> + (inputs (list qtbase-5 openssl qtwebsockets))
> + (home-page "https://github.com/alex-spataru/qmqtt")
> + (synopsis "MQTT Client")
"MQTT client"
> + (description "Provides an MQTT Client with SSL support and Websockets.")
> + (license license:expat)))
Please use make it a full sentence starting with a subject, e.g. "This
package provides an MQTT client with [...]".
--
Thanks,
Maxim
guix-patches <at> gnu.org:bug#59762; Package guix-patches.
(Mon, 16 Jan 2023 14:38:02 GMT) Full text and rfc822 format available.Message #14 received at 59762 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: phodina <phodina <at> protonmail.com> Cc: 59762 <at> debbugs.gnu.org Subject: Re: bug#59762: Add serial-studio Date: Mon, 16 Jan 2023 09:37:23 -0500
Hello, Reviewing patch 3 out of 4! phodina <phodina <at> protonmail.com> writes: > Hi, > > this adds a nice GUI program to monitor communication on serial interfaces (e.g. UART). > > ---- > Petr > > > From d4846647b80f28f71f9a0e779a98716f7e582f56 Mon Sep 17 00:00:00 2001 > From: Petr Hodina <phodina <at> protonmail.com> > Date: Fri, 21 Jan 2022 06:31:23 +0100 > Subject: [PATCH 3/4] gnu: qwt: Update to 6.2.0. > > * gnu/packages/qt.scm (qwt): Update to 6.2.0. > > diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm > index 71c8aafe3c..4621de0dad 100644 > --- a/gnu/packages/qt.scm > +++ b/gnu/packages/qt.scm > @@ -3731,7 +3731,7 @@ (define-public qtsolutions > (define-public qwt > (package > (name "qwt") > - (version "6.1.5") > + (version "6.2.0") > (source > (origin > (method url-fetch) > @@ -3739,7 +3739,7 @@ (define-public qwt > (string-append "mirror://sourceforge/qwt/qwt/" > version "/qwt-" version ".tar.bz2")) > (sha256 > - (base32 "0hf0mpca248xlqn7xnzkfj8drf19gdyg5syzklvq8pibxiixwxj0")))) > + (base32 "0c0mnyqa8rvs2bx946mbmfbh2ik4a0bmhwgn01rzvl2m758zd54i")))) > (build-system gnu-build-system) > (inputs > (list qtbase-5 qtsvg-5 qttools-5)) LGTM (you can push this one already), if you've verified it doesn't break dependent packages. -- Thanks, Maxim
guix-patches <at> gnu.org:bug#59762; Package guix-patches.
(Mon, 16 Jan 2023 14:43:01 GMT) Full text and rfc822 format available.Message #17 received at 59762 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: phodina <phodina <at> protonmail.com> Cc: 59762 <at> debbugs.gnu.org Subject: Re: bug#59762: Add serial-studio Date: Mon, 16 Jan 2023 09:42:32 -0500
Hi again!
phodina <phodina <at> protonmail.com> writes:
> Hi,
>
> this adds a nice GUI program to monitor communication on serial interfaces (e.g. UART).
>
> ----
> Petr
>
>
>
> From 6b25d088bd158ad2c5baa7548c49a04bd0f09664 Mon Sep 17 00:00:00 2001
> From: Petr Hodina <phodina <at> protonmail.com>
> Date: Wed, 19 Jan 2022 12:01:22 +0100
> Subject: [PATCH 2/4] gnu: Add qtcsv.
>
> * gnu/packages/qt.scm (qtcsv): New variable.
>
> diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
> index 6aaa9c47c2..71c8aafe3c 100644
> --- a/gnu/packages/qt.scm
> +++ b/gnu/packages/qt.scm
> @@ -1283,6 +1283,58 @@ (define-public qtconnectivity
> (description "The Qt Connectivity modules provides modules for interacting
> with Bluetooth and NFC.")))
>
> +(define-public qtcsv
> + (package
> + (name "qtcsv")
> + (version "1.6")
> + (source (origin
> + (method git-fetch)
> + (uri (git-reference
> + (url "https://github.com/iamantony/qtcsv")
> + (commit (string-append "v" version))))
> + (file-name (git-file-name name version))
> + (sha256
> + (base32
> + "109nbspnhaczm85wp1fqazmamdj2bh9wfv3anlslvll606lrfgir"))))
> + (build-system cmake-build-system)
> + (arguments
> + `(#:phases (modify-phases %standard-phases
Please switch to list + gexps.
> + (replace 'configure
> + (lambda* (#:key outputs #:allow-other-keys)
> + (invoke "qmake")))
Use plain lambda without arguments (lambda _).
> + (add-after 'unpack 'fix-install
> + (lambda* (#:key outputs #:allow-other-keys)
Ditto.
> + (let* ((out (assoc-ref outputs "out")))
>
Use gexps variables.
> + (substitute* "qtcsv.pro"
> + (("/usr") out)
> + (("\\$\\$PWD") out)))))
> + (replace 'install
> + (lambda* (#:key outputs #:allow-other-keys)
> + (let* ((out (assoc-ref outputs "out")) (lib (string-append
> + out "/lib"))
Ditto :-).
> + (include (string-append out "/include")))
> + (mkdir-p include)
> + (mkdir-p lib)
> + (chdir "..") ;we were in dir tests
Same comment bout active tense as earlier... but it seems that'd no
longer be needed with the next comment implemented?
> + (copy-recursively "include" include)
> + (for-each (lambda (file)
> + (install-file (string-append file) lib))
> + '("libqtcsv.so" "libqtcsv.so.1"
> + "libqtcsv.so.1.6" "libqtcsv.so.1.6.0")))))
> + (replace 'check
> + (lambda* (#:key tests? test-options parallel-tests?
> + #:allow-other-keys)
> + (when tests?
> + (chdir "tests")
> + (invoke "qmake")))))))
If qmake has a switch like make to change directory, e.g. -C, that'd be
preferable, else "with-directory-excursion" to avoid keeping state in
other phases.
Otherwise, this patch LGTM.
--
Thanks,
Maxim
guix-patches <at> gnu.org:bug#59762; Package guix-patches.
(Tue, 21 Mar 2023 13:54:02 GMT) Full text and rfc822 format available.Message #20 received at 59762 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: phodina <phodina <at> protonmail.com> Cc: 59762 <at> debbugs.gnu.org Subject: Re: bug#59762: Add serial-studio Date: Tue, 21 Mar 2023 09:53:03 -0400
Hello,
Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:
> Hi again!
>
> phodina <phodina <at> protonmail.com> writes:
>
>> Hi,
>>
>> this adds a nice GUI program to monitor communication on serial interfaces (e.g. UART).
>>
>> ----
>> Petr
>>
>>
>>
>> From 6b25d088bd158ad2c5baa7548c49a04bd0f09664 Mon Sep 17 00:00:00 2001
>> From: Petr Hodina <phodina <at> protonmail.com>
>> Date: Wed, 19 Jan 2022 12:01:22 +0100
>> Subject: [PATCH 2/4] gnu: Add qtcsv.
>>
>> * gnu/packages/qt.scm (qtcsv): New variable.
>>
>> diff --git a/gnu/packages/qt.scm b/gnu/packages/qt.scm
>> index 6aaa9c47c2..71c8aafe3c 100644
>> --- a/gnu/packages/qt.scm
>> +++ b/gnu/packages/qt.scm
>> @@ -1283,6 +1283,58 @@ (define-public qtconnectivity
>> (description "The Qt Connectivity modules provides modules for interacting
>> with Bluetooth and NFC.")))
>>
>> +(define-public qtcsv
>> + (package
>> + (name "qtcsv")
>> + (version "1.6")
>> + (source (origin
>> + (method git-fetch)
>> + (uri (git-reference
>> + (url "https://github.com/iamantony/qtcsv")
>> + (commit (string-append "v" version))))
>> + (file-name (git-file-name name version))
>> + (sha256
>> + (base32
>> + "109nbspnhaczm85wp1fqazmamdj2bh9wfv3anlslvll606lrfgir"))))
>> + (build-system cmake-build-system)
>> + (arguments
>> + `(#:phases (modify-phases %standard-phases
>
> Please switch to list + gexps.
>
>> + (replace 'configure
>> + (lambda* (#:key outputs #:allow-other-keys)
>> + (invoke "qmake")))
>
> Use plain lambda without arguments (lambda _).
>
>> + (add-after 'unpack 'fix-install
>> + (lambda* (#:key outputs #:allow-other-keys)
>
> Ditto.
>> + (let* ((out (assoc-ref outputs "out")))
>>
>
> Use gexps variables.
>
>> + (substitute* "qtcsv.pro"
>> + (("/usr") out)
>> + (("\\$\\$PWD") out)))))
>> + (replace 'install
>> + (lambda* (#:key outputs #:allow-other-keys)
>> + (let* ((out (assoc-ref outputs "out")) (lib (string-append
>> + out "/lib"))
>
> Ditto :-).
>
>> + (include (string-append out "/include")))
>> + (mkdir-p include)
>> + (mkdir-p lib)
>> + (chdir "..") ;we were in dir tests
>
> Same comment bout active tense as earlier... but it seems that'd no
> longer be needed with the next comment implemented?
>
>> + (copy-recursively "include" include)
>> + (for-each (lambda (file)
>> + (install-file (string-append file) lib))
>> + '("libqtcsv.so" "libqtcsv.so.1"
>> + "libqtcsv.so.1.6" "libqtcsv.so.1.6.0")))))
>> + (replace 'check
>> + (lambda* (#:key tests? test-options parallel-tests?
>> + #:allow-other-keys)
>> + (when tests?
>> + (chdir "tests")
>> + (invoke "qmake")))))))
>
> If qmake has a switch like make to change directory, e.g. -C, that'd be
> preferable, else "with-directory-excursion" to avoid keeping state in
> other phases.
>
> Otherwise, this patch LGTM.
Gently ping :-). There are also other review comments up-thread.
--
Thanks,
Maxim
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.