[PATCH 0/2] Add cyclone scheme

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

Change-Id: I85d37ebc48882d4b830ef34851ae6d37ecf2e6e5

---

gnu/packages/scheme.scm | 34 ++++++++++++++++++++++++++++++++++

1 file changed, 34 insertions(+)

Change-Id: Icf3890ded917b728177c2831231e87b52d996532

---

gnu/packages/scheme.scm | 23 +++++++++++++++++++++++

1 file changed, 23 insertions(+)

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

Change-Id: I85d37ebc48882d4b830ef34851ae6d37ecf2e6e5

---

gnu/packages/scheme.scm | 41 +++++++++++++++++++++++++++++++++++++++++

1 file changed, 41 insertions(+)

Change-Id: Icf3890ded917b728177c2831231e87b52d996532

---

gnu/packages/scheme.scm | 27 +++++++++++++++++++++++++++

1 file changed, 27 insertions(+)

LGTM!

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.

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

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.

TakeV <takev@disroot.org> writes:

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

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 --------

Sent: March 13, 2024 8:45:03 AM EDT

To: TakeV <takev@disroot.org>

Cc: 69552@debbugs.gnu.org, guix-patches@gnu.org

TakeV <takev@disroot.org> writes:

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

TakeV <takev@disroot.org> writes:

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.

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.