travis: WIP - build and run tests on os: osx #13816

scravy · 2018-07-31T13:14:42Z

This pull requests enables building and running the project with os: osx, sort of natively on os x instead of cross-compiling and not running the tests.

To achieve that the before_install, install, before_script, and script parts were externalized into .travis/before_install etc. These scripts exist for both linux (which runs everything in docker) and osx (which runs everything directly).

Having the steps externalized in individual scripts makes them more readable and subject to shellcheck (lint-shell.sh).

The shellcheck executable segfaults on me, which is why it's now being run via docker. This has a nice side effect, namely that a more recent version of shellcheck can be used. The runtime of the lint step is not affected to much by it (it's still around 1 minute as it was before).

The newer shellcheck also comes with more lint rules of which I disabled some as I did not want to touch too much stuff in this single pull request, which is primarily about a reorganization of the travis build.

scravy · 2018-07-31T13:25:14Z

A whitespace slipped in, removed that one (will squash these commits)

maflcko · 2018-07-31T13:29:39Z

.travis.yml

        GOAL="install"
        BITCOIN_CONFIG="--enable-glibc-back-compat --enable-reduce-exports"
-# Cross-Mac
-    - >-
-        HOST=x86_64-apple-darwin14


I'd prefer if we kept the cross compile on linux, since something similar is used for producing the release binaries.

I see. So it should cross-build on linux and for the purpose of this PR do the build on osx additionally, not instead of!

Yes, both would be fine with me.

ken2812221 · 2018-07-31T13:47:13Z

In my opinion, you could make it run on macos and linux both. Also, please remove the first two commits, e0d9191 and 3144b15.

promag

I also think also building on macos is good. The change is pretty big and so this is going to take a while to be merged. Maybe split in multiple PRs, like Replace travis matrix with jobs?

promag · 2018-07-31T14:11:18Z

test/lint/lint-shell.sh

-shellcheck -e SC2001,SC2004,SC2005,SC2006,SC2016,SC2028,SC2046,SC2048,SC2066,SC2086,SC2116,SC2148,SC2162,SC2166,SC2181 \
-    $(git ls-files -- "*.sh" | grep -vE 'src/(secp256k1|univalue)/')
+
+docker run -t -v $(pwd):/code -t koalaman/shellcheck-alpine:v0.5.0 \


NACK this change. Skip in macos for now?

@promag I am unsure about what you mean. This part does not have anything to do with macos. I'd happily factor this one out into a separate pull request.

The lint step is executed in linux/trusty and uses a shellcheck that comes with the distro. It kept segfaulting for me (in travis, not on macos), so I am using a different version which runs in docker and has a different (and more modern) executable in it.

This runs on my mac locally as well as in a linux build in travis equally.

Please don't introduce a Docker dependency. If shellcheck does not run on Mac then just skip it.

It does run on Mac. I did not introduce this step because it does not run on mac (it does) – as can be seen in #13728 ). For some reason it does segfault though in the os: linux/dist: trusty build.

Here is a build where it segfaults: https://travis-ci.org/scravy/bitcoin/jobs/410232096#L623

@practicalswift What is the problem with a docker dependency? To me it seems like the shellcheck binary in ubuntu trusty is broken and segfaults under certain conditions. Here is an issue: koalaman/shellcheck#1053 The docker image pulled in is the official build of shellcheck.

For me to understand: Introducing a dependency for example using brew on osx - is that okay or not?

@scravy With the suggested change everyone running the linter would effectively be vulnerable if the koalaman Docker Hub account is compromised.

So a clear NACK from me on this specific change. Try to find a more lightweight workaround that doesn't introduce any additional trust or dependencies.

You could run all the linters in a docker on travis, if that helps?

@MarcoFalke Running all the linters in a docker container sounds intriguing to me. I might tackle that in a separate pull request. Simply to pull in more recent versions.

IMO docker should not be a developer dependency and so running linters should also be possible without docker.

