[bug#69552] [PATCH 0/2] Add cyclone scheme

[bug#69552] [PATCH 0/2] Add cyclone scheme

[TakeV via Guix-patches via]

I saw someone wished for cyclone scheme to be added to the guix package repo. It is a R7RS scheme which compiles to c. Seems pretty neat!

TakeV (2):
  gnu: Add cyclone-bootstrap.
  gnu: Add cyclone.

 gnu/packages/scheme.scm | 57 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)


base-commit: ff3c592710120de2ac05972a85892cdf4b1db101
-- 
2.41.0

[bug#69552] [PATCH 1/2] gnu: Add cyclone-bootstrap.

[TakeV via Guix-patches via]

* gnu/packages/scheme.scm (cyclone-bootstrap): New variable.

Change-Id: I85d37ebc48882d4b830ef34851ae6d37ecf2e6e5
---
 gnu/packages/scheme.scm | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/gnu/packages/scheme.scm b/gnu/packages/scheme.scm
index ad06d7db06..7931ac1cc1 100644
--- a/gnu/packages/scheme.scm
+++ b/gnu/packages/scheme.scm
@@ -23,6 +23,7 @@
 ;;; Copyright © 2023 Andrew Whatson <whatson@tailcall.au>
 ;;; Copyright © 2023 Juliana Sims <juli@incana.org>
 ;;; Copyright © 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2024 TakeV <takev@disroot.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -59,6 +60,7 @@ (define-module (gnu packages scheme)
   #:use-module (gnu packages base)
   #:use-module (gnu packages bash)
   #:use-module (gnu packages bdw-gc)
+  #:use-module (gnu packages c)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages databases)
   #:use-module (gnu packages emacs)
@@ -1276,3 +1278,35 @@ (define-public stklos
 Machine.  STklos can also be compiled as a library and embedded in an
 application.")
     (license gpl2+)))
+
+;; cyclone is self-hosted. To build it, we require the bootstrapped compiler.
+(define cyclone-bootstrap
+  (package
+    (name "cyclone-bootstrap")
+    (version "0.36.0")
+    ;; TODO Use system's libtommath after
+    ;; https://github.com/justinethier/cyclone/issues/458 is resolved
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/justinethier/cyclone-bootstrap.git")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "0fv0mnrn5shbx77383f4mbkvc4i9yyj1bjm3dfyhipnaqapbhqpi"))))
+    (inputs (list ck))
+    (build-system gnu-build-system)
+    (arguments
+     (list
+      #:phases #~(modify-phases %standard-phases
+                   (delete 'configure))
+      #:test-target "test"
+      #:make-flags #~(list "CC=gcc"
+                           (string-append "PREFIX="
+                                          #$output))))
+    (home-page "https://github.com/justinethier/cyclone-bootstrap")
+    (synopsis "Install Cyclone Scheme on your machine.")
+    (description "Bootstrap the generate the cyclone scheme compiler")
+    (license expat)))
+
-- 
2.41.0

[bug#69552] [PATCH 2/2] gnu: Add cyclone.

[TakeV via Guix-patches via]

* gnu/packages/scheme.scm (cyclone): New variable.

Change-Id: Icf3890ded917b728177c2831231e87b52d996532
---
 gnu/packages/scheme.scm | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/gnu/packages/scheme.scm b/gnu/packages/scheme.scm
index 7931ac1cc1..997906aa02 100644
--- a/gnu/packages/scheme.scm
+++ b/gnu/packages/scheme.scm
@@ -1310,3 +1310,26 @@ (define cyclone-bootstrap
     (description "Bootstrap the generate the cyclone scheme compiler")
     (license expat)))
 
+(define-public cyclone
+  (package
+    ;; the bootstrapped compiler and final compiler share most build reqs
+    (inherit cyclone-bootstrap)
+    (name "cyclone")
+    (version "0.36.0")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/justinethier/cyclone.git")
+             (commit "v0.36.0")))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "0qz5sdcnqkvw78gx86k3g1f0di5aqagdxqvzc35j50h1q5kj67z6"))))
+    (native-inputs (list cyclone-bootstrap))
+    (home-page "http://justinethier.github.io/cyclone/")
+    (synopsis "Fast R7RS scheme which compiles to C")
+    (description
+     "A brand-new compiler that allows practical application
+development using R7RS Scheme. We provide modern features and a stable
+system capable of generating fast native binaries.")
+    (license expat)))
-- 
2.41.0

[bug#69552] [PATCH 2/2] gnu: Add cyclone.

[Juliana Sims]

Hello,

Thanks for this patch! It's always good to get more Scheme into Guix :)

 From a purely technical perspective, there's nothing wrong with these 
patches. They build fine, are reproducible, and seem well-enough-styled 
for all practical purposes. Make sure to run `guix lint` over the 
patches; I get a couple warnings when I do that locally.

As a sidenote, Cyclone fails to build on a supported architecture 
(aarch64) because the `ck` package fails to build there. Interestingly, 
it builds fine on 32-bit ARM (armhf). I note this merely for posterity 
and anyone interested in fixing `ck`. Everything builds just fine on 
x86(_64).

What follows is largely stylistic feedback.

First patch:

 > ;; cyclone is self-hosted. To build it, we require the bootstrapped 
compiler.

I would capitalize "Cyclone" here and elsewhere; it seems to be 
capitalized in the project's prose about it. Also, this is the 
"bootstrap" compiler -- the "bootstrapped" compiler would be the 
compiler compiled with this one.

 >     (synopsis "Install Cyclone Scheme on your machine.")

This is inaccurate as this package does not install Cyclone Scheme on 
the user's machine. Perhaps simply "Cyclone Scheme bootstrap compiler" 
or "Bootstrap Cyclone Scheme" would work? Synopses are, notably, not 
meant to be complete sentences and thus should not have final 
punctuation.

 >     (description "Bootstrap the generate the cyclone scheme 
compiler")

This description is a bit short. I would just combine the first two 
paragraphs of the project README into one with minor edits to get the 
following description string:

"Cyclone Scheme is a brand-new, R7RS Scheme-to-C compiler that uses a 
variant of Cheney on the MTA to implement full tail recursion, 
continuations, and generational garbage collection.  This package uses 
intermediate code generated by compiling the Scheme source files to 
build and install Cyclone Scheme.  The compiler is self-hosting and 
cannot be built directly on a system without Cyclone binaries 
installed."

Strictly speaking, "brand new" does not need a hyphen, but that's the 
way the project writes it so I've left it as-is. You probably also want 
to add a linebreak after `description` so the string starts on its own 
line; imo this makes the line alignment more esthetically pleasing when 
complying with the 80-column width limit.

Second patch:

 >     ;; the bootstrapped compiler and final compiler share most build 
reqs

"bootstrapped" -> "bootstrap"

 >     (synopsis "Fast R7RS scheme which compiles to C")

Capitalize "Scheme." This could also just be "Fast R7RS Scheme-to-C 
compiler" if you want.

 >     (description
 > "A brand-new compiler that allows practical application
 > development using R7RS Scheme. We provide modern features and a 
stable
 > system capable of generating fast native binaries.")

I would restore the README's full "Cyclone Scheme is a" at the 
beginning of this; the description can refer to the package in the 
third person. Also, first-person language should probably be avoided 
here. "We" in a Guix package description would most directly imply the 
Guix project; this is inaccurate. Perhaps replace "We provide" with 
"Cyclone provides." It may also be fruitful to cut the second sentence 
entirely and enumerate some of the compiler's said features. I would 
try to modify the language of the second paragraph in the project's 
README (replacing passive voice with active voice, for example), and/or 
add a list highlighting some (all would be a bit too long I think) of 
the "Features" section of the aforementioned README. They seem very 
proud of the "Cheney on the MTA" algorithm so make sure to highlight it.

Other than those stylistic notes, this patch series looks good to me!

Thanks,
Juli

[bug#69552] [PATCH vREVISION 1/2] gnu: Add cyclone-bootstrap.

[TakeV via Guix-patches via]

* gnu/packages/scheme.scm (cyclone-bootstrap): New variable.

Change-Id: I85d37ebc48882d4b830ef34851ae6d37ecf2e6e5
---
 gnu/packages/scheme.scm | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/gnu/packages/scheme.scm b/gnu/packages/scheme.scm
index ad06d7db06..e0151e954c 100644
--- a/gnu/packages/scheme.scm
+++ b/gnu/packages/scheme.scm
@@ -23,6 +23,7 @@
 ;;; Copyright © 2023 Andrew Whatson <whatson@tailcall.au>
 ;;; Copyright © 2023 Juliana Sims <juli@incana.org>
 ;;; Copyright © 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
+;;; Copyright © 2024 TakeV <takev@disroot.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -59,6 +60,7 @@ (define-module (gnu packages scheme)
   #:use-module (gnu packages base)
   #:use-module (gnu packages bash)
   #:use-module (gnu packages bdw-gc)
+  #:use-module (gnu packages c)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages databases)
   #:use-module (gnu packages emacs)
@@ -1276,3 +1278,42 @@ (define-public stklos
 Machine.  STklos can also be compiled as a library and embedded in an
 application.")
     (license gpl2+)))
+
+;; Cyclone is self-hosted. To build it, we require the bootstrap compiler.
+(define cyclone-bootstrap
+  (package
+    (name "cyclone-bootstrap")
+    (version "0.36.0")
+    ;; TODO Use system's libtommath after
+    ;; https://github.com/justinethier/cyclone/issues/458 is resolved
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/justinethier/cyclone-bootstrap.git")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "0fv0mnrn5shbx77383f4mbkvc4i9yyj1bjm3dfyhipnaqapbhqpi"))))
+    (inputs (list ck))
+    (build-system gnu-build-system)
+    (arguments
+     (list
+      #:phases #~(modify-phases %standard-phases
+                   (delete 'configure))
+      #:test-target "test"
+      #:make-flags #~(list "CC=gcc"
+                           (string-append "PREFIX="
+                                          #$output))))
+    (home-page "https://github.com/justinethier/cyclone-bootstrap")
+    (synopsis "Cyclone Scheme bootstrap compiler")
+    (description
+     "Cyclone Scheme is a brand-new, R7RS Scheme-to-C compiler that uses a
+variant of Cheney on the MTA to implement full tail recursion,
+continuations, and generational garbage collection.  This package uses
+intermediate code generated by compiling the Scheme source files to
+build and install Cyclone Scheme.  The compiler is self-hosting and
+cannot be built directly on a system without Cyclone binaries
+installed.")
+    (license expat)))
+

base-commit: 201f90f34f90242bb74baa5ca44bda6131b3a035
-- 
2.41.0

[bug#69552] [PATCH vREVISION 2/2] gnu: Add cyclone.

[TakeV via Guix-patches via]

* gnu/packages/scheme.scm (cyclone): New variable.

Change-Id: Icf3890ded917b728177c2831231e87b52d996532
---
 gnu/packages/scheme.scm | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/gnu/packages/scheme.scm b/gnu/packages/scheme.scm
index e0151e954c..29b8623310 100644
--- a/gnu/packages/scheme.scm
+++ b/gnu/packages/scheme.scm
@@ -1317,3 +1317,30 @@ (define cyclone-bootstrap
 installed.")
     (license expat)))
 
+(define-public cyclone
+  (package
+    ;; the bootstrap compiler and final compiler share most build reqs
+    (inherit cyclone-bootstrap)
+    (name "cyclone")
+    (version "0.36.0")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/justinethier/cyclone.git")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32 "0qz5sdcnqkvw78gx86k3g1f0di5aqagdxqvzc35j50h1q5kj67z6"))))
+    (native-inputs (list cyclone-bootstrap))
+    (home-page "http://justinethier.github.io/cyclone/")
+    (synopsis "Fast R7RS Scheme-to-C compiler")
+    (description
+     "Cyclone Scheme is a brand-new compiler that allows real-world application
+development using the R7RS Scheme Language standard.
+
+Cyclone's runtime uses Cheney on the MTA to implement full tail recursion,
+continuations, multiple native threads, and generational garbage collection.
+The on-the-fly garbage collector manages the second-generation heap and 
+performs major collections without 'stopping the world'.")
+    (license expat)))
-- 
2.41.0

[bug#69552] [PATCH vREVISION 1/2] gnu: Add cyclone-bootstrap.

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 2415 bytes --]

tags 69552 + moreinfo
user guix
usertag 69552 - reviewed-looks-good
quit

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

> * gnu/packages/scheme.scm (cyclone-bootstrap): New variable.
>
> Change-Id: I85d37ebc48882d4b830ef34851ae6d37ecf2e6e5
> ---
>  gnu/packages/scheme.scm | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/gnu/packages/scheme.scm b/gnu/packages/scheme.scm
> index ad06d7db06..e0151e954c 100644
> --- a/gnu/packages/scheme.scm
> +++ b/gnu/packages/scheme.scm
> @@ -23,6 +23,7 @@
>  ;;; Copyright © 2023 Andrew Whatson <whatson@tailcall.au>
>  ;;; Copyright © 2023 Juliana Sims <juli@incana.org>
>  ;;; Copyright © 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com>
> +;;; Copyright © 2024 TakeV <takev@disroot.org>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -59,6 +60,7 @@ (define-module (gnu packages scheme)
>    #:use-module (gnu packages base)
>    #:use-module (gnu packages bash)
>    #:use-module (gnu packages bdw-gc)
> +  #:use-module (gnu packages c)
>    #:use-module (gnu packages compression)
>    #:use-module (gnu packages databases)
>    #:use-module (gnu packages emacs)
> @@ -1276,3 +1278,42 @@ (define-public stklos
>  Machine.  STklos can also be compiled as a library and embedded in an
>  application.")
>      (license gpl2+)))
> +
> +;; Cyclone is self-hosted. To build it, we require the bootstrap compiler.
> +(define cyclone-bootstrap
> +  (package
> +    (name "cyclone-bootstrap")
> +    (version "0.36.0")
> +    ;; TODO Use system's libtommath after
> +    ;; https://github.com/justinethier/cyclone/issues/458 is resolved
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/justinethier/cyclone-bootstrap.git")
> +             (commit (string-append "v" version))))
> +       (file-name (git-file-name name version))
> +       (sha256
> +        (base32 "0fv0mnrn5shbx77383f4mbkvc4i9yyj1bjm3dfyhipnaqapbhqpi"))))

I've had a look at this and I'm concerned about the bootstrapping
approach. If you look at cyclone.c, I'm pretty sure that's not the
preferred form for editing and it doesn't look like that from the
commits.

Maybe there are things already in Guix that are similarly bad, but I
don't think that alone is sufficient to include more.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

[bug#69552] [PATCH vREVISION 1/2] gnu: Add cyclone-bootstrap.

[TakeV via Guix-patches via]

[-- Attachment #1.1: Type: text/plain, Size: 744 bytes --]

On 3/13/24 08:12, Christopher Baines wrote:

> tags 69552 + moreinfo
> user guix
> usertag 69552 - reviewed-looks-good
> quit
>
> TakeV via Guix-patches via <guix-patches@gnu.org> writes:
> I've had a look at this and I'm concerned about the bootstrapping
> approach. If you look at cyclone.c, I'm pretty sure that's not the
> preferred form for editing and it doesn't look like that from the
> commits.
>
> Maybe there are things already in Guix that are similarly bad, but I
> don't think that alone is sufficient to include more.

Do you mean the package is wrong, or cyclone itself is not going about 
it the correct way? The instructions for building from source specify 
that this is the intended way of handling it.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

[bug#69552] [PATCH vREVISION 1/2] gnu: Add cyclone-bootstrap.

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]


TakeV <takev@disroot.org> writes:

> On 3/13/24 08:12, Christopher Baines wrote:
>
>> tags 69552 + moreinfo
>> user guix
>> usertag 69552 - reviewed-looks-good
>> quit
>>
>> TakeV via Guix-patches via <guix-patches@gnu.org> writes:
>> I've had a look at this and I'm concerned about the bootstrapping
>> approach. If you look at cyclone.c, I'm pretty sure that's not the
>> preferred form for editing and it doesn't look like that from the
>> commits.
>>
>> Maybe there are things already in Guix that are similarly bad, but I
>> don't think that alone is sufficient to include more.
>
> Do you mean the package is wrong, or cyclone itself is not going about
> it the correct way? The instructions for building from source specify
> that this is the intended way of handling it.

It's not that cyclone is going about it in an incorrect way, just that
the approach used doesn't match up with Guix's requirement to build from
source (which is the prefered form for modification) in all but
exceptional cases.

Contrast this with Guile for example [1], rather than using some
generated C file for bootstrapping, it uses a Scheme interpreter
implemented in C.

1: https://bootstrappable.org/best-practices.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

[bug#69552] [PATCH vREVISION 1/2] gnu: Add cyclone-bootstrap.

[TakeV via Guix-patches via]

I think I am a bit confused. Are you saying that we need to be able to generate the file ourselves from scratch, rather than using the upstream source code?

Or do you mean that we package the first version of the bootstrap compiler which is not automatically generated, then use the result to generate the cyclone binary, then use that to generate the bootstrap compiler, and so forth until we are at the latest version?

Mostly uncertain because the self-hosted compiler does generate the bootstrap compiler's source, but it is not needed to build the bootstrap compiler itself, and thus seems to be the same approach as guile aside from how the source code being written by a person vs a computer. 



-------- Original Message --------
From: Christopher Baines <mail@cbaines.net>
Sent: March 13, 2024 8:45:03 AM EDT
To: TakeV <takev@disroot.org>
Cc: 69552@debbugs.gnu.org, guix-patches@gnu.org
Subject: Re: [bug#69552] [PATCH vREVISION 1/2] gnu: Add cyclone-bootstrap.


TakeV <takev@disroot.org> writes:

> On 3/13/24 08:12, Christopher Baines wrote:
>
>> tags 69552 + moreinfo
>> user guix
>> usertag 69552 - reviewed-looks-good
>> quit
>>
>> TakeV via Guix-patches via <guix-patches@gnu.org> writes:
>> I've had a look at this and I'm concerned about the bootstrapping
>> approach. If you look at cyclone.c, I'm pretty sure that's not the
>> preferred form for editing and it doesn't look like that from the
>> commits.
>>
>> Maybe there are things already in Guix that are similarly bad, but I
>> don't think that alone is sufficient to include more.
>
> Do you mean the package is wrong, or cyclone itself is not going about
> it the correct way? The instructions for building from source specify
> that this is the intended way of handling it.

It's not that cyclone is going about it in an incorrect way, just that
the approach used doesn't match up with Guix's requirement to build from
source (which is the prefered form for modification) in all but
exceptional cases.

Contrast this with Guile for example [1], rather than using some
generated C file for bootstrapping, it uses a Scheme interpreter
implemented in C.

1: https://bootstrappable.org/best-practices.html

[bug#69552] [PATCH vREVISION 1/2] gnu: Add cyclone-bootstrap.

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 1146 bytes --]


TakeV <takev@disroot.org> writes:

> I think I am a bit confused. Are you saying that we need to be able to
> generate the file ourselves from scratch, rather than using the
> upstream source code?
>
> Or do you mean that we package the first version of the bootstrap
> compiler which is not automatically generated, then use the result to
> generate the cyclone binary, then use that to generate the bootstrap
> compiler, and so forth until we are at the latest version?

I don't have a solution to suggest unfortunately, I'm just raising this
as a problem.

Either of the approaches you suggest above would work.

> Mostly uncertain because the self-hosted compiler does generate the
> bootstrap compiler's source, but it is not needed to build the
> bootstrap compiler itself, and thus seems to be the same approach as
> guile aside from how the source code being written by a person vs a
> computer.

It's the "how the source code being written by a person vs a computer"
bit that's key, source code generated by a computer isn't the preferred
form for editing, so building from it is not building from source, even
though it's source code.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

[bug#69552] [PATCH vREVISION 2/2] gnu: Add cyclone.

[Juliana Sims via Guix-patches via]

LGTM!