From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id 73A126DE0361 for ; Mon, 25 Sep 2017 22:28:02 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: 0.487 X-Spam-Level: X-Spam-Status: No, score=0.487 tagged_above=-999 required=5 tests=[AWL=-0.165, SPF_NEUTRAL=0.652] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id e-yAMsOPKyzI for ; Mon, 25 Sep 2017 22:28:01 -0700 (PDT) Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) by arlo.cworth.org (Postfix) with ESMTP id 276216DE0352 for ; Mon, 25 Sep 2017 22:28:00 -0700 (PDT) Received: from guru.guru-group.fi (localhost [IPv6:::1]) by guru.guru-group.fi (Postfix) with ESMTP id 899B11000E0; Tue, 26 Sep 2017 08:28:11 +0300 (EEST) From: Tomi Ollila To: Jani Nikula , notmuch@notmuchmail.org Subject: Re: [PATCH 24/24] devel: add script to test out-of-tree builds In-Reply-To: <578efe3a1c55a7030f4f58d56be8ae8a3149cd5d.1506370901.git.jani@nikula.org> References: <578efe3a1c55a7030f4f58d56be8ae8a3149cd5d.1506370901.git.jani@nikula.org> User-Agent: Notmuch/0.25+89~g5a0c015 (https://notmuchmail.org) Emacs/25.2.1 (x86_64-unknown-linux-gnu) X-Face: HhBM'cA~ MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Sep 2017 05:28:02 -0000 On Mon, Sep 25 2017, Jani Nikula wrote: > Something I used for 'git bisect run', but we should really add this > as part of our process. > > Can also be used for running out-of-tree tests with e.g.: > > $ devel/out-of-tree-build-check.sh V=1 test This series looks pretty good (there are some ""-quote inconsistensies in some of the smaller-numbered patches, but probably not adding to the current state of the art -- just note that if these were my patches that would be intolerable >;D) So, IMO, the above can be pushed as is (On Fedora 26 the test results are identical with and without these patches applied (855 succeeded, 7 expected failures, 2 tests skipped on one of my systems) This last one (in addition to quote inconsistencies) has also slight naming inconsistency compared w/ other files in that directory; To match IMO this should be named as check-out-of-tree-build.sh... And since I had to comment on this I'll express my nitpicks of the content, too :D: Tomi > --- > devel/out-of-tree-build-check.sh | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > create mode 100755 devel/out-of-tree-build-check.sh > > diff --git a/devel/out-of-tree-build-check.sh b/devel/out-of-tree-build-check.sh > new file mode 100755 > index 000000000000..917752ff72cb > --- /dev/null > +++ b/devel/out-of-tree-build-check.sh > @@ -0,0 +1,16 @@ > +#!/bin/sh > +# test out-of-tree builds in a temp directory > +# passes all args to make > + > +set -eu > + > +srcdir="$(cd "$(dirname "$0")"/.. && pwd)" quotes unnecessary above, but may clarify things... > +builddir=$(mktemp -d) ... but the above is then inconsistent as it does not have ""-quotes > + > +cd $builddir due to $(mktemp -d) not outputting any $IFS characters the above always works without "$builddir" but imo not having quotes there is a bad example (and inconsistent to what I'm going to write below) > +$srcdir/configure $srcdir, OTOH could have $IFS characters (usually spaces) there, so above line would break in such a case > +make "$@" > + > +cd $srcdir ditto > +rm -rf $builddir consistency > -- > 2.11.0