fa082d0a57afedca9122fac4aecd6a3070f06b04 travis: Remove valgrind (MarcoFalke)
Pull request description:
When the valgrind run was added, it took 2 hours. Travis kindly raised the timeout limit to the maximum possible of 3 hours.
Today, a full build of Bitcoin Core with all tests takes more than three hours. Thus, it is impossible to run all tests on travis.
Moreover, the feedback loop for developers that create a pull request takes at least 2 hours, but in some cases (when the travis queue is full) until the next day. This is unacceptable.
Fix both issues by removing the build from travis.
Please note that the `ci/test/` configurations are *not* removed. They will stay in the repo and can be executed anywhere (just not on travis).
ACKs for top commit:
jamesob:
ACK fa082d0a57
jnewbery:
utACK fa082d0a57afedca9122fac4aecd6a3070f06b04
Tree-SHA512: 9acaa0e2d3926014fadb7dd2e86c4e01df382e9399f6ae99f989fa609da66a77bdd1b75d6ff42d2686f38f730b8564e6dc722aa597a473290c9d30c2abe7ef0f
b0a01299ed8cc2026e91a8dcad880b700e462cee test: Disable s390 build on travis (Wladimir J. van der Laan)
Pull request description:
Travis is consistently failing on s390 due to out of disk space issues,
which causes false positives. Disable the s390 build for now until
this is fixed some other way.
ACKs for top commit:
MarcoFalke:
ACK b0a01299ed8cc2026e91a8dcad880b700e462cee 🍠
jonasschnelli:
ACK b0a01299ed8cc2026e91a8dcad880b700e462cee
Tree-SHA512: ae8a78fa03bc620d802e4992d46929a0a4b801b50a6066d8f093f93fc5ebbc010d221c04787cbfc75436a2c40f9292257bf85fbb251593e21afff11756b6d8d4
ef63f5fc1136ad2a2cd080d44142a2ee3945c238 ci: Combine 32-bit build with CentOS 7 build (Sebastian Falbesoner)
Pull request description:
Combines the CentOS build with the 32-bit (i686) build to avoid Travis bottlenecks, as suggested in #17757 by MarcoFalke. This keeps most of the properties of the 32-bit build (dash as config shell, building QT5 GUI) and just builds it with depends inside the CentOS docker container.
Making the depends in `05_before_script.sh` with unset config shell (`CONFIG_SHELL=`)
6196e93001/ci/test/05_before_script.sh (L28)
caused problems for building the library libevent (resulting in a Makefile with no shell set (`SHELL=`)), that's why I set it explicitely to `/bin/bash` if we have a CentOS Docker container.
A Travis output of this 32-bit CentOS build can be seen here: https://travis-ci.org/theStack/bitcoin/jobs/634472394 (has been restarted once due to too long build time and appearance of the `CACHE_ERR_MSG`).
For anyone wanting to verify the outputs, I found these instructions useful to reproduce a Travis build locally: https://github.com/erdc/proteus/wiki/Replicating-the-TravisCI-Environment-on-your-Local-Machine (steps 1-3). In this case it's a bit tricky since you run Docker inside Docker -- within the Travis Docker container, the CentOS Docker container is created. To make this possible, the Docker socket has to be exposed to the Travis container via bind-mounting (`docker run -v /var/run/docker.sock:/var/run/docker.sock ...`), as suggested in https://stackoverflow.com/a/33003273.
Top commit has no ACKs.
Tree-SHA512: af508241cec3a10a66c37673d56691717b78375340e910fcdd3fb3870741eba623a436e1e85b26b54f013375611896f5411c5a7fec2437d367d27172230129fe
faa079db92e614bddfe8bd323ddcd2adb77e0961 travis: Disable feature_block in tsan run (MarcoFalke)
Pull request description:
The `feature_block` test causes the travis machine to OOM, when run with the thread sanitizer.
The stderr says:
```
==27237==ERROR: ThreadSanitizer failed to allocate 0xf6000 (1007616) bytes of LargeMmapAllocator (error code: 12)
...
FATAL: ThreadSanitizer CHECK failed: /build/llvm-toolchain-3.8-_PD09B/llvm-toolchain-3.8-3.8/projects/compiler-rt/lib/sanitizer_common/sanitizer_common.cc:183 "((0 && "unable to mmap")) != (0)" (0x0, 0x0)
ERROR: Failed to mmap
```
(from https://travis-ci.org/bitcoin/bitcoin/jobs/588194563#L10505)
Fix this by disabling `feature_block` on travis. Longer term, I'd like to move away from travis, but I'll leave this for a follow-up.
ACKs for top commit:
fanquake:
ACK faa079db92e614bddfe8bd323ddcd2adb77e0961
Tree-SHA512: c0dc2272853aac53f68eb9e110c8500c4a92211ba89d856660bacdf6e959d875477e422b3280b743d85fc8a65e083bf9153911f12039d026e2501f426540dac4
BIP70 is removed in "merge #17165: Remove BIP70 support (#4023)".
So, this commit contains only some follow-ups to unify codebase
e09913f1c47e693b0c6fafef55b9ca78e5f3abc0 doc: specify protobuf as optional in build docs (fanquake)
376f4929f8f75011b72b2f9c3164980db482278a build: disable BIP70 support by default (fanquake)
Pull request description:
Disable BIP70 support in the GUI by default for `0.19.0` (for eventual removal in `0.20.0`?).
Users who want to compile with BIP70 support enabled can pass `--enable-bip70` to `./configure`.
I've inverted the current `--disable-bip70` test to instead pass `--enable-bip70`.
Tested configurations on `macOS` (`protobuf` installed with `brew`).
Protobuf available and `./configure`:
```
Options used to compile and link:
with wallet = yes
with gui / qt = yes
with bip70 = no
```
Protobuf available and `./configure --enable-bip70`:
```
Options used to compile and link:
with wallet = yes
with gui / qt = yes
with bip70 = yes
```
Protobuf not available (i.e `brew unlink protobuf`) and `./configure`:
```
Options used to compile and link:
with wallet = yes
with gui / qt = yes
with bip70 = no
```
Protobuf not available and `./configure --enable-bip70`:
```
checking whether to build test_bitcoin-qt... yes
checking whether to build BIP70 support... configure: error: protobuf missing
```
TODO:
- [x] Remove `protobuf` from other Travis builds
- [ ] Documentation updates (mention that `protobuf` is now optional)?
- [ ] Could split release notes into GUI and build
ACKs for top commit:
laanwj:
ACK e09913f1c47e693b0c6fafef55b9ca78e5f3abc0
elichai:
ACK e09913f1c47e693b0c6fafef55b9ca78e5f3abc0 Read the autotools changes. awesome that this removes the protobuf requirement.
practicalswift:
ACK e09913f1c47e693b0c6fafef55b9ca78e5f3abc0 -- diff looks correct
Tree-SHA512: 7bf87ae8555e24db2da2e89cc4d4e90d09be27499ad386ad65879d05df8f96d9a1384379891ac8963d17728c90e55961560813df97e849e631e2de8c08e210c8
fad88e6f86d1dd32cf01db2287df9c63e66c5116 ci: Remove use of cd (MarcoFalke)
fa2941bbf47a8a6b79b8db4a87e1aedcf6a29a5e ci: Remove unmaintained extended_lint (MarcoFalke)
fa041875284d01602647519cc452185ba1ad5a8f scripted-diff: Use ci DEPENDS_DIR, remove BASE_BUILD_DIR (MarcoFalke)
fa0656d1ebc2a01d900fead4ce055d59eb818a1f ci: Add DEPENDS_DIR variable, Add documentation for folders (MarcoFalke)
faeeca87b65dd98e0efbc54443b3f8854cae9c00 scripted-diff: Move various folders to ci scratch dir (MarcoFalke)
Pull request description:
Some minor cleanups:
* Remove unused and unmaintained extended_lint to avoid ci bottlenecks and waste of CPU
* Move all folders that hold temporary ci files to the ci scratch dir (except for the build dirs)
* Add some documentation to folders and remove the `BASE_BUILD_DIR` alias for the root directory
* Fixes#17178 by removing `cd`
Top commit has no ACKs.
Tree-SHA512: f6eb9d47ab98c08af59a79c2a4bf62fc331f0f6f3174909a28f2c2f48b0234423da5aee876225d2e188619b71b008d882a20d29a7aca68248039ea5080be5af5
e1900008699bd45031b7faa9ef3d0a81d54091b0 ci-s390x: Add qemu and depends support in the ci script (Elichai Turkel)
Pull request description:
Related: #17599
This adds qemu support just like we have in arm and compile the depends.
other than that I also fixed some missing includes to make the depends compile.
ACKs for top commit:
MarcoFalke:
ACK e190000 (first commit only, didn't look at second commit)
Tree-SHA512: 2b8a39772b86408569f52cdc33832dbce7e5e9cdd710524295f3d259628cdfc017e740f6f94941307d7f8e413236814a95ba851153c617eb5fb75b4bd9a7e52f
fabd71076cd9493bd2d30a198467f5ea621b27aa ci: Print free disk space (MarcoFalke)
fad9fdbea5dfb19328282afda9588edc6f1d0ddf test: Properly deserialize integers in little-endian (MarcoFalke)
fa94fc10c881e502e6c9a71f3b7719aa955900f9 ci: Run functional tests on s390x (MarcoFalke)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: 98ba77eb56f283131fdaeb393fda86cc308f1bf9781e1e0e5736b8d616528dc8ff2e494d55ba107c138083025c66a59e382fcfa9962d4349a5fd6cbbc52484c3
## Issue being fixed or feature implemented
During reviewing TODO were found some TODOes that can be done now.
## What was done?
- fix: follow-up dash#3467 - replaced commented code to disabled code
- follow-up bitcoin#16394 - uncommented code related to `watchonly`
feature
- removed out-dated TODO in `rpc/masternode.cpp` (already done)
- fix: renamed name of clean up test_unittests: removed TODO and updated
name of variable TRAVIS
- rewritten todo inside `.travis.yml`
- fix: adds a missing description for result of rpc `mnsync`
Last commit (`mnsync`) is an only candidate for backport to v19, other
changes are non significant.
## How Has This Been Tested?
Run functional/unit tests
## Breaking Changes
No breaking changes
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
fa2056af1c travis: Properly cache and error on timeout (MarcoFalke)
fa36a333ee travis: Switch to ubuntu keyserver to avoid timeouts (MarcoFalke)
Pull request description:
The other keyserver is consistently timing out on travis: https://travis-ci.org/bitcoin/bitcoin/jobs/512689710#L405
Attempt to fix it by using a different server.
Also:
* fixes#15372
* fixes#15738
ACKs for commit fa2056:
ryanofsky:
utACK fa2056af1c71aded3a821a07ec4de71c4be0bca3. All good changes (changing keyserver, getting rid of keyserver while loop, clarifying travis error, moving travis documentation to code comment).
Tree-SHA512: ac8436616ecfee0ed579114e19f03c53ceb688fbcd95a60cffe8f15b4e569772a6ba673f353bbd789e79fe27fc5626c77fab4086768844dd51e0c6c108b52fb2
f2472f64604a0c583f950c56e8753d0bee246388 tests: Improve test runner output in case of target errors (practicalswift)
733bbec34fbec85574cc456832b2b2f807e5dce9 tests: Add --exclude integer,parse_iso8601 (temporarily) to make Travis pass until uninitialized read issue in FormatISO8601DateTime is fixed (practicalswift)
5ea81449f30a6fe6db3b6df5e8009f21a782ff44 tests: Add support for excluding fuzz targets using -x/--exclude (practicalswift)
555236f769c13518db70f5df36e5688d63486bd5 tests: Remove -detect_leaks=0 from test/fuzz/test_runner.py - no longer needed (practicalswift)
a3b539a924f8611abb3096f2bd9d35094b5577e3 ci: Run fuzz testing test cases under valgrind (practicalswift)
Pull request description:
Run fuzz testing [test cases (bitcoin-core/qa-assets)](https://github.com/bitcoin-core/qa-assets) under `valgrind`.
This would have caught `util: Avoid potential uninitialized read in FormatISO8601DateTime(int64_t) by checking gmtime_s/gmtime_r return value` (#18162) and similar cases.
ACKs for top commit:
MarcoFalke:
ACK f2472f64604a0c583f950c56e8753d0bee246388 👼
Tree-SHA512: bb0879d40167cf6906bc0ed31bed39db83c39c7beb46026f7b0ee53f28ff0526ad6fabc3f4cb3f5f18d3b8cafdcbf5f30105b35919f4e83697c71e838ed71493
711e0449cf4a0f15cabe0d64094e3add24ad44b0 ci: Remove trusty build (Hennadii Stepanov)
7f3ae224685efaeb6fe714de90e8871d12e55f34 ci: Add CentOS 7 build (Hennadii Stepanov)
Pull request description:
Arguably, CentOS is the most conservative distro of all the popular ones. Thus, it could be a good way to check the Bitcoin Core compatibility with aged dependencies.
Currently, CentOS 7 has:
- Berkeley DB == 4.8.30
- Boost == 1.53.0
- GCC == 4.8.5
- libevent == 2.0.21 < minimum required [2.0.22](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md), but tests passed
- MiniUPnPc == 2.0
- Python == 3.6.8
- qrencode == 3.4.1
- Qt == 5.9.7
- ZeroMQ == 4.1.4
~Please note that this PR is based on the bugfix #17634.~
Also trusty build has been removed for the following reasons:
- https://github.com/bitcoin/bitcoin/issues/17628#issuecomment-559448201:
> Maybe it'd make sense to replace Ubuntu Trusty with Centos 7 as the "check ancient backward compatibililty" Travis run. It's supported until 2024, apparently.
- https://github.com/bitcoin/bitcoin/pull/17635#discussion_r354811792:
> Our travis is currently running at its limit and this doesn't seem like it is adding a lot new coverage compared to the other builds.
Close#17628
ACKs for top commit:
MarcoFalke:
ACK 711e0449cf4a0f15cabe0d64094e3add24ad44b0 🚠
Tree-SHA512: 614ec8394943f482a5867067f7119bffd052924a51e32ffda9a08e10c392c4a955a3539e2f8907cb65bfd9347dadf0ba62f6d1530bbc49927c347360a5a7f73c
9999a790749c583845a3ff7a265d540c2905aa12 travis: Rework CACHE_ERR_MSG (MarcoFalke)
Pull request description:
This should fix the misconception that we can fix build problems in forked repos. See https://github.com/bitcoin/bitcoin/pull/17418#issuecomment-551917920
Top commit has no ACKs.
Tree-SHA512: e0c86f6613fa38f4d4984420fa1fdc999829407c71024c3b92fd1d35afa98f4b5073af72656c83f607d5f6fab15f9ea629ca8be0b4e892c2fb65a226036c470a
facc0da63a8fa4bd6fc2782cbe92eb9f920f2256 travis: Run unit and functional tests on native arm (MarcoFalke)
fafa064d2a8dbe24303545ab582ec84cde52ab5b ci: Remove ccache requirement on the host (MarcoFalke)
Pull request description:
This keeps the cross-compilation to make it easy to run the ci on non-arm hardware. To run this locally in qemu-user as it used to be, just `export QEMU_USER_CMD="qemu-arm -L /usr/arm-linux-gnueabihf/"`.
ACKs for top commit:
laanwj:
LGTM ACK facc0da63a8fa4bd6fc2782cbe92eb9f920f2256
practicalswift:
ACK facc0da63a8fa4bd6fc2782cbe92eb9f920f2256 -- diff looks correct and Travis seems happy
Tree-SHA512: 0dc1bc82eb93e2bd8b159e044f20fe3055f8cdfd73aaa238bd2e178397582144dfc0c6a87bd8270115dafea1a623e642bde5d5f30254f94140f1a2cdb12fc2da
fa677d1801fb9153a95a1fc9855fd5f21fc440c0 ci: Remove redundant check for TRAVIS_OS_NAME (MarcoFalke)
fadccb263baf6b8694f750623add42f966e423a3 doc: Document that GNU tools are required for linters (MarcoFalke)
4444704ca9f66cdc24ab2d444941354db1dfed06 ci: Cleanup macOS runs (MarcoFalke)
Pull request description:
* Remove a commented out cleanup task in `before_cache`
* Remove the linter run on macOS, and document that GNU tools are required to run the linters
ACKs for top commit:
Sjors:
Code review ACK fa677d1801fb9153a95a1fc9855fd5f21fc440c0
laanwj:
ACK fa677d1801fb9153a95a1fc9855fd5f21fc440c0
ryanofsky:
Code review ACK fa677d1801fb9153a95a1fc9855fd5f21fc440c0 for new third commit replacing TRAVIS_OS_NAME check with NO_DEPENDS setting
Tree-SHA512: 9122a63bbe7887d9e379123152ea4ba44324cb18033b9e6b45bfdb1af665c10ea598564b9fcd57330d208a08e4696e41b4d6175f05f0843a3a76530da114f8c6
da1f153e5e260f1744ee1bf4f24ca3a74ffea465 Add s390x tests to travis (Elichai Turkel)
2fa65e0de94f01d502e8ace89be3c5dc963dd764 Add ci script to install on s390x (Elichai Turkel)
Pull request description:
Discovered this as part of #17402 and a conversation with gmaxwell.
You can see here that the platform is indeed BE: https://travis-ci.org/elichai/bitcoin/jobs/616656410#L36
This closes https://github.com/bitcoin/bitcoin/issues/6466
ACKs for top commit:
MarcoFalke:
ACK da1f153e5e260f1744ee1bf4f24ca3a74ffea465
Tree-SHA512: e7e94e54e220257d91b24fddc79eab2bcaaadf0b2d1e7e6872d9757808ab2541728f00b1f3ab7e343305c0e7d91bb48a17a3f9621f6fff6c9fe6cde6682de408
119d360aab travis: Document whether functional tests are run in the job name (Ben Woosley)
64f28545e3 Revert "travis: Compile trusty with depends for now" (Ben Woosley)
267eac00f9 Prefer boost::optional#get_value_or over #value_or (Ben Woosley)
1971f5ba04 Piecewise construct to avoid invalid construction (Ben Woosley)
Pull request description:
In light of #14979, I realized that only qt 5.5+ was being tested under CI, while compatibility lists 5.2+.
In #15276, Marco added Trusty to CI, building with depends. This changes that build to system libraries, in order to ensure ongoing compatibility with our claimed minimum required versions.
Fixes#14983, previously open as #14998
Tree-SHA512: 6cff5e28c756ecb8bf797c8f6eb77c1944ba61a8dd6d7d4984e63eef384f6429dc79c505da3241c05b9c4db31c72b2a9846c7365aba9280f2e0620e5f3998d07
84edfc72e5eba3dde824ebd0626e97929a0b1bca Update doc and CI config (qmma)
48bcb2ac249e0e666ce638bb29124558b3283c16 Disable other targets when enable-fuzz is set (qmma)
Pull request description:
This is to fix https://github.com/bitcoin/bitcoin/issues/16094
When the `enable-fuzz` flag is set, disable all other binary targets.
ACKs for top commit:
MarcoFalke:
ACK 84edfc72e5eba3dde824ebd0626e97929a0b1bca (only checked that travis compiled this)
Tree-SHA512: f4ac80526388a67709986b22de88b00bf93ab44ae31a20bd4d8923a4982ab97e015a9f13010081d6ecf6c23ae8afeac7ca9d849d198ce6ebe239aa3127151efc
c4606b84329d760d7cee144bebe05807857edaae Add Travis check for single parameter constructors not marked "explicit" (practicalswift)
Pull request description:
Make single parameter constructors `explicit` (C++11).
Rationale from the developer notes:
> - By default, declare single-argument constructors `explicit`.
> - *Rationale*: This is a precaution to avoid unintended conversions that might
> arise when single-argument constructors are used as implicit conversion
> functions.
ACKs for top commit:
laanwj:
ACK c4606b84329d760d7cee144bebe05807857edaae
Tree-SHA512: 3e6fd51935fd93b2604b2188664692973d0897469f814cd745b5147d71b99ea5d73c1081cfde9f6393f51f56969e412fcda35d2d54e938a3235b8d40945f31fd
fad0ce59e9 tests: Fail if RPC has been added without tests (MarcoFalke)
Pull request description:
Need to be run with --coverage
ACKs for commit fad0ce:
ryanofsky:
utACK fad0ce59e9154f9b7e61907a71c740a942c60282. New comment in travis.yml is the only change since last review.
Tree-SHA512: b53632dfe9865ec06991bfcba2fd67238bebbb866b355f09624eaf233257b2bca902caac6c24abb358b2f4c1c43f28ca75e30982765911e1a117102df65276d9
faa4043c66 qa: Run more tests with wallet disabled (MarcoFalke)
Pull request description:
Instead of skipping the whole test, only skip the wallet specific section of a test if the wallet is not compiled in. This is mostly an indentation change, so can be reviewed with `--ignore-all-space`.
Tree-SHA512: 5941a8b6b00dca5cf9438c5f6f010ba812115188a69e427d7ade4c1ab8cfe7a57c73daf52c66235dbb24b1cd9ab7c7a17c49bc23d931e041b605d79116a71f66
Merge #14324: qa: Run more tests with wallet disabled
faa4043c66 qa: Run more tests with wallet disabled (MarcoFalke)
Pull request description:
Instead of skipping the whole test, only skip the wallet specific section of a test if the wallet is not compiled in. This is mostly an indentation change, so can be reviewed with `--ignore-all-space`.
Tree-SHA512: 5941a8b6b00dca5cf9438c5f6f010ba812115188a69e427d7ade4c1ab8cfe7a57c73daf52c66235dbb24b1cd9ab7c7a17c49bc23d931e041b605d79116a71f66
fae8b8bb1a qa: Add tool-prefix to functional test readme (MarcoFalke)
faf3d22725 test_runner: Remove unused --force option (MarcoFalke)
Pull request description:
When someone calls the script they already have all intention to call it, no need to specify a redundant `--force`.
The functional tests are still disabled on the travis windows cross builds, where they'd run into issues when run under Wine.
Tree-SHA512: ada0dd9b3c0cd28c5832a12c5e04c029dc3bfe5ddf366fd0abc24fb7914d2e0f0a873fe756ade7ba780a561abe9bc731838c289accc421deda481269e08514cd