DrahtBot · 2018-07-31T15:13:35Z

Reviewers, this pull request conflicts with the following ones:

lint: Run the CI lint stage on mac #13728 (Scripts and tools: Run the CI lint stage on mac by Empact)
Build previous releases and run functional tests #12134 (Build previous releases and run functional tests by Sjors)

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.

maflcko · 2018-07-31T16:40:54Z

src/chainparams.h

-    double dTxRate;
+    int64_t nTime;    //!< UNIX timestamp of last known number of transactions
+    int64_t nTxCount; //!< total number of transactions between genesis and that timestamp
+    double dTxRate;   //!< estimated number of transactions per second after that timestamp


Please rebase on master to get rid of these changes.

Sjors · 2018-07-31T17:56:11Z

Concept ACK, I encounter macOS specific problems quite often.

+1 for keeping the cross-compile version around

There's also some difference in compiler noisiness between using depends and not using depends, so it might be worth running make for both (but not the full test suite). If you keep (homebrew) system dependencies and Xcode versions up to date we'll also notice if those start causing problems. Xcode updates have a habit of breaking builds.

practicalswift · 2018-08-01T14:39:57Z

Concept ACK assuming the Docker dependency added in test/lint/lint-shell.sh is removed :-)

Linting should not require Docker :-)

scravy · 2018-08-01T14:59:42Z

I did remove the docker dependency and am curious on whether the build will fail now and shellcheck segfault.

I seem to not have communicated this properly: The lint step is run using os: linux just as before. shellcheck segfaults for me in travis, not on a mac. There is a PR which makes the lint stage also run on osx ( #13728 ).

Looking at this now I am wondering why the shellcheck segfaults on this branch but does not (also in linux) on that branch.

scravy · 2018-08-01T15:20:45Z

The lint step indeed failed now. I printed uname -a:

16.14s$ test/lint/lint-all.sh
Linux travis-job-3b3edecb-cf13-406f-8e5d-e8031fd1d16d 4.4.0-101-generic #124~14.04.1-Ubuntu SMP Fri Nov 10 19:05:36 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
test/lint/lint-shell.sh: line 33:  8542 Segmentation fault      (core dumped) shellcheck -e SC1087,SC1117,SC2001,SC2004,SC2005,SC2006,SC2016,SC2028,SC2046,SC2048,SC2066,SC2086,SC2116,SC2148,SC2162,SC2166,SC2181,SC2206,SC2207,SC2230 $(git ls-files '*.sh' | grep -vE 'src/(secp256k1|univalue)/')
^---- failure generated from test/lint/lint-shell.sh
The command "test/lint/lint-all.sh" exited with 1.

I will try to install a more recent version of shellcheck via apt.

practicalswift · 2018-08-01T15:27:45Z

@scravy Are we relying on Travis' pre-installed shellcheck version? Perhaps the output of these can help:

$ dpkg -S $(which shellcheck)
$ dpkg -s shellcheck
$ shellcheck --version

maflcko · 2018-08-01T16:19:32Z

re shellcheck on travis:

If you want to install it with apt, you'd have to set sudo: true, but then you might as well run the linter in some of the minimal docker images?
If you want to install it with the apt extension on sudo: false, it is just as broken as the version that is already installed, so no need to add the extension

Wouldn't it work if you just left it the way it was run before?

maflcko · 2018-08-01T16:23:31Z

It seems that you extract the travis.yml into bash scripts. Imo it would help review to split this specific change into a separate commit/pull (for just linux).

ken2812221 · 2018-08-02T07:52:19Z

.travis.yml

      language: python
      python: '3.6'
      install:
        - travis_retry pip install flake8
-        - travis_retry apt-get install shellcheck -y -q
+        - echo $USER
+        - travis_retry sudo apt-get install shellcheck -y -q


This does not require sudo if you use

addons: apt: packages: - shellcheck

practicalswift · 2018-09-11T14:15:48Z

.travis/test_06_script.sh

+
+echo "configure"
+BEGIN_FOLD configure
+../configure --cache-file=config.cache $BITCOIN_CONFIG_ALL $BITCOIN_CONFIG || ( cat config.log && false)


~~Double quote to prevent globbing and word splitting. Same below. Fix throughout :-)~~

