mapbox / node-cpp-skel

Skeleton for bindings to C++ libraries for Node.js using node-addon-api
Creative Commons Zero v1.0 Universal
72 stars 10 forks source link

Improve the clang-tidy script #128

Closed springmeyer closed 6 years ago

springmeyer commented 6 years ago

This fixes a longstanding issue in the scripts/clang-tidy.sh script. Previously if the build failed it would leave behind an empty build/compile_commands.json. Then - when you went to run the tidy target again - the script logic would see that build/compile_commands.json existed and would avoid running the build again, leading clang-tidy to spuriously report nothing.

This change improves the script to avoid the possibility that it will write an empty build/compile_commands.json

Context: The scripts/generate_compile_commands.py is needed to reformat the build out into this specific format (https://clang.llvm.org/docs/JSONCompilationDatabase.html). Some build systems can do this automatically (like cmake https://cmake.org/cmake/help/v3.5/variable/CMAKE_EXPORT_COMPILE_COMMANDS.html) but here we use gyp, which cannot. So the scripts/generate_compile_commands.py is my attempt to bolt this functionality onto gyp.

/cc @allieoop for final PR. This PR is a subset created via https://github.com/mapbox/node-cpp-skel/pull/118/files#diff-297d66ab0c294f76d1a7ab83006a7383

springmeyer commented 6 years ago

fe692a0 is based on some learning trying to debug hanging clang-tidy jobs in another repo. I think what was happening was:

So, using tee ensures that the normal build output is still streamed to travis logs, but also to a file (so that we can then generate the compile_commands.json when needed).

/cc @GretaCB

sssoleileraaa commented 6 years ago

Ah, I see. Thanks for the clear explanation of how tee solves the problem of travis seeing no output during a long make. And this also addresses the issue we discussed in the larger PR. Good find.

lgtm!