Skip to content

Conversation

@Empact
Copy link
Contributor

@Empact Empact commented Jul 20, 2018

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

@Empact
Copy link
Contributor Author

Empact commented Jul 20, 2018

Inspired by and built on #13720

@Empact Empact changed the title Run the CI lint stage on mac and linux both Scripts and tools: Run the CI lint stage on mac and linux both Jul 20, 2018
@Empact Empact force-pushed the bsd-bash-compatibility branch from 052ed45 to 1598438 Compare July 21, 2018 17:04
@practicalswift
Copy link
Contributor

practicalswift commented Jul 21, 2018

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 Empact force-pushed the bsd-bash-compatibility branch 2 times, most recently from 7436739 to 3a9c712 Compare July 21, 2018 19:35
@sipa
Copy link
Member

sipa commented Jul 21, 2018

/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 Empact force-pushed the bsd-bash-compatibility branch from 3a9c712 to 18591b2 Compare July 21, 2018 21:54
@Empact
Copy link
Contributor Author

Empact commented Jul 22, 2018

wip: I’m rolling back to 10.10

@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 22, 2018

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

Conflicts

No conflicts as of last run.

@Empact Empact force-pushed the bsd-bash-compatibility branch from 18591b2 to f94eafa Compare July 24, 2018 01:28
@maflcko
Copy link
Member

maflcko commented Jul 24, 2018

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

@Empact Empact force-pushed the bsd-bash-compatibility branch 2 times, most recently from 3c87951 to 3d8c654 Compare July 28, 2018 18:19
Copy link
Contributor

@scravy scravy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 Empact force-pushed the bsd-bash-compatibility branch from 3d8c654 to d5d7665 Compare August 1, 2018 15:20
@scravy
Copy link
Contributor

scravy commented Aug 1, 2018

ACK d5d76655105d6d338dbc367ae0b54b7433ffeb38

@practicalswift
Copy link
Contributor

utACK d5d7665

Closing my PR #13720 in favour of this one.

@maflcko
Copy link
Member

maflcko commented Aug 27, 2018

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

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

@Empact Empact force-pushed the bsd-bash-compatibility branch from d5d7665 to daef159 Compare August 27, 2018 17:21
@Empact
Copy link
Contributor Author

Empact commented Aug 27, 2018

@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
Copy link
Contributor

Needs rebase

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a good idea to add a travis_retry here as well

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@Empact Empact Oct 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

@Empact Empact force-pushed the bsd-bash-compatibility branch from 27acd37 to fe91b3a Compare October 9, 2019 14:48
Empact added 3 commits October 9, 2019 15:49
This helps ensure ongoing compatibility with macOS-distributed version
of GNU bash.
Particularly `--with-pcre2` is needed to run `git grep --perl-regexp` in
`test/link/check-doc.py`
@Empact Empact force-pushed the bsd-bash-compatibility branch from fe91b3a to cd82f75 Compare October 9, 2019 14:51
@maflcko
Copy link
Member

maflcko commented Oct 9, 2019

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

@maflcko
Copy link
Member

maflcko commented Oct 9, 2019

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

ACK cd82f75

Show signature and timestamp

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK cd82f75a431967030d25bcfe7c4aaefe5b345a99
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUij+wv8DunewWxtXKFPL59lT0GJQYyfc4ueELUQTYrEf8pyG9Ryd6FPs4jd67u0
eb6D4q7M/8vZFt9TfO5MfhG9CVNEhV7sY6N1b8iXKCl7giyI8eezo2/QLYInrbFs
/zpFbu0jlgcQUC8UjsfZjdckmyIKgFQtsM5j0aXDgkTxdgOOTztNHPw4J8HVypvX
kzrZXg7gN63RgEhS08toErpAKtIssahjn8swpks7iZpsckNUmwuA4qV5251K+ZcV
phd8lViW5VkH9Gk75gDo1UThFLs4Fpg2xN9JVG4sKi1vSFpdhZV+fvPfkwWl152p
hXmfwmn+UGlKnzLhgZNMYirBSkS5EEtBQSM1nlpR48+G9ABCCeRnehFchfC/DWSu
585fGTGyHQ1JJn64JnxeAw2ECURMn0t7Gmbr/A4pXUNGnunWhMQvOq3LD67iXrHV
KzYV3pGcXXn2bSJUEOoWPfFJi5Vi2likTTbFfRd6rGlxX1hqUZFfuTgWoFdDGTGx
1HiNi67t
=EoXU
-----END PGP SIGNATURE-----

Timestamp of file with hash 83acb4bc3587c0498eb8a1fc412634b436d9456759eec0634bddf78144e5b4e4 -

@fanquake
Copy link
Member

fanquake commented Oct 9, 2019

It takes about 10 minutes to boot the macOS on travis.

@MarcoFalke Any idea why it takes that long?

@maflcko
Copy link
Member

maflcko commented Oct 9, 2019

@MarcoFalke Any idea why it takes that long?

because travis 🤷

maflcko pushed a commit that referenced this pull request Oct 9, 2019
cd82f75 lint: Install grep and git via brew on mac for --perl-regexp (Ben Woosley)
eafa747 lint: Fix shellcheck SC2155 (Ben Woosley)
615ff4e lint: Run the linters against Mac OS on Travis (Ben Woosley)

Pull request description:

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

ACKs for top commit:
  MarcoFalke:
    ACK cd82f75

Tree-SHA512: 8d56d2303bbebedba8ea2291f4ab35b7fdf3245b7a4c3f04557eee4f19d83573798ad32facc92bfa060aaeb294e6d2c95e6d1c3b795fd7951dcf3aa1cccec107
@maflcko maflcko merged commit cd82f75 into bitcoin:master Oct 9, 2019
@maflcko
Copy link
Member

maflcko commented Oct 10, 2019

@ryanofsky
Copy link
Contributor

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

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

@Sjors
Copy link
Member

Sjors commented Oct 17, 2019

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

@maflcko
Copy link
Member

maflcko commented Oct 17, 2019

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

@maflcko maflcko mentioned this pull request Oct 17, 2019
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.