envoy icon indicating copy to clipboard operation
envoy copied to clipboard

fix clang-tidy reported errors on master

Open lizan opened this issue 6 years ago • 15 comments

Description: clang-tidy runs against diffs in CI, but it will caught all errors on the lines the PR author touched. Ideally we should make a full clang-tidy run error free.

lizan avatar Oct 26 '18 01:10 lizan

I am very interested, I can come to fix the issue

zyfjeff avatar Oct 27 '18 02:10 zyfjeff

@zyfjeff Great, I think you can just run clang-tidy with -fix flag with the checks marked as error.

lizan avatar Oct 27 '18 09:10 lizan

Not directly relevant, but would be nice for instructions on clang-tidy to be added in the same places as instructions on fix_format, e.g. https://github.com/envoyproxy/envoy/blob/15706964a29e595c045a5d7ef0646d04f347dcc1/support/README.md

auni53 avatar Nov 01 '18 21:11 auni53

@auni53 Definitely, do you want to take that part?

lizan avatar Nov 01 '18 21:11 lizan

Can do :)

auni53 avatar Nov 02 '18 00:11 auni53

I have no idea how to run the clang-tidy script locally. Documenting here to try and prevent others running into the same issue...

$ ./ci/run_envoy_docker.sh './ci/do_ci.sh bazel.clang_tidy'         
      
No remote cache bucket is set, skipping setup remote cache.
ENVOY_SRCDIR=/source
fatal: Not a git repository (or any parent up to mount point /build)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).```

auni53 avatar Nov 02 '18 12:11 auni53

ah, rm -rf /tmp/envoy-docker-build did the trick.

auni53 avatar Nov 02 '18 13:11 auni53

OK yeah that did as far as running the script, but I'm continually running into errors. I have no idea how any of this works, but here's what I understand:

  • My tests time out: https://circleci.com/gh/envoyproxy/envoy/118039

  • By default, $ ./ci/run_envoy_docker.sh './ci/run_clang_tidy.sh' elicits

Generating compilation database...
./ci/run_clang_tidy.sh: line 12: /tools/gen_compilation_database.py: No such file or directory

This is because ENVOY_SRCDIR isn't set for me by default. Fixed it by replacing the ${ENVOY_SRCDIR} in run_clang_tidy.sh with ${ENVOY_SRCDIR:-.}, not sure if that should be addressed elsewise.

  • Every failed attempt at the script adds a blank line build to .bazelrc (from the line echo "build ${BAZEL_BUILD_OPTIONS}" >> .bazelrc). Should be either .gitignore'd or addressed I suppose. I'm now wondering whether the reason I'm cursed is that I have my local `~/.bazelrc

auni53 avatar Nov 03 '18 01:11 auni53

@auni53 'ci/run_clang_tidy.sh' is written for running from do_ci.sh so it may depend on env vars from there. Usually I do tools/gen_compilation_database.py --run_bazel_build --include_header && run-clang-tidy-7, let me know (or ping me on Slack) when you face any issue.

lizan avatar Nov 03 '18 05:11 lizan

hah, took a day to realize that should be run-clang-tidy -7 (in case anyone else comes along this)

auni53 avatar Nov 04 '18 15:11 auni53

@auni53 no run-clang-tidy-7 (without space) is the right one if you're in envoy-build-docker or have llvm installed via official apt repository.

lizan avatar Nov 04 '18 22:11 lizan

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

stale[bot] avatar Dec 04 '18 23:12 stale[bot]

latest status of master errors that I'm getting with clang-tidy-8

