perf: enable more multi-threading and caching in linters #4807
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PastaPastaPasta
force-pushed
the
faster-linting
branch
from
1a1359e to
8b072cd
Compare
|
This pull request has conflicts, please rebase. |
PastaPastaPasta
force-pushed
the
faster-linting
branch
from
8b072cd to
c6ff676
Compare
UdjinM6
reviewed
Apr 27, 2022
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
This was referenced Nov 26, 2024
PastaPastaPasta
added a commit
that referenced
this pull request
Dec 4, 2024
, bitcoin#24844, bitcoin#24853, bitcoin#24895, bitcoin#24802, bitcoin#24932, partial bitcoin#23212, bitcoin#23462 (lint backports) 852f55e merge bitcoin#24932: Convert lint-locale-dependence.sh to Python (Kittywhiskers Van Gogh) f745b7f merge bitcoin#24802: convert format strings linter test to python (Kittywhiskers Van Gogh) a0b051b partial revert dash#4807: enable more multi-threading and caching in linters (Kittywhiskers Van Gogh) 7864c21 merge bitcoin#24895: Convert lint-includes.sh to Python (Kittywhiskers Van Gogh) f63bafe merge bitcoin#24853: Convert lint-git-commit-check.sh to Python (Kittywhiskers Van Gogh) d463d7d fix: clone full history on GitHub Actions build job, track `develop` (Kittywhiskers Van Gogh) d23b661 merge bitcoin#24844: Convert lint-whitespace.sh to Python (Kittywhiskers Van Gogh) fe16516 merge bitcoin#24849: Convert lint-logs.sh to Python (Kittywhiskers Van Gogh) dbfd0b0 merge bitcoin#24766: convert spellchecking lint test to python (Kittywhiskers Van Gogh) 5c23add merge bitcoin#24778: Convert Python dead code linter test to Python (Kittywhiskers Van Gogh) 829dcfb partial bitcoin#23462: Enable SC2046 and SC2086 shellcheck rules (Kittywhiskers Van Gogh) df6be0e partial bitcoin#23212: enable mypy import checking (Kittywhiskers Van Gogh) f77eca9 fix: make sure that parallelized `lint-all` runs incl. python scripts (Kittywhiskers Van Gogh) e220175 merge bitcoin#22861: Update test README and lint script (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependency for #6429 * Even though support for Python scripts was extended to `lint-all` in [bitcoin#24762](f226e8d) ([dash#6023](#6023)), the changes needed were not extended to parallelized linting added in [dash#4637](#4637). This meant the match expression didn't include non-shell scripts and additionally, `parallel` interpreted all scripts as Bash scripts. This has been resolved in this PR. * [bitcoin#23212](bitcoin#23212) is partial as `--ignore-missing-imports` has not been removed from `mypy` arguments as the version of `mypy` used in the PR (0.910) isn't syntax aware to `imports` like [here](https://github.com/dashpay/dash/blob/f9d044d5ec7cc492d10fd4a89f85927b00305e83/test/functional/test_framework/crypto/bip324_cipher.py#L16-L17) and [here](https://github.com/dashpay/dash/blob/f9d044d5ec7cc492d10fd4a89f85927b00305e83/test/functional/test_framework/crypto/muhash.py#L9). <details> <summary>Lint errors:</summary> ``` dash@96b217d539f7:/src/dash$ ./test/lint/lint-python.sh Consider install flake8-cached for cached flake8 results. [...] test/functional/test_framework/crypto/muhash.py:9: error: Skipping analyzing ".chacha20": found module but no type hints or library stubs [import] test/functional/test_framework/crypto/ellswift.py:15: error: Cannot find implementation or library stub for module named "test_framework.crypto.secp256k1" [import] test/functional/test_framework/crypto/bip324_cipher.py:16: error: Skipping analyzing ".chacha20": found module but no type hints or library stubs [import] test/functional/test_framework/crypto/bip324_cipher.py:17: error: Skipping analyzing ".poly1305": found module but no type hints or library stubs [import] test/functional/test_framework/messages.py:29: error: Cannot find implementation or library stub for module named "test_framework.crypto.siphash" [import] test/functional/test_framework/messages.py:32: error: Cannot find implementation or library stub for module named "dash_hash" [import] test/functional/test_framework/script.py:20: error: Cannot find implementation or library stub for module named "test_framework.crypto.ripemd160" [import] test/functional/test_framework/key.py:16: error: Cannot find implementation or library stub for module named "test_framework.crypto" [import] test/functional/test_framework/v2_p2p.py:9: error: Cannot find implementation or library stub for module named "test_framework.crypto.bip324_cipher" [import] test/functional/test_framework/v2_p2p.py:10: error: Cannot find implementation or library stub for module named "test_framework.crypto.chacha20" [import] test/functional/test_framework/v2_p2p.py:11: error: Cannot find implementation or library stub for module named "test_framework.crypto.ellswift" [import] test/functional/test_framework/v2_p2p.py:12: error: Cannot find implementation or library stub for module named "test_framework.crypto.hkdf" [import] test/functional/p2p_v2_encrypted.py:22: error: Cannot find implementation or library stub for module named "test_framework.crypto.chacha20" [import] test/functional/feature_utxo_set_hash.py:12: error: Cannot find implementation or library stub for module named "test_framework.crypto.muhash" [import] Found 17 errors in 12 files (checked 275 source files) ``` </details> And upgrading to the latest version of `mypy` used upstream (1.4.1, [source](https://github.com/bitcoin/bitcoin/blob/f7144b24be09e7db2173a0b15d73f99a08b98118/ci/lint/04_install.sh#L52)) brings syntax awareness but new errors. <details> <summary>Lint errors:</summary> ``` dash@96b217d539f7:/src/dash$ ./test/lint/lint-python.sh Consider install flake8-cached for cached flake8 results. [...] test/functional/test_framework/coverage.py:23: error: Incompatible default for argument "coverage_logfile" (default has type "None", argument has type "str") [assignment] test/functional/test_framework/coverage.py:23: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True test/functional/test_framework/coverage.py:23: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase test/functional/test_framework/util.py:322: error: Incompatible default for argument "timeout" (default has type "None", argument has type "int") [assignment] test/functional/test_framework/util.py:322: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True test/functional/test_framework/util.py:322: note: Use https://github.com/hauntsaninja/no_implicit_optional to automatically upgrade your codebase test/functional/test_framework/util.py:322: error: Incompatible default for argument "coveragedir" (default has type "None", argument has type "str") [assignment] test/functional/test_framework/messages.py:32: error: Cannot find implementation or library stub for module named "dash_hash" [import] test/functional/test_framework/test_framework.py:122: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked] test/functional/test_framework/test_framework.py:123: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked] test/functional/test_framework/test_framework.py:124: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked] test/functional/test_framework/test_framework.py:125: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked] test/functional/p2p_message_capture.py:48: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked] Found 6 errors in 5 files (checked 275 source files) ``` </details> * [bitcoin#23462](bitcoin#23462) is partial as neither `SC2046` nor `SC2086` have been enabled. This backport was done for the sole purpose of backporting changes to shell scripts that would be replaced with their Python counterparts in later backports. The necessary changes needed to enable `SC2046` and `SC2086` will be done in a later pull request. * `actions/checkout` does not do a full clone nor tracks the default branch by default. It is already documented that `git merge-base` (used by `lint-git-commit-check.py` and `lint-whitespace.py`, [source](https://github.com/dashpay/dash/blob/4aaa314a575a318683f3d15f16f396ac1a44a913/test/lint/lint-git-commit-check.py#L45)) doesn't play nice with it (see [actions/checkout#423](https://github.com/actions/checkout/discussions/423)). No such issue exists on GitLab ([build](https://gitlab.com/dashpay/dash/-/jobs/8456260939#L113)), but it remains present on GitHub Actions ([build](https://github.com/dashpay/dash/actions/runs/11996049800/job/33440106874?pr=6428#step:7:103)). This PR does a full clone, switches to the `develop` branch, then switches back to the intended commit, as a workaround (see working build [here](#6428 (comment))) * Changes to `run-lint-format-strings.py` made in [dash#4807](#4807) were reverted as they were interfering with `lint-format-strings.py` <details> <summary>Lint error:</summary> ``` dash@96b217d539f7:/src/dash$ ./test/lint/lint-format-strings.py Traceback (most recent call last): File "/src/dash/test/lint/run-lint-format-strings.py", line 308, in <module> main() File "/src/dash/test/lint/run-lint-format-strings.py", line 304, in main sys.exit(max(exit_codes)) ValueError: max() arg is an empty sequence Traceback (most recent call last): File "/src/dash/test/lint/run-lint-format-strings.py", line 308, in <module> main() File "/src/dash/test/lint/run-lint-format-strings.py", line 304, in main sys.exit(max(exit_codes)) ValueError: max() arg is an empty sequence ``` </details> ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: knst: utACK 852f55e UdjinM6: utACK 852f55e Tree-SHA512: e508311c00249e7e48f91ca23e6f62117c07497f9c4471d07659b233e43a9f4d09ee2bfe0dd76f0c9746209cdba5895cb002a708924daa19d7aa76869815a7d9
PastaPastaPasta
added a commit
that referenced
this pull request
Dec 4, 2024
, bitcoin#24915, bitcoin#24916, bitcoin#24929, bitcoin#23506, bitcoin#24840, bitcoin#24982, partial bitcoin#25288 (lint backports: part 2) 2fa480a partial bitcoin#25288: Reliably don't start itself (lint-all.py runs all tests twice) (Kittywhiskers Van Gogh) bda1e03 merge bitcoin#24982: Port `lint-all.sh` to `lint-all.py` (Kittywhiskers Van Gogh) b054a0d merge bitcoin#24840: port `lint-shell.sh` to python (Kittywhiskers Van Gogh) 973ca7b merge bitcoin#23506: Make more shell scripts verifiable by the `shellcheck` tool (Kittywhiskers Van Gogh) 694c1a4 merge bitcoin#24929: convert shell locale linter test to Python (Kittywhiskers Van Gogh) 2a7d32a merge bitcoin#24916: Convert lint-python-utf8-encoding.sh to Python (Kittywhiskers Van Gogh) 0321fa0 merge bitcoin#24915: Convert lint-circular-dependencies.sh to Python (Kittywhiskers Van Gogh) e3dc4b1 merge bitcoin#24902: Convert lint-include-guards.sh to Python (Kittywhiskers Van Gogh) fc48a13 merge bitcoin#23524: Fix typos in endif header comments (Kittywhiskers Van Gogh) 1f8c3b5 merge bitcoin#24794: Convert Python linter to Python (Kittywhiskers Van Gogh) 110b6ac partial revert dash#4807: enable more multi-threading and caching in linters (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Depends on #6428 * The introduction in `flake8-cached` for `lint-python.sh` in [dash#4807](#4807) was reverted as this logic wasn't going to be ported over to the Python replacement as the `flake8-cached` repo has been archived since April 2023 ([source](https://github.com/jnoortheen/flake8-cached)) and we don't use it in CI through GitLab ([build](https://gitlab.com/dashpay/dash/-/jobs/8456994796#L144)) or GitHub Actions ([build](https://github.com/dashpay/dash/actions/runs/11981121905/job/33406844883#step:7:75)). * [bitcoin#25288](bitcoin#25288) has been marked as partial as the change of the glob pattern from `{mod_path}/lint-*` to `{mod_path}/lint-*.py` as we still have `lint-cppcheck-dash.sh` around ([source](https://github.com/dashpay/dash/blob/b88d9910a8362ad7a47cf2ea822a00649793350b/test/lint/lint-cppcheck-dash.sh)) (and the original `cppcheck` linter upstream was removed in [bitcoin#25091](bitcoin#25091)). A Python port of that linter would allow for completing [bitcoin#25288](bitcoin#25288). ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 2fa480a PastaPastaPasta: utACK 2fa480a Tree-SHA512: 48ddf11be11232df26051b39dfadac9f363d2f201b9f303cad6ddd54550e2f1881947061155da9d4eaf3f5a87cdd371368dc36b4d70eb81ff4c48a7a93af63ae
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Utilize multi-threading in the individual linters, and caching where available. This resulted in my incremental linting runs going from 20s to 6s (much more sensible to run as pre-commit now)