Skip to content

Backports 0.17 pr18#3636

Merged
UdjinM6 merged 31 commits into
dashpay:developfrom
PastaPastaPasta:backports-0.17-pr18
Aug 1, 2020
Merged

Backports 0.17 pr18#3636
UdjinM6 merged 31 commits into
dashpay:developfrom
PastaPastaPasta:backports-0.17-pr18

Conversation

@PastaPastaPasta
Copy link
Copy Markdown
Member

Linter added in this PR fails, will take a look tomorrow

MarcoFalke and others added 15 commits July 28, 2020 20:08
…y::vch where possible

6755569 trivial: Replace CPubKey::operator[] with CPubKey::vch where possible (Nikolay Mitev)

Pull request description:

  Use variable name instead of calling operator[] through &(*this)[0]

Tree-SHA512: 7054ffda0fa33fb45d4d9f3b29698643f02fd1421d78d5197a0881f2c368dc410647fd2e1a6feb8048e30f8ab8bc2fa8749bf42b9ccbe42c30de8ff80ac45274
…he same thread

01a06d6 Avoid locking mutexes that are already held by the same thread (practicalswift)

Pull request description:

  Avoid locking mutexes that are already held by the same thread. These are reentrant mutexes, but still no need to lock them more than once per thread :-)

Tree-SHA512: e2fb85882e8800892fd8e8170f3c13128d6acfeb14d7b69fb9555f2b7ad0884fb201cf945b8144ffaf6fb1253c28af7c8c6c435319a7ae30ca003f28aa645a98
0454b56 trivial: remove unneeded include (Nikolay Mitev)

Pull request description:

  Remove dead include

Tree-SHA512: 66380fe25259d37a19f955142ad53da24d4927064a84249989f54bebc21d9d688236fb60979acc79f219b05692c4c73b3ebab0872b8d03ab2447b0b44a06c8ed
-BEGIN VERIFY SCRIPT-

sed --in-place'' --regexp-extended 's/[[:space:]]+$//g' $(git grep -I --files-with-matches --extended-regexp '[[:space:]]+$' -- src test  ':!*.svg' ':!src/crypto/sha256_sse4*' ':!src/leveldb' ':!src/qt/locale' ':!src/secp256k1' ':!src/univalue')

-END VERIFY SCRIPT-

Signed-off-by: pasta <pasta@dashboost.org>
aa85dcf build: sync ax_boost_chrono/unit_test (fanquake)

