diff mbox series

[bug#56989,v2] gnu: bqn: Add bqn.scm and dbqn package.

Message ID 20220805054616.30620-1-yewscion@gmail.com
State Accepted
Headers show
Series [bug#56989,v2] gnu: bqn: Add bqn.scm and dbqn package. | expand

Checks

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

Commit Message

Christopher Rodriguez Aug. 5, 2022, 5:46 a.m. UTC
Thanks to some help from upstream and on IRC (thanks, dzaima and lilyp!) I
was able to remove the non-determinism issue. Here's an updated patch.

---
 gnu/packages/bqn.scm | 102 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)
 create mode 100644 gnu/packages/bqn.scm


base-commit: 116c0268ffd387c88b6b47135203fb330eb422f0

Comments

Liliana Marie Prikler Aug. 5, 2022, 7:15 a.m. UTC | #1
merge 56989 56990 56991 56992 56993
thanks

Regarding the patch title, just one level of grouping is enough.
That is, use "gnu: Add dbqn."

Also, you're missing a ChangeLog, i.e.

* gnu/packages/bqn.scm: New file.
* gnu/local.mk (GNU_SYSTEM_MODULES): Register it here.

Note that the second change is lacking from your patch.

Am Freitag, dem 05.08.2022 um 01:46 -0400 schrieb Christopher
Rodriguez:
> Thanks to some help from upstream and on IRC (thanks, dzaima and
> lilyp!) I
> was able to remove the non-determinism issue. Here's an updated
> patch.
> 
> ---
>  gnu/packages/bqn.scm | 102
> +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
>  create mode 100644 gnu/packages/bqn.scm
> 
> diff --git a/gnu/packages/bqn.scm b/gnu/packages/bqn.scm
> new file mode 100644
> index 0000000000..261f29ece5
> --- /dev/null
> +++ b/gnu/packages/bqn.scm
> @@ -0,0 +1,102 @@
> +(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* ((tag "0.2.1")
> +         (revision "1")
> +         (commit "0bbe096fc07d278b679a8479318f1722d096a03e")
> +         (hash
> "1kxzxz2hrd1871281s4rsi569qk314aqfmng9pkqn8gv9nqhmph0")
> +         (version (git-version tag revision commit)))
Don't let-bind tag, version and hash, use them inline.
> +    (package
> +      (name "dbqn")
> +      (version version)
> +      (source (origin
> +                (method git-fetch)
> +                (uri (git-reference
> +                      (url "https://github.com/dzaima/BQN")
> +                      (commit commit)))
> +                (file-name (git-file-name name version))
Note that version will be bound here even if you use the version field
to do so.
> +                (sha256
> +                 (base32
> +                  hash))))
> +      (outputs '("out"))
> +      (build-system gnu-build-system)
> +      (arguments
> +       (list #:tests? #f ;While there is a "test" directory, there
> is no
> +             ;; mechanism to run the tests other than to feed the
> files into the
> +             ;; binary and check for an error. This is outside the
> scope of a
> +             ;; packaging workflow, and would need to be fixed
> upstream
> +             ;; instead. Issue Reported:
> https://github.com/dzaima/BQN/issues/12
> +             ;; Maintainer says many of the tests fail, and so they
> will remain off
> +             ;; until this is sorted out.
You could do
  (replace 'check
    (lambda* (#:key tests? #:allow-other-keys)
      (when tests?
        (for-each (lambda (known-good-test)
                    (invoke my-glorious-bin known-good-test))
                  known-good-tests))))
FSVO my-glorious-bin and known-good-tests.
> +             #: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")))
> +                          (add-after 'build 'strip-jar-timestamps
> +                            (lambda* (#:key outputs #:allow-other-
> keys)
> +                              (write %standard-phases)))
> +                          (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))))
> +                          (add-after 'install 'subjars
> +                            (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")))
> +                                (substitute* (string-append dest-bin
> "/dbqn")
> +                                  (("BQN.jar")
> +                                   (string-append dest-jar
> "/BQN.jar"))))))
Could this be done in/before install?
> +                          (add-after 'subjars 'reorder-jar-content
> +                            (lambda* (#:key outputs #:allow-other-
> keys)
> +                              (apply (cdr (assoc 'reorder-jar-
> content
> +                                                 ant:%standard-
> phases))
> +                                     #:outputs (list outputs))))
You can use #:rest args to bind args for apply.  Also use assoc-ref
rather than cdr + assoc.
> +                          (add-after 'reorder-jar-content 'jar-
> indices
> +                            (lambda* (#:key outputs #:allow-other-
> keys)
> +                              (apply (cdr (assoc 'generate-jar-
> indices
> +                                                 ant:%standard-
> phases))
> +                                     #:outputs (list outputs))))
> +                          (add-after 'jar-indices 'fix-jar-
> timestamps
> +                            (lambda* (#:key outputs #:allow-other-
> keys)
> +                              (apply (cdr (assoc 'reorder-jar-
> content
> +                                                 ant:%standard-
> phases))
> +                                     #:outputs (list outputs)))))))
> +      (native-inputs (list `(,openjdk17 "jdk") coreutils zip))
Is OpenJDK 17 required?
> +      (synopsis "BQN implementation based on dzaima/APL")
> +      (description "BQN implementation based on dzaima/APL.")
> +      (home-page "https://github.com/dzaima/BQN")
> +      (license license:expat))))
Cheers
Christopher Rodriguez Aug. 5, 2022, 3:12 p.m. UTC | #2
Liliana Marie Prikler <liliana.prikler@ist.tugraz.at> writes:

> merge 56989 56990 56991 56992 56993
> thanks

Is this notation something anyone can do? I would very much like to be
able to fix my own mistakes in the future.

> Regarding the patch title, just one level of grouping is enough.
> That is, use "gnu: Add dbqn."
>
> Also, you're missing a ChangeLog, i.e.
>
> * gnu/packages/bqn.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Register it here.

I have amended the commit message to reflect the above. Thanks for the
tips!

I've also added `%D%/packages/bqn.scm` to `gnu/local.mk`, as requested.

> Don't let-bind tag, version and hash, use them inline.
...
> Note that version will be bound here even if you use the version field
> to do so.

This makes sense, and I've removed the let binding entirely. My only
uncertainty is where "revision" should go; I've currently attached it to
the upstream version tag (version "0.2.1-1"), where "0.2.1" is the tag
and "1" is the revision. Is this correct?

> You could do
>   (replace 'check
>     (lambda* (#:key tests? #:allow-other-keys)
>       (when tests?
>         (for-each (lambda (known-good-test)
>                     (invoke my-glorious-bin known-good-test))
>                   known-good-tests))))
> FSVO my-glorious-bin and known-good-tests.

I plan to do this once I've been able to look at each test and the
entire source and see if I can get it working. I've added an issue
upstream[1] where the author of the package has confirmed it is on "just
enough life support" to build the recommended implementation from
source.

As it stands, I would have to test each test individually anyway, and
only add it to the package if it arbitrarily passes on my machine for
some reason. I don't think there is value there, as tests are meant to
ensure consistency and I cannot do that using such a workflow.

And though this *is* and *should be* a public package, it is *not* the
recommended interpreter for the language. It is primarily included here
to build the recommended one (CBQN) from source, along with some other
tools I've yet to package that require it during build.

> Could this be done in/before install?

It could, in fact. I've moved it to the above step, and deleted subjars
entirely.

> You can use #:rest args to bind args for apply.  Also use assoc-ref
> rather than cdr + assoc.

I had, for some reason, flipped the arguments on assoc-ref (which
obviously didn't work) and when that failed fell back to cdr + assoc. I
woke up this morning and noticed my mistake; It is fixed now.

As for the #:rest args recommendation: I cannot figure out how to
explicitly bind (list options) to #:options in the apply call using
#:rest. This is probably ignorance on my part; I am still learning the
some of the mechanisms in scheme, and have not used #:rest (or the dot
notation for it) much at all.

Is there an example You could point me to so I can educate myself?

> Is OpenJDK 17 required?

Really, only a JDK 7+ is required. openjdk17 carries the "openjdk" label
currently, and so I defaulted to that one. Is there another I should use
in my packages instead?


> Cheers

Thank You for the speedy response!

--

Christopher Rodriguez
\( Aug. 5, 2022, 3:37 p.m. UTC | #3
On Fri Aug 5, 2022 at 4:12 PM BST, Christopher Rodriguez wrote:
> > merge 56989 56990 56991 56992 56993
> > thanks
>
> Is this notation something anyone can do? I would very much like to be
> able to fix my own mistakes in the future.

Yes! :) You can send 'control messages' to <control@debbugs.gnu.org>
to perform operations such as `close`, `merge`, `reopen`, and `unarchive`.

For example:

> To: control@debbugs.gnu.org
>
> close 19832
> merge 98123 83720 64932
> reopen 10284
> unarchive 29177
> thanks
>
>     -- (

("thanks" just stops the processing of commands and treats the rest as
normal text.)

    -- (
Christopher Rodriguez Aug. 5, 2022, 3:50 p.m. UTC | #4
"(" <paren@disroot.org> writes:

> On Fri Aug 5, 2022 at 4:12 PM BST, Christopher Rodriguez wrote:
>> > merge 56989 56990 56991 56992 56993
>> > thanks
>>
>> Is this notation something anyone can do? I would very much like to be
>> able to fix my own mistakes in the future.
>
> Yes! :) You can send 'control messages' to <control@debbugs.gnu.org>
> to perform operations such as `close`, `merge`, `reopen`, and `unarchive`.
>
> For example:
>
>> To: control@debbugs.gnu.org
>>
>> close 19832
>> merge 98123 83720 64932
>> reopen 10284
>> unarchive 29177
>> thanks
>>
>>     -- (
>
> ("thanks" just stops the processing of commands and treats the rest as
> normal text.)
>
>     -- (
Christopher Rodriguez Aug. 5, 2022, 3:50 p.m. UTC | #5
"(" <paren@disroot.org> writes:

> Yes! :) You can send 'control messages' to <control@debbugs.gnu.org>
> to perform operations such as `close`, `merge`, `reopen`, and `unarchive`.

Ah, okay, thanks for the examples! I'll have to read up so I know what
all my options are. 

--

Christopher Rodriguez
Liliana Marie Prikler Aug. 5, 2022, 10:33 p.m. UTC | #6
Am Freitag, dem 05.08.2022 um 11:12 -0400 schrieb Christopher
Rodriguez:
> 
> > Don't let-bind tag, version and hash, use them inline.
> ...
> > Note that version will be bound here even if you use the version
> > field to do so.
> 
> This makes sense, and I've removed the let binding entirely. My only
> uncertainty is where "revision" should go; I've currently attached it
> to the upstream version tag (version "0.2.1-1"), where "0.2.1" is the
> tag and "1" is the revision. Is this correct?
You should let-bind revision and commit.  You should nt let-bind tag,
version and hash.  Use git-version like (git-version "0.2.1" revision
commit).

> 
> > You could do
> >   (replace 'check
> >     (lambda* (#:key tests? #:allow-other-keys)
> >       (when tests?
> >         (for-each (lambda (known-good-test)
> >                     (invoke my-glorious-bin known-good-test))
> >                   known-good-tests))))
> > FSVO my-glorious-bin and known-good-tests.
> 
> I plan to do this once I've been able to look at each test and the
> entire source and see if I can get it working. I've added an issue
> upstream[1] where the author of the package has confirmed it is on
> "just enough life support" to build the recommended implementation
> from source.
> 
> As it stands, I would have to test each test individually anyway, and
> only add it to the package if it arbitrarily passes on my machine for
> some reason. I don't think there is value there, as tests are meant
> to ensure consistency and I cannot do that using such a workflow.
Fair enough.

> And though this *is* and *should be* a public package, it is *not*
> the recommended interpreter for the language. It is primarily
> included here to build the recommended one (CBQN) from source, along
> with some other tools I've yet to package that require it during
> build.
That isn't really a good argument not to have tests though.  While
package maintainers should check that dependant packages still build,
having early failure for a broken package (courtesy of the check phase)
goes a long way.

> 
> > You can use #:rest args to bind args for apply.  Also use assoc-ref
> > rather than cdr + assoc.
> 
> I had, for some reason, flipped the arguments on assoc-ref (which
> obviously didn't work) and when that failed fell back to cdr + assoc.
> I woke up this morning and noticed my mistake; It is fixed now.
Ah, yes, the infamous flip :)

> As for the #:rest args recommendation: I cannot figure out how to
> explicitly bind (list options) to #:options in the apply call using
> #:rest. This is probably ignorance on my part; I am still learning
> the some of the mechanisms in scheme, and have not used #:rest (or
> the dot notation for it) much at all.
(cons* #:options your-options rest) ?

> Is there an example You could point me to so I can educate myself?
> 
> > Is OpenJDK 17 required?
> 
> Really, only a JDK 7+ is required. openjdk17 carries the "openjdk"
> label currently, and so I defaulted to that one. Is there another I
> should use in my packages instead?
If there is no *variable* named "openjdk", I'd suggest using the lowest
one that works.  If people are so inclined to use a newer jdk they can
modify the package graph (which is easier than walking back to the
earliest supported version), plus it's lower bootstrap for those of us
building from source.

Cheers
Christopher Rodriguez Aug. 6, 2022, 1:47 a.m. UTC | #7
Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

> You should let-bind revision and commit.  You should nt let-bind tag,
> version and hash.  Use git-version like (git-version "0.2.1" revision
> commit).

Copy that, I've implemented this change in the upcoming patch.

> That isn't really a good argument not to have tests though.  While
> package maintainers should check that dependant packages still build,
> having early failure for a broken package (courtesy of the check phase)
> goes a long way.
>

I agree with this.

I did check each test, and there were three failing in the cloned repo
on my local machine… Until I ran them inside the package build
environment, where they all passed 100%. I have run this with --check
--rounds=20 and have not seen a failure inside of the build environment,
so I will leave them enabled.

> Ah, yes, the infamous flip :)
> (cons* #:options your-options rest) ?

I've left these as (#:outputs (list outputs)), as that is less
characters than the above and I'm just passing in a single variable
(inside a list, as that is what the procedure is expecting) without any
(other) modifications.

> If there is no *variable* named "openjdk", I'd suggest using the lowest
> one that works.  If people are so inclined to use a newer jdk they can
> modify the package graph (which is easier than walking back to the
> earliest supported version), plus it's lower bootstrap for those of us
> building from source.

I've successfully built it using icedtea-8 (icedtea-7 failed), so I've
switched the patch over to that.

--

Christopher Rodriguez
diff mbox series

Patch

diff --git a/gnu/packages/bqn.scm b/gnu/packages/bqn.scm
new file mode 100644
index 0000000000..261f29ece5
--- /dev/null
+++ b/gnu/packages/bqn.scm
@@ -0,0 +1,102 @@ 
+(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* ((tag "0.2.1")
+         (revision "1")
+         (commit "0bbe096fc07d278b679a8479318f1722d096a03e")
+         (hash "1kxzxz2hrd1871281s4rsi569qk314aqfmng9pkqn8gv9nqhmph0")
+         (version (git-version tag revision commit)))
+    (package
+      (name "dbqn")
+      (version version)
+      (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
+                  hash))))
+      (outputs '("out"))
+      (build-system gnu-build-system)
+      (arguments
+       (list #:tests? #f ;While there is a "test" directory, there is no
+             ;; mechanism to run the tests other than to feed the files into the
+             ;; binary and check for an error. This is outside the scope of a
+             ;; packaging workflow, and would need to be fixed upstream
+             ;; instead. Issue Reported: https://github.com/dzaima/BQN/issues/12
+             ;; Maintainer says many of the tests fail, and so they will remain off
+             ;; until this is sorted out.
+             #: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")))
+                          (add-after 'build 'strip-jar-timestamps
+                            (lambda* (#:key outputs #:allow-other-keys)
+                              (write %standard-phases)))
+                          (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))))
+                          (add-after 'install 'subjars
+                            (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")))
+                                (substitute* (string-append dest-bin "/dbqn")
+                                  (("BQN.jar")
+                                   (string-append dest-jar "/BQN.jar"))))))
+                          (add-after 'subjars 'reorder-jar-content
+                            (lambda* (#:key outputs #:allow-other-keys)
+                              (apply (cdr (assoc 'reorder-jar-content
+                                                 ant:%standard-phases))
+                                     #:outputs (list outputs))))
+                          (add-after 'reorder-jar-content 'jar-indices
+                            (lambda* (#:key outputs #:allow-other-keys)
+                              (apply (cdr (assoc 'generate-jar-indices
+                                                 ant:%standard-phases))
+                                     #:outputs (list outputs))))
+                          (add-after 'jar-indices 'fix-jar-timestamps
+                            (lambda* (#:key outputs #:allow-other-keys)
+                              (apply (cdr (assoc 'reorder-jar-content
+                                                 ant:%standard-phases))
+                                     #:outputs (list outputs)))))))
+      (native-inputs (list `(,openjdk17 "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))))