ci: subtree lint scope #34928

issue Sjors opened this issue on March 26, 2026
  1. Sjors commented at 5:24 PM on March 26, 2026: member

    In #34804 the locale linter tripped over a missing LC_ALL in libmultiprocess. That was trivial to fix, and perhaps a good thing, but it raised the question to what extend we should be linting subtrees: https://github.com/bitcoin-core/libmultiprocess/pull/265#issuecomment-4136321000

    Currently it's rather inconsistent; each linter has their own list of subtree exceptions, which doesn't include some old libraries (e.g. crc32c) and misses some new ones (e.g. libmultiprocess).

    Should we make one list and exclude it from all linters? Or do we strive to keep the same code style in subtrees and make exceptions only as needed?

  2. maflcko commented at 5:45 PM on March 26, 2026: member

    Currently it's rather inconsistent; each linter has their own list of subtree exceptions

    I don't think this is true. There are common/shared constants for those in Rust and Python:

    $ git grep -e 'pub fn get_subtrees(' -e 'SHARED_EXCLUDED_SUBTREES ='
    test/lint/lint_ignore_dirs.py:SHARED_EXCLUDED_SUBTREES = ["src/leveldb/",
    test/lint/test_runner/src/util.rs:pub fn get_subtrees() -> Vec<&'static str> {
    

    So if a linter is missing those, it should be added.

    In any case, libmultiprocess is probably written in the same C++ that Bitcoin Core is written in, so it could make sense to apply some of the linters there.

  3. maflcko added the label Tests on Mar 26, 2026
  4. ryanofsky commented at 9:48 AM on March 27, 2026: contributor

    I agree with Marco it's nice for these lint checks to apply. The thing that's not ideal is just the way the checks are currently applied because they aren't performed until the subtree is updated. One way to improve this might be to extend https://github.com/bitcoin-core/libmultiprocess/pull/253 to run the Bitcoin core lint job in addition to other jobs when libmultiprocess prs are opened to provide earlier feedback. (It could also be good to make Bitcoin core CI code more modular so the checks could be easier to reuse and probably easier to maintain.)

  5. savagemechanic commented at 5:54 AM on May 18, 2026: none

    I did a small audit of current master to check what still seems open here.

    There are already shared subtree lists in both lint stacks:

    • Python: test/lint/lint_ignore_dirs.py has SHARED_EXCLUDED_SUBTREES
    • Rust lint runner: test/lint/test_runner/src/util.rs has get_subtrees() / get_pathspecs_default_excludes()

    A number of checks already consume these lists, for example lint-locale-dependence.py, lint-includes.py, lint-include-guards.py, and the Rust lint-runner checks that use the default excludes.

    The part that still seems inconsistent is the older Python linters with local or ad hoc subtree filtering. In particular, lint-shell-locale.py still scans all *.sh files and only skips src/(secp256k1|minisketch)/, so it still applies to the src/ipc/libmultiprocess/ci/scripts/*.sh files. Current master passes because bitcoin-core/libmultiprocess#265 added the expected LC_ALL lines, but the check is still being applied to that subtree.

    So I think the remaining issue is probably narrower than "create one list": the shared lists exist, but some Python linters still bypass them or maintain separate exclusion logic. A small follow-up could move the ad hoc subtree filtering to SHARED_EXCLUDED_SUBTREES, while leaving explicit opt-ins for checks maintainers want to keep applying to a compatible subtree like libmultiprocess.

    That would make the policy clearer:

    subtree paths
        |
        +-- default: excluded through one shared list
        |
        +-- opt-in: individual linters intentionally include a subtree
                    when that check is useful there
    

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:52 UTC