From: Xinglu Chen <public@yoctocell.xyz>
To: "Ludovic Courtès" <ludo@gnu.org>
Cc: 48697@debbugs.gnu.org, raingloom <raingloom@riseup.net>
Subject: [bug#48697] [PATCH] import: Add CHICKEN egg importer.
Date: Sat, 29 May 2021 21:51:06 +0200 [thread overview]
Message-ID: <87eedppbdh.fsf@yoctocell.xyz> (raw)
In-Reply-To: <87zgwd7anf.fsf@gnu.org>
[-- Attachment #1: Type: text/plain, Size: 6790 bytes --]
On Sat, May 29 2021, Ludovic Courtès wrote:
> Hello!
>
> Xinglu Chen <public@yoctocell.xyz> skribis:
>
>> * guix/import/egg.scm: New file.
>> * guix/scripts/import/egg.scm: New file.
>> * tests/egg.scm: New file.
>> * Makefile.am (MODULES, SCM_TESTS): Register them.
>> * guix/scripts/import.scm (importers): Add egg importer.
>> * doc/guix.texi (Invoking guix import, Invoking guix refresh): Document it.
>
> Woohoo, very nice!
>
>> This patch adds recursive importer for CHICKEN eggs, the generated
>> packages aren’t entirely complete, though. It gets information from The
>> PACKAGE.egg, which is just a Scheme file that contains a list of lists
>> that specify the metadata for an egg. However, it doesn’t specify a
>> description, so I have just set the ‘description’ field to #f for now.
>> The licensing policy for eggs is also a bit vague[1], there is no strict
>> naming format for licenses, and a lot of eggs just specify ‘BSD’ rather
>> than ‘BSD-N-Clause’.
>
> On IRC, Mario Goulart of CHICKEN fame mentioned that they are working on
> it, linking to this discussion:
>
> https://lists.nongnu.org/archive/html/chicken-hackers/2020-10/msg00003.html
That’s great to see!
>> The PACKAGE.egg file can also specify system dependencies, but there is
>> no consistent format for this, sometimes strings are used, other times
>> symbols are used, and sometimes the version of the package is also
>> included. The user will have to double check the names and make sure
>> they are correct. I am also unsure about whether the system
>> dependencies should be ‘propagated-inputs’ or just ‘inputs’.
>
> Probably just ‘inputs’, no? Why would they need to be propagated? For
> Guile (and Python, etc.) they have to be propagated because you need
> .scm and .go files to be in the search path; but with CHICKEN, I believe
> you end up with .so files, so there’s probably no need for propagation?
> Not sure actually.
Ah, that makes sense, I am actually not that familiar with CHICKEN. :)
>> + #:use-module (ice-9 string-fun)
>
> Uh, first time I see this one (!). Maybe add #:select to clarify why
> it’s used for.
It’s needed for the ‘string-replace-substring’ procedure. Here is the
relevant part of the Guile manual (in case you were curios)
The following additional functions are available in the module
‘(ice-9 string-fun)’. They can be used with:
(use-modules (ice-9 string-fun))
-- Scheme Procedure: string-replace-substring str substring replacement
Return a new string where every instance of SUBSTRING in string STR
has been replaced by REPLACEMENT. For example:
(string-replace-substring "a ring of strings" "ring" "rut")
⇒ "a rut of struts"
>> +(define %eggs-home-page
>> + (make-parameter "https://api.call-cc.org/5/doc"))
>
> On IRC, Mario suggested that a better value may be
> "https://wiki.call-cc.org/egg/".
Indeed, that looks like a better page.
>> +(define (get-eggs-repository)
>> + "Update or fetch the latest version of the eggs repository and return the path
>> +to the repository."
>> + (let*-values (((url) "git://code.call-cc.org/eggs-5-latest")
>> + ((directory commit _)
>> + (update-cached-checkout url)))
>> + directory))
>
> I’d call it ‘eggs-repository’ (without ‘get-’).
>
> Also, I recommend using srfi-71 instead of srfi-11 in new code.
Oh, I didn’t know about that, definitely looks nicer.
>> +(define (find-latest-version name)
>> + "Get the latest version of the egg NAME."
>> + (let ((directory (scandir (egg-directory name))))
>> + (if directory
>> + (last directory)
>> + (begin
>> + (format #t (G_ "Package not found in eggs repository: ~a~%") name)
>> + #f))))
>
> This should be rendered with ‘warning’ from (guix diagnostics).
>
> Or maybe it should be raised as a ‘formatted-message’ exception?
Not sure if it should be an exception or not, if you run ‘guix import egg’
on a package that doesn’t exist, it will already throw an error
--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix import egg lasdkfj
Package not found in eggs repository: lasdkfj
guix import: error: failed to download meta-data for package 'lasdkfj'
--8<---------------cut here---------------end--------------->8---
>> +(define (get-metadata port)
>> + "Parse the egg metadata from PORT."
>> + (let ((content (read port)))
>> + (close-port port)
>> + content))
>> +
>> +(define (egg-metadata name)
>> + "Return the package metadata file for the egg NAME."
>> + (let ((version (find-latest-version name)))
>> + (if version
>> + (get-metadata (open-file
>> + (string-append (egg-directory name) "/"
>> + version "/" name ".egg")
>> + "r"))
>> + #f)))
>
> Rather:
>
> (call-with-input-file (string-append …)
> read)
>
> … and you can remove ‘get-metadata’.
Good idea.
>> + ;; letters in them.
>> + ;;
>> + ;; There will probably still be some weird edge cases.
>> + (string-replace-substring (string-downcase name*) " " "")))
>
> How about:
>
> (string-map (lambda (chr)
> (case chr
> ((#\space) #\-)
> (else chr)))
> (string-downcase name))
>
> ?
That would also work, and then we don’t have to import (ice-9
string-fun). :)
>> + (define egg-native-inputs
>> + (let* ((test-dependencies (assoc-ref egg-content
>> + 'test-dependencies))
>> + (build-dependencies (assoc-ref egg-content
>> + 'build-dependencies))
>> + (test+build-dependencies (safe-append
>> + test-dependencies
>> + build-dependencies)))
>> + (if (list? test+build-dependencies)
>> + (map egg-parse-dependency
>> + test+build-dependencies)
>> + '())))
>
> Use ‘match’. Arrange so ‘test-dependencies’ and ‘build-dependencies’
> are always lists so you don’t need ‘safe-append’.
>
> Last, could you add files that contain translatable strings to
> ‘po/guix/POTFILES.in’?
Sure!
> Otherwise LGTM. Could you send an updated patch?
>
> Thank you!
>
> Ludo’.
Thanks for the review! v2 will be coming. :)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]
next prev parent reply other threads:[~2021-05-29 19:52 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-27 12:48 [bug#48697] [PATCH] import: Add CHICKEN egg importer Xinglu Chen
2021-05-27 19:21 ` raingloom
2021-05-29 16:44 ` Ludovic Courtès
2021-05-29 19:51 ` Xinglu Chen [this message]
2021-05-31 16:15 ` Ludovic Courtès
2021-05-31 18:00 ` Xinglu Chen
2021-05-31 18:29 ` [bug#48697] [PATCH v3] " Xinglu Chen
2021-06-01 21:10 ` Ludovic Courtès
2021-06-01 22:05 ` Xinglu Chen
2021-06-02 15:18 ` [bug#48697] [PATCH v4] " Xinglu Chen
2021-06-03 11:09 ` bug#48697: " Ludovic Courtès
2021-06-03 13:58 ` [bug#48697] " Xinglu Chen
[not found] <id:df2632846c36a40d0effae2c9d7036281c48868f.1622118826.git.public@yoctocell.xyz>
2021-05-29 21:41 ` [bug#48697] [PATCH] " Xinglu Chen
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87eedppbdh.fsf@yoctocell.xyz \
--to=public@yoctocell.xyz \
--cc=48697@debbugs.gnu.org \
--cc=ludo@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 external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.