lint: Run the CI lint stage on mac

Empact commented at 10:35 pm on July 20, 2018: member

This helps ensure ongoing compatibility with macOS-distributed version of GNU bash.

Empact commented at 10:35 pm on July 20, 2018: member

Inspired by and built on #13720

Empact renamed this:
~~Run the CI lint stage on mac and linux both~~
Scripts and tools: Run the CI lint stage on mac and linux both
on Jul 20, 2018

fanquake added the label Scripts and tools on Jul 21, 2018

in .travis.yml:115 in 052ed45193 outdated

109@@ -105,3 +110,34 @@ jobs:
110               while read LINE; do travis_retry gpg --keyserver hkp://subset.pool.sks-keyservers.net --recv-keys $LINE; done < contrib/verify-commits/trusted-keys &&
111               travis_wait 50 contrib/verify-commits/verify-commits.py;
112           fi
113+    - stage: lint BSD
114+      os: osx
115+      # Use macOS consistent with the minimum supported version: 10.11

fanquake commented at 4:38 am on July 21, 2018:

If you mean our next supported minimum macOS version, shouldn’t this be 10.10?

Empact commented at 5:06 pm on July 21, 2018:

Thanks - was relying on faulty memory. Also it happens to be the earliest version supported by Travis. Updated. #13617

Empact force-pushed on Jul 21, 2018

practicalswift commented at 5:59 pm on July 21, 2018: contributor

Concept ACK

Nit: macOS ships with GNU bash, albeit an ancient version. Please change “helps ensure ongoing compatibility with BSD bash as well as GNU bash” to something along the lines of “helps ensure ongoing compatibility with the macOS BSD user land and the ancient version of GNU bash shipped with macOS”.

Empact force-pushed on Jul 21, 2018

sipa commented at 9:10 pm on July 21, 2018: member

