Message ID | f19329dc2f36a10f3fdc9985887cb2cf66dce094.1651594312.git.philip@philipmcgrath.com |
---|---|
State | New |
Headers |
Return-Path: <guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org> X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id 4E70B27BBEA; Tue, 3 May 2022 19:36:27 +0100 (BST) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-3.7 required=5.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=3.4.6 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTPS id E710F27BBE9 for <patchwork@mira.cbaines.net>; Tue, 3 May 2022 19:36:26 +0100 (BST) Received: from localhost ([::1]:36520 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from <guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org>) id 1nlxNp-0000fs-QO for patchwork@mira.cbaines.net; Tue, 03 May 2022 14:36:26 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49310) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1nlxMU-00009w-MJ for guix-patches@gnu.org; Tue, 03 May 2022 14:35:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:47160) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1nlxMU-00071V-Cd for guix-patches@gnu.org; Tue, 03 May 2022 14:35:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1nlxMU-0002nF-8g for guix-patches@gnu.org; Tue, 03 May 2022 14:35:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#55248] [PATCH 2/7] gnu: racket: Fix out-of-source build. Resent-From: Philip McGrath <philip@philipmcgrath.com> Original-Sender: "Debbugs-submit" <debbugs-submit-bounces@debbugs.gnu.org> Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 03 May 2022 18:35:02 +0000 Resent-Message-ID: <handler.55248.B55248.165160288110639@debbugs.gnu.org> Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 55248 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 55248@debbugs.gnu.org Cc: Philip McGrath <philip@philipmcgrath.com> Received: via spool by 55248-submit@debbugs.gnu.org id=B55248.165160288110639 (code B ref 55248); Tue, 03 May 2022 18:35:02 +0000 Received: (at 55248) by debbugs.gnu.org; 3 May 2022 18:34:41 +0000 Received: from localhost ([127.0.0.1]:41042 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <debbugs-submit-bounces@debbugs.gnu.org>) id 1nlxM9-0002lQ-5c for submit@debbugs.gnu.org; Tue, 03 May 2022 14:34:41 -0400 Received: from mail-vs1-f42.google.com ([209.85.217.42]:36607) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <philip@philipmcgrath.com>) id 1nlxM1-0002ko-T9 for 55248@debbugs.gnu.org; Tue, 03 May 2022 14:34:34 -0400 Received: by mail-vs1-f42.google.com with SMTP id a127so17318198vsa.3 for <55248@debbugs.gnu.org>; Tue, 03 May 2022 11:34:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=philipmcgrath.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=BC5GQY0TXltZBMOkoXODUxQlNsYii5czqkkZnBMyZ8A=; b=So01hS0avRQyq5BR0bZJHikyiz3xrEh5Om5n8810uteBCKnot9WSJhkfZdxB9OcUf4 gNxQ34KwfiYPvufbKeA0jwOKyGBo0qPQq2baymfh3CRB+Cpc+8FPFYka1tLHT9YkQ2L8 CsVl9fwJVohWO//fz5lXU5nPC5es1rVLxf/3JzL+lDVh1+ZOM7HXsyKZl3Zcs6vVXxIL tBIlNYSFcVILgfFuvzqHk404M8m5SXs+O0iDVMwCzXisrxnuXa9CwASP1ZXIYpir475m yJFRey4it5A14i1gI0zdUXM2RcG0EZboJTzSob+6y1YPPj20f0UVlUWvpOmC1aO/98e9 ErgQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=BC5GQY0TXltZBMOkoXODUxQlNsYii5czqkkZnBMyZ8A=; b=empu5H/a4LaCQUFaJ2oC0TCVXdZ5Coprd6PPAn6BkjjWse41bBafqUD4Apj2DiJOna XTcWFsBpr0U2YC8vhrw0B/JshgjqPq182YdXt+0B3HA/EI+JIwxhXbjI5axdtRKn77lC 6JsSNcNLp8tJ/zu76J7i1Iu1xX8VhkHqfSgskoJHXCZaELfwDA7t73fB3/3s//IcVRu0 /B48pgsFPNktEUGcKrI6C7/EYPh0NFq/OnNBYjYANxt4vug1xreVhtDwwZEvbT5RyLbn rMayZgW/3OFJ4ZNfhLT6D9vlteSfz6YpVgMUG2uldmEy1lvqB11Z08eQvrZ3fNyJtDcs ZQVA== X-Gm-Message-State: AOAM53038+P2uHcZPnrEj6uDQgbGRBBXl2gc362YVzjTlNtz6+FAOUlF zCl1AIvsGahBpI61/x1inLiH+VhZXqCG7qqv X-Google-Smtp-Source: ABdhPJxI4SER/0TzrJzO6yfod5+AK6RQm16XhiTNOt4h3vjFt+0kuXGeV/0ZvqK3VSIGbNpZtHyNeA== X-Received: by 2002:a67:be0b:0:b0:32c:d82f:6723 with SMTP id x11-20020a67be0b000000b0032cd82f6723mr5825286vsq.67.1651602868262; Tue, 03 May 2022 11:34:28 -0700 (PDT) Received: from localhost (c-73-125-98-51.hsd1.fl.comcast.net. [73.125.98.51]) by smtp.gmail.com with UTF8SMTPSA id x19-20020ab035d3000000b003626f894df6sm3078916uat.36.2022.05.03.11.34.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 03 May 2022 11:34:28 -0700 (PDT) From: Philip McGrath <philip@philipmcgrath.com> Date: Tue, 3 May 2022 14:33:41 -0400 Message-Id: <f19329dc2f36a10f3fdc9985887cb2cf66dce094.1651594312.git.philip@philipmcgrath.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <cover.1651594312.git.philip@philipmcgrath.com> References: <cover.1651594312.git.philip@philipmcgrath.com> MIME-Version: 1.0 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: <guix-patches.gnu.org> List-Unsubscribe: <https://lists.gnu.org/mailman/options/guix-patches>, <mailto:guix-patches-request@gnu.org?subject=unsubscribe> List-Archive: <https://lists.gnu.org/archive/html/guix-patches> List-Post: <mailto:guix-patches@gnu.org> List-Help: <mailto:guix-patches-request@gnu.org?subject=help> List-Subscribe: <https://lists.gnu.org/mailman/listinfo/guix-patches>, <mailto:guix-patches-request@gnu.org?subject=subscribe> Errors-To: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org Sender: "Guix-patches" <guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org> X-getmail-retrieved-from-mailbox: Patches |
Series |
gnu: Update Racket to 8.5 and Chez Scheme to 9.5.8.
|
|
Commit Message
Philip McGrath
May 3, 2022, 6:33 p.m. UTC
* gnu/packages/racket.scm (racket-vm-cgc)[arguments]: Add a 'symlink-src' phase to put license files where 'install-license-files' expects to find them. Supply '#:out-of-source? #t'. --- gnu/packages/racket.scm | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
Comments
Philip McGrath schreef op di 03-05-2022 om 14:33 [-0400]: > - ;; help with debugging, but it confuses `install-license-files`. [...] > + ;; workaround for install-license-files > + (lambda* (#:key out-of-source? #:allow-other-keys) > + (when out-of-source? > + (with-directory-excursion ".." > + (symlink "src" > + (package-name->name+version > + (strip-store-file-name #$output)))))))))) Surely we could fix this bug/limitation of install-license-files upstream (in this case, upstream=Guix)? Greetings, Maxime.
Hi, On 5/4/22 05:29, Maxime Devos wrote: > Philip McGrath schreef op di 03-05-2022 om 14:33 [-0400]: >> - ;; help with debugging, but it confuses `install-license-files`. > [...] >> + ;; workaround for install-license-files >> + (lambda* (#:key out-of-source? #:allow-other-keys) >> + (when out-of-source? >> + (with-directory-excursion ".." >> + (symlink "src" >> + (package-name->name+version >> + (strip-store-file-name #$output)))))))))) > > Surely we could fix this bug/limitation of install-license-files > upstream (in this case, upstream=Guix)? > I'd certainly be in favor of that! I've never edited '(guix build gnu-build-system)' but I assume that would be a 'core-updates' change. I'm also not sure what the best solution would be in general. In Racket's specific case, the source directory for the Racket VM is (relative to the repository root), "racket/src/", where other notable directories present include "racket/collects/" and "pkgs/". An out-of-source build ends up happening in "racket/build/". The license files are in the source directory, which is what install-license-files expects, but the find-source-directory helper function fails to guess the location of the source directory: > (define (find-source-directory package) > ;; For an out-of-source build, guess the source directory location > ;; relative to the current directory. Return #f on failure. > (match (scandir ".." > (lambda (file) > (and (not (member file '("." ".." "build"))) > (file-is-directory? > (string-append "../" file))))) > (() ;hmm, no source > #f) > ((source) ;only one other file > (string-append "../" source)) > ((directories ...) ;pick the most likely one > ;; This happens for example with libstdc++, which lives within the GCC > ;; source tree. > (any (lambda (directory) > (and (string-prefix? package directory) > (string-append "../" directory))) > directories)))) Some possibilities I can imagine: * We could add a case to find-source-directory specifically for "src". * We could change configure to communicate the location of the source directory, e.g. via an environment variable, rather than trying to guess. * We could change install-license-files to do more searching in general, e.g. trying multiple directories or preferring a directory that contains at least one match for #:license-file-regexp. We'd have to consider the potential for false positives, too (e.g. sibling directories with projects under different licenses.) * We could add an argument to install-license-files to specify the directory explicitly, as a complement to #:license-file-regexp. Another scenario that might be worth considering is projects that follow the REUSE specification[1], which I don't think would be handled by the current install-license-files. But actually, for Racket, I forgot that a patch I'd sent upstream to handle this in `make install` should be in 8.5, so we may not need this patch at all: I'll confirm before I send v2. -Philip [1]: https://reuse.software/ [2]: https://github.com/racket/racket/pull/4127
Am Donnerstag, dem 05.05.2022 um 14:53 -0400 schrieb Philip McGrath: > Hi, > > On 5/4/22 05:29, Maxime Devos wrote: > > Philip McGrath schreef op di 03-05-2022 om 14:33 [-0400]: > > > - ;; help with debugging, but it confuses `install-license- > > > files`. > > [...] > > > + ;; workaround for install-license-files > > > + (lambda* (#:key out-of-source? #:allow-other-keys) > > > + (when out-of-source? > > > + (with-directory-excursion ".." > > > + (symlink "src" > > > + (package-name->name+version > > > + (strip-store-file-name > > > #$output)))))))))) > > > > Surely we could fix this bug/limitation of install-license-files > > upstream (in this case, upstream=Guix)? > > > > I'd certainly be in favor of that! I've never edited '(guix build > gnu-build-system)' but I assume that would be a 'core-updates' change. > It would. > I'm also not sure what the best solution would be in general. > > In Racket's specific case, the source directory for the Racket VM is > (relative to the repository root), "racket/src/", where other notable > directories present include "racket/collects/" and "pkgs/". An > out-of-source build ends up happening in "racket/build/". The license > files are in the source directory, which is what install-license-files > expects, but the find-source-directory helper function fails to guess > the location of the source directory: I think the issue here is that we're in a chdir into the source, where the typical assumption that there's only a single directory besides it fails. > > (define (find-source-directory package) > > ;; For an out-of-source build, guess the source directory > > location > > ;; relative to the current directory. Return #f on failure. > > (match (scandir ".." > > (lambda (file) > > (and (not (member file '("." ".." "build"))) > > (file-is-directory? > > (string-append "../" file))))) > > (() ;hmm, no source > > #f) > > ((source) ;only one other > > file > > (string-append "../" source)) > > ((directories ...) ;pick the most > > likely one > > ;; This happens for example with libstdc++, which lives > > within the GCC > > ;; source tree. > > (any (lambda (directory) > > (and (string-prefix? package directory) > > (string-append "../" directory))) > > directories)))) I don't think (lambda (directory) (and (string-prefix? package directory) (string-append "../" directory))) does what we think it does. Even so, it's debatable whether that works for Racket. > Some possibilities I can imagine: > > * We could add a case to find-source-directory specifically for > "src". Not a fan personally. > * We could change configure to communicate the location of the > source directory, e.g. via an environment variable, rather than > trying to guess. That would work, but gratuitous environment variables could have unexpected consequences. However, note that the build systems themselves already need to capture the information of where the source directory lives! For gnu-build-system, you could check for abs_srcdir in the Makefile for instance. If this ever becomes necessary (reading ahead it doesn't seem to be, but let that if be an if), I'd suggest splitting install-license-files into a helper procedure that assumes we're in the correct directory and a top level procedure, that performs the guess. This top level procedure would need to be implemented once per build system, while the helper could be inherited. > * We could change install-license-files to do more searching in > general, e.g. trying multiple directories or preferring a > directory that contains at least one match for > #:license-file-regexp. We'd have to consider the potential for > false positives, too (e.g. sibling directories with projects > under different licenses.) > * We could add an argument to install-license-files to specify the > directory explicitly, as a complement to #:license-file-regexp. I would not do either. An alternative to adding another phase is wrapping install-license-files in a directory excursion within this package. That'd be less code which achieves the same thing. > Another scenario that might be worth considering is projects that > follow the REUSE specification[1], which I don't think would be > handled by the current install-license-files. I don't think we can rely on projects consistently following them, sadly. > But actually, for Racket, I forgot that a patch I'd sent upstream to > handle this in `make install` should be in 8.5, so we may not need > this patch at all: I'll confirm before I send v2. That is of course the best solution :)
Philip McGrath schreef op do 05-05-2022 om 14:53 [-0400]: > Another scenario that might be worth considering is projects that follow > the REUSE specification[1], which I don't think would be handled by the > current install-license-files. It doesn't copy .reuse/dep5 and LICENSES yet, though that can be implemented: <https://issues.guix.gnu.org/54234#4>. I don't think that helps for this particular package though, given that Racket doesn't ship those files. Greetings, Maxime.
Liliana Marie Prikler schreef op do 05-05-2022 om 21:52 [+0200]: > > Another scenario that might be worth considering is projects that > > follow the REUSE specification[1], which I don't think would be > > handled by the current install-license-files. > I don't think we can rely on projects consistently following them, > sadly. There's plenty of projects that don't follow REUSE (e.g., Guix itself), and there might be some that have the .reuse/dep5 and LICENSES but aren't doing things 100% according to the spec, but I don't think it matters much to Guix if SPDX-License-Identifier is present in every file and such, given that all Guix has to do here is copy license files, so Guix could be taught to copy .reuse/dep5 and LICENSES (in addition to LICENSE, COPYING, ...). Greetings, Maxime
On 5/5/22 16:33, Maxime Devos wrote: > Philip McGrath schreef op do 05-05-2022 om 14:53 [-0400]: >> Another scenario that might be worth considering is projects that follow >> the REUSE specification[1], which I don't think would be handled by the >> current install-license-files. > > It doesn't copy .reuse/dep5 and LICENSES yet, though that can be > implemented: <https://issues.guix.gnu.org/54234#4>. I don't think that > helps for this particular package though, given that Racket doesn't > ship those files. > Right, it wouldn't help Racket, it was just another example of a case not handled by the current implementation. (Though I think it would be much more justifiable to add a special case for REUSE than for Racket.) Glad people are already thinking about it! -Philip
diff --git a/gnu/packages/racket.scm b/gnu/packages/racket.scm index adf3ccfd74..2f4f7cebd8 100644 --- a/gnu/packages/racket.scm +++ b/gnu/packages/racket.scm @@ -272,8 +272,8 @@ (define-public racket-vm-cgc ;; main-distribution-test that aren't part of the main ;; distribution. #:tests? #f - ;; Upstream recommends #:out-of-source?, and it does - ;; help with debugging, but it confuses `install-license-files`. + ;; Upstream recommends #:out-of-source?, and helps a lot with debugging + #:out-of-source? #t #:modules '((ice-9 match) (ice-9 regex) (guix build gnu-build-system) @@ -310,7 +310,15 @@ (define-public racket-vm-cgc #f))))))) (add-before 'configure 'chdir (lambda _ - (chdir "racket/src")))))) + (chdir "racket/src"))) + (add-after 'chdir 'symlink-src + ;; workaround for install-license-files + (lambda* (#:key out-of-source? #:allow-other-keys) + (when out-of-source? + (with-directory-excursion ".." + (symlink "src" + (package-name->name+version + (strip-store-file-name #$output)))))))))) (home-page "https://racket-lang.org") (synopsis "Old Racket implementation used for bootstrapping") (description "This variant of the Racket BC (``before Chez'' or