From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp11.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id MMXqOJtpaGIPLwEAbAwnHQ (envelope-from ) for ; Tue, 26 Apr 2022 23:52:27 +0200 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp11.migadu.com with LMTPS id 2GYKOZtpaGJu4QAA9RJhRA (envelope-from ) for ; Tue, 26 Apr 2022 23:52:27 +0200 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 77BD83A98D for ; Tue, 26 Apr 2022 23:52:27 +0200 (CEST) Received: from localhost ([::1]:49160 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1njT6g-0002JR-Bl for larch@yhetil.org; Tue, 26 Apr 2022 17:52:26 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:60880) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1njT6d-0002JD-JE for gwl-devel@gnu.org; Tue, 26 Apr 2022 17:52:23 -0400 Received: from smtp.polymtl.ca ([132.207.4.11]:39854) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1njT6a-0005uO-DF for gwl-devel@gnu.org; Tue, 26 Apr 2022 17:52:22 -0400 Received: from localhost (modemcable094.169-200-24.mc.videotron.ca [24.200.169.94]) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 23QLqCgf006699 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 26 Apr 2022 17:52:17 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 23QLqCgf006699 To: Ricardo Wurmus Cc: gwl-devel@gnu.org Subject: Re: [PATCH v2 1/2] packages: Support for full Guix specification In-Reply-To: <875ymv5z0l.fsf@elephly.net> References: <20220421195158.22407-1-olivier.dion@polymtl.ca> <20220422184359.7929-1-olivier.dion@polymtl.ca> <20220422184359.7929-2-olivier.dion@polymtl.ca> <87ilqv66yd.fsf@elephly.net> <875ymvhds4.fsf@laura> <875ymv5z0l.fsf@elephly.net> Date: Tue, 26 Apr 2022 17:52:12 -0400 Message-ID: <8735hzh5rn.fsf@laura> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Poly-FromMTA: (modemcable094.169-200-24.mc.videotron.ca [24.200.169.94]) at Tue, 26 Apr 2022 21:52:12 +0000 Received-SPF: pass client-ip=132.207.4.11; envelope-from=olivier.dion@polymtl.ca; helo=smtp.polymtl.ca X-Spam_score_int: -41 X-Spam_score: -4.2 X-Spam_bar: ---- X-Spam_report: (-4.2 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: gwl-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gwl-devel-bounces+larch=yhetil.org@gnu.org Sender: "gwl-devel" Reply-to: Olivier Dion From: Olivier Dion via X-Migadu-Flow: FLOW_IN X-Migadu-To: larch@yhetil.org X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1651009947; h=from:from:sender:sender:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=GkiaHt8jiAyjUaQFQW23wec1CFRL7WcAr31fMScxMZE=; b=bAon53HIZJ98f18RJdp5ADNomtF+LnlCTv0Iz0K1+uZzic26ndAzIaaEX6fseqS1bdUCSS b9YqZEl1wgF9Y8X2dme40HWNoraARHiMfZr5OYH2tu/msBHSeewh0EBu+3T1Urorq4FMas j7HA0Mj6B7/WB/sChqgjY2MZRzw4QogJpfd1v/VgeS4ST4qQuaWATI/hD9mtyceA2CsP/i pKveRMurjqEgXwACxmij43k1CG7SBUkXCpO1BQo8jAVapk9bVD+235DeeueNbCSrMFHyED gaYptSXqOIhP2pcQ+DJnPb8ppKnprjqNSEkrKc1kRKRrFglMCVIM8Dn+q32I0w== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1651009947; a=rsa-sha256; cv=none; b=OlztFdrxItl92x64kti+4yBpUUlg64UzQUWjaPaC32rVs2+vD8MIe9AyYC0UH528xSAG1c +olaFPgdRHbHtTwQ/nFMWoaGz3QqU7B5/4GtRj6NGWViSosWSR9u9hGMTx/yJH0o6L0sL8 z1s0Qq3Moa25mf3PkSEjxaD968sluhKyKF+AV/5XxQ0tRvyTHkbUAk9/RhI1Z60Cwvvbpr Oj2z53WpyfvnYsM8tE/wbGmRgizdOYE03wnZmQVM3QXSsy22TSkw8R6X2pjc1DKjz+LzaM 0s93EGMO0AHTe/OQjrauLlOHUvQ4q/tVXa5Ig//9jT43LlycGXMX9a5sjwG1yQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of "gwl-devel-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="gwl-devel-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: -3.00 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=pass (policy=none) header.from=gnu.org; spf=pass (aspmx1.migadu.com: domain of "gwl-devel-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="gwl-devel-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: 77BD83A98D X-Spam-Score: -3.00 X-Migadu-Scanner: scn1.migadu.com X-TUID: +XRXkIFdWkqS On Tue, 26 Apr 2022, Ricardo Wurmus wrote: > PACKAGES->MANIFEST=20 > So that=E2=80=99s why you=E2=80=99re making it return a tuple of package/= string tuples =E2=80=93 > for compatibility with that procedure. Yes that's indeed what I had in mind. >> I do think it would be better to wait for (guix inferior) to support >> selecting outputs. However, I do need selection of outputs for my >> use case right now! Specificaly, I need to have debug symbols of >> many packages. The quick hack above does the work for me but I >> understand that it would be preferable if (guix inferior) has support >> for outputs instead. > > I understand. > > So =E2=80=A6 I think we can figure something out that won=E2=80=99t be fa= r removed from > what you proposed. I=E2=80=99d probably split it into smaller procedures, > though, to make it a bit more obvious what=E2=80=99s going on. > > Let=E2=80=99s see the diff again=E2=80=A6 > >> -(define (lookup-package specification) >> +(define (%lookup-package name+version output) >> + (list (match (apply lookup-inferior-packages >> + (cons (current-guix) (string-split name+version #= \@))) >> + ((first . rest) first) >> + (_ (raise (condition >> + (&gwl-package-error >> + (package-spec (string-append name+version output)= )))))) >> + output)) >> >> +(define* (lookup-package specification #:optional (output "out")) >> (log-event 'guix (G_ "Looking up package `~a'~%") specification) >> - (match (lookup-inferior-packages (current-guix) specification) >> - ((first . rest) first) >> - (_ (raise (condition >> - (&gwl-package-error >> - (package-spec specification))))))) >> + (match (string-split specification #\:) >> + ((name+version sub-drv) (%lookup-package name+version sub-drv)) >> + ((name+version) (simple-package (%lookup-package name+version outpu= t))))) > > I=E2=80=99m struggling to figure out a cleaner way to do this=E2=80=A6 > Why are we processing the specification *and* accept an optional OUTPUT > argument? It seems to me that SUB-DRV and OUTPUT *should* be the same, > but it=E2=80=99s possible to call LOOKUP-PACKAGE in a way that they diffe= r, > which doesn=E2=80=99t make much sense to me. Hmm you're right. I don't know why I've put this here. I think I got confused with how one can do e.g. `(,git "send-email") instead of "git:send-email". One use the package object followed by the output, the other is a full speficication. So really what we want here is no optional output. > Another thing that bothers me a bit is all that string splitting; once > for version, again for the output. The (guix ui) module has > PACKAGE-SPECIFICATION->NAME+VERSION+OUTPUT, which is dedicated for this > task. It returns multiple values; let=E2=80=99s use LET* from SRFI-71. = What do > you think of this? Yes. If there's a standard function in Guix, then it's better to use it. Less burden on ourself. > --8<---------------cut here---------------start------------->8--- > (import (srfi srfi-71) > (define (lookup-package specification) > "Look up SPECIFICATION in an inferior and return a matching package. I= f the > specification declares a specific output return a tuple consisting of the > package value and the output. If no matching package is found, raise a > &GWL-PACKAGE-ERROR." > (log-event 'guix (G_ "Looking up package `~a'~%") specification) > (let* ((name version output (package-specification->name+version+outpu= t specification)) > (package > (match (lookup-inferior-packages (current-guix) name version) > ((first . rest) first) > (_ (raise (condition > (&gwl-package-error > (package-spec specification)))))))) > (if output > (list package output) > package))) > --8<---------------cut here---------------end--------------->8--- > What do you think of that? Looks good. However, output is already "out" if no output is provided. So the alternative branch is never took here. I would change it to: --8<---------------cut here---------------start------------->8--- (match output ("out" package) (specific (list package specific))) --8<---------------cut here---------------end--------------->8--- >> (define (valid-package? val) >> - (or (package? val) >> - (inferior-package? val))) >> + (or >> + (and (list? val) >> + (valid-package? (car val)) >> + (string? (cadr val))) >> + (package? val) >> + (inferior-package? val))) >> + > > I suggest rewriting this whole thing with MATCH so that the structure of > VAL becomes apparent. Perhaps something like this? > > (match > ((maybe-package (? string? output)) > (valid-package? maybe-package)) > (_ > (or (package? val) > (inferior-package? val)))) Yes. Matching patterns are awesome! >> +(define (simple-package pkg) >> + (if (list? pkg) (car pkg) pkg)) > > I still don=E2=80=99t like this :) Not only the implementation but the f= act > that it appears to be needed. At least implementation-wise I=E2=80=99d p= refer > something like this: > > (define (just-package maybe-package+output) > (match maybe-package+output > (((? package? package) (? string? output)) package) > ((? package? package) package) > (_ (error "what is this?")))) Yes match or record's getter are better. > > There are a few places where we need to be careful that we=E2=80=99re dea= ling > with the right type and that we handle both cases equally well: when a > tuple is encountered and when a plain package value is encountered. If we need to deal in different way depending on the type, wouldn't generics help here? If not, we can use a record. Then, it's just a matter of using a getter for the underlying package without the output. Ideally, we have to determined what we use here. Generic, record or matching patterns?=20 > Ideally we=E2=80=99d also have tests for this. For testing, I think you have a lots of unit testing. Which is great on its own. However, I believe that the best way of testing a software is to develop scenarios and run them, just like an user would. We could start by adding a test suite that run all of the examples in the documentation directory and the tutorial. I believe that the latter is even more crucial since this is what new user will try first. If the tutorial does not work for them, chance are they are going to uninstall gwl before trying anything! Finer grained scenarios could be added that way in the future. This also augment the set of examples for users. What do you think? --=20 Olivier Dion oldiob.dev