From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH] import: Add PyPI importer. Date: Sat, 27 Sep 2014 23:49:18 +0200 Message-ID: <87eguwentt.fsf@gnu.org> References: <8761g8ojde.fsf@izanagi.i-did-not-set--mail-host-address--so-tickle-me> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:48421) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XXzrs-0007j2-FO for guix-devel@gnu.org; Sat, 27 Sep 2014 17:49:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XXzrn-0002wC-IG for guix-devel@gnu.org; Sat, 27 Sep 2014 17:49:28 -0400 Received: from hera.aquilenet.fr ([2a01:474::1]:39275) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XXzrn-0002vn-3m for guix-devel@gnu.org; Sat, 27 Sep 2014 17:49:23 -0400 In-Reply-To: <8761g8ojde.fsf@izanagi.i-did-not-set--mail-host-address--so-tickle-me> (David Thompson's message of "Sat, 27 Sep 2014 17:15:25 -0400") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: David Thompson Cc: guix-devel@gnu.org David Thompson skribis: > I spent my day working on generalizing the 'guix import' UI to allow for > using a PyPI importer in addition to the pre-existing Nix importer. > It's now at the point where I stop coding and open it up for review. :) Heheh, cool! :-) Overall looks good to me. Some comments below: > From b3ec259fd097034631cf311040af7aa12f7c5ebc Mon Sep 17 00:00:00 2001 > From: David Thompson > Date: Sat, 27 Sep 2014 10:16:23 -0400 > Subject: [PATCH] import: Add PyPI importer. > > * guix/snix.scm: Delete. > * guix/import/snix.scm: New file. > * guix/scripts/import/nix.scm: New file. This is good. > * guix/import/pypi.scm: New file. > * guix/scripts/import/pypi.scm: New file. Nice too. I wonder if there may be shared options between all the importers (like an option for import & live build.) That can still be addressed by exporting an option list from (guix scripts import), like (guix scripts build) does, I think. > * Makefile.am (MODULES): Add new files and remove 'guix/snix.scm'. > * guix/scripts/import.scm (%default-options, %options): Delete. > (importers): New variable. > (show-help): List importers. > (guix-import): Factor out Nix-specific logic. Delegate to correct impo= rter > based upon first argument. Make sure to adjust tests/snix.scm. Also, it=E2=80=99d be cool to have tests for (guix import pypi), at least f= or the part that is concerned with parsing JSON and producing a package object. > +(define tarball-url->string-append > + (let ((tar.gz-regex (make-regexp "\\.tar\\.gz$")) > + (tarball-regex (make-regexp ".*-(.*)\\.tar\\.gz"))) > + (lambda (url name version) > + "Return a `string-append' s-expression used for building a generic= form > +of URL for the package NAME where VERSION is replaced by a `version' > +variable." This is similar to what snix has, and i think it should be shared (see below.) > +(define (make-pypi-sexp name version source-url home-page synopsis > + description license) > + "Return the `package' s-expression for a python package with the given= NAME, Namely, what do you think of having importers return directly a =E2=80=98package=E2=80=99 object? Then there could be a shared =E2=80=98pa= ckage->sexp=E2=80=99 procedure, that would to the fancy =E2=80=98string-append=E2=80=99 thing li= ke above. And, eventually, we can add an option to do live builds of the generated package objects. That can also be done in the next iteration, though. > +(define (factorize-uri uri version) > + "Factorize URI, a package tarball URI as a string, such that any occur= rences > +of the string VERSION is replaced by the symbol 'version." This one from snix is redundant with =E2=80=98tarball-url->string-append=E2= =80=99 (and maybe less sophisticated?). Thanks! Ludo=E2=80=99.