From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp10.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id YHrLBDDGdmLV9gAAbAwnHQ (envelope-from ) for ; Sat, 07 May 2022 21:19:12 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp10.migadu.com with LMTPS id kG29AzDGdmKmcgAAG6o9tA (envelope-from ) for ; Sat, 07 May 2022 21:19:12 +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 1FED73839E for ; Sat, 7 May 2022 21:19:11 +0200 (CEST) Received: from localhost ([::1]:36478 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nnPxO-0004sg-9P for larch@yhetil.org; Sat, 07 May 2022 15:19:10 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56238) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nnPxH-0004oW-1L for guix-patches@gnu.org; Sat, 07 May 2022 15:19:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:59068) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nnPxG-00034Y-OU for guix-patches@gnu.org; Sat, 07 May 2022 15:19:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1nnPxG-0000nF-I3 for guix-patches@gnu.org; Sat, 07 May 2022 15:19:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#55248] [PATCH 7/7] gnu: chez-scheme-for-system: Adjust support logic. Resent-From: Philip McGrath Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sat, 07 May 2022 19:19:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 55248 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Liliana Marie Prikler , 55248@debbugs.gnu.org, Maxime Devos Received: via spool by 55248-submit@debbugs.gnu.org id=B55248.16519511163015 (code B ref 55248); Sat, 07 May 2022 19:19:02 +0000 Received: (at 55248) by debbugs.gnu.org; 7 May 2022 19:18:36 +0000 Received: from localhost ([127.0.0.1]:52965 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nnPwp-0000mY-BX for submit@debbugs.gnu.org; Sat, 07 May 2022 15:18:36 -0400 Received: from wout4-smtp.messagingengine.com ([64.147.123.20]:35809) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nnPwm-0000mL-Rl for 55248@debbugs.gnu.org; Sat, 07 May 2022 15:18:34 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id 124C5320010B; Sat, 7 May 2022 15:18:25 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Sat, 07 May 2022 15:18:26 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= philipmcgrath.com; h=cc:content-transfer-encoding:content-type :date:date:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to; s=fm1; t=1651951105; x=1652037505; bh=xwiqcTwbHnxk4JNJPf7s15QD+ oYRX1nofVxn0iNXU5U=; b=GWk0T2tHtA3oZszCg+rw+OlMQFPzixthwQHdYWiky 3lfEzjNhCHBGJNr4w8fNbpevApRBEZplDXtDsNLAFI+5/JqeFtWsW3Wt6gBmSXHO 2U2T/Ya/8/XcpNKY7AISSUn5yjlBRMoCuhwrO3r9hBMGCDwfrLWjSUOUbV7u9WXx aVl9gFCKQrxvYZPtaCsuOgZnyLPdR8f10uLucLhK5qnFe2xryBitZiJ14LpvHMhM HuiPrhBCBljBbMxis9mcBT9VFJofQLbNA0i/C5uZVTOEFaBn62f641AhZVDuOzdm vPAMmYwF+sZUCjreZOHpyW/UbBHYhtDNBre1/8ECC9kRA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:date:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1651951105; x=1652037505; bh=xwiqcTwbHnxk4JNJPf7s15QD+oYR X1nofVxn0iNXU5U=; b=xPhnqX8LZua6XZklTDkR1ImtJa0HRwvalCPhVW/X8jN6 Itcwm5PFvkC1Z8rUZsufXcjrwRqfrpsg3VXbrESaKv9bA9+nNL/vBUFRwWji6ddx 4auow4f+ez0/ckvvZGUEe0CNA2kz/f/c5hwq6iRb6ju8m7ylbDN2oaoaL9s9I8GZ QBgpwjiBFGYbgO82krCTV0JCloI7KmSNB9mSguWkki3P8OPuhVOGxRXHS5tcn/8H 5LJdZMsyGc2iQ8Aq97/V0j4s/p3AZlPeq7FRTDs44YU4NXOmTgpe/l4PZ8dIhRaE 7HghijCi6DvzB3TtuQxxEPeuC6WuRTy4vI4mlOcaGw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrfeehgddufeeiucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne goufhushhpvggtthffohhmrghinhculdegledmnecujfgurhepkfffgggfuffvfhfhjggt gfesthekredttdefjeenucfhrhhomheprfhhihhlihhpucfotgfirhgrthhhuceophhhih hlihhpsehphhhilhhiphhmtghgrhgrthhhrdgtohhmqeenucggtffrrghtthgvrhhnpefh gfdtkeeukedtfedvhfefkeefhedvffeutdfffefgtdetudejieefkeduieetueenucffoh hmrghinhepghhnuhdrohhrghdpghhoohhglhgvrdgtohhmnecuvehluhhsthgvrhfuihii vgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhhihhlihhpsehphhhilhhiphhmtg hgrhgrthhhrdgtohhm X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sat, 7 May 2022 15:18:24 -0400 (EDT) Message-ID: <5db08b29-a064-76ed-20a6-d55836071d4c@philipmcgrath.com> Date: Sat, 7 May 2022 15:18:24 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1 Content-Language: en-US References: <9ba89b23f86a163679047f113e3524b4352df55d.1651594312.git.philip@philipmcgrath.com> <1dc114e6acd0320c553837e1d9f5d94e4e8c800d.camel@ist.tugraz.at> <770e2e49-e27c-dbfb-ee03-e2886df94f42@philipmcgrath.com> <5919abe72ececaa4464a52869ecee7397e93b372.camel@ist.tugraz.at> From: Philip McGrath In-Reply-To: <5919abe72ececaa4464a52869ecee7397e93b372.camel@ist.tugraz.at> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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-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=1651951151; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: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=xwiqcTwbHnxk4JNJPf7s15QD+oYRX1nofVxn0iNXU5U=; b=HIn2k/PpQDJaR18XZs23zhXx5PIRYsVNVbafLXMiEtQVHZdRakvJsuGvlGqs2gv7fz0DHv z1A0XvPCIeP8oECdTYimiobVbyCqWUpzgCfFxUTMBOVpTx0KmcbrRcMw5zl1uVbXmq7BRB xRSUD5cNM8Wor9vohl0vs8NRgc3nsfNquaxZ0Qi147ItVYoX0iecWDjN/g7kCBDaqttRKj Kr8Gc/FgmVwG+hgfM7017+cMzkE+aRDeof+0idJT1E7keoeY1PWfIo5Io23drgOSNY+0tJ 9WBUwgxXjxKU+lxhPnLccOS2evQmaQrvmutNsmAQ+R3Nibmng03FkxJtjb/bnQ== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1651951151; a=rsa-sha256; cv=none; b=h1MtCBV/a5H1xWvxiXqDPV245hgnTD3wFXLhW2LIoyUD0oyqBrgjk7DBBhks7zon6iZXBi e8jYCtY4U7u7RxBGYddLrANGT3IX/lH11eJoYQWyN5v0HqYd3XfcpuJj8icj6ZmKuHoRcW Qfaf4OhkKpr6OAms6SJcwG5qd4cczZbnUgpQ9CvMsoiIsRrVjOHnPu5PSn4FhaPQGuO+Nb 7++nMRQa1xIQfI3FN03f82BHQqqHsVNoTvQTFNZSh2pXblxtYEA4psSfquAf3GvXWy72oc M4/EVbSebadoKS1iRYbEs+jAGzH4/r90/7Hu2ZAbKBnr1qMvP6s1qsBytgr3tg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=fm1 header.b=GWk0T2tH; dkim=fail ("headers rsa verify failed") header.d=messagingengine.com header.s=fm1 header.b=xPhnqX8L; dmarc=none; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" X-Migadu-Spam-Score: 2.40 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=philipmcgrath.com header.s=fm1 header.b=GWk0T2tH; dkim=fail ("headers rsa verify failed") header.d=messagingengine.com header.s=fm1 header.b=xPhnqX8L; dmarc=none; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org" X-Migadu-Queue-Id: 1FED73839E X-Spam-Score: 2.40 X-Migadu-Scanner: scn1.migadu.com X-TUID: mmcwo0erwsQN Hi, On 5/6/22 03:08, Liliana Marie Prikler wrote: > Am Donnerstag, dem 05.05.2022 um 16:42 -0400 schrieb Philip McGrath: >> Hi, >> >> On 5/4/22 03:21, Liliana Marie Prikler wrote: >>> Am Dienstag, dem 03.05.2022 um 14:33 -0400 schrieb Philip McGrath: >>> >>>> This is a follow-up to commit >>>> b8fc9169515ef1a6d6037c84e30ad308e5418b6f: >>>> see . Thanks to Liliana Marie >>>> Prikler for pointing out various issues, e.g. that being able to >>>> represent a Nix system as a Chez Scheme machine type does not >>>> necessarily mean the system is supported! >>> The issue in that commit is a different one: nix-system->chez-machine >>> can fail if there's no conversion.  Anyway... >>> >> >> The issue fixed in the commit is different, but this issue hadn't >> occurred to me until you wrote in >> : >> >>> I pushed that definition upstream, but a rewrite is still needed.  I >>> also think this logic should be a little decoupled from the >>> question of whether or not a given nix-system is supported.  While >>> surely this function returning #f means it's not, there are still >>> other questions to consider. > Ahh, in that case the commit message is pointing people to the wrong > location. I think this needs to be communicated more clearly, e.g. > > "This commit is a follow-up to > b8fc9169515ef1a6d6037c84e30ad308e5418b6f. While that commit did fix a > breaking build, this one addresses the assumptions that lead to the > failure, see also ." > > Thereafter go on to describe what's actually done. > Yes, that seems better. (I was trying to focus the summary on user-facing functions, rather than replaced internals, but I think it can be better.) > >>>> [...] >>>> ;; Commentary: >>>> @@ -73,96 +71,17 @@ (define* (chez-scheme-for-system #:optional >>>>                                                (%current- >>>> system)))) >>>>     "Return 'chez-scheme' unless only 'chez-scheme-for-racket' >>>> supports SYSTEM, >>>>   including support for native threads." >>>> -  (if (or >>>> -       ;; full support upstream >>>> -       (and=> (chez-upstream-features-for-system system) >>>> -              (cut memq 'threads <>)) >>>> -       ;; no support anywhere >>>> -       (not (nix-system->chez-machine system))) >>>> +  (if (and=> (chez-upstream-features-for-system system) >>>> +             (lambda (features) >>>> +               (every (cut memq <> features) >>>> +                      '(threads >>>> +                        ;; We can cross-compile for platforms >>>> without >>>> +                        ;; bootstrap bootfiles, but we can't >>>> self- >>>> host >>>> +                        ;; on them short of adding more binary >>>> seeds. >>>> +                        bootstrap-bootfiles)))) >>>>         chez-scheme >>>>         chez-scheme-for-racket)) >>> Does it make sense to require 'threads always? >>> >> >> I guess there are a few notions of "always". >> >> In 'chez-scheme-for-racket', yes, because Racket CS needs thread >> support for "futures" and "places". (Racket BC had a notion of >> platforms where those features were not available, but AFAIK there >> isn't support for a non-threaded configuration of Racket CS.) >> >> For 'chez-scheme', every distribution I'm aware of packages the >> threaded version (only) on platforms where thread support is >> available. The only reason to use the nonthreaded version is if you >> know for sure that your application doesn't use threads---IIRC, that >> may even include any FFI libraries not using threads internally---AND >> the small performance gain from not implementing thread safety >> internally makes a difference. >> >> For 'chez-scheme-for-system', I don't have a strong view, but the >> fact that I think the benefits of thread support are significant >> makes me lean that way. Concretely, the answer to this question only >> affects armhf-linux, so I think we should not change this at least >> until we re-enable it in upstream Chez's 'supported-system'. > In other words, there aren't that many uses of Chez scheme in embedded > spaces, so we might as well always require threads? > At least, I think it's a sufficiently niche use-case that its reasonable for those users to use a package transformation (and check that it works for all of the libraries they use) while allowing general Chez code to assume threads are available, since they are well supported overall. (I have heard stories about embedded Chez, e.g. to control a Disney World virtual reality ride.[1] But I'm not aware of any free software Chez projects that don't work with threads.) >>>> -(define* (nix-system->chez-machine #:optional >>>> -                                   (system (or (%current-target- >>>> system) >>>> -                                               (%current- >>>> system)))) >>>> -  "Return the Chez Scheme machine type corresponding to the Nix >>>> system >>>> -identifier SYSTEM, or @code{#f} if the translation of SYSTEM to >>>> a >>>> Chez Scheme >>>> -machine type is undefined. >>>> - >>>> -It is unspecified whether the resulting string will name a >>>> threaded >>>> or a >>>> -nonthreaded machine type: when the distinction is relevant, use >>>> -@code{chez-machine->nonthreaded} or @code{chez-machine- >>>>> threaded} to >>>> adjust >>>> -the result." >>>> -  (let* ((hyphen (string-index system #\-)) >>>> -         (nix-arch (substring system 0 hyphen)) >>>> -         (nix-os (substring system (+ 1 hyphen))) >>>> -         (chez-arch (assoc-ref %nix-arch-to-chez-alist nix- >>>> arch)) >>>> -         (chez-os (assoc-ref %nix-os-to-chez-alist nix-os))) >>>> -    (and chez-arch chez-os (string-append chez-arch chez-os)))) >>>> - >>> The replacement code should go here for readability imho.  At the >>> very least I was confused why this was first above and now below. >>> >> >> Happy to move things. Specifically, do you want 'target-chez-arch' >> and 'target-chez-os' (and '%chez-features-table'?) before >> 'chez-upstream-features-for-system' and >> 'racket-cs-native-supported-system?'? > This is my personal bias coming from a C background, but I read source > files top to bottom with helper procedures at the top and the main > thing at the bottom. If you look closely, much of Guix also follows > that pattern. For instance, build systems have their phases declared > at the bottom, "guix build" expects the last line to evaluate to a > package, and so on. > My personal preference vacillates between defining helpers before using them and putting high-level or public definitions above internal utilities. >>>> + >>> For the sake of completeness, we might want to still have nix- >>> system- >>>> chez-machine (with a threaded? argument) defined in terms of >>>> target- >>> chez-arch and target-chez-os.  See 6/7 for motivation. >>> >> >> Eventually, I imagine we will want to have a function like >> 'nix-system->chez-machine', but I think it would be better to wait >> until we have a concrete use-case. In particular, what I'd written >> here: >> >>  >> -Note that this function only handles Chez Scheme machine types >> in >>  >> the >>  >> -strictest sense, not other kinds of descriptors sometimes used >> in >>  >> place of a >>  >> -Chez Scheme machine type by Racket, such as @code{\"pb\"}, >>  >> @code{#f}, or >>  >> -@code{\"racket\"}.  (When using such extensions, the Chez Scheme >>  >> machine type >>  >> -for the host system is often still relevant.)" >> >> is no longer necessarily true, thanks to the improvements in the >> "portable bytecode" backends. > In other words, nix-system->chez-scheme would get an extra #:features > argument, which would be a sequence of 'threads and 'portable-bytecode, > no? This question also has relevance w.r.t. 6/7 and potentially a > chez-build-system, where this machine-type could actually be a > discriminating factor. > It may end up being more complex than that, depending on how many of the underlying options we want to expose via Guix. For example, there is "pb" for a fully machine-independent bytecode, but e.g. "tpb64l" for a specialized bytecode for 64-bit little-endian machines with threads. I also don't yet understand when, if ever, we might want to supply a native machine type (if one is defined) in addition to a pb-based machine types. The uncertainty is why I'd rather avoid nix-system->chez-machine until we actually need it. >>> >> [...] >>> All in all, the individual logic of this patch seems fine, but >>> overall it appears as though it's doing three separate things >>> (chez-scheme-for-system, chez features, racket-cs stuff).  IMO it >>> would make sense to split this patch according to those lines. >>> WDYT? >>> >> >> I don't think I'm picturing what you have in mind. >> >> The way I've been thinking of this patch is replacing the Chez >> features and machine type functions based on '%chez-features-table', >> then updating other things accordingly. > Sure, but if we retain nix-system->chez-machine as a function, I think > we can make a cut here and proceed with the second patch thereafter. > I don't think we should retain nix-system->chez-machine beyond this series, but I guess we can delay removing it to split here. >> I guess there is a distinguishable change to the behavior of >> 'chez-scheme-for-system' for systems with no native-code backed. I >> could separate that, if you want. On the other hand, it continues to >> return a package that can't actually be built for the specified >> system, so the change seems mostly theoretical. > This should be the second patch imo. Regardless of theoreticness in > value, I think the change itself is one that deserves its own commit > message. It would also be easier to review and reason about later that > way. > This is the part that makes the most sense to me to put in its own commit. >> In terms of "racket-cs stuff", 'racket-cs-native-supported-system?' >> seemed better than any name I could come up with based on >> 'chez-scheme-for-racket', but the answer is based only on Racket's >> variant of Chez scheme. The old version based on >> 'nix-system->chez-machine' was just wrong (it would falsely claim to >> support e.g. "powerpc-w64-mingw32"), and we didn't have a way to >> implement a correct function until adding the information in >> '%chez-features-table'. > This would be the third patch according to my initial suggestion. That > way, racket-cs-native-supported-system? would remain wrong for patch > 7/9, but be corrected in patch 9/9, which imo would more clearly > communicate that it was previously wrong. > > WDYT? I will try to come up with a v2 more or less along these lines. -Philip [1]: https://groups.google.com/g/comp.lang.scheme/c/Xud6nGrF0Ss/m/BaJDopHMYYAJ