lint: Make all linters work under the default macOS dev environment (build-osx.md) #14115

pull practicalswift wants to merge 4 commits into bitcoin:master from practicalswift:linter-macos changing 5 files +26 −6

practicalswift commented at 11:04 PM on August 30, 2018: contributor

The linters are thoroughly tested under Ubuntu which is what we use in Travis. When reading #14041 I understood that some developers were experiencing problems when running the linters on their local machines.

Assuming these local machines were running macOS I installed a fresh macOS VM, followed the instructions in build-osx.md and ran the linters.

This PR contains the changes needed to make lint-all.sh run as expected.

Ideally the linters would continuously run also under a Travis macOS environment to make sure we catch these kind of issues before merge.
macOS fix: Avoid mapfile due to ancient version of bash shipped with macOS b57d7d92fe
macOS fix: Work around empty (sub)expression error when using BSD grep ec4d57bbb3
macOS fix: Add excludes for checks added in the newer shellcheck version installed by brew 908a559f33
macOS fix: Check for correct version of flake8 to avoid spurious warnings. The brew installed flake8 version is Python 2 based and does not work. 341f7c7b0e
fanquake added the label Scripts and tools on Aug 30, 2018
practicalswift cross-referenced this on Aug 30, 2018 from issue Linter fixes by kallewoof
DrahtBot commented at 2:21 AM on August 31, 2018: contributor
Note to reviewers: This pull request conflicts with the following ones:
- #13728 (Scripts and tools: Run the CI lint stage on mac and linux both by Empact)
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.
DrahtBot cross-referenced this on Aug 31, 2018 from issue lint: Run the CI lint stage on mac by Empact
practicalswift cross-referenced this on Aug 31, 2018 from issue wallet: Remove unused local variable old_label by practicalswift
laanwj commented at 11:28 AM on August 31, 2018: member

Concept ACK
MarcoFalke commented at 11:49 AM on August 31, 2018: member

utACK 341f7c7b0e77edcc02cb3429fb9a3d49745332cc
Empact commented at 8:12 PM on September 1, 2018: member

See also: #13728 which is similar but fixes the linter errors rather than disabling them, and runs the linters on macos via CI.
fanquake commented at 5:38 AM on September 2, 2018: member
Tested on macOS 10.13.6, master https://github.com/bitcoin/bitcoin/commit/2070a545e2c4e8c2dff4c0fb40b8fd2f68fc2cad has a bunch of warnings/failures:
```
./test/lint/lint-all.sh
test/lint/lint-format-strings.sh: line 36: mapfile: command not found
grep: empty (sub)expression
/usr/local/lib/python3.7/site-packages/pycodestyle.py:113: FutureWarning: Possible nested set at position 1
  EXTRANEOUS_WHITESPACE_REGEX = re.compile(r'[[({] | []}),;:]')
grep: empty (sub)expression
Missing "export LC_ALL=C" (to avoid locale dependence) as first non-comment non-empty line in .travis/lint_04_install.sh
grep: empty (sub)expression
^---- failure generated from test/lint/lint-shell-locale.sh
```
Using https://github.com/bitcoin/bitcoin/pull/14115/commits/341f7c7b0e77edcc02cb3429fb9a3d49745332cc and after brew install spellcheck and pip3 install flake8, all linters seem to run, with no warnings.

Did not check if this breaks anything on a different OS etc. tACK https://github.com/bitcoin/bitcoin/pull/14115/commits/341f7c7b0e77edcc02cb3429fb9a3d49745332cc
practicalswift commented at 7:44 AM on September 5, 2018: contributor

Please review :-)
MarcoFalke merged this on Sep 5, 2018
MarcoFalke closed this on Sep 5, 2018
MarcoFalke referenced this in commit 97ccd2b84e on Sep 5, 2018
Bushstar cross-referenced this on Oct 11, 2018 from issue Updates from bitcoin/master by Bushstar
hebasto cross-referenced this on Jul 2, 2019 from issue scripts and tools: Update ShellCheck linter by hebasto
UdjinM6 referenced this in commit 889990fbef on Jul 29, 2020
UdjinM6 referenced this in commit 8e05f8b288 on Jul 29, 2020
PastaPastaPasta referenced this in commit 481254c86d on Jul 29, 2020
str4d cross-referenced this on Oct 27, 2020 from issue Backport useful lints from upstream by str4d
zkbot referenced this in commit 43ac2062f9 on Oct 28, 2020
zkbot referenced this in commit 84a5830aaa on Nov 9, 2020
practicalswift deleted the branch on Apr 10, 2021
gades referenced this in commit c558370b0e on Apr 20, 2022
bitcoin locked this on Aug 18, 2022

Contributors

Labels

Linked (view graph)

#14041 Linter fixes #14117 wallet: Remove unused local variable old_label #16327 scripts and tools: Update ShellCheck linter