~~Verify with shellcheck .travis/test_06_script.sh | grep SC2086.~~

I think the config arguments should be split off. No?

Oh, sorry. Was a bit too quick with that comment :-)

maflcko · 2018-10-26T15:40:57Z

Should be marked "up for grabs"?

83c48d9 fix locale for lint-shell (Julian Fleischer) Pull request description: A piece of code from bitcoin#13816 which I am hereby splitting into smaller PRs. The `shellcheck` executable shipped with travis's trusty linux environment (contains shellcheck `0.3.1` in `/usr/local/bin` as opposed to the distros `0.3.3` in `/usr/bin`) segfaults when `LC_ALL=C`. This makes sure that in travis, no matter from where the script is called, `LC_ALL` is left unset. Comment changed accordingly. Tree-SHA512: 86afa9247f2adbeefa75bf3d56a94766f8e8e1839f40b73763ff7b893a09c848ee64648fc06ce3e6bd0f650127365f508b37fdefb48d61e49f5d551c074cb16e

fanquake · 2021-03-02T08:10:18Z

Removing "Up for grabs" as we have left Travis and have a "native" macOS build via Cirrus.

…s/` subject to shellcheck 4143269 use export LC_ALL=C.UTF-8 (Julian Fleischer) 728c82d make script exit if a command fails (Julian Fleischer) 506890b move remaining travis build steps into individual files (Julian Fleischer) 272306e number .travis/ script according to build lifecycle and add README to explain (Julian Fleischer) 519e273 move lint stage up to resemble travis build ui (Julian Fleischer) 86d34f0 abort script in END_FOLD on non-zero exit code (Julian Fleischer) 4f2f88c move script sections info individual files and comply with shellcheck (Julian Fleischer) Pull request description: This PR is extracted from bitcoin#13816 to make that one easier to review. It follows on bitcoin#13849 and bitcoin#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. Tree-SHA512: c497e1687ceb1c1d795de177d3fc35af908bc8e3f781a871afabdecf031e581d4db229290627249e35ef7c09952bc34884e4734ea91d40f57b4a9efb85bba2e3 # Conflicts: # .travis.yml # test/lint/lint-shell-locale.sh

…s/` subject to shellcheck 4143269 use export LC_ALL=C.UTF-8 (Julian Fleischer) 728c82d make script exit if a command fails (Julian Fleischer) 506890b move remaining travis build steps into individual files (Julian Fleischer) 272306e number .travis/ script according to build lifecycle and add README to explain (Julian Fleischer) 519e273 move lint stage up to resemble travis build ui (Julian Fleischer) 86d34f0 abort script in END_FOLD on non-zero exit code (Julian Fleischer) 4f2f88c move script sections info individual files and comply with shellcheck (Julian Fleischer) Pull request description: This PR is extracted from bitcoin#13816 to make that one easier to review. It follows on bitcoin#13849 and bitcoin#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. Tree-SHA512: c497e1687ceb1c1d795de177d3fc35af908bc8e3f781a871afabdecf031e581d4db229290627249e35ef7c09952bc34884e4734ea91d40f57b4a9efb85bba2e3 # Conflicts: # .travis.yml # .travis/test_06_script.sh # test/lint/lint-shell-locale.sh

