[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#48697] [PATCH] import: Add CHICKEN egg importer.
From: |
Xinglu Chen |
Subject: |
[bug#48697] [PATCH] import: Add CHICKEN egg importer. |
Date: |
Sat, 29 May 2021 21:51:06 +0200 |
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. :)
signature.asc
Description: PGP signature