diff mbox series

[bug#43976] Chicken build system + some example eggs

Message ID 20201013105220.7606ee5a@riseup.net
State Accepted
Headers show
Series [bug#43976] Chicken build system + some example eggs | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job

Commit Message

Csepp Oct. 13, 2020, 8:52 a.m. UTC
Here it is, chicken-build-system.

# What works
Building eggs, dependencies, importing them from search path.

# What's broken
Cross-compilation has not been attempted beacuse the Go build system I
based this on does not support it either.

# Necessary improvements
The Go build system removes some references. I was not sure if this is
needed for Chicken, so for now I left it out.

# Eggs
Some were selected because I'll need for the 9p egg, the rest because
the agrep egg was the first one I found that had dependencies, so it
was selected as a perfect test case.
I don't know if all eggs will work. Eggs that bind to native libraries
(like SDL) still need to be tested.

## SRFI-14 licensing
SRFI-14 has a weird license. The metadata says it's "BSD" but it's
clearly not BSD. I'm not sure what it is or if it's compatible with
Guix. If not, we could probably contact the authors and ask them to
relicense it.

# Chicken 4
Not attempted. Nix supports it but it's old and looks mostly abandoned.
I think we can skip it.

Comments

Ludovic Courtès Oct. 18, 2020, 4:10 p.m. UTC | #1
Hi!

raingloom <raingloom@riseup.net> skribis:

> Here it is, chicken-build-system.

Woohoo, really nice!  Great to welcome another Scheme in our home.  :-)

Overall the series LGTM.  Inline below are a few suggestions for minor
issues.

> # What's broken
> Cross-compilation has not been attempted beacuse the Go build system I
> based this on does not support it either.

That’s fine, it can come later if/when someone feels like it.

> # Necessary improvements
> The Go build system removes some references. I was not sure if this is
> needed for Chicken, so for now I left it out.

You can check the output of ‘guix size chicken-srfi-14’ (say).  If it
contains things that shouldn’t be there, like GCC or whatever, then
we should do something about it.

[...]

>>From 502235505c75446758cc1932bd1ba333644423ca Mon Sep 17 00:00:00 2001
> From: raingloom <raingloom@riseup.net>
> Date: Mon, 12 Oct 2020 04:11:59 +0200
> Subject: [PATCH 01/11] gnu: Added search paths for Chicken Scheme.
>
> * gnu/packages/chicken.scm (chicken): Added search paths
>   [native-search-paths]: added CHICKEN_REPOSITORY_PATH and CHICKEN_INCLUDE_PATH

[...]

> +    (native-search-paths
> +     (list (search-path-specification
> +            (variable "CHICKEN_REPOSITORY_PATH")
> +            ;; TODO extract binary version into a module level definition.
> +            (files (list "var/lib/chicken/11")))
> +           (search-path-specification
> +            (variable "CHICKEN_INCLUDE_PATH")
> +            (files '("share")))))

Is it just share/, not share/chicken/ or something?  A Chicken-specific
directory name would be better, but if that’s really what Chicken
expects, then so be it.

Otherwise LGTM!

> From a734e591ad066c20a53f9d0f955716ba8422bac5 Mon Sep 17 00:00:00 2001
> From: raingloom <raingloom@riseup.net>
> Date: Tue, 13 Oct 2020 09:26:52 +0200
> Subject: [PATCH 02/11] guix: Added chicken-build-system.
>
> * guix/build-system/chicken.scm: New file.
> * guix/build/chicken-build-system.scm: New file.
> * Makefile.am: Add them.

Please mention it in doc/guix.texi under “Build Systems” with a
paragraph explaining the basics, as is done for the other build systems.

> --- /dev/null
> +++ b/guix/build/chicken-build-system.scm
> @@ -0,0 +1,103 @@
> +(define-module (guix build chicken-build-system)

Please add the GPLv3+ copyright header.

> +;; TODO how do we run tests???
> +
> +;; TODO remove references

You can remove the second TODO unless/until we have reasons to believe
this has to be done.

The first TODO is more problematic though.  Is there a standard way to
run tests?  It would be great if you could skim the packages you added
to see how they handle tests, so that ‘chicken-build-system’ can have a
‘check’ phase that follows common practice.

Otherwise LGTM!

>>From a7e3b91b76625e01eed749c2c83a94862e3ef738 Mon Sep 17 00:00:00 2001
> From: raingloom <raingloom@riseup.net>
> Date: Tue, 13 Oct 2020 09:55:42 +0200
> Subject: [PATCH 04/11] gnu: Added imports for chicken eggs.
>
> * gnu/packages/chicken.scm: New module imports.

Usually we’d import modules in the patch where we first make use of
them.  Otherwise one might think this patch has no effect.

> +    (home-page "http://wiki.call-cc.org/eggref/5/srfi-69")
> +    (synopsis "An implementation of SRFI 69 with SRFI 90 extensions")

I think ‘guix lint’ won’t like that…

> +    (description
> +     "Hash table implementation and binary search")

… and a full sentence here would be welcome.  :-)

  https://guix.gnu.org/manual/devel/en/html_node/Synopses-and-Descriptions.html

Same for the other packages.

> +             (url (string-append "https://code.call-cc.org/svn/chicken-eggs/"
> +                                 "release/5/srfi-14/tags/" version))
> +             (revision 39057)
> +             (user-name "anonymous")
> +             (password "")))
> +       (sha256
> +        (base32
> +         "0wjsqfwawh9bx6vvii1gwag166bxkflc0ib374fbws14914g2ac1"))))
> +    (build-system chicken-build-system)
> +    (arguments '(#:egg-name "srfi-14"))
> +    (home-page "http://wiki.call-cc.org/eggref/5/srfi-14")
> +    (synopsis "Character set library")
> +    (description
> +     "Character sets can be created, extended, tested for the membership of
> +a characters and be compared to other character sets")
> +    (license (license:non-copyleft
> +              "file://srfi-14.scm"
> +              "See end of srfi-14.scm in the distribution."))))

You can use <http://wiki.call-cc.org/eggref/5/srfi-14#license> instead
of <file://...>.

The license looks weird indeed, but it looks like a valid free software
license.  The only discussion I found is at:
<https://srfi.schemers.org/srfi-14/mail-archive/msg00029.html>.

> From 52a27d11eb3d4d0ced3deda01fe901bf2f1937fd Mon Sep 17 00:00:00 2001
> From: raingloom <raingloom@riseup.net>
> Date: Mon, 12 Oct 2020 04:19:46 +0200
> Subject: [PATCH 11/11] gnu: Added myself to chicken.scm copyright.
>
> ---
>  gnu/packages/chicken.scm | 1 +
>  1 file changed, 1 insertion(+)

Please do that along with your first changes to the file.

That’s it.

Could you send a v2?

Thank you for working on it!

Ludo’.
Csepp Nov. 20, 2020, 4:51 a.m. UTC | #2
Well, CHICKEN_IMPORT_PATH seems to just be some kind of historical
baggage, at least as far as I can tell. It is not mentioned anywhere on
https://wiki.call-cc.org/man/5 and from what I deciphered from the
sources, the only time it's used it gets prepended to
CHICKEN_REPOSITORY_PATH.

I removed the search path and things still seem to work. It was
probably used in earlier version of Chicken and Nix just never removed
it. Didn't find any related issues either.
I think I'll comment it out for now but leave it in? If there are no
issues with it a few months after merging, it can probably be deleted
entirely...

Possibly more important: I noticed some warnings in some builds about
cp(1). So far it hasn't caused any issues. Gonna fix this, then I think
it'll be mergeable.

```
building srfi-18
   /tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18/build-srfi-18
-host -D compiling-extension -J -s -regenerate-import-libraries
-setup-mode -I /tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18 -C
-I/tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18 -O2 -d1
srfi-18.scm -o
/tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18/srfi-18.so
/tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18/build-srfi-18: line
8: srfi-18.types: Permission denied
/tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18/build-srfi-18
-regenerate-import-libraries -M -setup-mode -static -I
/tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18 -emit-link-file
/tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18/srfi-18.link -host
-D compiling-extension -c -unit srfi-18 -D compiling-static-extension
-C -I/tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18 -O2 -d1
srfi-18.scm -o
/tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18/srfi-18.static.o
cp: cannot create regular file 'srfi-18.types': Permission denied
/tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18/build-srfi-18: line
8: srfi-18.types: Permission denied
/gnu/store/aags0k5s6pnk1askg8k3czyhi34gz4pg-chicken-5.2.0/bin/csc
-setup-mode -s -host -I
/tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18 -C
-I/tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18 -O2 -d0
srfi-18.import.scm -o
/tmp/guix-build-chicken-srfi-18-0.1.5.drv-0/srfi-18/srfi-18.import.so
```
Ludovic Courtès Nov. 21, 2020, 11:23 a.m. UTC | #3
Hi,

raingloom <raingloom@riseup.net> skribis:

> Well, CHICKEN_IMPORT_PATH seems to just be some kind of historical
> baggage, at least as far as I can tell. It is not mentioned anywhere on
> https://wiki.call-cc.org/man/5 and from what I deciphered from the
> sources, the only time it's used it gets prepended to
> CHICKEN_REPOSITORY_PATH.
>
> I removed the search path and things still seem to work. It was
> probably used in earlier version of Chicken and Nix just never removed
> it. Didn't find any related issues either.
> I think I'll comment it out for now but leave it in? If there are no
> issues with it a few months after merging, it can probably be deleted
> entirely...

I’m not familiar with Chicken.  The only advice I could give is to refer
to the Chicken documentation before checking how other distros do it,
it’s probably safer.

> Possibly more important: I noticed some warnings in some builds about
> cp(1). So far it hasn't caused any issues. Gonna fix this, then I think
> it'll be mergeable.

Yup weird.

Anyway, let me know when you have a v2!

Thanks,
Ludo’.
Csepp Nov. 21, 2020, 8:45 p.m. UTC | #4
On Fri, 20 Nov 2020 05:51:17 +0100
raingloom <raingloom@riseup.net> wrote:

> Possibly more important: I noticed some warnings in some builds about
> cp(1). So far it hasn't caused any issues. Gonna fix this, then I
> think it'll be mergeable.

Seems to be a srfi-18 specific issue, so I'm calling
chicken-build-system final for now. You can merge it if it stands up to
a review.
The other eggs also seem fine, srfi-18 just has a somewhat shoddily
written auxiliary build script to generate some type information,
probably based on the current Chicken version? I'm not sure what it's
doing to be honest. It doesn't fail to build because the script doesn't
check for errors, which is why it took a while to notice it.

This is why we always `set -e` in our Bash scripts, kids.

Anyways, all other eggs seem to be fine, and none of them depend on
srfi-18.
Efraim Flashner Nov. 21, 2020, 8:58 p.m. UTC | #5
On Sat, Nov 21, 2020 at 09:45:43PM +0100, raingloom wrote:
> On Fri, 20 Nov 2020 05:51:17 +0100
> raingloom <raingloom@riseup.net> wrote:
> 
> > Possibly more important: I noticed some warnings in some builds about
> > cp(1). So far it hasn't caused any issues. Gonna fix this, then I
> > think it'll be mergeable.
> 
> Seems to be a srfi-18 specific issue, so I'm calling
> chicken-build-system final for now. You can merge it if it stands up to
> a review.

Well that's exciting! That's the patches at the beginning of the email
thread?

> The other eggs also seem fine, srfi-18 just has a somewhat shoddily
> written auxiliary build script to generate some type information,
> probably based on the current Chicken version? I'm not sure what it's
> doing to be honest. It doesn't fail to build because the script doesn't
> check for errors, which is why it took a while to notice it.
> 
> This is why we always `set -e` in our Bash scripts, kids.
> 
> Anyways, all other eggs seem to be fine, and none of them depend on
> srfi-18.

You can always rant at upstream or your monitor, your choice :)
Csepp Nov. 21, 2020, 10:13 p.m. UTC | #6
On Sat, 21 Nov 2020 22:58:58 +0200
Efraim Flashner <efraim@flashner.co.il> wrote:

