Adds an additional Travis machine to run the functional test suite on native macOS
Homebrew is not particularly Travis compatible, but I found some useful hints here: https://discourse.brew.sh/t/best-practice-for-homebrew-on-travis-brew-update-is-5min-to-build-time/5215/11
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
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.
Are there travis tests which can proof it? Like your fork? But I generally like this idea as the current situation with tuxes on the side is a bit confusing
235 | + osx_image: xcode11 236 | + language: 237 | + - ruby 238 | + - python 239 | + python: 240 | + - "3.6.5"
Should this be 3.5.6 to match our minimum supported Python?
Concept ACK. Will take a look at the base PR first.
Travis is ignoring this PR and my branch atm.
Can you enable travis for your forked repository?
@emilengler I tried that, but it wasn't building there either. I'll wait until the previous PR is merged and then try again.
@Sjors Ok I will fork your repository, then remove it from the parent and try to enable travis then
@Sjors Doesn't work, take a look https://travis-ci.org/emilengler/bitcoin-sjors-test/requests. I also forked your repo here as a playgroud, I also invited you for commit access, will delete it once it is done.
6 | @@ -7,7 +7,7 @@ 7 | export LC_ALL=C.UTF-8 8 | 9 | export HOST=x86_64-apple-darwin14 10 | -export PACKAGES="cmake imagemagick libcap-dev librsvg2-bin libz-dev libbz2-dev libtiff-tools python3-dev python3-setuptools" 11 | +export BREW_PACKAGES="automake berkeley-db4 libtool boost miniupnpc openssl pkg-config protobuf qt qrencode librsvg pyenv"
why is this changed?
3 | @@ -4,6 +4,11 @@ 4 | # Distributed under the MIT software license, see the accompanying 5 | # file COPYING or http://www.opensource.org/licenses/mit-license.php. 6 | 7 | +if [ "$TRAVIS_OS_NAME" == "osx" ]; then 8 | + travis_retry brew install $BREW_PACKAGES
${CI_RETRY_EXE} brew install $PACKAGES
Concept ACK
3 | @@ -4,6 +4,11 @@ 4 | # Distributed under the MIT software license, see the accompanying 5 | # file COPYING or http://www.opensource.org/licenses/mit-license.php. 6 | 7 | +if [ "$TRAVIS_OS_NAME" == "osx" ]; then 8 | + ${CI_RETRY_EXE} brew install $BREW_PACKAGES 9 | + return 0 10 | + fi 11 | +
should move this below the LC_ALL line
164 | + - stage: test 165 | + name: 'macOS 10.14 native' 166 | + os: osx 167 | + # Use the most recent version: 168 | + # Xcode 11, macOS 10.14, JDK 12.0.1 169 | + # https://docs.travis-ci.com/user/reference/osx/#OS-X-Version
# https://docs.travis-ci.com/user/reference/osx/#macos-version
172 | + - ruby 173 | + - python 174 | + python: 175 | + - "3.5.6" 176 | + rvm: 177 | + - 2.6.0
This config doesn't boot (even after waiting an hour)
Maybe you need to install python via brew upgrade python?
Trying with pyenv install 3.5.6
That doesn't seem to boot either.
In def61133e18a4487af1c9406ffa3f6bf19b702fd I removed both of these and it fails with:
Warning: automake 1.16.1_1 is already installed and up-to-date
To reinstall 1.16.1_1, run `brew reinstall automake`
Warning: berkeley-db@4 4.8.30 is already installed and up-to-date
To reinstall 4.8.30, run `brew reinstall berkeley-db@4`
Warning: libtool 2.4.6_1 is already installed and up-to-date
To reinstall 2.4.6_1, run `brew reinstall libtool`
Warning: boost 1.71.0 is already installed and up-to-date
To reinstall 1.71.0, run `brew reinstall boost`
Warning: miniupnpc 2.1 is already installed and up-to-date
To reinstall 2.1, run `brew reinstall miniupnpc`
Warning: openssl 1.0.2t is already installed and up-to-date
To reinstall 1.0.2t, run `brew reinstall openssl`
Warning: pkg-config 0.29.2 is already installed and up-to-date
To reinstall 0.29.2, run `brew reinstall pkg-config`
Warning: qt 5.13.1 is already installed and up-to-date
To reinstall 5.13.1, run `brew reinstall qt`
Warning: qrencode 4.0.2 is already installed and up-to-date
To reinstall 4.0.2, run `brew reinstall qrencode`
Warning: librsvg 2.46.1 is already installed and up-to-date
To reinstall 2.46.1, run `brew reinstall librsvg`
Warning: pyenv 1.2.13_1 is already installed and up-to-date
To reinstall 1.2.13_1, run `brew reinstall pyenv`
Error: protobuf 3.9.1_1 is already installed
To upgrade to 3.10.0, run `brew upgrade protobuf`.
The command "brew install automake berkeley-db4 libtool boost miniupnpc openssl pkg-config protobuf qt qrencode librsvg pyenv" failed. Retrying, 3 of 3.
Oh boy, Homebrew is a pain. I lifted some stuff from: https://discourse.brew.sh/t/best-practice-for-homebrew-on-travis-brew-update-is-5min-to-build-time/5215/11 and https://github.com/vadi2/Mudlet/blob/857d1ae29fe6e55f1507399200c45db318b28ae6/CI/travis.osx.install.sh#L11-L35
@vadi2 @keneanung let me know if you prefer not to be mentioned as co-authored-by in the commit.
(assuming we ever get this to work :-)
It's fine! Let us know if we can help you out with this.
I'm good with this as well. As Vadim said, let us know if we can be of help.
Thanks! I could use some help with update 7 below.
Hm. I'm unsure whether we had this error... And since we don't use this deep caching anymore, we don't have any further experience in this dark corner of homebrew :ghost:
5ea77cd56d failed at travis_fold:end:autogen without any error. I added a check for OSX in order to run ./autogen.sh without DOCKER_EXEC; who knows...
Update: no luck, trying some other things. Apparently sometimes set -o errexit can cause Travis to exit before the log is updated with the last error.
Update2: it couldn't find OpenSSL. Continuing on a PR-less branch to avoid overloading Travis.
Update3: Travis macOS comes with OpenSSL 1.1 preinstalled via Homebrew. I removed openssl from the BREW_PACKAGES list, and added the appropriate PKG_CONFIG_PATH.
Update4: Got all the way to make deploy, but the Apple script just hangs. Switching goal to install. Caching homebrew and PyEnv seems to work, and save a lot of time.
Update5: There is something weird about the configure step that cause Travis to rage quit without information. I put set +o errexit around those places. Also, the output of make distdir doesn't look very healthy, though I get the same on my local machine, despite exit status 0. (fatal: current working directory is untracked).
Update6: It wasn't ./configure. It looks like this pattern is the culprit: cd build || (echo "could not enter build directory"; exit 1). Afaik this is redundant because all scripts are already called with set -o errexit; source. But removing the check doesn't help. Instead, I just wrapped it in set +o errexit. Clearly cd build is working, so I have no idea why Travis bails on that line.
Update 7: got all the way through the functional test suite. The code that tries to prune brew cache doesn't seem to behave, see error: find: -delete: /usr/local/Homebrew: relative path potentially not safe. I commented the line out to preserve my own sanity :-)
5 | +# file COPYING or http://www.opensource.org/licenses/mit-license.php. 6 | + 7 | +export LC_ALL=C.UTF-8 8 | + 9 | +export HOST=x86_64-apple-darwin14 10 | +export BREW_PACKAGES="automake berkeley-db4 libtool boost miniupnpc openssl pkg-config protobuf qt qrencode librsvg pyenv ccache"
No reason to install protobuf if we're not enabling BIP70.
Dropped (in other branch, will push later)
If we're building/testing only once natively, we should enable everything...
I added it back and enabled BIP70 in the build.
0 | @@ -0,0 +1,15 @@ 1 | +#!/usr/bin/env bash 2 | +# 3 | +# Copyright (c) 2019 The Bitcoin Core developers 4 | +# Distributed under the MIT software license, see the accompanying 5 | +# file COPYING or http://www.opensource.org/licenses/mit-license.php. 6 | + 7 | +export LC_ALL=C.UTF-8 8 | +
Even thought it'll be unused, could add NO_DEPENDS=1 here to make it clear that we aren't actually using depends.
Travis macOS passed! I added pip install zmq for the functional ZMQ tests...
pyenv was causing more headaches than it solved. There doesn't appear to be any `language: python python: '3.x'. packages available, so I'm installing via homebrew instead.
72 | @@ -26,7 +73,7 @@ elif [[ $BITCOIN_CONFIG = *--with-sanitizers=*address* ]]; then # If ran with (A 73 | DOCKER_ADMIN="--cap-add SYS_PTRACE" 74 | fi 75 | 76 | -if [ -z "$RUN_CI_ON_HOST" ]; then 77 | +if [ -z "$RUN_CI_ON_HOST" ] && [ "$TRAVIS_OS_NAME" != "osx" ]; then
I'd prefer if you set RUN_CI_ON_HOST=1 in the env file or travis.yml
42 | + else 43 | + echo "Retrying..." 44 | + fi 45 | + done 46 | + 47 | + done
homebrew :man_facepalming:
This can't be written shorter, right?
Probably, but I'm no bash-master. Homebrew doesn't have a "install or update if possible, don't complain otherwise" feature, because ... reason.
Could you just run brew update then brew install package-name --force for each package you're trying to install?
Reinstalling these packages at every Travis run seems very slow:
-f, --force
Remove a previously cached version and re-fetch.
I shortened it a bit; we don't really care if brew upgrade fails, and we can use CI_RETRY_EXE for install.
I'd be fine with a solution that works, but isn't the fastest. We can optimize for speed in a follow up pull request.
ACK 26068f31ad60c957ab7e6aef0a757fea4397da13 (looked at the diff on GitHub)
7 | @@ -8,6 +8,8 @@ export LC_ALL=C 8 | 9 | if [ "$TRAVIS_OS_NAME" == "osx" ]; then 10 | # update first to install required ruby dependency 11 | + cd /usr/local/Homebrew 12 | + git reset --hard origin/master
Why is the linter modified?
Oops, wrong one.
5 | @@ -6,6 +6,29 @@ 6 | 7 | export LC_ALL=C.UTF-8 8 | 9 | +if [ "$TRAVIS_OS_NAME" == "osx" ]; then 10 | + cd /usr/local/Homebrew || exit 1 11 | + git reset --hard origin/master 12 | + ${CI_RETRY_EXE} brew update
@MarcoFalke brew update blew up on a previous run (can't find it anymore) and recommended these two lines to "fix" it.
It might be worth rebasing the PR for a while to see if any other surprises show up.
5 | @@ -6,7 +6,10 @@ 6 | 7 | export LC_ALL=C.UTF-8 8 | 9 | +# Temporarily disable errexit, because Travis macOS fails without error message 10 | +set +o errexit
This is where your sanity gives in...
Review hint:
git show -w
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
Co-authored-by: keneanung <keneanung@googlemail.com>
Co-authored-by: Vadim Peretokin <vperetokin@gmail.com>
re-ACK 1f6c650c99eb94fdd0cbdd528aa3405201de08c2
5 | @@ -6,6 +6,32 @@ 6 | 7 | export LC_ALL=C.UTF-8 8 | 9 | +if [ "$TRAVIS_OS_NAME" == "osx" ]; then 10 | + set +o errexit 11 | + pushd /usr/local/Homebrew || exit 1 12 | + git reset --hard origin/master 13 | + popd || exit 1 14 | + set -o errexit
What happens if you remove this?
Nothing initially because the brew cache is in a good state. But at some random other moment homebrew will be in a bad state and you'll need this.
38 | + - $HOME/Library/Caches/Homebrew 39 | + - /usr/local/Homebrew 40 | +before_cache: 41 | + - if [ "${TRAVIS_OS_NAME}" = "osx" ]; then brew cleanup; fi 42 | + # Cache only .git files under "/usr/local/Homebrew" so "brew update" does not take 5min every build 43 | + # - if [ "${TRAVIS_OS_NAME}" = "osx" ]; then find /usr/local/Homebrew \! -regex ".+\.git.+" -delete; fi
Is this still needed?
The whole process takes 2 min without it: https://travis-ci.org/bitcoin/bitcoin/jobs/597848502#L100
I don't know if it builds up any cruft over time, but if time doesn't creep up, then we can drop this.
Well it is commented out already, so I removed it in #17176
19 | @@ -20,7 +20,11 @@ fi 20 | END_FOLD 21 | 22 | mkdir -p build 23 | + 24 | +# Temporarily disable errexit, because Travis macOS fails without error message 25 | +set +o errexit 26 | cd build || (echo "could not enter build directory"; exit 1) 27 | +set -o errexit
The cd command succeeds and returns an error code? I guess this is calling some travis-wrapped cd alias rather than the actual cd command? Could debug with type cd. If a workaround is necessary, it would seem nicer to wrap the workaround in a safe_cd function than repeat this errexit stuff different places.
Filed as good first issue. See #17178
13 | +else 14 | + DOCKER_EXEC echo \> \$HOME/.bitcoin 15 | +fi 16 | 17 | -mkdir -p depends/SDKs depends/sdk-sources 18 | +if [ "$TRAVIS_OS_NAME" != "osx" ]; then
Maybe should add a comment explaining this check. I'm confused about why all of this is skipped on osx. Does the depends build not work there? It would seem cleaner to use NO_DEPENDS variable to skip depends steps than to tie the decision to the travis operating system name.
Thanks for the review @ryanofsky . This check looks indeed redundant. Removed in #17176
Post merge code review ACK 1f6c650c99eb94fdd0cbdd528aa3405201de08c2
Since this adds homebrew, I wonder if it fixes the scripted diff lint error I ran into #13728 (comment)