From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id E2F9E3858D37 for ; Wed, 19 Apr 2023 14:04:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E2F9E3858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [10.0.0.170] (unknown [167.248.160.41]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 510A71E128; Wed, 19 Apr 2023 10:04:55 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1681913095; bh=7XMEEU1Uk9/RIrJ00l8jexVK1HJvXHrXsYgya6db1mU=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=Zw0qjTLzRAIlol8pFwaKdZ1EfQuPnSoQt43kX32DGTCotLdY5OASPPcK5OzemI5dU WYS3Nlcb8gFUvPZzJ5E5OrAF3TAzqlgCP6B3WFdBbMOcF0TiDUrwlx8j0P+Kxar3qW sOkq2c2pObQ2FU94tzmXeuSfgtONjtkWCQOXY+Fg= Message-ID: <2eef24aa-bbe4-478a-c965-82783d3dea9f@simark.ca> Date: Wed, 19 Apr 2023 10:04:54 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [PATCH 1/1] [gdb/contrib] Add make-check-all.sh Content-Language: fr To: Tom de Vries , gdb-patches@sourceware.org Cc: Andrew Burgess References: <20230403150957.28921-1-tdevries@suse.de> <20230403150957.28921-2-tdevries@suse.de> <87wn2rc22u.fsf@redhat.com> <97c65614-e1a5-a9cb-b5a3-36020e8ed259@suse.de> From: Simon Marchi In-Reply-To: <97c65614-e1a5-a9cb-b5a3-36020e8ed259@suse.de> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 4/18/23 08:43, Tom de Vries wrote: > On 4/5/23 11:01, Tom de Vries via Gdb-patches wrote: >> On 4/4/23 13:35, Andrew Burgess wrote: >>> Tom de Vries via Gdb-patches writes: >>> >>>> Add script gdb/contrib/make-check-all.sh, that's intended to function as a >>>> drop-in replacement of make check, but excercising all host/target boards in >>>> gdb/testsuite/boards. >>>> >>>> Shell-checked and tested on x86_64-linux. >>> >>> Hi Tom, >>> >>> Thanks for putting this together, I think this could be really useful. >>> >> >> That's great to hear :) >> >>> I'm not a fan of the way you've split the patch description into email >>> 0/1 and not included it with this commit. I think there's lots of >>> useful information in there, and I'd much rather have the whole >>> description included in the commit message -- it's much easier to find >>> then rather than having to hunt on the mailing list in the future. >>> >> >> Ack, I've reworked this into a patch rather than patch series. >> >>>> --- >>>> gdb/contrib/make-check-all.sh | 255 ++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 255 insertions(+) >>>> create mode 100755 gdb/contrib/make-check-all.sh >>>> >>>> diff --git a/gdb/contrib/make-check-all.sh b/gdb/contrib/make-check-all.sh >>>> new file mode 100755 >>>> index 00000000000..1befe418b81 >>>> --- /dev/null >>>> +++ b/gdb/contrib/make-check-all.sh >>>> @@ -0,0 +1,255 @@ >>>> +#!/bin/bash >>>> + >>>> +# Copyright (C) 2023 Free Software Foundation, Inc. >>>> +# This program is free software; you can redistribute it and/or modify >>>> +# it under the terms of the GNU General Public License as published by >>>> +# the Free Software Foundation; either version 3 of the License, or >>>> +# (at your option) any later version. >>>> +# >>>> +# This program is distributed in the hope that it will be useful, >>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>> +# GNU General Public License for more details. >>>> +# >>>> +# You should have received a copy of the GNU General Public License >>>> +# along with this program. If not, see . >>>> + >>>> +# Run make check with all boards from gdb/testsuite/boards. >>> >>> I think you need some kind of usage test here too. Imagine a new >>> developer finds this file and looks inside. Ideally, I think they >>> should know how to use it, and what it will do for them without having >>> to read any of the actual script content. >>> >> >> Added. >> >>> On implementation, I wonder if it would be useful to provide a mechanism >>> by which the gdb.sum and gdb.log files for each test run could be >>> preserved? >>> >> >> Done, I've renamed --keep to --keep-tmp, and added this new functionality under --keep. Maybe call it "keep-results"? Otherwise it's not clear what --keep keeps. >> >> I've also added "set -e" to make sure ^C aborts the script rather than just one make check invocation, and moved the tmpdir removal to a cleanup function that's executed on exit, to make sure ^C doesn't leave tmpdirs behind. >> > > I'd like to commit this at the end of the week, if there are no further review comments. > > Simon, I remember discussing something similar with you a few years ago ( https://sourceware.org/pipermail/gdb-patches/2020-May/169003.html ), so I wondered if you have any comments on this approach. I don't recall this discussion at all :). But I like the idea of your script, it lets people run tests on all target boards without having to specifically know about each board. I never really test on boards other than unix, native-gdbserver and native-extended-gdbserver, but I might now with your script (probably not whole testsuite runs, but at least when working on a test). When applying: Applying: Add make-check-all.sh .git/rebase-apply/patch:232: trailing whitespace. else warning: 1 line adds whitespace errors. Would it make more sense to have this file under gdb/testsuite? I don't understand the comment about using --host-user and --target-user. Are you suggesting that I create new users on my machine (e.g. gdb-test-host and gdb-test-target), so that running with the remote host and remote target boards pollute those users' home directories instead of mine? I also don't understand those `if true` conditions in main. Finally, I'd appreciate a few comments here and there in the script, to make further modifications easier. But other than that, it looks good to me, it can't hurt to have that in the tree. Simon