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 ms0.migadu.com with LMTPS id AzJaBdqdNGGgzgAAgWs5BA (envelope-from ) for ; Sun, 05 Sep 2021 12:37:14 +0200 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 iLthANqdNGECVwAAB5/wlQ (envelope-from ) for ; Sun, 05 Sep 2021 10:37:14 +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 D2AA71F17C for ; Sun, 5 Sep 2021 12:37:13 +0200 (CEST) Received: from localhost ([::1]:34128 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mMpWS-0001sd-Me for larch@yhetil.org; Sun, 05 Sep 2021 06:37:12 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48978) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mMpWI-0001r2-1O for guix-patches@gnu.org; Sun, 05 Sep 2021 06:37:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:37822) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mMpWH-0007Af-QJ for guix-patches@gnu.org; Sun, 05 Sep 2021 06:37:01 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mMpWH-0003Hl-Nf for guix-patches@gnu.org; Sun, 05 Sep 2021 06:37:01 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#50359] [PATCH] import: Add 'generic-git' updater. Resent-From: Xinglu Chen Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sun, 05 Sep 2021 10:37:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 50359 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Sarah Morgensen Cc: 50359@debbugs.gnu.org Received: via spool by 50359-submit@debbugs.gnu.org id=B50359.163083819912595 (code B ref 50359); Sun, 05 Sep 2021 10:37:01 +0000 Received: (at 50359) by debbugs.gnu.org; 5 Sep 2021 10:36:39 +0000 Received: from localhost ([127.0.0.1]:49368 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mMpVu-0003H3-BC for submit@debbugs.gnu.org; Sun, 05 Sep 2021 06:36:38 -0400 Received: from h87-96-130-155.cust.a3fiber.se ([87.96.130.155]:34286 helo=mail.yoctocell.xyz) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mMpVs-0003Gj-Lc for 50359@debbugs.gnu.org; Sun, 05 Sep 2021 06:36:37 -0400 From: Xinglu Chen DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=yoctocell.xyz; s=mail; t=1630838188; bh=eXYEC//qZ2lxCKHOpD/Aij4T6gIZE8/Mcyd/QwTDA68=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=uT2UZp/QQBJ+dxk7Tj9GLEfHRF0msM/uZuZkEYo+fpvPRkSJ169xnebdhf9b529qP 5+UXfYRRSEcGFNtPYYH1R9NkTqK5x9b8ecuiRqgB6MFA51O9VkZjv9/KRXRHzIovr4 rpdXDdtdrDsuGO56O5fH9sv8J8k/GNWr4a0Bv1II= In-Reply-To: <86k0jvkh5v.fsf@mgsn.dev> References: <86k0jvkh5v.fsf@mgsn.dev> Date: Sun, 05 Sep 2021 12:36:24 +0200 Message-ID: <87h7ez48d3.fsf@yoctocell.xyz> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" 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 X-Migadu-Spam-Score: -4.00 Authentication-Results: aspmx1.migadu.com; none X-Migadu-Queue-Id: D2AA71F17C X-Spam-Score: -4.00 X-Migadu-Scanner: scn1.migadu.com X-TUID: 53GUGeRroM43 --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Sat, Sep 04 2021, Sarah Morgensen wrote: > Hello, > > Thanks for the patch! Glad to see this idea becoming more polished. > > Xinglu Chen writes: > >> * guix/import/git.scm: New file. >> * doc/guix.texi (Invoking guix refresh): Document it. >> * Makefile.am (MODULES): Register it. >> --- >> This patch adds a new =E2=80=98generic-git=E2=80=99 updater which can ch= eck for new tags >> for package hosted on Git repos. However, it cannot download Git repos >> and update the package definitions, i.e. =E2=80=98guix refresh -u=E2=80= =99. There is a >> pending patch that would add this feature though[1]. >> >> =E2=80=98guix refresh -L=E2=80=99 now reports >> >> Available updaters: >> [=E2=80=A6] >> 94.5% of the packages are covered by these updaters. >> >> We are getting close to 100% :-) > > Wow, that is close! > >> >> See it in action! >> >> $ ./pre-inst-env guix refresh harmonist scdoc gmnisrv >> gnu/packages/web.scm:7931:4: warning: no tags were found for package `gm= nisrv' >> gnu/packages/web.scm:7931:4: warning: 'generic-git' updater failed to de= termine available releases for gmnisrv >> gnu/packages/man.scm:339:12: scdoc would be upgraded from 1.10.1 to 1.11= .1 >> gnu/packages/games.scm:9433:2: warning: failed to fetch Git repository f= or package `harmonist' >> gnu/packages/games.scm:9433:2: warning: 'generic-git' updater failed to = determine available releases for harmonist > > FWIW, harmonist and a few other packages fail to work because they use > an old git protocol which is not supported by libgit2. > > [...] >> + >> +@itemize >> +@item @code{tag-prefix}: a regular expression for matching a prefix of >> +the tag name. >> + >> +@item @code{tag-suffix}: a regular expression for matching a suffix of >> +the tag name. >> + >> +@item @code{tag-version-delimiter}: a string used as the delimiter in >> +the tag name for separating the numbers of the version. >> +@end itemize >> + >> +@lisp >> +(package >> + (name "foo") >> + ;; ... >> + (properties >> + '((tag-prefix . "^release0-") >> + (tag-suffix . "[a-z]?$") >> + (tag-version-delimiter . ":")))) >> +@end lisp > ^ extra whitespace > > I do like the selection of (prefix, suffix, delimiter), though I think > there are only one or two packages which use a different delimiter. > > [...] >> +;;; Errors & warnings >> + >> +(define-condition-type &git-tag-error &error >> + git-tag-error? >> + (kind git-tag-error-kind)) >> + >> +(define (git-tag-error kind) >> + (raise (condition (&message (message (format "bad `~a' property"))) >> + (&git-tag-error >> + (kind kind))))) > > When I trigger this error, I get: > --8<---------------cut here---------------start------------->8--- > In ice-9/exceptions.scm: > 406:15 6 (latest-git-release _) > In ice-9/boot-9.scm: > 1752:10 5 (with-exception-handler _ _ #:unwind? _ # _) > In guix/import/git.scm: > 59:39 4 (get-version _ _ #:prefix _ #:suffix _ #:delim _) > In unknown file: > 3 (simple-format #f "bad `~a' property") > In ice-9/boot-9.scm: > 1685:16 2 (raise-exception _ #:continuable? _) > 1683:16 1 (raise-exception _ #:continuable? _) > 1685:16 0 (raise-exception _ #:continuable? _) > > ice-9/boot-9.scm:1685:16: In procedure raise-exception: > In procedure simple-format: FORMAT: Missing argument for ~a > --8<---------------cut here---------------end--------------->8--- Oops, it should be (format "bad `~a' property" kind) =20=20 >> + >> +(define (git-tag-warning package c) >> + (warning (package-location package) >> + (G_ "~a for package `~a'~%") >> + (condition-message c) >> + (package-name package))) >> + >> +(define-condition-type &git-no-tags-error &error >> + git-no-tags-error?) >> + >> +(define (git-no-tags-error) >> + (raise (condition (&message (message "no tags were found")) >> + (&git-no-tags-error)))) >> + >> +(define (git-no-tags-warning package c) >> + (warning (package-location package) >> + (G_ "~a for package `~a'~%") >> + (condition-message c) >> + (package-name package))) >> + >> +(define (git-fetch-warning package) >> + (warning (package-location package) >> + (G_ "failed to fetch Git repository for package `~a'~%") >> + (package-name package))) >> + >> + >> +;;; Helper functions >> + >> +(define (string-split* str delim) >> + "Like `string-split', but DELIM is a string instead of a >> +char-set." >> + (filter (lambda (str) (not (equal? str ""))) >> + (string-split str (string->char-set delim)))) > > (string-split* "1:2.3" ":.") -> ("1" "2" "3") > (string-split* "1a2b3" "ab") -> ("1" "2" "3") > > Is this what you intended? The documentation above makes it sound like > the whole string serves as the delimiter. It=E2=80=99s not what I wanted, indeed. I will try to fix it. >> + >> +(define* (get-version package tag #:key prefix suffix delim) > > PACKAGE is not used by this procedure. Good catch, it was some leftover I forgot to remove. >> + (define delim* (if delim delim ".")) >> + (define prefix-regexp "^[^0-9]*") >> + (define suffix-regexp (string-append "[^0-9" (regexp-quote delim*) "]= *$")) > > With a delimiter of '.', this would say the suffix of '1.2.3.prerelease' > is 'prerelease', not '.prerelease'. Is this correct? (I would be > tempted to just remove delim* from this.) Good point, I think removing =E2=80=98delim*=E2=80=99 would be a good idea. >> + (define delim-regexp (string-append "^[0-9]+" (regexp-quote delim*) "= [0-9]+")) > > This fails to account for versions which use non-numerics, such as (all > taken from the package-version field of packages using git-fetch and > which use this version as the tag): > > 1.0.0-beta.0 > 0.0.9.4f > 4.4-git.1 > 5.2.0-alpha > 0.2.0-alpha-199-g3e7a475 > 20200701.154658.b0d6223 > 12-068oasis4 > 4.0.0.dev8 > 0.32-14-gcdfe14e > 2.8-fix-2 > > There are about 50-60 packages like this. > > I'm not sure how much effort should be spent including them, and for > some of them I'm not sure what our ideal behavior *is*. Even if we > could reliably detect them, should "alpha" or "dev" packages be returned > by the updater? I don=E2=80=99t think we usually include alpha or rc releases, so updater probably shouldn=E2=80=99t return them either. Not sure how we would try to detect alpha/beta/rc releases, though, besides running something like (string-match? "(alpha|beta|rc|dev)" TAG) On each tag. > Upon investigation, there is a deeper problem: version-compare thinks > "5.2.0" is a lower version than "5.2.0-alpha", and that "4.0.0" is lower > than "4.0.0.dev8". > > scheme@(guile-user)> (version-compare "5.1.9" "5.2.0") > $5 =3D < > scheme@(guile-user)> (version-compare "5.2.0" "5.2.0-alpha") > $6 =3D < > scheme@(guile-user)> (version-compare "4.0.0" "4.0.0.dev8") > $7 =3D < Maybe we should filter the tags before comparing them; that should get rid of these pre-release tags. >> + >> + (define no-prefix >> + (let ((match (string-match (or prefix prefix-regexp) tag))) >> + (if match >> + (regexp-substitute #f match 'post) >> + (git-tag-error 'tag-prefix)))) >> + >> + (define no-suffix >> + (let ((match (string-match (or suffix suffix-regexp) no-prefix))) >> + (if match >> + (regexp-substitute #f match 'pre) >> + (git-tag-error 'tag-suffix)))) >> + >> + (define no-delims >> + (if (string-match delim-regexp no-suffix) >> + (string-split* no-suffix delim*) >> + (git-tag-error 'tag-version-delimiter))) > > This throws an error if the version doesn't have any delimiter. Setting the =E2=80=98tag-version-delimiter=E2=80=99 prefix to an empty stri= ng would solve this, right? Or, maybe we should just get rid of the delimiter thing since only a few packages use a different delimiter. > Actually, it throws an error in a lot of other cases too, often saying > the 'tag-version-delimiter is wrong when it's something else. Consider > the tags from the "openjpeg" package, sorted by 'sort-tags': > > arelease > opj0-97 > start > v2.1.1 > v2.1.2 > v2.2.0 > v2.3.0 > v2.3.1 > v2.4.0 > version.1.0 > version.1.1 > version.1.2 > version.1.3 > version.1.4 > version.1.5 > version.1.5.1 > version.1.5.2 > version.2.0 > version.2.0.1 > version.2.1 > wg1n6848 > > At first, 'get-version' throws an error because "wg1n6848" doesn't have > a delimiter. But even disregarding that, it would return "version.2.1" > -> "2.1" as the latest version. > > Probably we should process all tags with 'get-version' (simply skipping > any that don't parse) and use that to sort the tags. If none parse with > 'get-version' we could use the "no tags" error or have a separate error > for "there were tags but we couldn't process them". Ah, yes, that would be a good idea. > And this lets us just do something like (untested): > > (define* (get-version tag #:key prefix suffix delim) > (define delim-rx (regexp-quote (or delim "."))) > (define prefix-rx (or prefix "[^[:digit:]]*")) > (define suffix-rx (or suffix ".*")) > (define version-char-rx > (string-append "[^" delim-rx "[:punct:]]")) > > (define tag-rx > (string-append "^" prefix "(" version-char-rx "+(" > delim-rx version-char-rx ")*)" suffix-rx "$")) > > (and=3D> (string-match tag-rx tag) > (cut match-substring <> 1))) > > Though at this point, 'tag-rx' should probably be constructed and > compiled outside the loop. > >> + >> + (string-join no-delims ".")) >> + >> +(define (sort-tags tags) >> + "Sort TAGS, a list if Git tags, such that the latest tag is the last = element." >> + (sort tags (lambda (a b) >> + (eq? (version-compare a b) '<)))) >> + >> + >> +;;; Updater >> + >> +(define (get-remote url git-uri) >> + "Given a URL and GIT-URI, a record, return the ``orig= in'' remote." >> + (let* ((checkout (update-cached-checkout url >> + #:recursive? >> + (git-reference-recursive? gi= t-uri))) >> + (repository (repository-open checkout))) >> + (remote-lookup repository "origin"))) > > We surely don't want 'update-cached-checkout' since that fetches the > whole repo history! I've attached a patch below (based on top of this > one) which brings the total time-per-package to under 1s. I moved it to > (guix git) to make use of 'with-libgit2' which ensures we use system > certificates. > > Apologies for such a long reply. I hope it was helpful :) No worries, it definitely helped a lot, thank you! --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQJJBAEBCAAzFiEEAVhh4yyK5+SEykIzrPUJmaL7XHkFAmE0nagVHHB1YmxpY0B5 b2N0b2NlbGwueHl6AAoJEKz1CZmi+1x5vCAQAI0BtYCXoZojo9O8Y4O128gOfCvo ETQvtPRNMILtZ8mkv+6aY0gnB1foGIEUDWb7pdZs3IBY6Z7H9yAM/pIaWnlfldum 6x/3m6ESO1GVyEFsSZxZ4pPa1xeQ3+qIRrKqIPN8fpmyttxzEDR54djq8yBARlCR RhSjEf+r65nsUZqlOBRfWCDp/Gx7oreb6+RiFo7ZYeBsLESlcLt8SmUbJSXzw0b5 gvigA232i3povwdbYdfGiX3np9K6xhaXeXcGO5pPike5jlJlpLoSYmBP0aqhHOqb TgqUvmCoxHcnMc8ytEVR02CdSix3RBhP8z7+fbyb1emTGVDiLCGfHCUn8L1jg2pF 6zsxKEGbLshNHkkolSIlm1YuHELiPqo8s9aNCtPFYyfcpn5iNjjx/P6B0Ywb1Gm6 uFiZ4f4PT3+slS07WWwZz+AbJnts+NFuhP26TOSjDym/WzM2XNOYAxlmcjoEDriz +H8RFOmt6p8nnE2oBxLZkRq1+6jQBJHrCJc6+ZrJm3N3ZlFa0qdbRMwEeSkHrj7Y nJgp+ZdIBqm2zbvw/xyquzytysnCuu2mBlQBPuWlRzoa9HYTa4FW6YEa7lXX8FDf 75tgRA+A2o/Cjv19zn6XpJjtIYW1Rdg7NhJqWmqUoPpMMf9BqNw5vcXztXSM2/f6 A05zw5m5IumjEdiy =ZAEz -----END PGP SIGNATURE----- --=-=-=--