unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: "Ludovic Courtès" <ludo@gnu.org>
To: raingloom <raingloom@riseup.net>
Cc: 43976@debbugs.gnu.org
Subject: [bug#43976] [PATCH] Chicken build system + some example eggs
Date: Sun, 18 Oct 2020 18:10:19 +0200	[thread overview]
Message-ID: <87mu0j347o.fsf@gnu.org> (raw)
In-Reply-To: <20201013105220.7606ee5a@riseup.net> (raingloom@riseup.net's message of "Tue, 13 Oct 2020 10:52:20 +0200")

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




  reply	other threads:[~2020-10-18 16:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-13  8:52 [bug#43976] [PATCH] Chicken build system + some example eggs raingloom
2020-10-18 16:10 ` Ludovic Courtès [this message]
2020-11-20  4:51   ` raingloom
2020-11-21 11:23     ` Ludovic Courtès
2020-11-21 20:45     ` raingloom
2020-11-21 20:58       ` Efraim Flashner
2020-11-21 22:13         ` raingloom
2020-11-22 23:12       ` raingloom
2020-11-24 20:22         ` raingloom
2020-11-27  9:09           ` Ludovic Courtès
2020-12-01  4:14             ` raingloom
2020-12-03 16:04               ` bug#43976: " Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87mu0j347o.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=43976@debbugs.gnu.org \
    --cc=raingloom@riseup.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).