From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id eLbvGH20PmAqAwAA0tVLHw (envelope-from ) for ; Tue, 02 Mar 2021 21:56:13 +0000 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id iPSbFH20PmC4KwAAbx9fmQ (envelope-from ) for ; Tue, 02 Mar 2021 21:56:13 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 6013215A22 for ; Tue, 2 Mar 2021 22:56:12 +0100 (CET) Received: from localhost ([::1]:48000 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lHCzz-0001LM-5T for larch@yhetil.org; Tue, 02 Mar 2021 16:56:11 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:34040) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lHCzq-0001IU-Cq for guix-patches@gnu.org; Tue, 02 Mar 2021 16:56:02 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:42882) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lHCzq-0000j2-4c for guix-patches@gnu.org; Tue, 02 Mar 2021 16:56:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1lHCzq-0000AJ-3O for guix-patches@gnu.org; Tue, 02 Mar 2021 16:56:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#44178] Add a Go Module Importer Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 02 Mar 2021 21:56:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 44178 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: To: JOULAUD =?UTF-8?Q?Fran=C3=A7ois?= Cc: "44178@debbugs.gnu.org" <44178@debbugs.gnu.org>, Katherine Cox-Buday Received: via spool by 44178-submit@debbugs.gnu.org id=B44178.1614722107570 (code B ref 44178); Tue, 02 Mar 2021 21:56:02 +0000 Received: (at 44178) by debbugs.gnu.org; 2 Mar 2021 21:55:07 +0000 Received: from localhost ([127.0.0.1]:54428 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lHCyq-00008i-6l for submit@debbugs.gnu.org; Tue, 02 Mar 2021 16:55:06 -0500 Received: from eggs.gnu.org ([209.51.188.92]:44078) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1lHCyo-00008U-Ai for 44178@debbugs.gnu.org; Tue, 02 Mar 2021 16:54:58 -0500 Received: from fencepost.gnu.org ([2001:470:142:3::e]:49269) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lHCyi-0000VG-9R; Tue, 02 Mar 2021 16:54:52 -0500 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=60212 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1lHCyh-0002SV-MG; Tue, 02 Mar 2021 16:54:52 -0500 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <87sga5kpdp.fsf@gmail.com> <20210219161737.4l266imcd24gqxwn@fjo-extia-HPdeb.example.avalenn.eu> Date: Tue, 02 Mar 2021 22:54:49 +0100 In-Reply-To: <20210219161737.4l266imcd24gqxwn@fjo-extia-HPdeb.example.avalenn.eu> ("JOULAUD =?UTF-8?Q?Fran=C3=A7ois?="'s message of "Fri, 19 Feb 2021 16:21:03 +0000") Message-ID: <871rcxte52.fsf_-_@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1614722172; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:resent-cc: resent-from:resent-sender:resent-message-id:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post; bh=lcW9YLuBkMBjcdDb7SiB8E3Ug7LMqWS9MG4AxPvIB44=; b=jc6jiem2ab+ASmRJT/SweGk3WLomGhAfpJUZFKZFMpleofOGx4NFBQyhsqP2h6oYS2AJYj u6ZmznvgsKnpHJ+jGQe+koCEn2vr9D2vHVvwuGsQ7dpA6Wb8XX9g8Hnj71M1zppdmF39nE 7bh1va8L6vDBLgzdtWHXa858XqdMhmOntLYnsmYWAePSjJdibZMQoUik45cxnK3seN4oOg 8AoEqvJAvuz9B6M37hq/YT+QdwPa+CNcmaLKavqXHpLl9UqaquSyI/P6J9hcaeo8VOnYgZ YcWYGpsPyeAbpLeVceV97H1ZfWMlDZu5MN86d98XMVhv0VevwNXf6oMeWiyg4A== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1614722172; a=rsa-sha256; cv=none; b=PCtof6vHEByCPCS+dzAwieMeaJNkpcbE7LBxZ5zVqgpSMosksxqw0iT1V8LOTmH2DEh5er N5RReETu1ijKb+lbH7Qyal5G66Ktizb9bWUrStGy/4EujgZmD8vEzw7BAvsnm+o2Gjj/Yy Zv0wNJxrJ/nsinoOpK3/g1OkT5Jp60IsCZ6CHBUZbzyE9SNEOQi/+01eq9XcTdBMKUkQWL zDK4KKhB5EeSAVRz4Sgx5aTr/uNJT6d9JA+wxO4FkE+51CjMOfc7JvFNn+Mry4aVI1ul7r bfYoIDO8cUW5YwKP967HbcwhVgOxKa4ThJrMIuqY7A85dRG/PZVSOJ1UwwLDow== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Migadu-Spam-Score: -2.86 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Migadu-Queue-Id: 6013215A22 X-Spam-Score: -2.86 X-Migadu-Scanner: scn0.migadu.com X-TUID: cUtWU+6RtT5l Hi, JOULAUD Fran=C3=A7ois skribis: > This patch add a `guix import go` command. > > It was tested with several big repositories and mostly works. Several > features are lacking (see TODO in source code) but we will do the > improvments step-by-step in future patches. > > * doc/guix.texi: doc about go importer and guile-lib dependency > * gnu/packages/package-management.scm: added guile-lib dependency > * guix/self.scm: add guile-lib dependency > * guix/build-system/go.scm: go-version->git-ref function > * guix/import/go.scm: Created Go importer > * guix/scripts/import/go.scm: Subcommand for Go importer > * guix/scripts/import.scm: Declare subcommand guix import go > * tests/import-go.scm: Tests for parse-go.mod procedure Nitpick: please mention the sections (for documentation) or variables changed (see ). Some comments below, mostly stylistic as I=E2=80=99m not familiar with the actual file formats etc. that the importer implements. > +@item > +@uref{https://www.nongnu.org/guile-lib/doc/ref/htmlprag/, guile-lib} for > +the @code{crate} importer (@pxref{Invoking guix import}). s/crate/go/ s/guile-lib/Guile-Lib/ > + (re (string-concatenate > + (list > + "(v?[0-9]\\.[0-9]\\.[0-9])" ; "v" prefix can be omitted i= n version prefix > + "(-|-pre\\.0\\.|-0\\.)" ; separator > + "([0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]= [0-9][0-9][0-9])-" ; timestamp > + "([0-9A-Fa-f][0-9A-Fa-f][0-9A-Fa-f][0-9A-Fa-f][0-9A-Fa-f]= [0-9A-Fa-f][0-9A-Fa-f][0-9A-Fa-f][0-9A-Fa-f][0-9A-Fa-f][0-9A-Fa-f][0-9A-Fa-= f])"))) ; commit hash You can use =E2=80=98string-append=E2=80=99 instead of (string-concatenate = (list =E2=80=A6)). Use [[:xdigit:]] instead of [0-9A-Fa-f] for clarity and locale-independence. Also, you can arrange to use =E2=80=98make-regexp=E2=80=99 so that the rege= xp is compiled once for all, and then just =E2=80=98regexp-exec=E2=80=99: (define %go-version-rx (make-regexp =E2=80=A6)) (define (go-version->git-ref version) (=E2=80=A6 (regexp-exec %go-version-rx =E2=80=A6) =E2=80=A6)) It=E2=80=99s not critical though. > + (define (replace-directive results line) > + "Extract replaced modules and new requirements from replace directive > + in LINE and add to RESULTS." > + ;; ReplaceSpec =3D ModulePath [ Version ] "=3D>" FilePath newline > + ;; | ModulePath [ Version ] "=3D>" ModulePath Version ne= wline . > + (let* ((requirements (car results)) > + (replaced (cdr results)) Please use =E2=80=98match=E2=80=99 instead of car/cdr (throughout): https://guix.gnu.org/manual/en/html_node/Coding-Style.html > + (re (string-concatenate > + '("([^[:blank:]]+)([[:blank:]]+([^[:blank:]]+))?" > + "[[:blank:]]+" "=3D>" "[[:blank:]]+" > + "([^[:blank:]]+)([[:blank:]]+([^[:blank:]]+))?"))) > + (match (string-match re line)) As above, you should arrange to pre-compile the regexp. > + (module-path (match:substring match 1)) > + (version (match:substring match 3)) > + (new-module-path (match:substring match 4)) > + (new-version (match:substring match 6)) > + (new-replaced (acons module-path version replaced)) s/acons/alist-cons/ for consistency with the rest of the code. > + (re (string-concatenate > + '("^[[:blank:]]*" > + "([^[:blank:]]+)[[:blank:]]+([^[:blank:]]+)" > + "([[:blank:]]+//.*)?"))) > + (match (string-match re line)) Same as above. > + ;; TODO: handle module path with VCS qualifier as described in > + ;; https://golang.org/ref/mod#vcs-find and > + ;; https://golang.org/cmd/go/#hdr-Remote_import_paths > + (define-record-type > + (make-vcs url-prefix root-regex type) > + vcs? > + (url-prefix vcs-url-prefix) > + (root-regex vcs-root-regex) > + (type vcs-type)) You could rename =E2=80=98make-vcs=E2=80=99 above to =E2=80=98%make-vcs=E2= =80=99 and do: (define (make-vcs prefix regexp type) (%make-vcs prefix (make-regex regexp) type)) so that again you can rely on pre-compiled regexps. > + (let* ((known-vcs > + (list > + (make-vcs > + "github.com" > + "^(github\\.com/[A-Za-z0-9_.\\-]+/[A-Za-z0-9_.\\-]+)(/[A-Za-= z0-9_.\\-]+)*$" > + 'git) > + (make-vcs > + "bitbucket.org" > + "^(bitbucket\\.org/([A-Za-z0-9_.\\-]+/[A-Za-z0-9_.\\-]+))(/[= A-Za-z0-9_.\\-]+)*$" > + 'unknown) > + (make-vcs > + "hub.jazz.net/git/" > + "^(hub\\.jazz\\.net/git/[a-z0-9]+/[A-Za-z0-9_.\\-]+)(/[A-Za-= z0-9_.\\-]+)*$" > + 'git) > + (make-vcs > + "git.apache.org" > + "^(git\\.apache\\.org/[a-z0-9_.\\-]+\\.git)(/[A-Za-z0-9_.\\-= ]+)*$" > + 'git) > + (make-vcs > + "git.openstack.org" > + "^(git\\.openstack\\.org/[A-Za-z0-9_.\\-]+/[A-Za-z0-9_.\\-]+= )(\\.git)?(/[A-Za-z0-9_.\\-]+)*$" > + 'git))) > + (vcs (find (lambda (vcs) (string-prefix? (vcs-url-prefix vcs) m= odule-path)) > + known-vcs))) Keep =E2=80=98known-vcs=E2=80=99 in a global variable so it doesn=E2=80=99t= have to be recomputed every time. > + `(package > + (name ,guix-name) > + ;; Elide the "v" prefix Go uses > + (version ,(string-trim latest-version #\v)) > + (source > + ,(vcs->origin vcs-type vcs-repo-url latest-version temp)) > + (build-system go-build-system) > + (arguments > + '(#:import-path ,root-module-path)) > + ,@(maybe-inputs (map go-module->guix-package-name dependencie= s)) > + ;; TODO(katco): It would be nice to make an effort to fetch t= his > + ;; from known forges, e.g. GitHub > + (home-page ,(format #f "https://~a" root-module-path)) > + (synopsis "A Go package") ^ =E2=80=98guix lint=E2=80=99 wouldn=E2=80=99t like it. :-) Maybe "Write syn= opsis here" instead? > + (description ,(format #f "~a is a Go package." guix-name)) > + (license #f)) Is there no info about the license? > +++ b/guix/self.scm > @@ -814,6 +814,9 @@ itself." > (define guile-ssh > (specification->package "guile-ssh")) >=20=20 > + (define guile-lib > + (specification->package "guile-lib")) > + > (define guile-git > (specification->package "guile-git")) >=20=20 > @@ -842,7 +845,7 @@ itself." > (append-map transitive-package-dependencies > (list guile-gcrypt gnutls guile-git guile-avahi > guile-json guile-semver guile-ssh guile-sqlite3 > - guile-zlib guile-lzlib guile-zstd))) > + guile-lib guile-zlib guile-lzlib guile-zstd))) New dependency; it=E2=80=99s a bit of a commitment, but hopefully Guile-Lib= is stable enough and works on all the supported architectures. Please add guix/scripts/import/go.scm to =E2=80=98po/guix/POTFILES.in=E2=80= =99 so it can be translated. > +++ b/tests/import-go.scm Looks nice! It should be called =E2=80=98tests/go.scm=E2=80=99 for consist= ency, with: (test-begin "go") =E2=80=A6 (test-end "go") Would it be an option to also have an end-to-end test (checking the resulting =E2=80=98package=E2=80=99 sexp)? That=E2=80=99d be nice, but per= haps we can add it afterwards if you prefer. Let=E2=80=99s see how much of the comments above you can address for a v4, = and then we can get that in and improve it from there if needed! Thanks again, Ludo=E2=80=99.