Re: [PATCH 24/24] devel: add script to test out-of-tree builds

Subject: Re: [PATCH 24/24] devel: add script to test out-of-tree builds

Date: Tue, 26 Sep 2017 08:28:11 +0300

To: Jani Nikula, notmuch@notmuchmail.org

Cc:

From: Tomi Ollila


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
_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Thread: