From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id SPOnJrZIRGHRjAAAgWs5BA (envelope-from ) for ; Fri, 17 Sep 2021 09:50:14 +0200 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id wBpOIrZIRGFdXQAAB5/wlQ (envelope-from ) for ; Fri, 17 Sep 2021 07:50: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 5FDFCFBA4 for ; Fri, 17 Sep 2021 09:50:13 +0200 (CEST) Received: from localhost ([::1]:36572 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mR8dP-0003MK-MJ for larch@yhetil.org; Fri, 17 Sep 2021 03:50:11 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:38348) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mR8dG-0003LU-TB for guix-patches@gnu.org; Fri, 17 Sep 2021 03:50:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:46828) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mR8dG-00047w-Ia for guix-patches@gnu.org; Fri, 17 Sep 2021 03:50:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mR8dG-0000xi-Fb for guix-patches@gnu.org; Fri, 17 Sep 2021 03:50:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#50359] [PATCH 3/3] import: Add 'generic-git' updater. Resent-From: Xinglu Chen Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Fri, 17 Sep 2021 07:50:02 +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: Ludovic =?UTF-8?Q?Court=C3=A8s?= , 50359@debbugs.gnu.org Received: via spool by 50359-submit@debbugs.gnu.org id=B50359.16318649523634 (code B ref 50359); Fri, 17 Sep 2021 07:50:02 +0000 Received: (at 50359) by debbugs.gnu.org; 17 Sep 2021 07:49:12 +0000 Received: from localhost ([127.0.0.1]:58374 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mR8cR-0000wY-Hw for submit@debbugs.gnu.org; Fri, 17 Sep 2021 03:49:11 -0400 Received: from h87-96-130-155.cust.a3fiber.se ([87.96.130.155]:60596 helo=mail.yoctocell.xyz) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mR8cP-0000wJ-Rw for 50359@debbugs.gnu.org; Fri, 17 Sep 2021 03:49:10 -0400 From: Xinglu Chen DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=yoctocell.xyz; s=mail; t=1631864941; bh=RkqjigjBO+ib8clJXIDbwCx8ki4ITq+5anaKC2HP7ik=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=KyzwyKO2DRMDnq2BnTQ4TORkXyDwyirGLlrZ+oz5lWjgfeByYINVQepIicAbdHA6a ONEQeD6bISVVqMEExfrUvw/PJL6sK6ICZVIjpXXssQFCMsnY/L/JqO2DQPjkP04n4j puJmtWJlOXSqcKtJP+rrA9Q2+r6GZC4iwpoSZ2Kg= In-Reply-To: <867dfghytt.fsf@mgsn.dev> References: <5d10dd1e65b0a65ada4a8102310c10de42f53e8d.1631290349.git.public@yoctocell.xyz> <86czp8j38z.fsf@mgsn.dev> <87h7ekr8iw.fsf@yoctocell.xyz> <867dfghytt.fsf@mgsn.dev> Date: Fri, 17 Sep 2021 09:48:49 +0200 Message-ID: <8735q3r6am.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: 5FDFCFBA4 X-Spam-Score: -4.00 X-Migadu-Scanner: scn0.migadu.com X-TUID: JXDGBEcazTdl --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Thu, Sep 16 2021, Sarah Morgensen wrote: > Xinglu Chen writes: > >>>> Maybe we could have a =E2=80=98release-tag-date-scheme?=E2=80=99 prope= rty, that way we >>>> could just try to match dates? >>> >>> That seems like it might be the only way to handle it in some cases (if >>> they have both versions and dates with a "." delimiter). >> >> It doesn=E2=80=99t have to be =E2=80=9C.=E2=80=9D delimiter though; if t= hey both have the same >> delimiter it would be difficult to distinguish a version from a date, >> e.g., =E2=80=9C1-2-3=E2=80=9D vs =E2=80=9C2021-03-23=E2=80=9D. > > Sure, but I haven't seen the former :) > >>> (Though, we are actually interested in the *lack* of a date scheme. >>> If they use a date scheme now, other versions will be disregarded, so >>> we're fine; but if they use versions now and used a date scheme >>> before, the versions will be discarded.) >> >> I am not sure what you are trying to say, could you elaborate? > > Just that the important case is disallowing dates when > 'release-tag-date-scheme? is #f. > > If the tags of a repo are: > > 12.1 > 12.2 > 13.0 > 13.4 > 2018.01.01 > 2018.05.05 > > and we do nothing, the 2018.05.05 tag will be selected. This is correct > if we do want dates, but incorrect if we don't (in which case we would > set 'tag-version-date-scheme? to #f to get the correct result). Ah, that makes sense. :-) >>> Though it would be nice to see when such updates are available, is it >>> worth some bogus results? Are false positives better or false negatives >>> better? >> >> Hmm, good question! If in the future we have some kind of bot that >> automatically runs =E2=80=98guix refresh -u=E2=80=99, builds the updated= package, and >> send a patch to the mailing list, not having false positives might be >> more important. We could also have a =E2=80=98disable-tag-updater?=E2= =80=99 property to >> disable the updater for packages which gives false positive, or maybe >> that will result in to many properties. > > For these packages, it would probably easier to just use the existing > tag- properties. In fact, instead of this or the date-scheme above, > a 'tag-version-regex' would cover both cases. > > In fact, we could replace 'tag-version-delimiter' with > 'tag-version-regex' and instead provide convencience functions such as > (untested): > > (define (version-regex delim) > (let ((delim-rx (regexp-quote delim))) > (string-append "([[:digit:]][^" delim-rx "[:punct:]]*" > "(" delim-rx "[^[:punct:]" delim-rx "]+)" > (if (string=3D? delim-rx "") "*" "+")))) > > (define* (version-date-regex (delim ".")) > (let ((delim-rx (regexp-quote delim))) > (string-append "([0-9]{4}" delim-rx "(0[1-9]|11|12)" > delim-rx "(0[1-9]|[1-2][0-9])"))) > > WDYT? That sounds like a good idea! Where would we put these procedures, (guix packages)? >>> Unless you/we want to pursue one or both of the above changes now, the >>> latest patch LGTM (modulo my nits). >> >> I would prefer to wait a bit with the improvements mentioned above. The >> current patch has been in the works a week or two already, so it=E2=80= =99s >> probably a good idea to get it merged, and try to solve the less >> important issues later. :-) > > Sounds good to me, then! > >>> I discovered that this can segfault unless 'remote-disconnect' and >>> possibly 'repository-close!' are called *after* copying the data out. >>> I've attached a diff for this. >> >> I don=E2=80=99t see a diff attached; maybe you forgot? :-) >> > > I've actually attached it this time :) > >>>> + >>>> +(define (git-package? package) >>>> + "Whether the origin of PACKAGE is a Git repostiory." >>> >>> "Return true if PACKAGE is..." >> >> =E2=80=9CPACKAGE is a Git repository.=E2=80=9D doesn=E2=80=99t really so= und right, maybe =E2=80=9Cif >> PACKAGE is hosted on a Git repository=E2=80=9D?' > > Sorry, yes, that's what I meant, or "Return true if the origin..."; I > was just suggesting making it a full sentence. > >>> I tested this updater on all packages in .scm files starting with f >>> through z, and I found the following packages with possibly bogus >>> updates: >>> >>> --8<---------------cut here---------------start------------->8--- >>> javaxom >> >> I assume you meant =E2=80=98java-xom=E2=80=99 :-) >> >> That=E2=80=99s a weird scheme; setting the delimiter to =E2=80=9C.=E2=80= =9D doesn=E2=80=99t help since >> it thinks that =E2=80=9C127=E2=80=9D is greater than =E2=80=9C1.3.7=E2= =80=9D. > > 'tag-version-regex would allow fixing this ;) > >> >>> luakit >>> ocproxy >>> pitivi >> >> =E2=80=98pitivi=E2=80=99 has a pretty weird version string to begin with= ; it may be >> better to change it to the date: =E2=80=9C0.999.0-2021-05.0=E2=80=9D -> = =E2=80=9C2021-05.0=E2=80=9D. >> >>> eid-mw >>> libhomfly >>> gnuradio >>> welle-io >> >> Setting the delimiter to "." fixes the issue. >> >>> racket-minimal >> >> Setting the prefix to "v" fixes this. >> >>> milkytracker >>> cl-portal >>> kodi-cli >>> openjdk >>> java-bouncycastle >>> hurd >>> opencsg >> >> Setting the suffix to "-release" fixes this. >> >>> povray >>> gpsbabel >> >> Setting the prefix to "gpsbabel_" fixes this. >> >>> go >>> stepmania >>> ocaml-mcl >>> >>> many minetest packages (minetest will have its own updater, though) >>> >>> ocaml4.07-core-kernel, ocamlbuild and many other ocaml packages >>> (they seem to be covered by the github updater) >>> --8<---------------cut here---------------end--------------->8--- > > I'm glad to see that these are easily fixed with the properties, though! > That's some good validation. Yeah, it=E2=80=99s looking pretty good. :-) > Now I just have to give the (guix upstream) some attention... > > -- > Sarah > > diff --git a/guix/git.scm b/guix/git.scm > index dc3d3afd02..bbff4fc890 100644 > --- a/guix/git.scm > +++ b/guix/git.scm > @@ -593,6 +593,11 @@ is true, limit to only refs/tags." > (and (ref? ref) > (or (not tags?) (tag? ref)))) >=20=20 > + (define (remote-head->ref remote) > + (let ((name (remote-head-name remote))) > + (and (include? name) > + name))) > + > (with-libgit2 > (call-with-temporary-directory > (lambda (cache-directory) > @@ -600,14 +605,13 @@ is true, limit to only refs/tags." > ;; Create an in-memory remote so we don't touch disk. > (remote (remote-create-anonymous repository url))) > (remote-connect remote) > - (remote-disconnect remote) > - (repository-close! repository) > - > - (filter-map (lambda (remote) > - (let ((name (remote-head-name remote))) > - (and (include? name) > - name))) > - (remote-ls remote))))))) > + > + (let* ((remote-heads (remote-ls remote)) > + (refs (filter-map remote-head->ref remote-heads))) > + ;; Wait until we're finished with the repository before closin= g it. > + (remote-disconnect remote) > + (repository-close! repository) > + refs)))))) >=20=20 > > ;;; --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQJJBAEBCAAzFiEEAVhh4yyK5+SEykIzrPUJmaL7XHkFAmFESGEVHHB1YmxpY0B5 b2N0b2NlbGwueHl6AAoJEKz1CZmi+1x5PYkP/RWAH42qmxWT38TjfWw7DG3v7WXY gqpFQEvdnW7AFtjB6xBviU11NFF/20zuGX8z6SSibydOoj3lcuxW2WunOcoMOQuw n1yDrpQ/8EkSOkqIMqt+5zBGEocPyjwu+ggdqFuttPfSmXyRQp831woY0lGYalQc MtP6jqFdYlliShONpLaTrMOT8l8md1FpJXX8PVZUukFHgwHFhrLZLlEVCggEsZAk yxkjzIX4Hs/TMT/MY0b46/R2p9ocWHO8jcOlHhxerehRHt3IWB1SHn48ziVHXDou 166/uwJRGWPsAYn0en0UdEv9bPgOCABE+bGDBd4G41UFEYB9Ags3MW2/OoEAGmzC cWW5EhVXQFBx+EKOrYiHUuK1khh4ZMrM7Igb0tvUi7oce38exk9LM3GSWldEUKmG 0Bkhfmc6Glb6ytKiBY2f1RkhSjQ1qZt+BUjT/bCf/9JprXexPwgdEaZJ72qgQnSC QrJd90D76X3/P+JbXB4wgtvnJKhYQI4Jcf4rDIS5b4sxNrjhVd7H8P30rB0g8IYB c29b5DwlmC/GnZi/gGWpgMatFFA443MW6JF4huZOzUaTopaqF2keXXj9YU5LmPI0 CeNMFM7fKC/Asvp2QDNTUFJJM81YPRL2tG6JuAi0rTooXNo5dQxT22kf/mQPdA3D MsuVbuwHe8ZJ5wvL =5SH8 -----END PGP SIGNATURE----- --=-=-=--