Simplify include analysis by enforcing the developer guide's include syntax #13230

pull practicalswift wants to merge 3 commits into bitcoin:master from practicalswift:bracket-syntax-includes changing 7 files +33 −22
  1. practicalswift commented at 8:00 AM on May 14, 2018: contributor

    When analysing includes in the project it is often assumed that the preferred bracket include syntax (#include <foo.h>) mentioned in developer-docs.md is used consistently. @sipa:s excellent circular dependencies script circular-dependencies.py (#13228) is an example of a script making this reasonable assumption.

    This PR enables automatic Travis checking of the include syntax making sure that the bracket syntax includes (#include <foo.h>) is used consistently.

  2. practicalswift force-pushed on May 14, 2018
  3. in contrib/devtools/lint-includes.sh:9 in 9d8f70aafb outdated
       3 | @@ -4,10 +4,12 @@
       4 |  # Distributed under the MIT software license, see the accompanying
       5 |  # file COPYING or http://www.opensource.org/licenses/mit-license.php.
       6 |  #
       7 | -# Check for duplicate includes.
       8 | +# Check includes: Check for duplicate includes. Enforce bracket syntax includes.
       9 | +
      10 | +IGNORE_REGEXP="(leveldb|secp256k1|univalue)/"
    


    Empact commented at 8:24 AM on May 14, 2018:

    leading / missing


    practicalswift commented at 8:36 AM on May 14, 2018:

    Thanks! Fixed!

  4. practicalswift force-pushed on May 14, 2018
  5. fanquake added the label Scripts and tools on May 14, 2018
  6. practicalswift renamed this:
    Allow for easy include analysis by enforcing the developer guide's include syntax preference
    Simplify include analysis by enforcing the developer guide's include syntax
    on May 14, 2018
  7. practicalswift force-pushed on May 15, 2018
  8. in src/test/blockchain_tests.cpp:5 in 11875037fc outdated
       0 | @@ -1,9 +1,9 @@
       1 |  #include <boost/test/unit_test.hpp>
       2 |  
       3 | -#include "stdlib.h"
       4 | +#include <stdlib.h>
       5 |  
       6 | -#include "rpc/blockchain.cpp"
       7 | -#include "test/test_bitcoin.h"
       8 | +#include <rpc/blockchain.cpp>
    


    MarcoFalke commented at 4:56 PM on May 19, 2018:

    Including a cpp file seems extremely wrong. (I know this is not your fault, just leaving a note)


    Empact commented at 9:18 PM on May 20, 2018:
  9. sipa commented at 7:09 PM on May 19, 2018: member

    The developer guidelines say to use the #include <...> form when possible. It's not clear to me when it wouldn't be possible.

    If it's always possible, ACK on enforcing it.

  10. promag commented at 1:47 PM on May 20, 2018: member

    ☝️then update developer notes?

  11. Empact cross-referenced this on May 20, 2018 from issue rpc: Remove the need to include rpc/blockchain.cpp in order to put `GetDifficulty` under test by Empact
  12. practicalswift commented at 7:17 AM on May 21, 2018: contributor

    @sipa @promag Good point! Updated the developer guidelines :-)

  13. practicalswift force-pushed on May 22, 2018
  14. Empact cross-referenced this on May 23, 2018 from issue lint: Add linter to error on #include <*.cpp> by Empact
  15. MarcoFalke commented at 5:50 PM on May 29, 2018: member

    Needs rebase

  16. practicalswift force-pushed on May 29, 2018
  17. practicalswift commented at 10:00 PM on May 29, 2018: contributor

    Thanks @MarcoFalke. Rebased!

  18. Empact commented at 1:06 PM on June 1, 2018: member

    utACK 04016bf could squash

  19. MarcoFalke commented at 2:29 PM on June 2, 2018: member

    utACK 04016bf0b78aed2a6c39dfa284478b39192e2ec2

  20. DrahtBot cross-referenced this on Jun 4, 2018 from issue build: Guard against accidental introduction of new Boost dependencies by practicalswift
  21. practicalswift force-pushed on Jun 5, 2018
  22. practicalswift commented at 2:59 PM on June 5, 2018: contributor

    Rebased! Please re-review :-)

  23. in test/lint/lint-includes.sh:103 in 186b010185 outdated
      99 | @@ -97,4 +100,12 @@ for EXPECTED_BOOST_INCLUDE in "${EXPECTED_BOOST_INCLUDES[@]}"; do
     100 |      fi
     101 |  done
     102 |  
     103 | +QUOTE_SYNTAX_INCLUDES=$(git grep '^#include "' -- "**/*.cpp" "**/*.h" | grep -Ev "${IGNORE_REGEXP}")
    


    ken2812221 commented at 3:10 PM on June 5, 2018:

    Why not use *.cpp and *.h? Isn't it compatible with other git versions?


    practicalswift commented at 4:14 PM on June 5, 2018:

    Yes, you're right that -- "*.cpp" "*.h" would be equivalent in this specific case (since we're only matching in src/), but my suggestions is that we keep this PR as is since it already has two utACK:s that would need re-reviewing in case of a change. Makes sense? :-)


    practicalswift commented at 5:58 AM on June 6, 2018:

    Fixed when rebasing :-)

  24. MarcoFalke commented at 3:14 PM on June 5, 2018: member

    utACK 186b0101854824bb2bed55a7674aa5d6979d9f66

  25. ken2812221 commented at 3:25 PM on June 5, 2018: contributor

    utACK 186b010

  26. in src/bench/merkle_root.cpp:5 in 186b010185 outdated
       1 | @@ -2,11 +2,11 @@
       2 |  // Distributed under the MIT software license, see the accompanying
       3 |  // file COPYING or http://www.opensource.org/licenses/mit-license.php.
       4 |  
       5 | -#include "bench.h"
       6 | +#include <bench.h>
    


    laanwj commented at 6:06 PM on June 5, 2018:

    This doesn't compile locally here. Should be #include <bench/bench.h>


    practicalswift commented at 5:56 AM on June 6, 2018:

    Fixed!

  27. MarcoFalke commented at 10:31 PM on June 5, 2018: member

    Needs rebase

  28. MarcoFalke added the label Needs rebase on Jun 6, 2018
  29. practicalswift force-pushed on Jun 6, 2018
  30. practicalswift commented at 5:56 AM on June 6, 2018: contributor

    Rebased! Please re-review :-)

  31. practicalswift force-pushed on Jun 6, 2018
  32. Use bracket syntax includes ("#include <foo.h>") 906bee8e5f
  33. Enforce the use of bracket syntax includes ("#include <foo.h>") 6d10f43738
  34. Clarify include recommendation 16e3cd380a
  35. in src/test/blockchain_tests.cpp:5 in 655d4321e9 outdated
       0 | @@ -1,9 +1,9 @@
       1 |  #include <boost/test/unit_test.hpp>
       2 |  
       3 | -#include "stdlib.h"
       4 | +#include <stdlib.h>
       5 |  
       6 | -#include "rpc/blockchain.h"
       7 | -#include "test/test_bitcoin.h"
       8 | +#include <rpc/blockchain.cpp>
    


    ken2812221 commented at 6:15 AM on June 6, 2018:

    This seems incorrect


    practicalswift commented at 7:25 AM on June 6, 2018:

    In what way? :-)


    ken2812221 commented at 7:37 AM on June 6, 2018:

    It includes blockchain.h previously, but you include blockchain.cpp. Might because of #13288 merged.


    practicalswift commented at 9:10 AM on June 6, 2018:

    Ouch, now fixed! Thanks for catching this. I missed the .cpp extension!

  36. practicalswift force-pushed on Jun 6, 2018
  37. ken2812221 approved
  38. ken2812221 commented at 9:26 AM on June 6, 2018: contributor

    utACK 16e3cd380af570fb2f656e0344bab88829a4bcda

  39. MarcoFalke removed the label Needs rebase on Jun 6, 2018
  40. Empact commented at 12:04 AM on June 7, 2018: member

    re-utACK 16e3cd3

  41. promag commented at 1:46 AM on June 7, 2018: member

    utACK 16e3cd3.

  42. MarcoFalke commented at 4:02 AM on June 7, 2018: member

    utACK 16e3cd3

  43. laanwj merged this on Jun 11, 2018
  44. laanwj closed this on Jun 11, 2018

  45. laanwj referenced this in commit 7c32b414b6 on Jun 11, 2018
  46. Bushstar cross-referenced this on Jun 12, 2018 from issue commits from bitcoin/master by Bushstar
  47. PastaPastaPasta referenced this in commit 0295a8986a on Jun 17, 2020
  48. PastaPastaPasta referenced this in commit 6e5b3f3b9e on Jul 2, 2020
  49. practicalswift deleted the branch on Apr 10, 2021
  50. Fuzzbawls cross-referenced this on Feb 7, 2022 from issue [Lint] Introduce lint-includes.sh script by Fuzzbawls
  51. random-zebra referenced this in commit 0e757ad2c9 on Feb 9, 2022
  52. gades referenced this in commit 2bd3aef83d on Feb 21, 2022
  53. bitcoin locked this on Aug 18, 2022

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:55 UTC