From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id YGWcAjlwFmHDKAAAgWs5BA (envelope-from ) for ; Fri, 13 Aug 2021 15:14:33 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id +IX3OThwFmFPYwAAbx9fmQ (envelope-from ) for ; Fri, 13 Aug 2021 13:14:32 +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 899C07E97 for ; Fri, 13 Aug 2021 15:14:31 +0200 (CEST) Received: from localhost ([::1]:50164 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mEX13-00023Y-K5 for larch@yhetil.org; Fri, 13 Aug 2021 09:14:29 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:45602) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mEX0d-00020D-Ho for guix-patches@gnu.org; Fri, 13 Aug 2021 09:14:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:57549) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mEX0b-00086z-O4 for guix-patches@gnu.org; Fri, 13 Aug 2021 09:14:01 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mEX0b-0005P0-Ir for guix-patches@gnu.org; Fri, 13 Aug 2021 09:14:01 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#49958] [PATCH] More flexibility in opam importer Resent-From: Xinglu Chen Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Fri, 13 Aug 2021 13:14:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 49958 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Alice BRENON Cc: 49958@debbugs.gnu.org Received: via spool by 49958-submit@debbugs.gnu.org id=B49958.162886042520737 (code B ref 49958); Fri, 13 Aug 2021 13:14:01 +0000 Received: (at 49958) by debbugs.gnu.org; 13 Aug 2021 13:13:45 +0000 Received: from localhost ([127.0.0.1]:40862 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mEX0G-0005OL-I4 for submit@debbugs.gnu.org; Fri, 13 Aug 2021 09:13:44 -0400 Received: from h87-96-130-155.cust.a3fiber.se ([87.96.130.155]:39160 helo=mail.yoctocell.xyz) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mEX0A-0005O2-Uf for 49958@debbugs.gnu.org; Fri, 13 Aug 2021 09:13:39 -0400 From: Xinglu Chen DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=yoctocell.xyz; s=mail; t=1628860404; bh=yTUN3qsUeH8+oU2eD8kwglWY5vrWlvqft0BjC3fIMdU=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=WHiXyfPr/VF/VzFQCKlU74t0AhlZ3U+GBZiMnHIvL/3PWe4MKOqZ4hyS+i4xMhc3N YYLE3fE9qFsepgU8n+skTRTRT3abcS0Q1YYB+v4P3gwZyDs2ELSlZYzwYx0uJUrrfB 0vzr5V1a62LlCMRVdZiWf7+XJXPvswdUArDCegrA= In-Reply-To: <20210813131109.14c25204@ens-lyon.fr> References: <20210809140407.748fa019@ens-lyon.fr> <20210809171935.05fac773@ens-lyon.fr> <20210810140413.2f7d2f1b@ens-lyon.fr> <20210810184826.17d14aab@ens-lyon.fr> <87pmuhhk1k.fsf@yoctocell.xyz> <20210813131109.14c25204@ens-lyon.fr> Date: Fri, 13 Aug 2021 15:13:14 +0200 Message-ID: <87h7fth4id.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 ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1628860472; 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=vh4LLxzFlzVHxwUe7vplgif2GO1I/TLUleBPLHqBGpI=; b=t03Aj1P+ItTD1QUrsMvnFaBKNfDPGQqg8wtqknSkLsrOyd9Daftc6dADgW0IH1LJDjmLDf +Xdjr04Z8ESAj4Gc0cp2UGvIfZognxr978NsYAJCwj9yDuTy17U/yIONTpTlPtnoJLk70B HnHgoz8/nmQb3nKOZL98ssiEFgwyFCLGIskUQcHfKzeelwDbXiM0nu6tWymLPsrgPo+qPx VlSEBs/nMrnoE86r8pibY1u2mh4erBD6rhlAcnv0FGZ/NtU4sa2JCxPTTeC+B0EkrHmiWi s/erSxWvpxzU9r/tV2Ze/T0QPpU/a8UU81ILUjYP1GctWMen/94bkJaY5FHnBQ== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1628860472; a=rsa-sha256; cv=none; b=IDKQQJgnrnIVGP57kIGb4rRy8P7cxi9woZ3ThcyMP1Yq8cz0fmbDX42sYot+DXQDWZugHV XArQgIhzLtNuxgs88n5LdICeGvxbndfGutmQ4auhnKYAa/euJz/ETphmCfW8egCM8CPcS4 D6D9SDp3P6lE1r5eO9zatlf6yFjHvBmxhxbdL2z+dtfcN9E90H9GfRM4GLn9rVOKhq2GPs K4d9klVcZPLGW9MNLPOuB1d0fO+Dw5c4x3H4fURk2Zwgc4latG1/W5tPtT1IvJb4M5iG91 SVTbdHtsDhL4ABA3Id4OGGvR9AXQFGeoWKgGvrichW1kBlDooG/Udny4BWi/5A== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=yoctocell.xyz header.s=mail header.b=WHiXyfPr; dmarc=fail reason="SPF not aligned (relaxed)" header.from=yoctocell.xyz (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-Spam-Score: -3.41 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=yoctocell.xyz header.s=mail header.b=WHiXyfPr; dmarc=fail reason="SPF not aligned (relaxed)" header.from=yoctocell.xyz (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: 899C07E97 X-Spam-Score: -3.41 X-Migadu-Scanner: scn1.migadu.com X-TUID: /448AEW1mbxy --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Fri, Aug 13 2021, Alice BRENON wrote: > Le Fri, 13 Aug 2021 09:37:43 +0200, > Xinglu Chen a =C3=A9crit : > >> On Tue, Aug 10 2021, Alice BRENON wrote: >>=20 >> [=E2=80=A6] >> > include: + >> > +By default, packages are searched in the official OPAM repository. >> > This +option, which can be used more than once, lets you add other >> > +repositories where to look for packages.=20=20 >>=20 >> =E2=80=9Clets you add other repositories where to look for package=E2=80= =9D sounds a >> bit weird, maybe >>=20 >> lets you add other repositories which will be used to lookup >> packages. >>=20 >> ? > > Ok, as discussed on IRC, trying "lets you add other repositories which > will be searched for packages". > > >> What happens if I specify an additional repository, and a package >> exists in that repository _and_ the default opam repository? From >> which repository will the package be imported from? > > That is the beauty of it: the repositories are assumed to be passed by > order of preference, defaulting to the official opam repositories only > if packages haven't been found anywhere else. Writing this makes me > realize that indeed, starting with --repo=3Dopam isn't entirely > redundant: it could be used to prevent an otherwise interesting repo > from overriding stuff if opam already provides it (let's assume some > "super-opam" with a couple additional packages, and custom versions of > existing opam packages). > > Calling `--repo=3Dsuper-opam` would use the super-opam versions as soon > as a package exists in super-opam, while `--repo=3Dopam > --repo=3Dsuper-opam` would take the super-opam versions only when none > exist in opam. > > A much simpler use-case would be to locally override only some > packages in a repo, and pass --repo=3Doverriden-repo --repo=3Dnormal-repo. > > This behaviour relies on the implementation of opam-fetch and how folds > work in guile. > > Since in the importer script options are stacked as they are retrieved > from the CLI arguments, and repositories are then just filter-maped from > that list, they end up in a list by reverse order of preference. In > opam->guix-package, 'opam gets push on the top if it's not already > there somewhere. So what we get as input of opam-fetch is a list of > repositories-specs by reverse order of preference. Now fold applies the > accumulator to each item in order, so, last elements has the final say, > i.e. the last elements which yield results in find-latests are > preferred over the earlier elements of the list. This works for the > same reason why `(lambda (l) (fold cons '() l)` will reverse its input > list. It's slightly inefficient because it means all repositories are > searched, in reverse order of preference, but I haven't figured how to > get a lazy fold in guile. Granted, I could have written the recursion > explicitly to get that. Will fix if performance matters. > > Also, versions are not compared between repositories, as soon as a repo > provides one version of a given package, the latest of all the versions > this one provides is used in the output, no matter the contents of > other repositories. This is useful to allow "downgrades" by masking > parts of repository which have too recent versions. > > So, thanks for your remark, the documentation deserved a clearer > explanation of it. Thanks for the explanation! And great that you also documented this to avoid ambiguity. >> > -(define (latest-version versions) >> > - "Find the most recent version from a list of versions." >> > - (fold (lambda (a b) (if (version>? a b) a b)) (car versions) >> > versions)) +(define (get-version-and-file path) >> > + "Analyse a candidate path and return an list containing >> > information for proper >> > + version comparison as well as the source path for metadata." >> > + (and-let* ((metadata-file (string-append path "/opam")) >> > + (filename (basename path)) >> > + (version (string-join (cdr (string-split filename >> > #\.)) "."))) >> > + (and (file-exists? metadata-file) >> > + (eq? 'regular (stat:type (stat metadata-file))) >> > + (if (string-prefix? "v" version) >> > + `(V ,(substring version 1) ,metadata-file) >> > + `(digits ,version ,metadata-file)))))=20=20 >>=20 >> What happens if some other prefix is used, e.g., =E2=80=9Crelease-=E2=80= =9D or =E2=80=9CV-=E2=80=9D? >>=20 > > It would get marked as a 'digit. In a previous draft before I started > sending this series of patches, it was called 'R, standing for > "regular", then I thought it was not very meaningful, and, since the > versions were to my knowledge supposed to be either v[0-9]+(\.[0-9]+)* > or [0-9]+(\.[0-9]+)*, I thought I could call that default case "digits" > to clearly indicate what I was trying to refer to. I could change it to > 'other if it matters too much, but the important thing here is that we > distinguish between v-prefixed (the so-called "janestreet > re-versionning" mentioned inside the implementation of find-latest on > current d87d6d6 master) and other versions because =E2=AC=87=EF=B8=8F Oh, OK, I wasn=E2=80=99t aware of this =E2=80=9Cjanestreet re-versionning= =E2=80=9D thing, so only janestreet package have the =E2=80=9Cv=E2=80=9D prefix, right? That e= xplains why versions prefixed with =E2=80=9Cv=E2=80=9D are always greater than those no= t prefixed with anything (in =E2=80=98keep-max-version=E2=80=99). >> Also, why not just return the version number and the metadata file; we >> don=E2=80=99t really care about the prefix do we? >>=20 > > yes we do ! the former latest-version finder handled strings, and > dropped this prefix or put it back on the fly, but the logic > implemented was: if there are v-prefixed versions, find the greatest of > them, without the initial "v", if there aren't, just find the greatest > of all versions. This implies that v-prefixed versions are considered > more important and automatically greater than non-prefixed versions, no > matter what the numbers, which is why this information must be kept. Ah, understood. > I'm just playing ADTs in guile here, "parsing" the version string only > once to retain a symbolic representation of it that will at first > glance allow to identify the type of version used and access the > relevant digits for comparison. The comparison is used right after: > >> > +(define (keep-max-version a b) >> > + "Version comparison on the lists returned by the previous >> > function taking the >> > + janestreet re-versioning into account (v-prefixed come first)." >> > + (match (cons a b) >> > + ((('V va _) . ('V vb _)) (if (version>? va vb) a b)) >> > + ((('V _ _) . _) a) >> > + ((_ . ('V _ _)) b) >> > + ((('digits va _) . ('digits vb _)) (if (version>? va vb) a >> > b))))=20 > > and used in the reduce in find-latest-version. So keeping this 'V is > what will help janestreet-re-versionned packages "skip the line" by > being automatically greater than any non v-prefixed package (thus, > v0.0.1 is greater than 13.2, which is the current behaviour). > >> > (define (find-latest-version package repository) >> > "Get the latest version of a package as described in the given >> > repository." >> > - (let* ((dir (string-append repository "/packages/" package)) >> > - (versions (scandir dir (lambda (name) (not >> > (string-prefix? "." name)))))) >> > - (if versions >> > - (let ((versions (map >> > - (lambda (dir) >> > - (string-join (cdr (string-split dir >> > #\.)) ".")) >> > - versions))) >> > - ;; Workaround for janestreet re-versionning >> > - (let ((v-versions (filter (lambda (version) >> > (string-prefix? "v" version)) versions))) >> > - (if (null? v-versions) >> > - (latest-version versions) >> > - (string-append "v" (latest-version (map (lambda >> > (version) (substring version 1)) v-versions)))))) >> > - (begin >> > - (format #t (G_ "Package not found in opam repository: >> > ~a~%") package) >> > - #f)))) >> > + (let ((packages (string-append repository "/packages")) >> > + (filter (make-regexp (string-append "^" package "\\.")))) >> > + (reduce keep-max-version #f >> > + (filter-map >> > + get-version-and-file >> > + (find-files packages filter #:directories? #t))))) >> >=20=20 >> [=E2=80=A6] >> > + (filter-map get-opam-repository repositories-specs)) >> > + (leave (G_ "Package '~a' not found~%") name)))=20=20 >>=20 >> Nit: I wouldn=E2=80=99t capitalize =E2=80=9Cpackage=E2=80=9D, otherwise = the error message >> looks like this >>=20 >> guix import: error: Package 'equations' not found > > a very neat tip, thank you ! : ) You are welcome, and thank you for working on this! > From cde8b2a5d88d89bfea31c86d3ae94d37c1d3c83f Mon Sep 17 00:00:00 2001 > From: Alice BRENON > Date: Sat, 7 Aug 2021 19:50:10 +0200 > Subject: [PATCH] guix: opam: More flexibility in the importer. > > * guix/scripts/import/opam.scm: pass all instances of --repo as a list > to the importer. Nit: The word after the =E2=80=9C:=E2=80=9D is usually capitalized, so =E2= =80=9CPass=E2=80=9D instead of =E2=80=9Cpass=E2=80=9D in this case. Sorry for not noticing this earlier; = the person committing the patch can probably fixup the commit message, so no need to send a reroll just for this small fix. :-) > * guix/import/opam.scm (opam-fetch): stop expecting "expanded" > repositories and call get-opam-repository instead to keep values > "symbolic" as long as possible and factorize. > (get-opam-repository): use the same repository source as CLI opam does > (i.e. HTTP-served index.tar.gz instead of git repositories). > (find-latest-version): be more flexible on the repositories structure > instead of expecting packages/PACKAGE-NAME/PACKAGE-NAME.VERSION/. > * tests/opam.scm: update the call to opam->guix-package since repo is > now expected to be a list and remove the mocked get-opam-repository > deprecated by the support for local folders by the actual > implementation. > * doc/guix.texi: document the new semantics and valid arguments for the > --repo option. > --- > doc/guix.texi | 30 +++++-- > guix/import/opam.scm | 158 +++++++++++++++++++++-------------- > guix/scripts/import/opam.scm | 8 +- > tests/opam.scm | 68 ++++++++------- > 4 files changed, 160 insertions(+), 104 deletions(-) > > diff --git a/doc/guix.texi b/doc/guix.texi > index 78c1c09858..2d36561186 100644 > --- a/doc/guix.texi > +++ b/doc/guix.texi > @@ -94,6 +94,7 @@ Copyright @copyright{} 2021 Xinglu Chen@* > Copyright @copyright{} 2021 Raghav Gururajan@* > Copyright @copyright{} 2021 Domagoj Stolfa@* > Copyright @copyright{} 2021 Hui Lu@* > +Copyright @copyright{} 2021 Alice Brenon@* >=20=20 > Permission is granted to copy, distribute and/or modify this document > under the terms of the GNU Free Documentation License, Version 1.3 or > @@ -11612,14 +11613,31 @@ Traverse the dependency graph of the given upst= ream package recursively > and generate package expressions for all those packages that are not yet > in Guix. > @item --repo > -Select the given repository (a repository name). Possible values includ= e: > + > +By default, packages are searched in the official OPAM repository. This > +option, which can be used more than once, lets you add other > +repositories which will be searched for packages. It accepts as valid > +arguments: > + > @itemize > -@item @code{opam}, the default opam repository, > -@item @code{coq} or @code{coq-released}, the stable repository for coq p= ackages, > -@item @code{coq-core-dev}, the repository that contains development vers= ions of coq, > -@item @code{coq-extra-dev}, the repository that contains development ver= sions > - of coq packages. > +@item the name of a known repository - can be one of @code{opam}, > + @code{coq} (equivalent to @code{coq-released}), > + @code{coq-core-dev}, @code{coq-extra-dev} or @code{grew}. > +@item the URL of a repository as expected by the @code{opam repository > + add} command (for instance, the URL equivalent of the above > + @code{opam} name would be @uref{https://opam.ocaml.org}). > +@item the path to a local copy of a repository (a directory containing a > + @file{packages/} sub-directory). > @end itemize > + > +Repositories must be passed to this option by order of preference and do > +not replace the default @code{opam} which is always failed-back to. I suggest Repositories should be passed to this option by the order of preference. The additional repositories will not replace the default @code{opam} repository, which is always kept as a fallback. WDYT? > +Also, please note that versions are not compared accross repositories. > +The first repository (from left to right) that has at least one version > +of a given package will prevail over any others and the version imported ^ Missing comma. The rest looks good! :-) --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQJJBAEBCAAzFiEEAVhh4yyK5+SEykIzrPUJmaL7XHkFAmEWb+oVHHB1YmxpY0B5 b2N0b2NlbGwueHl6AAoJEKz1CZmi+1x5+lAP/3SaIU3lETZzT0Uud0i+ai/tqJ8q 6hZqXaa1HY652u9i/a9IYxUDdMRKX62b1WPrOLmSYw+QOhPZ7ob81iNlPTqxFnXT wXQYSr7zth/sMccZtoT3Zjkv79PWXkS0V4d8S/0dYvwfdI2tClL/qhoig0v2rSGL tYVL8qJRA4ZTcZhV3X4ICB5NnbublCZu5TlD0T5CxdqPU73ssOwU9bA1t3zQTuz4 TDjlIEEdDjdv2SCrHDHbnZpgnue2qaH2PMls+jz3gwZk4WNTHXGQiI3ikYRVMIhG XJjdIqG97e1lNpPGutz7szg3vz66yOlvOFiqzdK3H+GVsWHfmLV0ryWKcaYN8J2a TfglQJ5pu+Xcf92Iujeqm3vUdRvVCTZzPNYnISxewtG7jMBu/wsE5ly0Zncf0Jea DL8pLSok7Uq3CcRGxZ8JDxwPcCYCy6Tw9CHvLdaMddQtbXnlq1C85jswfXY6S1dY bW3tZdOCHNQnUU+JegVpK9zntOsZaPQwv6B/3FMkhS23o/NGMorz8LIDwQExPIji fMTDImb1Syff2Keky6BmLOQZjMQvBgwQigRf3dUGjjwhQY1ade+/gVHn7vc2wJOm mPD1lxu6o4qTcKvmpLwZn/evEXlnOxGb9A9R6vpkO13mp4uCVUdUQDI0vm+hgVR6 2SZ93oNIrGwzMNNI =i9iN -----END PGP SIGNATURE----- --=-=-=--