dereka@dev-dereka-2:~/code/envoy-oss$ make clang-tidy 2>&1 | grep " error: " | grep -v "chromium_url"
/home/dereka/.cache/bazel/_bazel_dereka/b4e08c1eaa8adbb40db2e2ca7ea0cbf2/execroot/envoy/include/envoy/server/admin.h:124:53: error: invalid case style for parameter 'access_log_path_' [readability-identifier-naming,-warnings-as-errors]
/home/dereka/.cache/bazel/_bazel_dereka/b4e08c1eaa8adbb40db2e2ca7ea0cbf2/execroot/envoy/include/envoy/server/admin.h:124:53: error: invalid case style for parameter 'access_log_path_' [readability-identifier-naming,-warnings-as-errors]
bazel-out/k8-fastbuild/bin/source/extensions/quic_listeners/quiche/_virtual_includes/envoy_quic_server_session_lib/extensions/quic_listeners/quiche/envoy_quic_server_session.h:16:10: error: 'extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h' file not found [clang-diagnostic-error]
bazel-out/k8-fastbuild/bin/source/extensions/quic_listeners/quiche/_virtual_includes/envoy_quic_server_session_lib/extensions/quic_listeners/quiche/envoy_quic_server_session.h:16:10: error: 'extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h' file not found [clang-diagnostic-error]
source/extensions/quic_listeners/quiche/platform/quic_stack_trace_impl.h:16:22: error: implicit instantiation of undefined template 'std::__cxx11::basic_ostringstream<char, std::char_traits<char>, std::allocator<char> >' [clang-diagnostic-error]
bazel-out/k8-fastbuild/bin/source/extensions/quic_listeners/quiche/_virtual_includes/envoy_quic_server_session_lib/extensions/quic_listeners/quiche/envoy_quic_server_session.h:16:10: error: 'extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h' file not found [clang-diagnostic-error]
bazel-out/k8-fastbuild/bin/source/extensions/quic_listeners/quiche/_virtual_includes/envoy_quic_server_session_lib/extensions/quic_listeners/quiche/envoy_quic_server_session.h:16:10: error: 'extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h' file not found [clang-diagnostic-error]
source/extensions/quic_listeners/quiche/platform/quic_stack_trace_impl.h:16:22: error: implicit instantiation of undefined template 'std::__cxx11::basic_ostringstream<char, std::char_traits<char>, std::allocator<char> >' [clang-diagnostic-error]
bazel-out/k8-fastbuild/bin/source/extensions/quic_listeners/quiche/_virtual_includes/envoy_quic_server_session_lib/extensions/quic_listeners/quiche/envoy_quic_server_session.h:16:10: error: 'extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h' file not found [clang-diagnostic-error]
bazel-out/k8-fastbuild/bin/source/extensions/quic_listeners/quiche/_virtual_includes/envoy_quic_server_session_lib/extensions/quic_listeners/quiche/envoy_quic_server_session.h:16:10: error: 'extensions/quic_listeners/quiche/quic_filter_manager_connection_impl.h' file not found [clang-diagnostic-error]
test/server/filter_chain_benchmark_test.cc:174:20: error: initializer for base class 'Envoy::Server::FilterChainBenchmarkFixture' is redundant [readability-redundant-member-init,-warnings-as-errors]
test/server/filter_chain_benchmark_test.cc:183:20: error: initializer for base class 'Envoy::Server::FilterChainBenchmarkFixture' is redundant [readability-redundant-member-init,-warnings-as-errors]
/home/dereka/.cache/bazel/_bazel_dereka/b4e08c1eaa8adbb40db2e2ca7ea0cbf2/execroot/envoy/test/common/upstream/cluster_manager_impl_test.cc:49:16: error: using decl 'ByRef' is unused [misc-unused-using-decls,-warnings-as-errors]
dereka@dev-dereka-2:~/code/envoy-oss$

derekargueta avatar Oct 08 '19 08:10 derekargueta

hi, anyone working on this issue ?

arvryna avatar Sep 12 '21 04:09 arvryna

Did anyone meet this issue when running full clang-tidy under docker?

$ ./ci/run_envoy_docker.sh "RUN_FULL_CLANG_TIDY=1 ./ci/do_ci.sh bazel.clang_tidy"
3de483a98c5e24973e710b4f97b2dabcd3cb621f: Pulling from envoyproxy/envoy-build-ubuntu
Digest: sha256:a0dfdec963cec68c0636578ec1587cc542f9839584f32bb56165284d64cbafe6
Status: Image is up to date for envoyproxy/envoy-build-ubuntu:3de483a98c5e24973e710b4f97b2dabcd3cb621f
docker.io/envoyproxy/envoy-build-ubuntu:3de483a98c5e24973e710b4f97b2dabcd3cb621f

real    0m3.934s
user    0m0.029s
sys     0m0.003s
No remote cache is set, skipping setup remote cache.
ENVOY_SRCDIR=/source
ENVOY_BUILD_TARGET=//source/exe:envoy-static
ENVOY_BUILD_ARCH=x86_64
$TEST_TMPDIR defined: output root default is '/build/tmp' and max_idle_secs default is '15'.
Starting local Bazel server and connecting to it...
WARNING: info command does not support starlark options. Ignoring options: [--@com_googlesource_googleurl//build_config:system_icu=0]
Skip setting up Envoy Filter Example.
building using 8 CPUs
building for x86_64
clang toolchain with libstdc++ configured
Generating compilation database...
$TEST_TMPDIR defined: output root default is '/build/tmp' and max_idle_secs default is '15'.
INFO: Analyzed 3758 targets (1466 packages loaded, 92970 targets configured).
INFO: Found 3758 targets...
INFO: Elapsed time: 36.237s, Critical Path: 0.58s
INFO: 1 process: 1 internal.
$TEST_TMPDIR defined: output root default is '/build/tmp' and max_idle_secs default is '15'.
WARNING: info command does not support starlark options. Ignoring options: [--@com_googlesource_googleurl//build_config:system_icu=0]
Running a full clang-tidy
python3: can't open file '/opt/llvm/share/clang/run-clang-tidy.py': [Errno 2] No such file or directory

gyohuangxin avatar Aug 09 '22 17:08 gyohuangxin