0/usr/local/Homebrew/Library/Homebrew/brew.rb:12:in `<main>': Homebrew must be run under Ruby 2.3! You're running 2.0.0. (RuntimeError)

Empact force-pushed on Jul 21, 2018

Empact commented at 1:07 am on July 22, 2018: member

~wip: I’m rolling back to 10.10~

DrahtBot commented at 1:18 pm on July 22, 2018: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

Empact force-pushed on Jul 24, 2018

MarcoFalke commented at 12:50 pm on July 24, 2018: member

Might as well do a full depends and bitcoind build when pulling in macos anyway?

Empact force-pushed on Jul 28, 2018

in .travis.yml:5 in 3d8c65470c outdated

0@@ -1,6 +1,8 @@
1 sudo: required
2 dist: trusty
3-os: linux
4+os:
5+  - linux
6+  - osx

scravy commented at 2:00 pm on August 1, 2018:

AFAIK it is not required to enumerate osx here.

in .travis.yml:16 in 3d8c65470c outdated

12@@ -11,6 +13,7 @@ cache:
13 stages:
14   - lint
15   - test
16+  - lint macOS

scravy commented at 2:01 pm on August 1, 2018:

you can use the same stage lint - it will have this stage with two jobs, one in linux, one in macOS

in .travis.yml:88 in 3d8c65470c outdated

83@@ -81,6 +84,8 @@ after_script:
84     - echo $TRAVIS_COMMIT_LOG
85 
86 jobs:
87+  exclude:
88+    - os: osx

scravy commented at 2:01 pm on August 1, 2018:

this would not be required either if not mentioning it in the os section globally

in .travis.yml:57 in 3d8c65470c outdated

109@@ -105,3 +110,41 @@ jobs:
110               while read LINE; do travis_retry gpg --keyserver hkp://subset.pool.sks-keyservers.net --recv-keys $LINE; done < contrib/verify-commits/trusted-keys &&
111               travis_wait 50 contrib/verify-commits/verify-commits.py;
112           fi
113+    - stage: lint macOS
114+      os: osx

scravy commented at 2:01 pm on August 1, 2018:

this osx suffices

in .travis.yml:113 in 3d8c65470c outdated

109@@ -105,3 +110,41 @@ jobs:
110               while read LINE; do travis_retry gpg --keyserver hkp://subset.pool.sks-keyservers.net --recv-keys $LINE; done < contrib/verify-commits/trusted-keys &&
111               travis_wait 50 contrib/verify-commits/verify-commits.py;
112           fi
113+    - stage: lint macOS

scravy commented at 2:02 pm on August 1, 2018:

this could be lint as mentioned above

scravy changes_requested

scravy commented at 2:04 pm on August 1, 2018: contributor

I very much like these changes - love the --exclude change, the -e made me go to the documentation the first time I came across it and I wished it was a more self-explanatory cli switch :-)

Empact force-pushed on Aug 1, 2018

scravy commented at 3:41 pm on August 1, 2018: contributor

ACK d5d76655105d6d338dbc367ae0b54b7433ffeb38

scravy approved

practicalswift commented at 3:31 pm on August 6, 2018: contributor

utACK d5d7665

Closing my PR #13720 in favour of this one.

MarcoFalke commented at 11:56 am on August 27, 2018: member

There are some non-travis related changes here, which should be submitted independently, imo.

I.e. all commits to b24ee7680cd1b5932cd3092113bcef2fe2db5d19 might go to a separate pull.

DrahtBot added the label Needs rebase on Aug 27, 2018

Empact force-pushed on Aug 27, 2018

Empact commented at 5:21 pm on August 27, 2018: member

@MarcoFalke FYI those changes are required to make the lint stage pass when also run on Mac, so I included them to make this PR green. Will extract a second PR for them in case that’s helpful.

DrahtBot removed the label Needs rebase on Aug 27, 2018

Empact force-pushed on Aug 27, 2018

Empact commented at 6:47 pm on August 27, 2018: member

~On second thought I instead set up CI runs against this PR with each of the prior commits excluded, to demonstrate the associated failure: sans 4874450 / empty subexpressions: https://travis-ci.org/Empact/bitcoin/builds/421218998 sans 28d520b / SC1087: https://travis-ci.org/Empact/bitcoin/builds/421219652 sans a442c33 / SC1117: https://travis-ci.org/Empact/bitcoin/jobs/421201432 sans 21ff32e / SC2206: https://travis-ci.org/Empact/bitcoin/builds/421219244 sans 2917ecd / SC2207: https://travis-ci.org/Empact/bitcoin/builds/421220532 sans f65efb2 / SC2230: https://travis-ci.org/Empact/bitcoin/builds/421219961~

DrahtBot added the label Needs rebase on Sep 5, 2018

Empact force-pushed on Sep 15, 2018

Empact commented at 4:53 pm on September 15, 2018: member

Rebased for #14115. This now only adds the mac-based lint run to CI.

Empact force-pushed on Sep 15, 2018

ken2812221 commented at 5:54 pm on September 15, 2018: contributor

utACK 453daa0. Maybe put it in test stage. It takes 3 minutes to boot and 4 minutes to run the script.

practicalswift commented at 6:06 pm on September 15, 2018: contributor

utACK 453daa0e24ae441175f413dd03460417eb4f4ee5

Will be great to have this in Travis!

nit: Remove Linux in PR title? :-)

DrahtBot removed the label Needs rebase on Sep 15, 2018

Empact force-pushed on Sep 16, 2018

Empact commented at 11:14 pm on September 16, 2018: member

Moved it into a 3rd/final “Compat” stage. Sound good?

in .travis.yml:147 in 949581e17e outdated

142+      # Xcode 7.3.1, macOS 10.11, JDK 1.8.0_92-b14
143+      # Earlier versions failed to make and install ghc for shellcheck
144+      # https://docs.travis-ci.com/user/reference/osx/#OS-X-Version
145+      osx_image: xcode7.3
146+      # TODO: if you're updating osx_image, try using rvm: to supply the version of ruby
147+      # required by homebrew. Could not get this working with xcode7.3 but believe it should

scravy commented at 6:52 am on September 17, 2018:

It says osx_image: xcode7.3 and at the same time could not get this working with xcode7.3

Empact commented at 4:21 am on September 19, 2018:

That’s correct - I could not get the rvm: 2.3.7 setting working in the version used here, so had to use brew update in the install script. Updated the comment to clarify what “this” is.

Empact force-pushed on Sep 19, 2018

Empact force-pushed on Sep 25, 2018

MarcoFalke commented at 3:41 pm on October 26, 2018: member

Should be marked “up for grabs”?

Empact renamed this:
~~Scripts and tools: Run the CI lint stage on mac and linux both~~
Scripts and tools: Run the CI lint stage on mac
on Oct 27, 2018

Empact force-pushed on Oct 27, 2018

fanquake commented at 9:30 am on October 28, 2018: member

@Empact Looks like you’ve rebased since Marco’s comment? Travis had errored out, so I restarted, but it now seems to be stalled while installing dependencies with brew?

Empact force-pushed on Oct 28, 2018

Empact force-pushed on Oct 29, 2018

in .travis/lint_04_install.sh:13 in cc6ececad2 outdated

10-travis_retry pip install flake8==3.5.0
11+if [ "$TRAVIS_OS_NAME" == "osx" ]; then
12+
13+# update first to install required ruby dependency
14+travis_retry brew update
15+travis_retry brew install

MarcoFalke commented at 4:58 pm on November 7, 2018:

This command requires a formula

Empact commented at 0:24 am on November 9, 2018:

Yeah, sorry this is WIP. I pulled the brew install command from some defunct docs I think.

DrahtBot added the label Needs rebase on Nov 7, 2018

Empact force-pushed on Nov 9, 2018

Empact renamed this:
~~Scripts and tools: Run the CI lint stage on mac~~
WIP: Scripts and tools: Run the CI lint stage on mac
on Nov 9, 2018

DrahtBot removed the label Needs rebase on Nov 9, 2018

DrahtBot added the label Needs rebase on Jan 17, 2019

Empact force-pushed on Jan 17, 2019

DrahtBot removed the label Needs rebase on Jan 17, 2019

Empact commented at 4:27 pm on June 9, 2019: member

@fanquake FYI I’m picking this back up.

Empact force-pushed on Jun 9, 2019

Empact commented at 5:03 pm on June 9, 2019: member

~Currently based on #15257 in order to give myself a clean build locally. Happy to base otherwise.~

Empact commented at 5:43 pm on June 9, 2019: member

~Outstanding work: need to install git with USE_LIBPCRE~

~fatal: cannot use Perl-compatible regexes when not compiled with USE_LIBPCRE~

(done)

Empact force-pushed on Jun 10, 2019

Empact force-pushed on Jun 11, 2019

Empact renamed this:
~~WIP: Scripts and tools: Run the CI lint stage on mac~~
lint: Run the CI lint stage on mac
on Jun 11, 2019

Empact force-pushed on Jun 11, 2019

Empact force-pushed on Jun 14, 2019

Empact force-pushed on Jun 17, 2019

Empact force-pushed on Jun 22, 2019

DrahtBot added the label Needs rebase on Jul 8, 2019

Empact force-pushed on Jul 8, 2019

DrahtBot removed the label Needs rebase on Jul 9, 2019

Sjors commented at 4:55 pm on August 13, 2019: member

ACK eefbc9d. Looking forward to seeing more stuff tested on the macOS native build.

in .travis.yml:107 in eefbc9d8a3 outdated

113+      language: ruby
114+      rvm:
115+        - 2.3.7
116+      # clear default env
117+      env:
118+      sudo: false

fanquake commented at 8:24 am on August 14, 2019:

I’m pretty sure you can drop sudo here.

in .travis/lint_04_install.sh:12 in eefbc9d8a3 outdated

10-travis_retry pip install flake8==3.5.0
11-travis_retry pip install vulture==0.29
12+if [ "$TRAVIS_OS_NAME" == "osx" ]; then
13+  # update first to install required ruby dependency
14+  travis_retry brew update
15+  travis_retry brew reinstall git -- --with-pcre2 # for  --perl-regexp

fanquake commented at 8:32 am on August 14, 2019:

I don’t think --with-pcre2 is a git install option anymore? Or is this being passed because we’re using an old version of brew or something?

Empact commented at 2:46 pm on October 9, 2019:

This build demonstrates --with-pcre2 is needed to run git grep --perl-regexp in test/link/check-doc.py.

fanquake commented at 8:34 am on August 14, 2019: member

I feel like I’ve Concept ACK’d this already, but re-Concept ACK anyways. Left a couple comments inline. It might be handy to base this off #16582, to make it easier to test.

DrahtBot added the label Needs rebase on Aug 15, 2019

Empact force-pushed on Sep 7, 2019

DrahtBot removed the label Needs rebase on Sep 7, 2019

Empact force-pushed on Sep 9, 2019

DrahtBot commented at 5:20 pm on September 18, 2019: member

DrahtBot added the label Needs rebase on Sep 18, 2019

laanwj added the label Waiting for author on Oct 8, 2019

Empact force-pushed on Oct 9, 2019

fanquake removed the label Needs rebase on Oct 9, 2019

fanquake removed the label Waiting for author on Oct 9, 2019

Empact force-pushed on Oct 9, 2019

in ci/lint/04_install.sh:21 in 27acd374c8 outdated

15+  travis_retry brew upgrade python
16+  PATH="$(brew --prefix python)/bin:$PATH"
17+  export PATH
18+else
19+  SHELLCHECK_VERSION=v0.6.0
20+  curl -s "https://storage.googleapis.com/shellcheck/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" | tar --xz -xf - --directory /tmp/

jonasschnelli commented at 1:56 pm on October 9, 2019:

Probably a good idea to add a travis_retry here as well

MarcoFalke commented at 2:08 pm on October 9, 2019:

style-nit: If you feel like running source ./ci/test/00_setup_env.sh, you can use ${CI_RETRY_EXE} instead, which should also work on non travis hosts (theoretically), obviously not yet in practise.

Empact commented at 2:56 pm on October 9, 2019:

Added travis_retry and switched -s to --silent for clarity. @MarcoFalke will plan on a follow-up converting the lint scripts: https://github.com/Empact/bitcoin/tree/travis-retry-lint

MarcoFalke commented at 6:35 pm on October 9, 2019:

Ah, looks like setup_env is already running (because the before_install step is inherited.

https://travis-ci.org/bitcoin/bitcoin/jobs/595662274#L125

in ci/lint/04_install.sh:27 in 27acd374c8 outdated

26 travis_retry pip3 install flake8==3.7.8
27-
28-SHELLCHECK_VERSION=v0.6.0
29-curl -s "https://storage.googleapis.com/shellcheck/shellcheck-${SHELLCHECK_VERSION}.linux.x86_64.tar.xz" | tar --xz -xf - --directory /tmp/
30-export PATH="/tmp/shellcheck-${SHELLCHECK_VERSION}:${PATH}"
31+travis_retry pip3 install vulture==0.29

MarcoFalke commented at 2:03 pm on October 9, 2019:

This has been removed, no?

fanquake commented at 2:20 pm on October 9, 2019:

Indeed. Removed in #16961.

Empact commented at 2:51 pm on October 9, 2019:

Indeed, removed in #16961, and here now too. :P

Empact force-pushed on Oct 9, 2019

lint: Run the linters against Mac OS on Travis

This helps ensure ongoing compatibility with macOS-distributed version
of GNU bash.

615ff4e7db

lint: Fix shellcheck SC2155

Declare and assign separately to avoid masking return values.

https://github.com/koalaman/shellcheck/wiki/SC2155

eafa747ca5

lint: Install grep and git via brew on mac for --perl-regexp

Particularly `--with-pcre2` is needed to run `git grep --perl-regexp` in
`test/link/check-doc.py`

cd82f75a43

Empact force-pushed on Oct 9, 2019

MarcoFalke commented at 6:29 pm on October 9, 2019: member

I am going to merge this under the “experimental umbrella”, as it might be needed for #16597

MarcoFalke commented at 6:33 pm on October 9, 2019: member

It takes about 10 minutes to boot the macOS on travis. If that is too long, we can revert this.

ACK cd82f75a431967030d25bcfe7c4aaefe5b345a99

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3ACK cd82f75a431967030d25bcfe7c4aaefe5b345a99
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUij+wv8DunewWxtXKFPL59lT0GJQYyfc4ueELUQTYrEf8pyG9Ryd6FPs4jd67u0
 8eb6D4q7M/8vZFt9TfO5MfhG9CVNEhV7sY6N1b8iXKCl7giyI8eezo2/QLYInrbFs
 9/zpFbu0jlgcQUC8UjsfZjdckmyIKgFQtsM5j0aXDgkTxdgOOTztNHPw4J8HVypvX
10kzrZXg7gN63RgEhS08toErpAKtIssahjn8swpks7iZpsckNUmwuA4qV5251K+ZcV
11phd8lViW5VkH9Gk75gDo1UThFLs4Fpg2xN9JVG4sKi1vSFpdhZV+fvPfkwWl152p
12hXmfwmn+UGlKnzLhgZNMYirBSkS5EEtBQSM1nlpR48+G9ABCCeRnehFchfC/DWSu
13585fGTGyHQ1JJn64JnxeAw2ECURMn0t7Gmbr/A4pXUNGnunWhMQvOq3LD67iXrHV
14KzYV3pGcXXn2bSJUEOoWPfFJi5Vi2likTTbFfRd6rGlxX1hqUZFfuTgWoFdDGTGx
151HiNi67t
16=EoXU
17-----END PGP SIGNATURE-----

Timestamp of file with hash 83acb4bc3587c0498eb8a1fc412634b436d9456759eec0634bddf78144e5b4e4 -

fanquake commented at 6:34 pm on October 9, 2019: member

It takes about 10 minutes to boot the macOS on travis. @MarcoFalke Any idea why it takes that long?

MarcoFalke commented at 6:36 pm on October 9, 2019: member

@MarcoFalke Any idea why it takes that long?

because travis 🤷

MarcoFalke referenced this in commit 42d0eca725 on Oct 9, 2019

MarcoFalke merged this on Oct 9, 2019

MarcoFalke closed this on Oct 9, 2019

MarcoFalke commented at 4:34 pm on October 10, 2019: member

The backlog for macOS can be seen here: https://www.traviscistatus.com/

note to myself: It failed here: https://travis-ci.org/bitcoin/bitcoin/jobs/595893060#L2470

ryanofsky commented at 2:50 am on October 17, 2019: member

I think it’s bad that this change seems to now require scripted diffs to run in the macos unix environment. It makes me less inclined to write scripted diffs if I have to somehow get them them to work in macos when I don’t use the operating system, and don’t have the hardware, and all the unix tools are strange and old.

Also, does this scripted-diff checking script itself not function on macos…?

https://travis-ci.org/bitcoin/bitcoin/jobs/598935472

0$ set -o errexit; source ./ci/lint/06_script.sh
1sed: 1: "/^-BEGIN VERIFY SCRIPT- ...": unexpected EOF (pending }'s)
2Error: missing script for: 3c5edd40ae63daee4de6c58ea4123c66be83119c
3Failed
4sed: 1: "/^-BEGIN VERIFY SCRIPT- ...": unexpected EOF (pending }'s)
5Error: missing script for: 7e271ea765b731c59edbc62790f568af284c7800
6Failed

Sjors commented at 8:18 am on October 17, 2019: member

There are some tools / homebrew packages to make macOS scripts behave more like Linux. E.g. https://formulae.brew.sh/formula/gnu-sed

MarcoFalke commented at 2:34 pm on October 17, 2019: member

Ok, then lets remove this and macOS users should use the gnu utils that are provided by brew

kittywhiskers referenced this in commit 51ef8744d5 on Dec 13, 2021

kittywhiskers referenced this in commit 6c4f3af527 on Dec 13, 2021

MarcoFalke locked this on Dec 16, 2021

lint: Run the CI lint stage on mac #13728

Conflicts