.travis/ subject to shellcheck
#13863
This PR is extracted from #13816 to make that one easier to review. It follows on #13849 and #13851
In here the shell script parts from travis.yml are extracted into .travis/before_install.sh, .travis/install.sh, .travis/before_script.sh, .travis/script.sh, and .travis/lint.sh.
This has the benefit that test/lint/lint-shell.sh will also shellcheck these parts. Also it makes the individual script parts more readable.
Concept ACK
Very good idea!
0@@ -0,0 +1,25 @@
1+#!/usr/bin/env bash
2+#
3+# Copyright (c) 2017 The Bitcoin Core developers
Big issue with this PR: Travis - sections check the return code on a per-command basis. The newly sourced files do not, so failures are not detected unless you detect them in the script and propagate them to Travis.
E.g. see https://travis-ci.org/Empact/bitcoin/jobs/411794591 where I deleted a file to force the test/lint/git-subtree-check.sh src/secp256k1 lint to fail
26@@ -27,15 +27,15 @@ env:
27 - WINEDEBUG=fixme-all
28 - DOCKER_PACKAGES="build-essential libtool autotools-dev automake pkg-config bsdmainutils curl git ca-certificates ccache"
29 before_install:
30- - source .travis/test_03_before_install.sh
31+ - set -e; source .travis/test_03_before_install.sh
set -o errexit as more self-documenting?
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
rebased.
in between #13859 was merged, I’m now invoking python with LC_ALL=C.UTF_8 in a31bdfcb918469c9d0400a2bd8433fbfddcc411b as it requires that to interpret the emoji correctly. or parse the source file in the first place. Otherwise nothing changed.
5+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
6+
7+export LC_ALL=C
8+
9+travis_retry docker pull "$DOCKER_NAME_TAG"
10+env | grep -E '^(CCACHE_|WINEDEBUG|LC_ALL|BOOST_TEST_RANDOM|CONFIG_SHELL)' | tee /tmp/env
LC_ALL=C.UTF-8 in the travis yaml, then set it to LC_ALL=C here, save it into a file, read that by docker, but then overwrite it again with LC_ALL=C.UTF-8 for the test runner.
lint-shell-locale.sh requires scripts to set LC_ALL=C. I could change it to also accept LC_ALL=C.UTF-8.
export LC_ALL=C.UTF-8 and allowed that in lint-shell-locale in 50a51061fd04549625dedeed02bdf890548e3506 (rebased, now 54949499c1fdb860a6019d0b213b187e51a0a1b6)
16@@ -16,7 +17,7 @@ for SHELL_SCRIPT in $(git ls-files -- "*.sh" | grep -vE "src/(secp256k1|univalue
17 continue
18 fi
19 FIRST_NON_COMMENT_LINE=$(grep -vE '^(#.*|)$' "${SHELL_SCRIPT}" | head -1)
20- if [[ ${FIRST_NON_COMMENT_LINE} != "export LC_ALL=C" ]]; then
21+ if [[ ${FIRST_NON_COMMENT_LINE} != "export LC_ALL=C" && ${FIRST_NON_COMMENT_LINE} != "export LC_ALL=C.UTF-8" ]]; then
if [[ ! ${FIRST_NON_COMMENT_LINE} =~ "export LC_ALL=C"(|\.UTF-8)$ ]]; then
what about
0 case "${FIRST_NON_COMMENT_LINE}" in
1 "export LC_ALL=C");;
2 "export LC_ALL=C.UTF-8");;
3 *)
4 echo "Missing \"export LC_ALL=C\" (to avoid locale dependence) as first non-comment non-empty line in ${SHELL_SCRIPT}"
5 EXIT_CODE=1
6 esac
if-statement is much clearer than the suggested case construction. I suggest reverting back to the original version which was perfectly fine and simple :-)
About conflicting pull requests:
This one I just rebased.
So I was wondering about that. If something touches the travis-definitions only – what is the difference between ACK and utACK here, what could I do to test it? I am rebasing this PR every day and the travis build works splendidly every time (except for this one time where everything in travis failed as it did not at all have any network connectivity).
Because I do not see what more I could do (as a reviewer) but look at travis whether it worked or not. Possibly I could imagine forking and pushing it in my own github-space and looking at travis for my repo, seeing if I can make it fail in the way it should fail (again, as a reviewer; obviously I did that but I’m the author).
I’m mostly asking out of curiosity.
Obviously it passes every time because there is no other development happening on this branch, and I’m rebasing against things which passed the CI check before. While reacting to change requests in the discussion in here it very well failed a few times.
For easier verification would it make sense to link these failing builds?
Again though, what is a reviewer supposed to do in order to test this? The only thing I can imagine is push it from your own repository to trigger a travis build for that and try to make it break there, right?
adjust indentation to 2 spaces
In https://github.com/bitcoin/bitcoin/pull/13863/commits/8ad042b4bebbfd2e38bb4cc7ed36bc047aaf09cd I am explicitly breaking the build by introducing an undocumented argument, which is supposed to fail the lint stage.
Here’s the build failing in the lint stage in my repository: https://travis-ci.org/scravy/bitcoin/jobs/421068804#L575
Here’s the build failing in the lint stage as a pull request: https://travis-ci.org/bitcoin/bitcoin/jobs/421068842#L580
Screenshots of the failing build:
In https://github.com/bitcoin/bitcoin/pull/13863/commits/236091eebfa9fb0234b0ff7549ec730e87d5a589 I’m undoing the lint-error such that the lint stage passes again and introduce a compilation error in src/init.cpp to fail the build in the test stage.
Here’s the build failing in the test stage in my repository: https://travis-ci.org/scravy/bitcoin/builds/421075688 (exact failure: https://travis-ci.org/scravy/bitcoin/jobs/421075692#L2865 )
Here’s the build failing in the test stage as a pull request: https://travis-ci.org/bitcoin/bitcoin/builds/421075705 (exact failure: https://travis-ci.org/bitcoin/bitcoin/jobs/421075707#L3120 )
In https://github.com/bitcoin/bitcoin/pull/13863/commits/b1ab0435ad245161c3a4906b0fe4a68bd211927a I undo the compile error and introduce a lint violation insisde on of the travis script files (not using LC_ALL=c, violating lint-shell-locale).
This is how it’s failing in my repository: https://travis-ci.org/scravy/bitcoin/jobs/421082198#L579
This is how it’s failing as a pull request: https://travis-ci.org/bitcoin/bitcoin/jobs/421082215#L585
42+
43+BEGIN_FOLD build
44+DOCKER_EXEC make $MAKEJOBS $GOAL || ( echo "Build failure. Verbose build follows." && DOCKER_EXEC make $GOAL V=1 ; false )
45+END_FOLD
46+
47+if [ "$RUN_TESTS" = "true" ]; then
58+
59+if [ "$TRAVIS_EVENT_TYPE" = "cron" ]; then
60+ extended="--extended --exclude feature_pruning,feature_dbcrash"
61+fi
62+
63+if [ "$RUN_TESTS" = "true" ]; then
RUN_TESTS is not exported? Will send a follow-up PR then.