… `.travis/` subject to shellcheck 4143269 use export LC_ALL=C.UTF-8 (Julian Fleischer) 728c82d make script exit if a command fails (Julian Fleischer) 506890b move remaining travis build steps into individual files (Julian Fleischer) 272306e number .travis/ script according to build lifecycle and add README to explain (Julian Fleischer) 519e273 move lint stage up to resemble travis build ui (Julian Fleischer) 86d34f0 abort script in END_FOLD on non-zero exit code (Julian Fleischer) 4f2f88c move script sections info individual files and comply with shellcheck (Julian Fleischer) Pull request description: This PR is extracted from bitcoin#13816 to make that one easier to review. It follows on bitcoin#13849 and bitcoin#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. Tree-SHA512: c497e1687ceb1c1d795de177d3fc35af908bc8e3f781a871afabdecf031e581d4db229290627249e35ef7c09952bc34884e4734ea91d40f57b4a9efb85bba2e3

…s/` subject to shellcheck 4143269 use export LC_ALL=C.UTF-8 (Julian Fleischer) 728c82d make script exit if a command fails (Julian Fleischer) 506890b move remaining travis build steps into individual files (Julian Fleischer) 272306e number .travis/ script according to build lifecycle and add README to explain (Julian Fleischer) 519e273 move lint stage up to resemble travis build ui (Julian Fleischer) 86d34f0 abort script in END_FOLD on non-zero exit code (Julian Fleischer) 4f2f88c move script sections info individual files and comply with shellcheck (Julian Fleischer) Pull request description: This PR is extracted from bitcoin#13816 to make that one easier to review. It follows on bitcoin#13849 and bitcoin#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. Tree-SHA512: c497e1687ceb1c1d795de177d3fc35af908bc8e3f781a871afabdecf031e581d4db229290627249e35ef7c09952bc34884e4734ea91d40f57b4a9efb85bba2e3 # Conflicts: # .travis.yml # .travis/test_06_script.sh # test/lint/lint-shell-locale.sh

… `.travis/` subject to shellcheck 4143269 use export LC_ALL=C.UTF-8 (Julian Fleischer) 728c82d make script exit if a command fails (Julian Fleischer) 506890b move remaining travis build steps into individual files (Julian Fleischer) 272306e number .travis/ script according to build lifecycle and add README to explain (Julian Fleischer) 519e273 move lint stage up to resemble travis build ui (Julian Fleischer) 86d34f0 abort script in END_FOLD on non-zero exit code (Julian Fleischer) 4f2f88c move script sections info individual files and comply with shellcheck (Julian Fleischer) Pull request description: This PR is extracted from bitcoin#13816 to make that one easier to review. It follows on bitcoin#13849 and bitcoin#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. Tree-SHA512: c497e1687ceb1c1d795de177d3fc35af908bc8e3f781a871afabdecf031e581d4db229290627249e35ef7c09952bc34884e4734ea91d40f57b4a9efb85bba2e3

…s/` subject to shellcheck 4143269 use export LC_ALL=C.UTF-8 (Julian Fleischer) 728c82d make script exit if a command fails (Julian Fleischer) 506890b move remaining travis build steps into individual files (Julian Fleischer) 272306e number .travis/ script according to build lifecycle and add README to explain (Julian Fleischer) 519e273 move lint stage up to resemble travis build ui (Julian Fleischer) 86d34f0 abort script in END_FOLD on non-zero exit code (Julian Fleischer) 4f2f88c move script sections info individual files and comply with shellcheck (Julian Fleischer) Pull request description: This PR is extracted from bitcoin#13816 to make that one easier to review. It follows on bitcoin#13849 and bitcoin#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. Tree-SHA512: c497e1687ceb1c1d795de177d3fc35af908bc8e3f781a871afabdecf031e581d4db229290627249e35ef7c09952bc34884e4734ea91d40f57b4a9efb85bba2e3

… `.travis/` subject to shellcheck 4143269 use export LC_ALL=C.UTF-8 (Julian Fleischer) 728c82d make script exit if a command fails (Julian Fleischer) 506890b move remaining travis build steps into individual files (Julian Fleischer) 272306e number .travis/ script according to build lifecycle and add README to explain (Julian Fleischer) 519e273 move lint stage up to resemble travis build ui (Julian Fleischer) 86d34f0 abort script in END_FOLD on non-zero exit code (Julian Fleischer) 4f2f88c move script sections info individual files and comply with shellcheck (Julian Fleischer) Pull request description: This PR is extracted from bitcoin#13816 to make that one easier to review. It follows on bitcoin#13849 and bitcoin#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. Tree-SHA512: c497e1687ceb1c1d795de177d3fc35af908bc8e3f781a871afabdecf031e581d4db229290627249e35ef7c09952bc34884e4734ea91d40f57b4a9efb85bba2e3

