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 yL75H6bJdGLKGAEAbAwnHQ (envelope-from ) for ; Fri, 06 May 2022 09:09:26 +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 YEQsIKbJdGINhQEA9RJhRA (envelope-from ) for ; Fri, 06 May 2022 09:09:26 +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 1B7672313B for ; Fri, 6 May 2022 09:09:26 +0200 (CEST) Received: from localhost ([::1]:45386 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nms5c-0007Yu-Aa for larch@yhetil.org; Fri, 06 May 2022 03:09:24 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:40892) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nms5H-0007Wn-JR for guix-patches@gnu.org; Fri, 06 May 2022 03:09:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:53047) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nms5G-00068P-DH for guix-patches@gnu.org; Fri, 06 May 2022 03:09:03 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1nms5G-0000ZY-6i for guix-patches@gnu.org; Fri, 06 May 2022 03:09:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#55248] [PATCH 7/7] gnu: chez-scheme-for-system: Adjust support logic. Resent-From: Liliana Marie Prikler Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Fri, 06 May 2022 07:09: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: Philip McGrath , 55248@debbugs.gnu.org, Maxime Devos Received: via spool by 55248-submit@debbugs.gnu.org id=B55248.16518209352187 (code B ref 55248); Fri, 06 May 2022 07:09:02 +0000 Received: (at 55248) by debbugs.gnu.org; 6 May 2022 07:08:55 +0000 Received: from localhost ([127.0.0.1]:46944 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nms58-0000ZC-P4 for submit@debbugs.gnu.org; Fri, 06 May 2022 03:08:55 -0400 Received: from mailrelay.tugraz.at ([129.27.2.202]:19055) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nms56-0000Z1-CQ for 55248@debbugs.gnu.org; Fri, 06 May 2022 03:08:53 -0400 Received: from lprikler-laptop.ist.intra (gw.ist.tugraz.at [129.27.202.101]) by mailrelay.tugraz.at (Postfix) with ESMTPSA id 4KvhW46kQhz3xSm; Fri, 6 May 2022 09:08:48 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tugraz.at; s=mailrelay; t=1651820929; bh=ibq6NAdgZSIxIE50bu1wX7gSXoXGSExmcNF/79kGp5k=; h=Subject:From:To:Date:In-Reply-To:References; b=kY7tDNK0T0ZzlCOAi0BMunGPzowXIDmrNjd5kQceIqtF43hkaiXO3rTvGG9YwB3fB 56YNI7NInDab1FRwWSSzEdEmbKRGaSXTStqKA+lbD2pHT5Py/EvTcTLr54DEyX0r2A M2r5xdnw7bovRwqNQRcNajTrQJPyYtCqDWCYjgWM= Message-ID: <5919abe72ececaa4464a52869ecee7397e93b372.camel@ist.tugraz.at> From: Liliana Marie Prikler Date: Fri, 06 May 2022 09:08:48 +0200 In-Reply-To: <770e2e49-e27c-dbfb-ee03-e2886df94f42@philipmcgrath.com> References: <9ba89b23f86a163679047f113e3524b4352df55d.1651594312.git.philip@philipmcgrath.com> <1dc114e6acd0320c553837e1d9f5d94e4e8c800d.camel@ist.tugraz.at> <770e2e49-e27c-dbfb-ee03-e2886df94f42@philipmcgrath.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.42.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-TUG-Backscatter-control: waObeELIUl4ypBWmcn/8wQ X-Scanned-By: MIMEDefang 2.74 on 129.27.10.116 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=1651820966; 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=ibq6NAdgZSIxIE50bu1wX7gSXoXGSExmcNF/79kGp5k=; b=CsBmn7n3qtqpDp/Xzjya5Hg2uYbWQn/8UjTmonD9HwmfwJ6rwSsvJXPJdDnRJzukKAwRVt m5s9R0gg5Yllw1ZyxKgVz23zYMqzLGDJYTKuYP3eOQNpz2hlwAHgue6n9hhmKgi3J3WcGJ JLOCyYHu5PfkcU+rj6n5Uk6+Kng8/de9EO+0EdcmMHJ0Fj/nhsKGj0jYOAaYoewo6cBHuv UDcwnEY3G4RYNMVs1re8jGZKLLJEdNUbOoWS7H/IS+VBpBtAG5wxy0XvE2BnLh/blgd9aF 19z03nS8TGxuuN7Y5lvD7WCWGbEcguZ9rO1ygYrvgbjFZwnXwcWdA7TPwdtWIA== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1651820966; a=rsa-sha256; cv=none; b=W2Obs8w1fvP+Gz3W5kEhjYjpJXbTE9zhL9nf4WVFuIMzArGAmUKPPCjtDg6AnZsQFx/xTR 5Ey5XZvO/JoldwP1xP4du4g1Yyw2huOI5BjdWfsUn2xxq8P8ISuqxHTfQByLPOT6cWrPTM NIB4pBNN+kosB+z41CSmOEUp2ZMNvlXdSMizIyziY5vBYTF7992iuCpZn+Qc8OWv/omgnU SAlj9MkklQhgyRoZ3aZJ51VLNJQHYPbc8K9FUW/2CtqnTBTbIL7mdvI7Lgp5gze073dBks dq8q/PZlYwOgPHoEqqUS+vODi3l6Zvy2xO8huKwpZb7wO7ISWr6+Cjz4X6GkCw== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=tugraz.at header.s=mailrelay header.b=kY7tDNK0; dmarc=fail reason="SPF not aligned (relaxed)" header.from=tugraz.at (policy=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: 5.20 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=tugraz.at header.s=mailrelay header.b=kY7tDNK0; dmarc=fail reason="SPF not aligned (relaxed)" header.from=tugraz.at (policy=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: 1B7672313B X-Spam-Score: 5.20 X-Migadu-Scanner: scn0.migadu.com X-TUID: 1Wm06ZeE0fv5 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. > > > [...] > > > ;; 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? > > > -(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. > > > + > > 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. > > > [...] > > 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 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. > 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?