> > The other eggs also seem fine, srfi-18 just has a somewhat shoddily
> > written auxiliary build script to generate some type information,
> > probably based on the current Chicken version? I'm not sure what
> > it's doing to be honest. It doesn't fail to build because the
> > script doesn't check for errors, which is why it took a while to
> > notice it.
> > 
> > This is why we always `set -e` in our Bash scripts, kids.
> > 
> > Anyways, all other eggs seem to be fine, and none of them depend on
> > srfi-18.  
> 
> You can always rant at upstream or your monitor, your choice :)
> 

Oh I will, once I figure out where to do that. So far I've been more
focused on just getting enough things working to hack on the 9p egg.
diff mbox series

Patch

From 52a27d11eb3d4d0ced3deda01fe901bf2f1937fd Mon Sep 17 00:00:00 2001
From: raingloom <raingloom@riseup.net>
Date: Mon, 12 Oct 2020 04:19:46 +0200
Subject: [PATCH 11/11] gnu: Added myself to chicken.scm copyright.

---
 gnu/packages/chicken.scm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/gnu/packages/chicken.scm b/gnu/packages/chicken.scm
index 09f51a10af..378449582c 100644
--- a/gnu/packages/chicken.scm
+++ b/gnu/packages/chicken.scm
@@ -1,6 +1,7 @@ 
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2020 Ekaitz Zarraga <ekaitz@elenq.tech>
 ;;; Copyright © 2020 Evan Hanson <evhan@foldling.org>
+;;; Copyright © 2020 raingloom <raingloom@riseup.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
-- 
2.28.0