| Message ID | 20220806022022.24054-1-yewscion@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#56989,v3] gnu: Add dbqn. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git-branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
Am Freitag, dem 05.08.2022 um 22:20 -0400 schrieb Christopher Rodriguez: > * gnu/packages/bqn.scm: New file. > * gnu/local.mk (GNU_SYSTEM_MODULES): Registered it here. Don't know if that's just my preference or everyone's, but I like present tense. > --- > gnu/local.mk | 1 + > gnu/packages/bqn.scm | 89 > ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 90 insertions(+) > create mode 100644 gnu/packages/bqn.scm > > diff --git a/gnu/local.mk b/gnu/local.mk > index 0e8b7b0447..c3f4cc782c 100644 > --- a/gnu/local.mk > +++ b/gnu/local.mk > @@ -138,6 +138,7 @@ GNU_SYSTEM_MODULES > = \ > %D%/packages/boost.scm \ > %D%/packages/bootloaders.scm \ > %D%/packages/bootstrap.scm \ > + %D%/packages/bqn.scm \ > %D%/packages/browser-extensions.scm \ > %D%/packages/build-tools.scm \ > %D%/packages/busybox.scm \ > diff --git a/gnu/packages/bqn.scm b/gnu/packages/bqn.scm > new file mode 100644 > index 0000000000..456983f71f > --- /dev/null > +++ b/gnu/packages/bqn.scm > @@ -0,0 +1,89 @@ > +(define-module (gnu packages bqn) > + #:use-module ((guix licenses) #:prefix license:) > + #:use-module (guix gexp) > + #:use-module (guix packages) > + #:use-module (guix download) > + #:use-module (guix git-download) > + #:use-module (guix build-system copy) > + #:use-module (guix build-system gnu) > + #:use-module (guix utils) > + #:use-module (guix deprecation) > + #:use-module (gnu packages) > + #:use-module (gnu packages libffi) > + #:use-module (gnu packages base) > + #:use-module (gnu packages pkg-config) > + #:use-module (gnu packages llvm) > + #:use-module (gnu packages java) > + #:use-module (gnu packages compression)) > +(define-public dbqn > + (let ((commit "0bbe096fc07d278b679a8479318f1722d096a03e") > + (revision "1")) > + (package > + (name "dbqn") > + (version (git-version "0.2.1" revision commit)) > + (source (origin > + (method git-fetch) > + (uri (git-reference > + (url "https://github.com/dzaima/BQN") > + (commit commit))) > + (file-name (git-file-name name version)) > + (sha256 > + (base32 > + > "1kxzxz2hrd1871281s4rsi569qk314aqfmng9pkqn8gv9nqhmph0")))) > + (outputs '("out")) > + (build-system gnu-build-system) > + (arguments > + (list #:imported-modules `(,@%gnu-build-system-modules (guix > build > + > syscalls) Put #:imported modules on a new line, same for (guix build syscalls). > + (guix build ant-build-system)) > + #:modules `((guix build gnu-build-system) > + ((guix build ant-build-system) > + #:prefix ant:) > + (guix build utils)) > + #:phases #~(modify-phases %standard-phases > + (delete 'configure) > + (replace 'build > + (lambda* _ > + (invoke "./build"))) > + (replace 'check > + (lambda* (:#key tests? > + #:allow-other-tags) > + (when tests? > + (chmod "./BQN" 493) Use octal notation for chmod. > + (system "./BQN ./test/test")))) > + (add-after 'install 'reorder-jar-content > + (lambda* (#:key outputs #:allow-other- > keys) > + (apply (assoc-ref ant:%standard-phases > + 'reorder-jar- > content) > + #:outputs (list outputs)))) > + (add-after 'reorder-jar-content 'jar- > indices > + (lambda* (#:key outputs #:allow-other- > keys) > + (apply (assoc-ref ant:%standard-phases > + 'generate-jar- > indices) > + #:outputs (list outputs)))) > + (add-after 'jar-indices 'fix-jar- > timestamps > + (lambda* (#:key outputs #:allow-other- > keys) > + (apply (assoc-ref ant:%standard-phases > + 'reorder-jar- > content) > + #:outputs (list outputs)))) > + (replace 'install > + (lambda* (#:key outputs #:allow-other- > keys) > + (let* ((out (assoc-ref outputs "out")) > + (dest-bin (string-append out > "/bin")) > + (dest-jar (string-append out > + > "/share/java"))) > + (mkdir-p dest-bin) > + (mkdir-p dest-jar) > + (copy-recursively "BQN" > + (string-append > dest-bin > + > "/dbqn")) > + (chmod (string-append dest-bin > "/dbqn") 493) > + (install-file "BQN.jar" dest-jar) > + (substitute* (string-append dest-bin > "/dbqn") > + (("BQN.jar") > + (string-append dest-jar > "/BQN.jar"))))))))) Other than that LGTM modulo not having tested anything as of yet. Cheers
On 06-08-2022 04:20, Christopher Rodriguez wrote: > +(define-module (gnu packages bqn) Copyright and license headers are missing. Also, usually we don't do per-package modules but rather thematic modules, though that's not a hard rule especially if there are technical problems with that. > + #:use-module ((guix licenses) #:prefix license:) > + #:use-module (guix gexp) > + #:use-module (guix packages) > + #:use-module (guix download) > + #:use-module (guix git-download) > + #:use-module (guix build-system copy) > + #:use-module (guix build-system gnu) > + #:use-module (guix utils) > + #:use-module (guix deprecation) I don't think you are using (guix deprecation) below? > + #:use-module (gnu packages) > + #:use-module (gnu packages libffi) > + #:use-module (gnu packages base) > + #:use-module (gnu packages pkg-config) > + #:use-module (gnu packages llvm) > + #:use-module (gnu packages java) > + #:use-module (gnu packages compression)) > +(define-public dbqn > + (let ((commit "0bbe096fc07d278b679a8479318f1722d096a03e") > + (revision "1")) > + (package > + (name "dbqn") > + (version (git-version "0.2.1" revision commit)) > + (source (origin > + (method git-fetch) > + (uri (git-reference > + (url"https://github.com/dzaima/BQN") > + (commit commit))) > + (file-name (git-file-name name version)) > + (sha256 > + (base32 > + "1kxzxz2hrd1871281s4rsi569qk314aqfmng9pkqn8gv9nqhmph0")))) I have looked at the 'v0.2.1' tag, and it points at the 0bbe096... commit, so you are actually packaging version 0.2.1, not some git commit after v0.2.1. As such, no need for (git-version ...), you can just write "0.2.1" there. Then 'revision' becomes unused and can be dropped, and 'commit' is only used in a single place anymore so it can be inlined. > + (native-inputs (list `(,icedtea-8 "jdk") coreutils zip)) coreutils is an implicit (native-)input, likely no need to to mention it. Also, the Makefile mentions that the executable to start things has #!/bin/bash -- to properly patch is when cross-compiling, you need 'bash-minimal' or 'bash' in inputs, otherwise it will be patched for the wrong architecture. Also, that script runs 'java' -- make sure it is patched too such that java will actually be found -- and to patch it, you need to have icedtea:out or openjdk:out in 'inputs'. > + (outputs '("out")) That's the default, no need to set it. > + (list #:imported-modules `(,@%gnu-build-system-modules (guix build > + syscalls) For formatting, (guix build syscalls) should be on a separate line. > + (synopsis "BQN implementation based on dzaima/APL") > + (description "BQN implementation based on dzaima/APL.") The synopsis and description are identical, and this doesn't explain much to people who don't know what 'BQN' is. Can it be rewritten such that people not familiar with BQN can decide whether this ‘BQN’ is something that's useful for them? '(guix)Synopses and Descriptions' has more information. > + (lambda* (:#key tests? > + #:allow-other-tags) Why the : before #:key? Also, no need to break it in separate lines. > + (add-after 'install 'reorder-jar-content > + (lambda* (#:key outputs #:allow-other-keys) > + (apply (assoc-ref ant:%standard-phases > + 'reorder-jar-content) > + #:outputs (list outputs)))) 'output's is a list of strings, now you are giving reorder-jar-content a list of lists of strings However, looking at (guix build ant-build-system), it looks like it just wants a list of strings. As such, maybe it should be: (add-after 'install 'reorder-jar-content (assoc-ref ant:%standard-phases 'reorder-jar-content)) ? (untested) Possibly likewise for the other phases. > (the implementation is even under single-license GPL!) [...] You are writing license:expat in the 'license' field, not license:gplN/license:gplN+. Is it Expat or is it GPL? > + (synopsis "Official BQN sources in BQN") If they are just sources, you can do (define bqn-bytecode-sources (origin ...)) > + (home-page"https://github.com/mlochbaum/BQN.git") > + (license license:gpl3)))) The 'LICENSE' file says something different. I don't think that's the home page, maybe <https://mlochbaum.github.io/BQN/> instead? > + (chmod "BQN" 493) Hexadecimal would be clearer. > + "The expected implementation for the BQN language, > +according to the official documentation of that specification.") expected -> standard (what's 'expected' depends on the user, they might want a different implementation), unless it's not the standard implementation. 'documentation of that specification' -> 'the specification (pleonasm) And maybe remove 'official', given the absence of 'official' in the descriptions of, say, guile, gcc, openjdk and java, this sounds marketing and unfair to me. (singeli-bootstrap:) > + (let* ((tag "0") > + (revision "1") > + (commit "fd17b144483549dbd2bcf23e3a37a09219171a99") > + (hash "1rr4l7ijzcg25n2igi1mzya6qllh5wsrf3m5i429rlgwv1fwvfji") > + (version (git-version tag revision commit))) I'm not seeing a '0' tag anywhere in the repository -- I dont see any tags at all tere. > + (inputs (list bqn-bytecode-sources libffi singeli-bootstrap)))) For cross-compilation, I would have expected sengili-bootstrap in native-inputs, not inputs, assuming that it is used as a compiler. Does cross-compilation (with --target) work? Greetings, Maxime.
> Maxime Devos <maximedevos@telenet.be> writes: > Copyright and license headers are missing. Also, usually we don't do > per-package modules but rather thematic modules, though that's not a hard > rule especially if there are technical problems with that. Ah, right, I forgot about those. Not used to creating new files. Will be in the patch I'm sending shortly. As for the per-package rule, this will not be the only package for this file. Aside from the 2 user-facing packages and 3 bootstrap packages in this series, I'm hoping to package a further (at least) 3 packages—a standard library, a DSL, and a primer (info). I'm also hoping to package an emacs mode, a few fonts, and an update to a few packages to add BQN support. But those have their own files, and won't be included here. Is that enough of a justification for a new file, or should I look to add this to another? I /suppose/ apl.scm would work, though technically that would be like scheme, common lisp, arc, clojure, et al being grouped in the same language. WDYT? > I don't think you are using (guix deprecation) below? I don't think I am either, now that You mention it. Somehow that made its way into my starting template; I will remove it in the forthcoming patch. > I have looked at the 'v0.2.1' tag, and it points at the 0bbe096... commit, so > you are actually packaging version 0.2.1, not some git commit after > v0.2.1. As such, no need for (git-version ...), you can just write "0.2.1" > there. Then 'revision' becomes unused and can be dropped, and 'commit' is > only used in a single place anymore so it can be inlined. This honestly confused me a bit at first, but I think I see what You mean now. Since it's an official upstream version, that can be the version, instead of using git-version. I will apply these changes as well. >> + (native-inputs (list `(,icedtea-8 "jdk") coreutils zip)) > coreutils is an implicit (native-)input, likely no need to to mention it. > > Also, the Makefile mentions that the executable to start things has > #!/bin/bash -- to properly patch is when cross-compiling, you need > 'bash-minimal' or 'bash' in inputs, otherwise it will be patched for the > wrong architecture. > > Also, that script runs 'java' -- make sure it is patched too such that java > will actually be found -- and to patch it, you need to have icedtea:out or > openjdk:out in 'inputs'. Per the feedback I received from Liliana, I will use icedtea:out to (as there is less bootstrapping required if building everything from source). I'll also add bash-minimal in place of the unneeded coreutils, and place both icedtea:out and bash-minimal in inputs rather than native, as they are expected for the target machine. >> + (outputs '("out")) > That's the default, no need to set it. >> + (list #:imported-modules `(,@%gnu-build-system-modules (guix build >> + syscalls) > For formatting, (guix build syscalls) should be on a separate line. >> + (synopsis "BQN implementation based on dzaima/APL") >> + (description "BQN implementation based on dzaima/APL.") > The synopsis and description are identical, and this doesn't explain much to > people who don't know what 'BQN' is. Can it be rewritten such that people not > familiar with BQN can decide whether this ‘BQN’ is something that's useful > for them? '(guix)Synopses and Descriptions' has more information. >> + (lambda* (:#key tests? >> + #:allow-other-tags) > Why the : before #:key? Also, no need to break it in separate lines. All of these points are addressed in the forthcoming (v4) patch. In particular, thanks for calling out the description: It had been a placeholder while I was getting things working, and I'm happy to replace it with something more descriptive. And the `:#key` was a typo, unsure how it has worked thus far, as I had tests disabled up until recently. Maybe I was using the stock check phase, then. I forget, tbh. >> + (add-after 'install 'reorder-jar-content >> + (lambda* (#:key outputs #:allow-other-keys) >> + (apply (assoc-ref ant:%standard-phases >> + 'reorder-jar-content) >> + #:outputs (list outputs)))) > 'output's is a list of strings, now you are giving reorder-jar-content a list > of lists of strings > However, looking at (guix build ant-build-system), it looks like it just > wants a list of strings. > > As such, maybe it should be: > > (add-after 'install 'reorder-jar-content > (assoc-ref ant:%standard-phases 'reorder-jar-content)) > > ? (untested) Possibly likewise for the other phases. This intrigues me. The list of outputs was a kludge to allow the function to accept the singleton output. If the singleton is still wrapped in a list, then I'm unsure why it fails. Perhaps I need to test this more; will do so before sending v4. >> (the implementation is even under single-license GPL!) [...] > You are writing license:expat in the 'license' field, not > license:gplN/license:gplN+. Is it Expat or is it GPL? So, this package (dbqn) is expat. The implementation I referenced above (which is recommended and actively developed) is a different one (cbqn) that relies on this one as a dependency for bootstrapping, and is under gpl. I really just wanted to add cbqn to guix, but can't do so without dbqn, which /is/ still functional, is depended on by a bunch of tooling for the language, and may as well also be a package because of the above (sort of like how someone using common lisp may view clisp as a slow dependency of sbcl, or they may instead choose to use it as their target implementation). >> + (synopsis "Official BQN sources in BQN") > If they are just sources, you can do (define bqn-bytecode-sources (origin > ...)) I had not realized the package was unneeded! That might simplify things greatly. I love packaging things for guix; I always learn something new. >> + (home-page"https://github.com/mlochbaum/BQN.git") >> + (license license:gpl3)))) > The 'LICENSE' file says something different. I don't think that's the home > page, maybe <https://mlochbaum.github.io/BQN/> instead? So, <https://mlochbaum.github.io/BQN> is the homepage of the language, but the sources we're using are only a part of that language definition. In particular, we're only using the src/ and test/ directories of that repository. We're ignoring the docs, commentary, editor plugins, fonts, implementation instructions, spec, tutorial, javascript implementation, etc. We really are just targetting the sources for the bytecode and the tests, which is why I thought the repo itself makes a better homepage. Should I change that to the homepage for the language? As for the LICENSE file, indeed, it is isc. Thanks for the catch. > >> + (chmod "BQN" 493) > Hexadecimal would be clearer. As in Your second message, I will use the octal notation per Liliana. >> + "The expected implementation for the BQN language, >> +according to the official documentation of that specification.") > expected -> standard (what's 'expected' depends on the user, they might want > a different implementation), unless it's not the standard implementation. > > 'documentation of that specification' -> 'the specification (pleonasm) > > And maybe remove 'official', given the absence of 'official' in the > descriptions of, say, guile, gcc, openjdk and java, this sounds marketing and > unfair to me. I will change this to "The standard implementation of the BQN language, according to the specification." (Also, thank You for teaching me 'pleonasm'!) > (singeli-bootstrap:) > >> + (let* ((tag "0") >> + (revision "1") >> + (commit "fd17b144483549dbd2bcf23e3a37a09219171a99") >> + (hash "1rr4l7ijzcg25n2igi1mzya6qllh5wsrf3m5i429rlgwv1fwvfji") >> + (version (git-version tag revision commit))) > I'm not seeing a '0' tag anywhere in the repository -- I dont see any tags at > all tere. There are none; I thought the absence of any tage necessitates a '0' for the version number, and wanted to keep the definitions standardized. Should I inline the 0 instead in the git-version? >> + (inputs (list bqn-bytecode-sources libffi singeli-bootstrap)))) > For cross-compilation, I would have expected sengili-bootstrap in > native-inputs, not inputs, assuming that it is used as a compiler. Does > cross-compilation (with --target) work? Singeli is written in BQN, which is an interpreted language. So, as long as the BQN interpreter is for the correct architecture, it will work. It will never be compiled itelf. Thank You for all of the feedback! I'll try to have a revised set of patches in by this evening EDT, in the next 8 hours. -- Christopher Rodriguez
On 08-08-2022 15:54, Christopher Rodriguez wrote: >> Maxime Devos<maximedevos@telenet.be> writes: >> Copyright and license headers are missing. Also, usually we don't do >> per-package modules but rather thematic modules, though that's not a hard >> rule especially if there are technical problems with that. > Ah, right, I forgot about those. Not used to creating new files. Will be > in the patch I'm sending shortly. > > As for the per-package rule, this will not be the only package for this > file. Aside from the 2 user-facing packages and 3 bootstrap packages in > this series, I'm hoping to package a further (at least) 3 packages—a > standard library, a DSL, and a primer (info). > > I'm also hoping to package an emacs mode, a few fonts, and an update to > a few packages to add BQN support. But those have their own files, and > won't be included here. > > Is that enough of a justification for a new file, or should I look to > add this to another? I/suppose/ apl.scm would work, though technically > that would be like scheme, common lisp, arc, clojure, et al being I didn't notice the [2/5, 3/5, 4/5, 5/5] patches, the new file should be fine I think Greetings, Maxime
On 08-08-2022 15:54, Christopher Rodriguez wrote: >>> + (commit "fd17b144483549dbd2bcf23e3a37a09219171a99") >>> + (hash "1rr4l7ijzcg25n2igi1mzya6qllh5wsrf3m5i429rlgwv1fwvfji") >>> + (version (git-version tag revision commit))) >> I'm not seeing a '0' tag anywhere in the repository -- I dont see any tags at >> all tere. > There are none; I thought the absence of any tage necessitates a '0' for > the version number, and wanted to keep the definitions > standardized. Should I inline the 0 instead in the git-version? > Yes, if upstream doesn't do versioning we use "0" or sometimes "0.0", in combination with git-version (or hg-version, ...). However, "0" is not a tag, so the variable 'tag' needs to be renamed or inlined. As it is only used in a signle place, I would recommend inlining it. Likewise for 'version'. Additionally, 'hash' needs to be inlined -- not only because it is used in a single place, but also because there is some compile-time error checking for hashes that only can work when it is inlined. Also, I've seen a v2 for the first patch, but what happened to 2/5, 3/5, 4/5 and 5/5? Greetings, Maxime.
On 09-08-2022 10:58, Maxime Devos wrote: > Also, I've seen a v2 for the first patch, but what happened to 2/5, > 3/5, 4/5 and 5/5? I meant v4. Greetings, Maxime.
diff --git a/gnu/local.mk b/gnu/local.mk index 0e8b7b0447..c3f4cc782c 100644 --- a/gnu/local.mk +++ b/gnu/local.mk @@ -138,6 +138,7 @@ GNU_SYSTEM_MODULES = \ %D%/packages/boost.scm \ %D%/packages/bootloaders.scm \ %D%/packages/bootstrap.scm \ + %D%/packages/bqn.scm \ %D%/packages/browser-extensions.scm \ %D%/packages/build-tools.scm \ %D%/packages/busybox.scm \ diff --git a/gnu/packages/bqn.scm b/gnu/packages/bqn.scm new file mode 100644 index 0000000000..456983f71f --- /dev/null +++ b/gnu/packages/bqn.scm @@ -0,0 +1,89 @@ +(define-module (gnu packages bqn) + #:use-module ((guix licenses) #:prefix license:) + #:use-module (guix gexp) + #:use-module (guix packages) + #:use-module (guix download) + #:use-module (guix git-download) + #:use-module (guix build-system copy) + #:use-module (guix build-system gnu) + #:use-module (guix utils) + #:use-module (guix deprecation) + #:use-module (gnu packages) + #:use-module (gnu packages libffi) + #:use-module (gnu packages base) + #:use-module (gnu packages pkg-config) + #:use-module (gnu packages llvm) + #:use-module (gnu packages java) + #:use-module (gnu packages compression)) +(define-public dbqn + (let ((commit "0bbe096fc07d278b679a8479318f1722d096a03e") + (revision "1")) + (package + (name "dbqn") + (version (git-version "0.2.1" revision commit)) + (source (origin + (method git-fetch) + (uri (git-reference + (url "https://github.com/dzaima/BQN") + (commit commit))) + (file-name (git-file-name name version)) + (sha256 + (base32 + "1kxzxz2hrd1871281s4rsi569qk314aqfmng9pkqn8gv9nqhmph0")))) + (outputs '("out")) + (build-system gnu-build-system) + (arguments + (list #:imported-modules `(,@%gnu-build-system-modules (guix build + syscalls) + (guix build ant-build-system)) + #:modules `((guix build gnu-build-system) + ((guix build ant-build-system) + #:prefix ant:) + (guix build utils)) + #:phases #~(modify-phases %standard-phases + (delete 'configure) + (replace 'build + (lambda* _ + (invoke "./build"))) + (replace 'check + (lambda* (:#key tests? + #:allow-other-tags) + (when tests? + (chmod "./BQN" 493) + (system "./BQN ./test/test")))) + (add-after 'install 'reorder-jar-content + (lambda* (#:key outputs #:allow-other-keys) + (apply (assoc-ref ant:%standard-phases + 'reorder-jar-content) + #:outputs (list outputs)))) + (add-after 'reorder-jar-content 'jar-indices + (lambda* (#:key outputs #:allow-other-keys) + (apply (assoc-ref ant:%standard-phases + 'generate-jar-indices) + #:outputs (list outputs)))) + (add-after 'jar-indices 'fix-jar-timestamps + (lambda* (#:key outputs #:allow-other-keys) + (apply (assoc-ref ant:%standard-phases + 'reorder-jar-content) + #:outputs (list outputs)))) + (replace 'install + (lambda* (#:key outputs #:allow-other-keys) + (let* ((out (assoc-ref outputs "out")) + (dest-bin (string-append out "/bin")) + (dest-jar (string-append out + "/share/java"))) + (mkdir-p dest-bin) + (mkdir-p dest-jar) + (copy-recursively "BQN" + (string-append dest-bin + "/dbqn")) + (chmod (string-append dest-bin "/dbqn") 493) + (install-file "BQN.jar" dest-jar) + (substitute* (string-append dest-bin "/dbqn") + (("BQN.jar") + (string-append dest-jar "/BQN.jar"))))))))) + (native-inputs (list `(,icedtea-8 "jdk") coreutils zip)) + (synopsis "BQN implementation based on dzaima/APL") + (description "BQN implementation based on dzaima/APL.") + (home-page "https://github.com/dzaima/BQN") + (license license:expat))))