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 ms0.migadu.com with LMTPS id o2FnD7DWQ2HaSwEAgWs5BA (envelope-from ) for ; Fri, 17 Sep 2021 01:43:44 +0200 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 8GtnCrDWQ2ECcgAAbx9fmQ (envelope-from ) for ; Thu, 16 Sep 2021 23:43:44 +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 5DF87ADA5 for ; Fri, 17 Sep 2021 01:43:43 +0200 (CEST) Received: from localhost ([::1]:49242 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mR12c-00035R-Gx for larch@yhetil.org; Thu, 16 Sep 2021 19:43:42 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:53552) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mR11z-00032q-Tr for guix-patches@gnu.org; Thu, 16 Sep 2021 19:43:05 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:46327) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mR11y-0005kY-BI for guix-patches@gnu.org; Thu, 16 Sep 2021 19:43:03 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mR11y-0004h4-92 for guix-patches@gnu.org; Thu, 16 Sep 2021 19:43:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#50359] [PATCH 3/3] import: Add 'generic-git' updater. Resent-From: Sarah Morgensen Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Thu, 16 Sep 2021 23:43: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: Xinglu Chen Cc: Ludovic =?UTF-8?Q?Court=C3=A8s?= , 50359@debbugs.gnu.org Received: via spool by 50359-submit@debbugs.gnu.org id=B50359.163183576818020 (code B ref 50359); Thu, 16 Sep 2021 23:43:02 +0000 Received: (at 50359) by debbugs.gnu.org; 16 Sep 2021 23:42:48 +0000 Received: from localhost ([127.0.0.1]:57873 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mR11j-0004gZ-GR for submit@debbugs.gnu.org; Thu, 16 Sep 2021 19:42:47 -0400 Received: from out2.migadu.com ([188.165.223.204]:34083) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mR11f-0004gP-SU for 50359@debbugs.gnu.org; Thu, 16 Sep 2021 19:42:45 -0400 X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mgsn.dev; s=key1; t=1631835762; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=BS09ZLZPxojuNp4q9Or7B6VHTO2+qDBBXy5mFsL59N0=; b=kni2L2m0ReE2FRvBp97aIE9Wbos+i7+/cRHFHm9TXc3vmkTYYs0IwoUUA2hetSSX3aj8t2 3vfpVygdHsMYFmJpSPMw+bglPhy9OIdxVpGP9Vo6TioT6+go5wPLv+nQfm0la+Jk7e1vQq 18NE+A1cxvgzzgHjX3V/ara84EAmEac= From: Sarah Morgensen References: <5d10dd1e65b0a65ada4a8102310c10de42f53e8d.1631290349.git.public@yoctocell.xyz> <86czp8j38z.fsf@mgsn.dev> <87h7ekr8iw.fsf@yoctocell.xyz> Date: Thu, 16 Sep 2021 16:42:38 -0700 In-Reply-To: <87h7ekr8iw.fsf@yoctocell.xyz> (Xinglu Chen's message of "Thu, 16 Sep 2021 14:48:23 +0200 (10 hours, 1 minute, 2 seconds ago)") Message-ID: <867dfghytt.fsf@mgsn.dev> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" 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=1631835823; 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: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: dkim-signature; bh=BS09ZLZPxojuNp4q9Or7B6VHTO2+qDBBXy5mFsL59N0=; b=SkFvp4Lq8/LLyH/XpxqGWIVCqkTsBxJAxXbJp8cYEpS7FMDcMH4+beBVo58f/9OEL4jJ3H SuZjCe8WxR7Z9xVjHiJVIMYv11jc6qaOfbw3FVREIbq0u3q1JUNycDQ5v7tiZiLJYTYy/b fYOm1R3rs/2yuVKoWDokJk3dB5nB3YZ7U1xMZstv0sMO/f5norwUcRziOXh6vSwdOjBP5H PXp3QVZvWZO39uK2cKHo8jMh4vvf5XMcZbIaAuQ6zlAIUr4B94kwjTOW0HeQIhEzjelUh5 0dORnWLFYblLNWhsZDPB9FZNoHemk/6VOlhiZhCPBf+PNnWGqLQbXLC5wV6upw== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1631835823; a=rsa-sha256; cv=none; b=MSfLRLgXgh2reDmoXKaMfKkbEIpWdda6ByWW8ir5m3HfGuLwBGFPI+w5+c0qVsBJu0YMlI 1787whfa4iA2taCDSRrnKRU5qJH9k4Enc89gYib9tP/Ddkz23qxLB03e/tLcz7OMEnLShL bvDKDHEmUBPW2TFNRHr/iMdvoRXm1py/GULUKR5eUFhKQscN01x99+QETehBkAkaSbRPVW u+NH9RdQcVT+C96UlsD0we8CKfKX3aMzwBINgAXKuxFT/W6201f1lmzMM+Ae+SEOeQPENB s2ZqPczAfk245dhCQGRHFHtgPQsTDJp1jDOPzEA6zVNrzeX8oALcs4vYUfOR4A== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=mgsn.dev header.s=key1 header.b=kni2L2m0; 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: -1.20 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=mgsn.dev header.s=key1 header.b=kni2L2m0; dmarc=fail reason="SPF not aligned (relaxed)" header.from=mgsn.dev (policy=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-Queue-Id: 5DF87ADA5 X-Spam-Score: -1.20 X-Migadu-Scanner: scn0.migadu.com X-TUID: 4m0fpD8vOW4Y --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Xinglu Chen writes: >>> Maybe we could have a =E2=80=98release-tag-date-scheme?=E2=80=99 proper= ty, 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 th= ey 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). >> 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? >> 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 sou= nd 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. Now I just have to give the (guix upstream) some attention... -- Sarah --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=generic-git-fix-segfault.patch Content-Description: Fix undeterministic segfaults in remote-refs. 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)))) + (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 closing it. + (remote-disconnect remote) + (repository-close! repository) + refs)))))) ;;; --=-=-=--