From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id kJroBUFLmV+BHAAA0tVLHw (envelope-from ) for ; Wed, 28 Oct 2020 10:43:13 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id ILekAUFLmV98AQAAB5/wlQ (envelope-from ) for ; Wed, 28 Oct 2020 10:43: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 B312E940366 for ; Wed, 28 Oct 2020 10:43:12 +0000 (UTC) Received: from localhost ([::1]:52944 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kXiv9-0006Vg-NT for larch@yhetil.org; Wed, 28 Oct 2020 06:43:11 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38916) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kXiv0-0006Ub-MP for guix-patches@gnu.org; Wed, 28 Oct 2020 06:43:04 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:35797) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kXiuz-0004Ab-V3 for guix-patches@gnu.org; Wed, 28 Oct 2020 06:43:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kXiuz-0003Gh-TY for guix-patches@gnu.org; Wed, 28 Oct 2020 06:43:01 -0400 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: Wed, 28 Oct 2020 10:43:01 +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: Katherine Cox-Buday Cc: 44178@debbugs.gnu.org Received: via spool by 44178-submit@debbugs.gnu.org id=B44178.160388175412521 (code B ref 44178); Wed, 28 Oct 2020 10:43:01 +0000 Received: (at 44178) by debbugs.gnu.org; 28 Oct 2020 10:42:34 +0000 Received: from localhost ([127.0.0.1]:47341 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kXiuX-0003Fs-Ri for submit@debbugs.gnu.org; Wed, 28 Oct 2020 06:42:34 -0400 Received: from eggs.gnu.org ([209.51.188.92]:38704) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kXiuV-0003Fg-TU for 44178@debbugs.gnu.org; Wed, 28 Oct 2020 06:42:32 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:41114) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kXiuQ-00040m-Kw; Wed, 28 Oct 2020 06:42:26 -0400 Received: from vpn-0-27.aquilenet.fr ([2a0c:e300:4:27::]:38492 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1kXiuQ-00054k-3K; Wed, 28 Oct 2020 06:42:26 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= In-Reply-To: <87sga5kpdp.fsf@gmail.com> (Katherine Cox-Buday's message of "Fri, 23 Oct 2020 09:06:58 -0500") References: <87sga5kpdp.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) Date: Wed, 28 Oct 2020 11:42:24 +0100 Message-ID: <878sbq4oof.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-Spam-Score: -3.3 (---) 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-Scanner: scn0 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-Spam-Score: -1.51 X-TUID: GzWPIg6dzGYX Hi Katherine, Katherine Cox-Buday skribis: >>>From cc92cbcf5ae89891f478f319e955419800bdfcf9 Mon Sep 17 00:00:00 2001 > From: Katherine Cox-Buday > Date: Thu, 22 Oct 2020 19:40:17 -0500 > Subject: [PATCH] * guix/import/go.scm: Created Go Importer * > guix/scripts/import.scm: Created Go Importer Subcommand * guix/import/go= .scm > (importers): Added Go Importer Subcommand Nice! I think that can make a lot of people happy. :-) Here=E2=80=99s a quick review. I won=E2=80=99t promise I can reply to foll= owups in the coming days because with the release preparation going on, I=E2=80=99d rath= er focus on that. So perhaps this patch will have to wait until after this release, but certainly before the next one! > +(define (escape-capital-letters s) > + "To avoid ambiguity when serving from case-insensitive file systems, t= he > +$module and $version elements are case-encoded by replacing every upperc= ase > +letter with an exclamation mark followed by the corresponding lower-case > +letter." > + (let ((escaped-string (string))) > + (string-for-each-index > + (lambda (i) > + (let ((c (string-ref s i))) > + (set! escaped-string > + (string-concatenate > + (list escaped-string > + (if (char-upper-case? c) "!" "") > + (string (char-downcase c))))))) > + s) > + escaped-string)) As a general comment, the coding style in Guix is functional =E2=80=9Cby default=E2=80=9D (info "(guix) Coding Style"). That means we almost never = use =E2=80=98set!=E2=80=99 and procedures that modify their arguments. We also avoid idioms like car/cdr and =E2=80=98do=E2=80=99, which are more = commonly used in other Lisps, as you know very well. ;-) In the case above, I=E2=80=99d probably use =E2=80=98string-fold=E2=80=99. = The resulting code should be easier to reason about and likely more efficient. > +(define (fetch-latest-version goproxy-url module-path) > + "Fetches the version number of the latest version for MODULE-PATH from= the > +given GOPROXY-URL server." > + (assoc-ref > + (json-fetch (format #f "~a/~a/@latest" goproxy-url > + (escape-capital-letters module-path))) > + "Version")) I=E2=80=99d suggest using =E2=80=98define-json-mapping=E2=80=99 from (json)= like in the other importers. > +(define (infer-module-root module-path) > + "Go modules can be defined at any level of a repository's tree, but qu= erying > +for the meta tag usually can only be done at the webpage at the root of = the > +repository. Therefore, it is sometimes necessary to try and derive a mod= ule's > +root path from its path. For a set of well-known forges, the pattern of = what > +consists of a module's root page is known before hand." > + ;; See the following URL for the official Go equivalent: > + ;; https://github.com/golang/go/blob/846dce9d05f19a1f53465e62a304dea21= b99f910/src/cmd/go/internal/vcs/vcs.go#L1026-L1087 > + (define-record-type > + (make-scs url-prefix root-regex type) > + scs? > + (url-prefix scs-url-prefix) > + (root-regex scs-root-regex) > + (type scs-type)) Maybe VCS as =E2=80=9Cversion control system=E2=80=9D? (It took me a while= to guess what =E2=80=9CSCS=E2=80=9D meant.) > +(define (fetch-module-meta-data module-path) > + "Fetches module meta-data from a module's landing page. This is necess= ary > +because goproxy servers don't currently provide all the information need= ed to > +build a package." > + (let* ((port (http-fetch (string->uri (format #f "https://~a?go-get=3D= 1" module-path)))) > + (module-metadata #f) > + (meta-tag-prefix " + (meta-tag-prefix-length (string-length meta-tag-prefix))) > + (do ((line (read-line port) (read-line port))) > + ((or (eof-object? line) > + module-metadata)) > + (let ((meta-tag-index (string-contains line meta-tag-prefix))) > + (when meta-tag-index > + (let* ((start (+ meta-tag-index meta-tag-prefix-length)) > + (end (string-index line #\" start))) > + (set! module-metadata > + (string-split (substring/shared line start end) #\space)))= ))) I=E2=80=99d suggest a named =E2=80=98let=E2=80=99 or =E2=80=98fold=E2=80=99= here. Likewise, instead of concatenating XML strings (which could lead to malformed XML), I recommend using SXML: you would create an sexp like (meta (@ (name "go-import") (content =E2=80=A6))) and at the end pass it to =E2=80=98sxml->sxml=E2=80=99 (info "(guile) Readi= ng and Writing XML"). > + (else > + (raise-exception (format #f "unsupported scs type: ~a" scs-type))))) =E2=80=98raise-exception=E2=80=99 takes an error condition. In this case, = we should use (srfi srfi-34) for =E2=80=98raise=E2=80=99 write something like: (raise (condition (formatted-message (G_ "=E2=80=A6" =E2=80=A6)))) > +(define* (go-module->guix-package module-path #:key (goproxy-url "https:= //proxy.golang.org")) > + (call-with-temporary-output-file > + (lambda (temp port) > + (let* ((latest-version (fetch-latest-version goproxy-url module-pat= h)) > + (go.mod-path (fetch-go.mod goproxy-url module-path latest-ve= rsion > + temp)) It seems that =E2=80=98go.mod-path=E2=80=99 isn=E2=80=99t used, and thus = =E2=80=98fetch-go.mod=E2=80=99 & co. aren=E2=80=99t used either, or am I overlooking something? > + (dependencies (map car (parse-go.mod temp))) Please use =E2=80=98match=E2=80=99 instead, or perhaps define a record type= for the abstraction at hand. > + (guix-name (to-guix-package-name module-path)) > + (root-module-path (infer-module-root module-path)) > + ;; SCS type and URL are not included in goproxy information.= For > + ;; this we need to fetch it from the official module page. > + (meta-data (fetch-module-meta-data root-module-path)) > + (scs-type (module-meta-data-scs meta-data)) > + (scs-repo-url (module-meta-data-repo-url meta-data goproxy-u= rl))) > + (values > + `(package > + (name ,guix-name) > + ;; Elide the "v" prefix Go uses > + (version ,(string-trim latest-version #\v)) > + (source > + ,(source-uri scs-type scs-repo-url temp)) > + (build-system go-build-system) > + ,@(maybe-inputs (map to-guix-package-name dependencies)) > + ;; 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") > + (description ,(format #f "~a is a Go package." guix-name)) Maybe something like =E2=80=9Cfill it out!=E2=80=9D so we don=E2=80=99t get= patch submissions with the default synopsis/description. :-) > + (license #f)) Likewise. Two more things: could you (1) and an entry under =E2=80=9CInvoking guix im= port=E2=80=9D in doc/guix.texi, and (2) add tests, taking inspiration from the existing importer tests? Thank you! Ludo=E2=80=99.