[PATCH] gnu: Add notcurses

OpenSubmitted by Blake Shaw.
Details
2 participants
  • Blake Shaw
  • Nicolas Goaziou
Owner
unassigned
Severity
normal
B
B
Blake Shaw wrote on 30 Nov 2021 01:24
(address . guix-patches@gnu.org)(name . Blake Shaw)(address . blake@nonconstructivism.com)
6a6031ead6f9f61bc8eed976374638089efdaf3f.1638231894.git.blake@nonconstructivism.com
---
gnu/packages/notcurses.scm | 71 ++++++++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)
create mode 100644 gnu/packages/notcurses.scm

Toggle diff (79 lines)
diff --git a/gnu/packages/notcurses.scm b/gnu/packages/notcurses.scm
new file mode 100644
index 0000000000..898903628c
--- /dev/null
+++ b/gnu/packages/notcurses.scm
@@ -0,0 +1,71 @@
+(define-module (gnu packages notcurses)
+  #:use-module (guix utils)
+  #:use-module (gnu packages)
+  #:use-module (guix packages)
+  #:use-module (guix build utils)
+  #:use-module (guix git-download)
+  #:use-module (guix build-system cmake)
+  #:use-module ((guix licenses) #:prefix license:)
+  #:use-module (gnu packages gcc)
+  #:use-module (gnu packages video)
+  #:use-module (gnu packages ncurses)
+  #:use-module (gnu packages pkg-config)
+  #:use-module (gnu packages compression)
+  #:use-module (gnu packages libunistring)
+  #:use-module (ice-9 match))
+
+(define-public notcurses
+  (let ((commit "d15eb6003cbd65f11163916261cf6cd5600c77fa"))
+    (package
+      (name "notcurses")
+      (version "2.4.9")
+      (source
+       (origin
+         (method git-fetch)
+         (uri (git-reference
+               (url "https://github.com/dankamongmen/notcurses")
+               (commit commit)))
+         (file-name (git-file-name name version))
+         (sha256
+          (base32
+           "10jf6iai1r0xafrgaz978y9bqlaw1gvd11gc0yynwwp6rcs97g17"))))
+      (build-system cmake-build-system)
+      (arguments
+       `(#:tests? #f
+                  #:build-type "-DVAR=val"
+                  #:make-flags
+                  (list
+                   (string-append "prefix="
+                                  (assoc-ref %outputs "out"))
+                   "CC=gcc")
+                  #:configure-flags
+                  (map (lambda (s)
+                         (string-append "-D" s))
+                       '("USE_CPP=off"     "USE_COVERAGE=off"
+                         "USE_DOXYGEN=off" "USE_DOCTEST=off"
+                         "USE_GPM=off"     "USE_MULTIMEDIA=ffmpeg"
+                         "USE_PANDOC=off"  "FSG_BUILD=ON"))
+                  #:phases
+                  (modify-phases %standard-phases
+                    (add-before 'build 'patch-makefile-shell
+                      (lambda _
+                        (setenv "HOME" "/tmp")))
+                    (add-before 'build 'set-prefix-in-makefile
+                      (lambda* (#:key outputs #:allow-other-keys)
+                        (let ((out (assoc-ref outputs "out")))
+                          (substitute* "Makefile"
+                            (("PREFIX =.*")
+                             (string-append "PREFIX = " out "\n")))
+                          #true))))))
+      (native-inputs
+       `(("ncurses" ,ncurses)
+         ("gcc-toolchain" ,gcc-10)
+         ("pkg-config" ,pkg-config)))
+      (inputs
+       `(("zlib" ,zlib)
+         ("ffmpeg" ,ffmpeg)
+         ("libunistring" ,libunistring)))
+      (synopsis "Not-ncurses: A library facilitating complex TUIs on modern terminals")
+      (description "Supporting vivid colors, multimedia, threads, & Unicode to the max.")
+      (home-page "https://notcurses.com/html/")
+      (license license:asl2.0))))
-- 
2.33.1
N
N
Nicolas Goaziou wrote on 1 Dec 2021 17:01
(name . Blake Shaw via Guix-patches via)(address . guix-patches@gnu.org)
87r1awxqkc.fsf@nicolasgoaziou.fr
Hello,

Blake Shaw via Guix-patches via <guix-patches@gnu.org> writes:

Thank you. Some comments follow.

Toggle quote (2 lines)
> gnu/packages/notcurses.scm | 71 ++++++++++++++++++++++++++++++++++++++

I don't think we should create a new file just for this package. Also,
new files need to be registered in "gnu/local.mk".

Maybe this should go into... "ncurses.scm" (!). At a later time, we may
rename ncurse.scm into tui.scm or some such.

Toggle quote (3 lines)
> +(define-public notcurses
> + (let ((commit "d15eb6003cbd65f11163916261cf6cd5600c77fa"))

Upstream use tags. It might be more readable. You'll need a variable for
the code name, tho. In any case, a comment is warranted explaining the
situation.

Toggle quote (4 lines)
> + (sha256
> + (base32
> + "10jf6iai1r0xafrgaz978y9bqlaw1gvd11gc0yynwwp6rcs97g17"))))

Nitpick: string should go on the same line as base32.

Toggle quote (5 lines)
> + (build-system cmake-build-system)
> + (arguments
> + `(#:tests? #f
> + #:build-type "-DVAR=val"

Indentation is off. You may want to use "etc/indent-code.el" script.
The build-type value above is suspicious.

Toggle quote (6 lines)
> + #:make-flags
> + (list
> + (string-append "prefix="
> + (assoc-ref %outputs "out"))
> + "CC=gcc")

This is not cross-compilation friendly. The above should be:

(list ,(string-append "CC=" (cc-for-target))
(string-append "prefix=" ...))

Toggle quote (13 lines)
> + #:configure-flags
> + (map (lambda (s)
> + (string-append "-D" s))
> + '("USE_CPP=off" "USE_COVERAGE=off"
> + "USE_DOXYGEN=off" "USE_DOCTEST=off"
> + "USE_GPM=off" "USE_MULTIMEDIA=ffmpeg"
> + "USE_PANDOC=off" "FSG_BUILD=ON"))
> + #:phases
> + (modify-phases %standard-phases
> + (add-before 'build 'patch-makefile-shell
> + (lambda _
> + (setenv "HOME" "/tmp")))

Is the phase above required for tests? If so, could you add a comment
about it?

Toggle quote (8 lines)
> + (add-before 'build 'set-prefix-in-makefile
> + (lambda* (#:key outputs #:allow-other-keys)
> + (let ((out (assoc-ref outputs "out")))
> + (substitute* "Makefile"
> + (("PREFIX =.*")
> + (string-append "PREFIX = " out "\n")))
> + #true))))))

The trailing #true is not required anywore. You can drop it.

Toggle quote (4 lines)
> + (native-inputs
> + `(("ncurses" ,ncurses)
> + ("gcc-toolchain" ,gcc-10)

Could you explain why gcc-10 must be used?

Toggle quote (6 lines)
> + ("pkg-config" ,pkg-config)))
> + (inputs
> + `(("zlib" ,zlib)
> + ("ffmpeg" ,ffmpeg)
> + ("libunistring" ,libunistring)))

Pleas order inputs alphabetically.

Toggle quote (2 lines)
> + (synopsis "Not-ncurses: A library facilitating complex TUIs on modern terminals")

I suggest:

"Library for building textual user interfaces on modern terminals"

Toggle quote (2 lines)
> + (description "Supporting vivid colors, multimedia, threads, & Unicode to the max.")

The description is not terribly useful, and sounds like an ad. Maybe:

Notcurses is a library for building complex, textual user interfaces
(TUIs) on modern terminal emulators. It does not use Ncurses, though
it does make use of libtinfo from that package.

The second sentence above may even be dropped. Up to you.

Could you send an updated patch?

Regards,
--
Nicolas Goaziou
B
B
Blake Shaw wrote on 2 Dec 2021 22:04
(name . Nicolas Goaziou)(address . mail@nicolasgoaziou.fr)(address . 52189@debbugs.gnu.org)
8735naafcg.fsf@nonconstructivism.com
Hi Nicolas,

Thanks for the review.

Toggle quote (6 lines)
> I don't think we should create a new file just for this package. Also,
> new files need to be registered in "gnu/local.mk".

> Maybe this should go into... "ncurses.scm" (!). At a later time, we may
> rename ncurse.scm into tui.scm or some such.

When I asked about where it should go in the IRC, specifically inquiring
if it should be placed with ncurses, <notmaximed> said that it shouldn't
go with Ncurses and it should be placed in its own file [1]. Are we now
sure that the opposite is the case? noted re: `gnu/local.mk` for the future.

Toggle quote (6 lines)
> Upstream use tags. It might be more readable. You'll need a variable for
> the code name, tho. In any case, a comment is warranted explaining the
> situation.

> Nitpick: string should go on the same line as base32.

Noted.

Toggle quote (2 lines)
> The build-type value above is suspicious.

It is recommended to set this value in `INSTALL.md`. What about it is suspicious?

Toggle quote (2 lines)
> This is not cross-compilation friendly.

Noted. I'll change it and try running it on other architectures using
QEMU, I had tried without success before; hopefully that will get it
building faithfully across platforms :)

Toggle quote (3 lines)
> Is the phase above required for tests? If so, could you add a comment
> about it?

The configure flags are set to build a slimmed down version of
Notcurses; it shaves about 80mb off. I had hoped to make different
outputs with different options, but when I asked about it in IRC I
didn't get a response, and couldn't find any package that is
configurable based on outputs to reference.

I just checked and the phase configuration can go entirely actually, I
just checked. But the build will fail without the configure flags set.

But alas, I just found out the 3.0 release was yesterday, which is
said to be a big improvement on many levels, so it seems like I should
just go ahead and build that one now with your suggestions and introduce
the package from this version. This package has been driving me crazy
tbh, because it updates nearly everytime I prepare to send it up
stream. but I was under the impression the 3.0 wouldn't be ready until
maybe 2022.


Thanks for the feedback and let me know about the above questions and
I'll send the new patch accordingly.

Best,
Blake

--
“In girum imus nocte et consumimur igni”
B
B
Blake Shaw wrote on 2 Dec 2021 23:20
(name . Nicolas Goaziou)(address . mail@nicolasgoaziou.fr)
87czmemyy6.fsf@nonconstructivism.com
Hi, thanks for the feedback.

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

Toggle quote (7 lines)
> I don't think we should create a new file just for this package. Also,
> new files need to be registered in "gnu/local.mk".

>
> Maybe this should go into... "ncurses.scm" (!). At a later time, we may
> rename ncurse.scm into tui.scm or some such.

I was originally packaging it with ncurses, but when I asked on the IRC
I was advised by <notmaximed> to create a new package [1]. Let me know
which is preferred and I will change things accordingly.

Toggle quote (14 lines)
>
>> +(define-public notcurses
>> + (let ((commit "d15eb6003cbd65f11163916261cf6cd5600c77fa"))
>
> Upstream use tags. It might be more readable. You'll need a variable for
> the code name, tho. In any case, a comment is warranted explaining the
> situation.
>
>> + (sha256
>> + (base32
>> + "10jf6iai1r0xafrgaz978y9bqlaw1gvd11gc0yynwwp6rcs97g17"))))
>
> Nitpick: string should go on the same line as base32.
>
noted.

Toggle quote (8 lines)
>> + (build-system cmake-build-system)
>> + (arguments
>> + `(#:tests? #f
>> + #:build-type "-DVAR=val"
>
> Indentation is off. You may want to use "etc/indent-code.el" script.
> The build-type value above is suspicious.

The build type is recommend in INSTALL.md[2]. I tried building without
it, and it seems to work fine, in case you'd like me to remove it.

Toggle quote (11 lines)
>
>> + #:make-flags
>> + (list
>> + (string-append "prefix="
>> + (assoc-ref %outputs "out"))
>> + "CC=gcc")
>
> This is not cross-compilation friendly. The above should be:
>
> (list ,(string-append "CC=" (cc-for-target))
> (string-append "prefix=" ...))
Oh great, I did not know about this. I was trying to build for other targets
using QEMU and couldn't; this seems to be why.
Toggle quote (17 lines)
>
>> + #:configure-flags
>> + (map (lambda (s)
>> + (string-append "-D" s))
>> + '("USE_CPP=off" "USE_COVERAGE=off"
>> + "USE_DOXYGEN=off" "USE_DOCTEST=off"
>> + "USE_GPM=off" "USE_MULTIMEDIA=ffmpeg"
>> + "USE_PANDOC=off" "FSG_BUILD=ON"))
>> + #:phases
>> + (modify-phases %standard-phases
>> + (add-before 'build 'patch-makefile-shell
>> + (lambda _
>> + (setenv "HOME" "/tmp")))
>
> Is the phase above required for tests? If so, could you add a comment
> about it?

The configure flags bring down the package size by about 80mb, and
FSG_BUILD=ON is to insure only FSF approved programs are used in the
build. I wasn't sure about comment etiquette, but I will add and return.

Toggle quote (10 lines)
>> + (add-before 'build 'set-prefix-in-makefile
>> + (lambda* (#:key outputs #:allow-other-keys)
>> + (let ((out (assoc-ref outputs "out")))
>> + (substitute* "Makefile"
>> + (("PREFIX =.*")
>> + (string-append "PREFIX = " out "\n")))
>> + #true))))))
>
> The trailing #true is not required anywore. You can drop it.

It seems it builds fine without either build phase, so I can remove them
in the future. I originally wrote this package back in October and it
had updated multiple times since then, so its difficult for me to recall the
purpose of these phases, although I do recall them solving some debugging issues early on.

Toggle quote (7 lines)
>
>> + (native-inputs
>> + `(("ncurses" ,ncurses)
>> + ("gcc-toolchain" ,gcc-10)
>
> Could you explain why gcc-10 must be used?

For some reason I had thought its important to pin a version for the sake of
reproducibility. If not, I will remove it.

Toggle quote (8 lines)
>
>> + ("pkg-config" ,pkg-config)))
>> + (inputs
>> + `(("zlib" ,zlib)
>> + ("ffmpeg" ,ffmpeg)
>> + ("libunistring" ,libunistring)))
>
> Pleas order inputs alphabetically.
Got it
Toggle quote (19 lines)
>
>> + (synopsis "Not-ncurses: A library facilitating complex TUIs on
>> modern terminals")
>
> I suggest:
>
> "Library for building textual user interfaces on modern terminals"
>
>> + (description "Supporting vivid colors, multimedia, threads, &
>> Unicode to the max.")
>
> The description is not terribly useful, and sounds like an ad. Maybe:
>
> Notcurses is a library for building complex, textual user interfaces
> (TUIs) on modern terminal emulators. It does not use Ncurses, though
> it does make use of libtinfo from that package.
>
> The second sentence above may even be dropped. Up to you.

Got it, I had just copied from their official release.

Toggle quote (3 lines)
>
> Could you send an updated patch?

I just found out that the newest version, which is a landmark version,
3.0, was released yesterday (I had thought it wasn't coming until
2022). so I'll write the new package with the above considerations,
after I hear back from you about the questions concerning putting it in
gnu/ncurses.scm.

Thanks!
Blake
Toggle quote (3 lines)
>
> Regards,

--
“In girum imus nocte et consumimur igni”
B
B
Blake Shaw wrote on 2 Dec 2021 23:30
(name . Nicolas Goaziou)(address . mail@nicolasgoaziou.fr)
875ys6myfy.fsf@nonconstructivism.com
Hi, thanks for the feedback.

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

Toggle quote (7 lines)
> I don't think we should create a new file just for this package. Also,
> new files need to be registered in "gnu/local.mk".

>
> Maybe this should go into... "ncurses.scm" (!). At a later time, we may
> rename ncurse.scm into tui.scm or some such.

I was originally packaging it with ncurses, but when I asked on the IRC
I was advised by <notmaximed> to create a new package [1]. Let me know
which is preferred and I will change things accordingly.

Toggle quote (14 lines)
>
>> +(define-public notcurses
>> + (let ((commit "d15eb6003cbd65f11163916261cf6cd5600c77fa"))
>
> Upstream use tags. It might be more readable. You'll need a variable for
> the code name, tho. In any case, a comment is warranted explaining the
> situation.
>
>> + (sha256
>> + (base32
>> + "10jf6iai1r0xafrgaz978y9bqlaw1gvd11gc0yynwwp6rcs97g17"))))
>
> Nitpick: string should go on the same line as base32.
>
noted.

Toggle quote (8 lines)
>> + (build-system cmake-build-system)
>> + (arguments
>> + `(#:tests? #f
>> + #:build-type "-DVAR=val"
>
> Indentation is off. You may want to use "etc/indent-code.el" script.
> The build-type value above is suspicious.

The build type is recommend in INSTALL.md[2]. I tried building without
it, and it seems to work fine, in case you'd like me to remove it.

Toggle quote (11 lines)
>
>> + #:make-flags
>> + (list
>> + (string-append "prefix="
>> + (assoc-ref %outputs "out"))
>> + "CC=gcc")
>
> This is not cross-compilation friendly. The above should be:
>
> (list ,(string-append "CC=" (cc-for-target))
> (string-append "prefix=" ...))
Oh great, I did not know about this. I was trying to build for other targets
using QEMU and couldn't; this seems to be why.
Toggle quote (17 lines)
>
>> + #:configure-flags
>> + (map (lambda (s)
>> + (string-append "-D" s))
>> + '("USE_CPP=off" "USE_COVERAGE=off"
>> + "USE_DOXYGEN=off" "USE_DOCTEST=off"
>> + "USE_GPM=off" "USE_MULTIMEDIA=ffmpeg"
>> + "USE_PANDOC=off" "FSG_BUILD=ON"))
>> + #:phases
>> + (modify-phases %standard-phases
>> + (add-before 'build 'patch-makefile-shell
>> + (lambda _
>> + (setenv "HOME" "/tmp")))
>
> Is the phase above required for tests? If so, could you add a comment
> about it?

The configure flags bring down the package size by about 80mb, and
FSG_BUILD=ON is to insure only FSF approved programs are used in the
build. I wasn't sure about comment etiquette, but I will add and return.

Toggle quote (10 lines)
>> + (add-before 'build 'set-prefix-in-makefile
>> + (lambda* (#:key outputs #:allow-other-keys)
>> + (let ((out (assoc-ref outputs "out")))
>> + (substitute* "Makefile"
>> + (("PREFIX =.*")
>> + (string-append "PREFIX = " out "\n")))
>> + #true))))))
>
> The trailing #true is not required anywore. You can drop it.

It seems it builds fine without either build phase, so I can remove them
in the future. I originally wrote this package back in October and it
had updated multiple times since then, so its difficult for me to recall the
purpose of these phases, although I do recall them solving some debugging issues early on.

Toggle quote (7 lines)
>
>> + (native-inputs
>> + `(("ncurses" ,ncurses)
>> + ("gcc-toolchain" ,gcc-10)
>
> Could you explain why gcc-10 must be used?

For some reason I had thought its important to pin a version for the sake of
reproducibility. If not, I will remove it.

Toggle quote (8 lines)
>
>> + ("pkg-config" ,pkg-config)))
>> + (inputs
>> + `(("zlib" ,zlib)
>> + ("ffmpeg" ,ffmpeg)
>> + ("libunistring" ,libunistring)))
>
> Pleas order inputs alphabetically.
Got it
Toggle quote (19 lines)
>
>> + (synopsis "Not-ncurses: A library facilitating complex TUIs on
>> modern terminals")
>
> I suggest:
>
> "Library for building textual user interfaces on modern terminals"
>
>> + (description "Supporting vivid colors, multimedia, threads, &
>> Unicode to the max.")
>
> The description is not terribly useful, and sounds like an ad. Maybe:
>
> Notcurses is a library for building complex, textual user interfaces
> (TUIs) on modern terminal emulators. It does not use Ncurses, though
> it does make use of libtinfo from that package.
>
> The second sentence above may even be dropped. Up to you.

Got it, I had just copied from their official release.

Toggle quote (3 lines)
>
> Could you send an updated patch?

I just found out that the newest version, which is a landmark version,
3.0, was released yesterday (I had thought it wasn't coming until
2022). so I'll write the new package with the above considerations,
after I hear back from you about the questions concerning putting it in
gnu/ncurses.scm.

Thanks!
Blake
Toggle quote (3 lines)
>
> Regards,

--
“In girum imus nocte et consumimur igni”
N
N
Nicolas Goaziou wrote on 3 Dec 2021 21:47
(name . Blake Shaw)(address . blake@nonconstructivism.com)(address . 52189@debbugs.gnu.org)
87h7bpquul.fsf@nicolasgoaziou.fr
Hello,

Blake Shaw <blake@nonconstructivism.com> writes:

Toggle quote (5 lines)
> When I asked about where it should go in the IRC, specifically inquiring
> if it should be placed with ncurses, <notmaximed> said that it shouldn't
> go with Ncurses and it should be placed in its own file [1]. Are we now
> sure that the opposite is the case?

No, I'm not sure. I guess <notmaximed> is right, then.

Toggle quote (3 lines)
> It is recommended to set this value in `INSTALL.md`. What about it is
> suspicious?

"-DVAR=VAL" is neither common nor self-explaining. It looks like some
copy-pasta. I suggest to add a comment explaining it is per INSTALL.md.

Regards,
--
Nicolas Goaziou
?