From patchwork Thu Sep 16 23:42:38 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Sarah Morgensen X-Patchwork-Id: 33058 Return-Path: X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id E306027BBE3; Fri, 17 Sep 2021 00:43:42 +0100 (BST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,T_DKIM_INVALID, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTPS id 5D53227BBE1 for ; Fri, 17 Sep 2021 00:43:42 +0100 (BST) Received: from localhost ([::1]:49242 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mR12b-00035R-G1 for patchwork@mira.cbaines.net; Thu, 16 Sep 2021 19:43:41 -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 X-Migadu-Flow: FLOW_OUT X-Migadu-Auth-User: iskarian@mgsn.dev 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+patchwork=mira.cbaines.net@gnu.org Sender: "Guix-patches" X-getmail-retrieved-from-mailbox: Patches Xinglu Chen writes: >>> Maybe we could have a ‘release-tag-date-scheme?’ property, 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’t have to be “.” delimiter though; if they both have the same > delimiter it would be difficult to distinguish a version from a date, > e.g., “1-2-3” vs “2021-03-23”. 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 ‘guix refresh -u’, 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 ‘disable-tag-updater?’ 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=? 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’s > 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’t 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..." > > “PACKAGE is a Git repository.” doesn’t really sound right, maybe “if > PACKAGE is hosted on a Git repository”?' 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 ‘java-xom’ :-) > > That’s a weird scheme; setting the delimiter to “.” doesn’t help since > it thinks that “127” is greater than “1.3.7”. 'tag-version-regex would allow fixing this ;) > >> luakit >> ocproxy >> pitivi > > ‘pitivi’ has a pretty weird version string to begin with; it may be > better to change it to the date: “0.999.0-2021-05.0” -> “2021-05.0”. > >> 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 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)))))) ;;;