Message ID | 20221122194712.31515-1-raingloom@riseup.net |
---|---|
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 E02D327BBED; Tue, 22 Nov 2022 19:48:34 +0000 (GMT) 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, URIBL_BLOCKED autolearn=ham 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 5CEDC27BBE9 for <patchwork@mira.cbaines.net>; Tue, 22 Nov 2022 19:48:34 +0000 (GMT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from <guix-patches-bounces@gnu.org>) id 1oxZFU-0004O0-G7; Tue, 22 Nov 2022 14:48:04 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) 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 1oxZFS-0004N3-Sh for guix-patches@gnu.org; Tue, 22 Nov 2022 14:48:02 -0500 Received: from debbugs.gnu.org ([209.51.188.43]) 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 1oxZFS-0005po-KX for guix-patches@gnu.org; Tue, 22 Nov 2022 14:48:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1oxZFS-0000XC-8C for guix-patches@gnu.org; Tue, 22 Nov 2022 14:48:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#59487] [PATCH 1/2] build-system/dune: Automatically deduce test-target in most cases. Resent-From: raingloom <raingloom@riseup.net> Original-Sender: "Debbugs-submit" <debbugs-submit-bounces@debbugs.gnu.org> Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 22 Nov 2022 19:48:02 +0000 Resent-Message-ID: <handler.59487.B.16691464691960@debbugs.gnu.org> Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 59487 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 59487@debbugs.gnu.org Cc: raingloom <raingloom@riseup.net> X-Debbugs-Original-To: guix-patches@gnu.org Received: via spool by submit@debbugs.gnu.org id=B.16691464691960 (code B ref -1); Tue, 22 Nov 2022 19:48:02 +0000 Received: (at submit) by debbugs.gnu.org; 22 Nov 2022 19:47:49 +0000 Received: from localhost ([127.0.0.1]:52579 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <debbugs-submit-bounces@debbugs.gnu.org>) id 1oxZFF-0000VX-2O for submit@debbugs.gnu.org; Tue, 22 Nov 2022 14:47:49 -0500 Received: from lists.gnu.org ([209.51.188.17]:58004) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from <raingloom@riseup.net>) id 1oxZFD-0000VL-KT for submit@debbugs.gnu.org; Tue, 22 Nov 2022 14:47:48 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from <raingloom@riseup.net>) id 1oxZFD-0004Lz-Dk for guix-patches@gnu.org; Tue, 22 Nov 2022 14:47:47 -0500 Received: from mx0.riseup.net ([198.252.153.6]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from <raingloom@riseup.net>) id 1oxZF9-0005U6-ST for guix-patches@gnu.org; Tue, 22 Nov 2022 14:47:46 -0500 Received: from fews2.riseup.net (fews2-pn.riseup.net [10.0.1.84]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "mail.riseup.net", Issuer "R3" (not verified)) by mx0.riseup.net (Postfix) with ESMTPS id 4NGvtH43BLz9rxw for <guix-patches@gnu.org>; Tue, 22 Nov 2022 19:47:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=riseup.net; s=squak; t=1669146455; bh=KOrPrVRRbJ9wuWK618gzP/Z9eV0DUpzlQl1JWlD9XdA=; h=From:To:Cc:Subject:Date:From; b=rHVrrayravJKpirH+AAXLl2jhx18exzfobJIQIbZR+uXWBWXuJ0gZ20uF/38lLRc/ hVBVZABny6uLuOX6Pl2JNqyxMF4UWLwwzfwa88IcrsLkHxlcmKwbVlUx/exniOLkiU ebUoeBhyj3zpgO0sfRHdHD2v8sA38QkZzYE73IJc= X-Riseup-User-ID: 46E8C14D0E2B56ECBB778A21436A87928694784B40C4B7F2A8F4668325DE8623 Received: from [127.0.0.1] (localhost [127.0.0.1]) by fews2.riseup.net (Postfix) with ESMTPSA id 4NGvtG4z2rz21FS; Tue, 22 Nov 2022 19:47:34 +0000 (UTC) From: raingloom <raingloom@riseup.net> Date: Tue, 22 Nov 2022 20:47:11 +0100 Message-Id: <20221122194712.31515-1-raingloom@riseup.net> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=198.252.153.6; envelope-from=raingloom@riseup.net; helo=mx0.riseup.net X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action 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-bounces+patchwork=mira.cbaines.net@gnu.org X-getmail-retrieved-from-mailbox: Patches |
Series |
[bug#59487,1/2] build-system/dune: Automatically deduce test-target in most cases.
|
|
Commit Message
Csepp
Nov. 22, 2022, 7:47 p.m. UTC
guix/build-system/dune.scm (dune-build): tests? defaults to #f. guix/build/dune-build-system.scm (check): Missing test-target is auto-detected. --- guix/build-system/dune.scm | 2 +- guix/build/dune-build-system.scm | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-)
Comments
Hi! This LGTM, but maybe put test-target inside the cond too, so we can remove the or? I've looked at how opam does things, and this seems to be the norm: ["dune" "runtest" "-p" name "-j" jobs] Maybe this would work better: (apply invoke program "runtest" (append (if test-target (list test-target) '()) ...)) WDYT? Le Tue, 22 Nov 2022 20:47:11 +0100, raingloom <raingloom@riseup.net> a écrit : > guix/build-system/dune.scm (dune-build): tests? defaults to #f. > guix/build/dune-build-system.scm (check): Missing test-target is > auto-detected. --- > guix/build-system/dune.scm | 2 +- > guix/build/dune-build-system.scm | 9 +++++++-- > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/guix/build-system/dune.scm b/guix/build-system/dune.scm > index 12100fd8e8..b531d3c337 100644 > --- a/guix/build-system/dune.scm > +++ b/guix/build-system/dune.scm > @@ -107,7 +107,7 @@ (define* (dune-build name inputs > (dune-release-flags ''()) > (tests? #t) > (test-flags ''()) > - (test-target "test") > + (test-target #f) > (install-target "install") > (validate-runpath? #t) > (patch-shebangs? #t) > diff --git a/guix/build/dune-build-system.scm > b/guix/build/dune-build-system.scm index e9ccc71057..8a3725a4e9 100644 > --- a/guix/build/dune-build-system.scm > +++ b/guix/build/dune-build-system.scm > @@ -42,12 +42,17 @@ (define* (build #:key (build-flags '()) (jbuild? > #f) build-flags))) > #t) > > -(define* (check #:key (test-flags '()) (test-target "test") tests? > +(define* (check #:key (test-flags '()) (test-target #f) tests? > (jbuild? #f) (package #f) (dune-release-flags '()) > #:allow-other-keys) > "Test the given package." > (when tests? > - (let ((program (if jbuild? "jbuilder" "dune"))) > + (let ((program (if jbuild? "jbuilder" "dune")) > + (test-target (or test-target > + (cond > + ((file-exists? "tests") "tests") > + ((file-exists? "test") "test") > + (else "."))))) > (apply invoke program "runtest" test-target > (append (if package (list "-p" package) > dune-release-flags)
Hi! Sorry, I missed these patches and only found them after realizing that your Mirage patches require these. I'm having trouble applying the second patch (of V2), maybe there have been changes that cause conflicts? Could you rebase on master or just resend the V2 patches to 59487@debbugs.gnu.org? Maybe with a short comment if and how they address Julien's suggestion? -pukkamustard raingloom <raingloom@riseup.net> writes: > guix/build-system/dune.scm (dune-build): tests? defaults to #f. > guix/build/dune-build-system.scm (check): Missing test-target is auto-detected. > --- > guix/build-system/dune.scm | 2 +- > guix/build/dune-build-system.scm | 9 +++++++-- > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/guix/build-system/dune.scm b/guix/build-system/dune.scm > index 12100fd8e8..b531d3c337 100644 > --- a/guix/build-system/dune.scm > +++ b/guix/build-system/dune.scm > @@ -107,7 +107,7 @@ (define* (dune-build name inputs > (dune-release-flags ''()) > (tests? #t) > (test-flags ''()) > - (test-target "test") > + (test-target #f) > (install-target "install") > (validate-runpath? #t) > (patch-shebangs? #t) > diff --git a/guix/build/dune-build-system.scm b/guix/build/dune-build-system.scm > index e9ccc71057..8a3725a4e9 100644 > --- a/guix/build/dune-build-system.scm > +++ b/guix/build/dune-build-system.scm > @@ -42,12 +42,17 @@ (define* (build #:key (build-flags '()) (jbuild? #f) > build-flags))) > #t) > > -(define* (check #:key (test-flags '()) (test-target "test") tests? > +(define* (check #:key (test-flags '()) (test-target #f) tests? > (jbuild? #f) (package #f) (dune-release-flags '()) > #:allow-other-keys) > "Test the given package." > (when tests? > - (let ((program (if jbuild? "jbuilder" "dune"))) > + (let ((program (if jbuild? "jbuilder" "dune")) > + (test-target (or test-target > + (cond > + ((file-exists? "tests") "tests") > + ((file-exists? "test") "test") > + (else "."))))) > (apply invoke program "runtest" test-target > (append (if package (list "-p" package) > dune-release-flags)
pukkamustard <pukkamustard@posteo.net> writes: > Hi! Sorry, I missed these patches and only found them after realizing > that your Mirage patches require these. > > I'm having trouble applying the second patch (of V2), maybe there have > been changes that cause conflicts? Could you rebase on master or just > resend the V2 patches to 59487@debbugs.gnu.org? Maybe with a short > comment if and how they address Julien's suggestion? > > -pukkamustard Sent a rebased V2 just now. The runtest invocation looks like Julien's suggestion, so I think I addressed it.
This removes the usage of the `test-target` argument from dune-build-system completely. Dune automatically finds tests for packages being built. The previous test-target was limiting the search of tests to specific folders. In some cases the wrong folders. I could not systematically check all affected packages, but have fixed tests for some (these are packages for which previously we were not running tests). -pukkamustard pukkamustard (5): gnu: Add ocaml-cinaps. gnu: ocaml-ppxlib: Clean up inputs. gnu: ocaml-cohttp: Run tests. guix: dune-build-system: Run all tests. gnu: ocaml-ppx-expect: Disable tests. raingloom (1): guix: dune-build-system: Remove usage of test-target argument. gnu/packages/ocaml.scm | 209 +++++++++++++------------------ guix/build/dune-build-system.scm | 4 +- 2 files changed, 86 insertions(+), 127 deletions(-) base-commit: 9ad5c3deeaebfb6f953579992a082d8327730611
I was able to check the dependents, so pushed to master as 7686b68f6bb1ae5dc206178805c32bceceea3743 - 79bf3b54a64e304a29334b5029c24d942d9dc051. Note that I moved patches around, to have the last one before removing test-target. I merged patches 3, 4 and 5 because intermediate states would have failing packages otherwise. Also, patch 5 had unrelated changes to lablgtk3 which I droped, because the package still builds. If that was intentional, please send a new patch for that :) I also listed all modified packages in the commit message, and removed the Co-authored line which listed the author already. Thanks!
diff --git a/guix/build-system/dune.scm b/guix/build-system/dune.scm index 12100fd8e8..b531d3c337 100644 --- a/guix/build-system/dune.scm +++ b/guix/build-system/dune.scm @@ -107,7 +107,7 @@ (define* (dune-build name inputs (dune-release-flags ''()) (tests? #t) (test-flags ''()) - (test-target "test") + (test-target #f) (install-target "install") (validate-runpath? #t) (patch-shebangs? #t) diff --git a/guix/build/dune-build-system.scm b/guix/build/dune-build-system.scm index e9ccc71057..8a3725a4e9 100644 --- a/guix/build/dune-build-system.scm +++ b/guix/build/dune-build-system.scm @@ -42,12 +42,17 @@ (define* (build #:key (build-flags '()) (jbuild? #f) build-flags))) #t) -(define* (check #:key (test-flags '()) (test-target "test") tests? +(define* (check #:key (test-flags '()) (test-target #f) tests? (jbuild? #f) (package #f) (dune-release-flags '()) #:allow-other-keys) "Test the given package." (when tests? - (let ((program (if jbuild? "jbuilder" "dune"))) + (let ((program (if jbuild? "jbuilder" "dune")) + (test-target (or test-target + (cond + ((file-exists? "tests") "tests") + ((file-exists? "test") "test") + (else "."))))) (apply invoke program "runtest" test-target (append (if package (list "-p" package) dune-release-flags)