fanquake added the Tests label Jul 31, 2018

scravy force-pushed the run-functional-tests-on-macos branch from 4c65446 to aaab3b6 Compare July 31, 2018 13:25

maflcko reviewed Jul 31, 2018

View reviewed changes

scravy force-pushed the run-functional-tests-on-macos branch from aaab3b6 to a1a1e83 Compare July 31, 2018 13:30

promag reviewed Jul 31, 2018

View reviewed changes

This was referenced Jul 31, 2018

travis: Run bench_bitcoin once #13811

Merged

lint: Run the CI lint stage on mac #13728

Merged

maflcko reviewed Jul 31, 2018

View reviewed changes

DrahtBot mentioned this pull request Aug 1, 2018

travis: Enable qt for all jobs #13515

Merged

scravy force-pushed the run-functional-tests-on-macos branch 5 times, most recently from 390da7f to 781a8ba Compare August 1, 2018 13:44

DrahtBot mentioned this pull request Aug 1, 2018

[WIP] depends: Add native_nsis to support unicode #13827

Closed

1 task

scravy force-pushed the run-functional-tests-on-macos branch from 631bd8b to 227d4e9 Compare August 1, 2018 15:34

DrahtBot mentioned this pull request Aug 2, 2018

Include tinyformat as a subtree #13845

Closed

ken2812221 reviewed Aug 2, 2018

View reviewed changes

scravy force-pushed the run-functional-tests-on-macos branch 2 times, most recently from 4e810f0 to b0fb725 Compare August 2, 2018 08:37

practicalswift reviewed Sep 11, 2018

View reviewed changes

DrahtBot mentioned this pull request Sep 16, 2018

travis: Save cache even when build or test fail #14231

Merged

This was referenced Oct 19, 2018

Run tests on pull requests dtr-org/unit-e-project#32

Closed

Set up release pipeline dtr-org/unit-e-project#35

Closed

DrahtBot mentioned this pull request Oct 20, 2018

Build previous releases and run functional tests #12134

Merged

fanquake added the Up for grabs label Oct 28, 2018

fanquake closed this Oct 28, 2018

scravy mentioned this pull request Nov 22, 2018

Run a native macos build dtr-org/unit-e#282

Open

fanquake removed the Up for grabs label Mar 2, 2021

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022

travis: WIP - build and run tests on os: osx #13816

travis: WIP - build and run tests on os: osx #13816

Uh oh!

Conversation

scravy commented Jul 31, 2018

Uh oh!

scravy commented Jul 31, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ken2812221 commented Jul 31, 2018

Uh oh!

promag left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

practicalswift Aug 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sjors commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Aug 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scravy commented Aug 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scravy commented Aug 1, 2018

Uh oh!

practicalswift commented Aug 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Aug 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Aug 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

practicalswift Sep 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented Oct 26, 2018

Uh oh!

fanquake commented Mar 2, 2021

Uh oh!

Uh oh!

promag left a comment •

edited

Loading

practicalswift Aug 1, 2018 •

edited

Loading

DrahtBot commented Jul 31, 2018 •

edited

Loading

Sjors commented Jul 31, 2018 •

edited

Loading

practicalswift commented Aug 1, 2018 •

edited

Loading

scravy commented Aug 1, 2018 •

edited

Loading

practicalswift commented Aug 1, 2018 •

edited

Loading

maflcko commented Aug 1, 2018 •

edited

Loading

maflcko commented Aug 1, 2018 •

edited

Loading

practicalswift Sep 11, 2018 •

edited

Loading