[Don‘t merge][CodeStyle] repro of cpplint CI issue

[SigureMo]

PR types

Others

PR changes

Others

Describe

• Flake8 tracking issue: [CodeStyle] Flake8 tracking issue #46039

本地发现有很多 cpplint 的问题（上千），测试下 CI 环境是否可以正常拦截 cpplint 的问题

[paddle-bot]

你的PR提交成功，感谢你对开源项目的贡献!
请关注后续CI自动化测试结果，详情请参考Paddle-CI手册。
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

[SigureMo]

PR-CI-Codestyle-Check 已通过（流水线见 https://xly.bce.baidu.com/paddlepaddle/paddle/newipipe/detail/6636204/job/18498526 ），CI 貌似确实无法拦截，而 paddle/fluid/distributed/ps/service/graph_brpc_server.cc 文件中存在一些 cpplint error，本地 pre-commit 是可以拦截的

Note

其余 CI 貌似因为 patchelf 啥大小超了的问题都挂掉了，目前大多数新提交的 PR 都因为这个问题挂了（包括我的另外两个 PR），与本 PR 无关 该问题已解决

问题描述

pre-commit 本身的逻辑是对改动的文件进行校验（传入相应的 hook），当然也可以通过 pre-commit run --files <file_path> 来强制对某一个或者多个文件进行校验

目前 CI（如 PR-CI-Codestyle-Check，PR-CI-Static-Check 目测一样）的逻辑如下：

Paddle/tools/codestyle/pre_commit.sh

Lines 32 to 44 in 92e1f64

	diff_files=$(git diff --numstat ${BRANCH} | awk '{print $NF}')
	echo -e "diff files between pr and ${BRANCH}:\n${diff_files}"
	echo "Checking code style by pre-commit ..."
	pre-commit run --files ${diff_files};check_error=$?
	if test ! -z "$(git diff)"; then
	echo -e '\n************************************************************************************'
	echo -e "These files have been formated by code format hook. You should use pre-commit to \
	format them before git push."
	echo -e '************************************************************************************\n'
	git diff 2>&1
	fi

通过对比与 develop 发生 change 的文件，然后利用 pre-commit run --files <file_path> 对这些文件进行校验

对于其余直接使用 pre-commit 传入文件路径的 hook 没有任何问题，但是 cpplint 是 hook 文件里自行计算 diff 的文件的（经测试，不走 TRAVIS_BRANCH 分支，CI 走的也是本地分支）

Paddle/tools/codestyle/cpplint_pre_commit.hook

Lines 9 to 30 in 92e1f64

	if [[ ! $TRAVIS_BRANCH ]]; then
	# install cpplint on local machine.
	if ! [[ $version == *"$VERSION"* ]]; then
	pip install cpplint==1.6.0
	fi
	# diff files on local machine.
	files=$(git diff --cached --name-status | awk '$1 != "D" {print $2}')
	else
	# diff files between PR and latest commit on Travis CI.
	branch_ref=$(git rev-parse "$TRAVIS_BRANCH")
	head_ref=$(git rev-parse HEAD)
	files=$(git diff --name-status $branch_ref $head_ref | awk '$1 != "D" {print $2}')
	fi
	# The trick to remove deleted files: https://stackoverflow.com/a/2413151
	for file in $files; do
	if [[ $file =~ ^(patches/.*) ]]; then
	continue;
	else
	cpplint --filter=-readability/fn_size,-build/include_what_you_use,-build/c++11,-whitespace/parens $file;
	TOTAL_ERRORS=$(expr $TOTAL_ERRORS + $?);
	fi
	done

这样的话，在 CI 上得到的 diff files 其实是空的，当然 CI 是一定通过的了

建议解决方案

貌似直接传入 TRAVIS_BRANCH 就可以解决（貌似以前使用 Travis CI 所设的分支？）

当然，如果可以的话，直接复用 pre-commit 计算得到的 diff 路径即可，这样和其他 hook 行为一致，而且能复用 pre-commit 的一些操作（如 pre-commit run cpplint-cpp-source --all-files 进行一键扫描）

我尝试了下后者，CI 上确实也可以拦截了

存量问题

如果没有统计错的话，目前是有 1000+ 处 cpplint 问题，涉及 154 个文件（cuh 文件没统计，如果加上的话 157 个），log 见 cpplint.log（太多了，我个人可能解决不了）

统计方式为，修改 tools/codestyle/cpplint_pre_commit.hook 内容如下（即直接复用 pre-commit 传入的参数）：

#!/bin/bash

cpplint --filter=-readability/fn_size,-build/include_what_you_use,-build/c++11,-whitespace/parens --quiet $@
#                                                                                                   ^ 不添加 --quiet 会把所有 PASS 的文件也都打印一遍，不利于分析，建议 pre-commit 配置里开启该选项

之后运行 pre-commit run cpplint-cpp-source --all-files 即可复现

修复方式也许可以同 #43222，先修复并 exclude 掉存在问题的文件，以确保 CI 上 cpplint 会运行，然后后续 PR 集中解决这些文件中的问题

其他

目测 pylint 有同样问题

Paddle/tools/codestyle/pylint_pre_commit.hook

Lines 16 to 21 in 92e1f64

	# The trick to remove deleted files: https://stackoverflow.com/a/2413151
	for file in $(git diff --name-status | awk '$1 != "D" {print $2}'); do
	pylint --disable=all --load-plugins=docstring_checker \
	--enable=doc-string-one-line,doc-string-end-with,doc-string-with-all-args,doc-string-triple-quotes,doc-string-missing,doc-string-indent-error,doc-string-with-returns,doc-string-with-raises $file;
	TOTAL_ERRORS=$(expr $TOTAL_ERRORS + $?);
	done