Pull request description:

  [ax_boost_chrono](https://github.com/bitcoin/bitcoin/blob/master/build-aux/m4/ax_boost_chrono.m4) and [ax_boost_unit_test_framework](https://github.com/bitcoin/bitcoin/blob/master/build-aux/m4/ax_boost_unit_test_framework.m4) were updated from upstream in bitcoin#12678. However some minor upstream changes were missed. Pull those changes in here so these files actually reflect their upstream serial.

Tree-SHA512: 71d9ee7a1616d9d36e6f63dedb6687918c3662bde724cdda1fdf3eb039c8973acd166273876a9b2671a7e087149fcf956552f9f2b946e5ee1835d12944c0065d
46340b3 [bench] Add benchmark for unserialize prevector (Akio Nakamura)

Pull request description:

  This PR adds benchmarks for the unserialization of the prevector.

  Note: Separated from bitcoin#12324.

Tree-SHA512: c055a283328cc2634c01eb60f26604a8665939bbf77d367b6ba6b4e01e77d4511fab69cc3ddb1e62969adb3c48752ed870f45ceba153eee192302601341e18a7
…18.04

1e60713 contrib: Fix test-security-check fail in Ubuntu 18.04 (Chun Kuan Lee)

Pull request description:

  - Fix test-security-check fail in Ubuntu 18.04. Those flags are enabled by default, so we must specify `-no` to make the executable does 'not' have those attributes.
  - Drop HIGH_ENTROPY_VA. After update our gitian system to Bionic, the compiler should support HIGH_ENTROPY_VA

Tree-SHA512: 78c1f2aae1253ddd52faa1af569b7151a503a217c7ccbe21b8004d8910c45d8a27ff04695eacbdadd7192d2c91c0d59941ca20c651dd2d5052b9999163a11ae4
284f424 Fix osslsigncode compile issue in gitian-build (Chun Kuan Lee)

Pull request description:

  Install libssl1.0-dev that is compatible with osslsigncode.

  Fixes bitcoin#13762

  Verifed that this gitian descriptor file can sign 0.16.2rc2.

Tree-SHA512: 3029b86e77567a4e033b5ad95826e60df12a0486ac3c4afcac48218f5c76ba49e7f1c1307ce93ffc465ca2f24e12c401e4542929263688e4bd6521aeca3ff73b
7bf22bf gui: Reject options dialog when key escape is pressed (João Barbosa)
4a43306 gui: Reject edit address dialog when key escape is pressed (João Barbosa)
f7a5531 gui: Add GUIUtil::ItemDelegate with keyEscapePressed signal (João Barbosa)

Pull request description:

  Currently `EditAddressDialog` and `OptionsDialog` don't close when the escape key is pressed. The `QDataWidgetMapper` instances prevents closing the dialogs because the escape key is used to reset the widgets values. More details and workarounds in https://stackoverflow.com/a/51487847 and http://qtramblings.blogspot.com/2010/10/qdatawidgetmapper-annoyances.html.

  The adopted solution is different from the above references. It turns out that `QDataWidgetMapper::setItemDelegate` sets the event filter for all mapped widgets. So in this PR the mapper's delegate are changed to a custom `GUIUtil::ItemDelegate` that offers the signal `keyEscapePressed`, which is connected to the `QDialog::reject` slot.

  Note that the installed event filter lets all events pass, so the current behaviour isn't changed, meaning that widgets values are reset in addition to closing the dialog.

Tree-SHA512: 9c961d488480b4ccc3880a11a8f1824b65f77570ee8918c7302c62775a1a73e52ae988a31a55ffff87b4170ddbecf833c2f09b66095c00eb6854a4d43f030f1f
…arnings in shell scripts

1499fdc Add shell script linting: Check for shellcheck warnings in shell scripts (practicalswift)

Pull request description:

  Add shell script linting: Check for `shellcheck` warnings in shell scripts.

Tree-SHA512: c7f3f5ed9933415666d2a02f5658cdc62b959ce8112f46b6327ff5f77bb5a66710704c0cde5fd8e719d1fa1fc4f0375a0c115faced166b78e81b75dfb862f08e
Signed-off-by: pasta <pasta@dashboost.org>
47776a9 Add linter: Make sure all shell scripts opt out of locale dependence using "export LC_ALL=C" (practicalswift)
3352da8 Add "export LC_ALL=C" to all shell scripts (practicalswift)

Pull request description:

  ~~Make sure `LC_ALL=C` is set when using `grep` range expressions.~~

  Make sure `LC_ALL=C` is set in all shell scripts.

  From the `grep(1)` documentation:

  > Within a bracket expression, a range expression consists of two characters separated by a hyphen. It matches any single character that sorts between the two characters, inclusive, using the locale's collating sequence and character set. For example, in the default C locale, `[a-d]` is equivalent to `[abcd]`. Many  locales sort characters in dictionary order, and in these locales `[a-d]` is typically not equivalent to `[abcd]`; it might be equivalent to `[aBbCcDd]`, for example. To obtain the traditional interpretation of bracket expressions, you can use the C locale by setting the `LC_ALL` environment variable to the value C.

  Context: [Locale issue found when reviewing bitcoin#13450](https://github.com/bitcoin/bitcoin/pull/13450/files#r194877736)

Tree-SHA512: fd74d2612998f9b49ef9be24410e505d8c842716f84d085157fc7f9799d40e8a7b4969de783afcf99b7fae4f91bbb4559651f7dd6578a6a081a50bdea29f0909
…exporting LC_ALL=C

7b23e6e Follow-up to bitcoin#13454: Fix broken build by exporting LC_ALL=C (practicalswift)

Pull request description:

  Follow-up to bitcoin#13454: Fix broken build by exporting `LC_ALL=C`.

Tree-SHA512: 5cca3182ba034dce28a0df5f4a4b343de6c2526048f17fee30e2f8d946e976b39d9cc54faae6c31bfe89022f9f4c360e9ec8e163a1690bc0656410a48bb81dbf
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
…ng bitcoin-qt

00db418 Add aarch64 qt depends support for cross compiling bitcoin-qt (TheCharlatan)

Pull request description:

  This also adds a generic qt linux target in packages.mk . I am a bit confused by the existing docs for the RISC addition. Are there boards that would support running bitcoin-qt, or at the very least forwarding X over ssh? Is everybody building depends with `NO_QT=1` when targeting RISC? If not, I will revert the change for a generic qt linux package definition back to the piecemeal solution.

  This pull request should close bitcoin#13495

Tree-SHA512: 519b951bf50f214ad725e5330094582a212333cd85b0ae442c67f9afec5629995dfad130258c7706a61f7b7cccbfa49bce69b9931f7e30cf12b382cd9a0a4749
bcd4b0f Add linting of WalletLogPrintf(...) format strings (practicalswift)
a3e4556 build: Add format string linter (practicalswift)

Pull request description:

  Add format string linter.

  This linter checks that the number of arguments passed to each variadic format string function matches the number of format specifiers in the format string.

  Example output:

  ```
  $ test/lint/lint-format-strings.sh
  src/init.cpp: Expected 2 argument(s) after format string but found 1 argument(s):
    LogPrintf("We have a mismatch here: foo=%s bar=%d\n", foo)
  src/init.cpp: Expected 1 argument(s) after format string but found 2 argument(s):
    LogPrint(BCLog::RPC, "RPC stopped. This is a mismatch: %s\n", s1, s2)
  $ echo $?
  1
  ```

Tree-SHA512: 19ab844a63f04bf193d66682ca42745a1c7d6c454b30222491b9fe8dc047054c4a6d3ee7921ec0676fb9ca2e7f6f93bd6c97996fb09667269bd491cb875349f3
@UdjinM6
Copy link
Copy Markdown

UdjinM6 commented Jul 29, 2020

Tried to fix linter errors and ended up with this https://github.com/UdjinM6/dash/commits/pr3636 so far. Was able to run it on mac with 0 linter errors, tests pass locally too but I'm not 100% sure about shellcheck fixes yet, waiting for travis/gitlab.

EDIT: note that I had to backport a bit more than just direct fixes to minimize merge conflicts

laanwj and others added 14 commits July 29, 2020 10:55
4b75dcf devtools: Make linter check LogPrint calls (MarcoFalke)
ff2ad2d Add missing newlines to LogPrint debug logging (Wladimir J. van der Laan)

Pull request description:

  ~~Don't we have a linter that should catch these?~~

Tree-SHA512: 1a58eca01ded9c1719e943c09447deeb59bb06dba00528cf460eefe857fdf95b42671fbdebc87cdd2f51e931e86942d06587ffd097cbb0d8dd9eb7a0ba17a8f0
…railing whitespace flake8 checks for Python files

0d31ef4 Enable W191 and W291 flake8 checks. Remove trailing whitespace from Python files. Convert tabs to spaces. (John Bampton)

Pull request description:

Tree-SHA512: d062434310d6232469d7ca8e5f2ddb7db7e85cb2a299e609d98bacc318368e43e0777c9f4966df03d50f526bbe27207faa87a7464e62e14671194459a06ad969
…les we are currently not violating

506c578 Enable Travis checking for two Python linting rules we are currently not violating (practicalswift)

Pull request description:

  Enable Travis checking for two Python linting rules we are currently not violating:
  * E101: indentation contains mixed spaces and tabs
  * E129: visually indented line with same indent as next logical line

Tree-SHA512: 955ea5ce4576a5bdd561f9d2bbcfaa82f66a23391c84ddb806830ed15e321e4742457ccc801f457819f626d4a66a1ffcaecee28c3b9f3f907ab8401323743485
fa3c910 test: Move linters to test/lint, add readme (MarcoFalke)

Pull request description:

  This moves the checks and linters from `devtools` to a subfolder in `test`. (Motivated by my opinion that the dev tools are mostly for generating code and updating the repo whereas the linters are read-only checks.)

  Also, adds a readme to clarify that checks and linters are only meant to prevent bugs and user facing issues, not merely stylistic preference or inconsistencies. (This is motivated by the diversity in developers and work flows as well as existing code styles. It would be too disruptive to change all existing code to a single style or too burdensome to force all developers to adhere to a single style. Also note that our style guide is changing, so locking in at the wrong style "too early" would only waste resources.)

Tree-SHA512: 9b10e89f2aeaf0c8a9ae248aa891d74e0abf0569f8e5dfd266446efa8bfaf19f0ea0980abf0b0b22f0d8416ee90d7435d21a9f9285b66df43f370b7979173406
…cOS dev environment (build-osx.md)

341f7c7 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. (practicalswift)
908a559 macOS fix: Add excludes for checks added in the newer shellcheck version installed by brew (practicalswift)
ec4d57b macOS fix: Work around empty (sub)expression error when using BSD grep (practicalswift)
b57d7d9 macOS fix: Avoid mapfile due to ancient version of bash shipped with macOS (practicalswift)

Pull request description:

  The linters are thoroughly tested under Ubuntu which is what we use in Travis. When reading bitcoin#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.

Tree-SHA512: b39c9a970d14d27db1fb592539923c0bc676b5217f415d02fda3f17bf54d46faa172376e8a3ecab07ca68a3acba9aebe00b2b1b2161b2a36b85fbb672e7efb5c
…lly (fix for OS X)

21be609 In lint-format-strings, open files sequentially (Glenn Willen)

Pull request description:

  In lint-format-strings, we use python argparse to read our file arguments. In
  this mode, argparse opens all the files simultaneously. On OS X, where the
  default filehandle limit is 128, this causes the lint to fail. Instead, ask
  argparse for our filename arguments as strings, and open them one at a time
  using 'with open'.

Tree-SHA512: 4c7dabf98818a7c5d83ab10c61b89a26957fe399e39e933e30c561cb45c5e8ba6f6aedcde8343da0c32ee340289a8897db6a33708e35ee381334ee27e3f4d356
@PastaPastaPasta
Copy link
Copy Markdown
Member Author

Cherry-picked, the commits looks good overall, however lint-all.sh is still failing for me locally

./test/lint/lint-all.sh 

In ci/build_src.sh line 41:
test -n "$USE_SHELL" && eval '"$USE_SHELL" -c "./autogen.sh"' || ./autogen.sh
                     ^-- SC2015: Note that A && B || C is not if-then-else. C may run when A is true.


In ci/test_integrationtests.sh line 9:
PASS_ARGS="$@"
          ^--^ SC2124: Assigning an array to a string! Assign as array, or use * instead of @ to concatenate.


In ci/test_integrationtests.sh line 45:
  for d in $(ls testdatadirs/$BASEDIR | grep -v '^cache$'); do
             ^-- SC2010: Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.


In contrib/macdeploy/detached-sig-create.sh line 17:
if [ ! -n "$1" ]; then
     ^-- SC2236: Use -z instead of ! -n.


In contrib/qos/tc.sh line 34:
if [ ! -z "${LOCALNET_V6}" ] ; then
     ^-- SC2236: Use -n instead of ! -z.


In contrib/qos/tc.sh line 57:
if [ ! -z "${LOCALNET_V6}" ] ; then
     ^-- SC2236: Use -n instead of ! -z.


In contrib/verify-commits/verify-commits.sh line 101:
				HASH=$(git cat-file blob "$CURRENT_COMMIT":"$FILE" | shasum -a 512 | { read FIRST OTHER; echo $FIRST; } )
                                                                                                                  ^---^ SC2034: OTHER appears unused. Verify use (or export if used externally).


In contrib/windeploy/detached-sig-create.sh line 11:
if [ ! -n "$1" ]; then
     ^-- SC2236: Use -z instead of ! -n.


In docker/build-docker.sh line 6:
cd $DIR/..
^--------^ SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

Did you mean: 
cd $DIR/.. || exit


In docker/push-docker.sh line 6:
cd $DIR/..
^--------^ SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

Did you mean: 
cd $DIR/.. || exit


In test/lint/git-subtree-check.sh line 22:
	while read a b junk; do
                       ^--^ SC2034: junk appears unused. Verify use (or export if used externally).


In test/lint/git-subtree-check.sh line 52:
old=$1
^-^ SC2034: old appears unused. Verify use (or export if used externally).

For more information:
  https://www.shellcheck.net/wiki/SC2010 -- Don't use ls | grep. Use a glob o...
  https://www.shellcheck.net/wiki/SC2034 -- OTHER appears unused. Verify use ...
  https://www.shellcheck.net/wiki/SC2124 -- Assigning an array to a string! A...
^---- failure generated from ./test/lint/lint-shell.sh

@UdjinM6
Copy link
Copy Markdown

UdjinM6 commented Jul 29, 2020

You missed the last one - 682c6b843e85fe0b524b22b504d4c122966207e5

@PastaPastaPasta
Copy link
Copy Markdown
Member Author

Yeah... That helps, looks good

@PastaPastaPasta
Copy link
Copy Markdown
Member Author

PastaPastaPasta commented Jul 29, 2020

I'm not sure why CI isn't happy... Tests are passing, yet it's coming back red. Same on travis https://travis-ci.com/github/PastaPastaPasta/dash/jobs/366325130

@UdjinM6
Copy link
Copy Markdown

UdjinM6 commented Jul 30, 2020

Pls see d394166d4963ffcca578e3a9384aae6dc447599d

@UdjinM6 UdjinM6 added this to the 17 milestone Jul 30, 2020
@PastaPastaPasta
Copy link
Copy Markdown
Member Author

Cherry-picked

Copy link
Copy Markdown

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

Note: ffc3830 should be backported to 0.16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants