Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP][clang-tidy] add clang-tidy check in the end of PR-CI-Build #55619

Open
wants to merge 21 commits into
base: develop
Choose a base branch
from
Open

[WIP][clang-tidy] add clang-tidy check in the end of PR-CI-Build #55619

wants to merge 21 commits into from
+212 −176

Conversation

ccsuzzh
Copy link
Contributor

@ccsuzzh ccsuzzh commented Jul 21, 2023 •
edited
Loading

PR types

Others

PR changes

Others

Description

No.70 add clang-tidy check in the end of PR-CI-Build

@paddle-bot paddle-bot bot added the contributor External developers label Jul 21, 2023
@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Jul 24, 2023
@luotao1 luotao1 mentioned this pull request Jul 24, 2023
Open
@luotao1
Copy link
Contributor

luotao1 commented Jul 26, 2023

image apt-get install clang-tidy-10 -y 这个命令装不上。 
pip3.7 --no-cache-dir install clang-tidy==13.0.1.1
Collecting clang-tidy==13.0.1.1
  Downloading clang_tidy-13.0.1.1-py2.py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (21.3 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 0.2/21.3 MB 33.6 kB/s eta 0:10:28

我本地用pip只能按照13以上的版本。

@GreatV 帮忙一起看看CI镜像中如何安装～

@luotao1
Copy link
Contributor

luotao1 commented Jul 26, 2023 

λ c06e91bdfe32 /Paddle {clang-tidy} pre-commit run clang-tidy
clang-tidy...........................................(no files to check)Skipped

本地镜像中我使用precommit看上去是安装了clang-tidy，但没有用apt-get安装过，那是用什么方式给安装上的？CI中可以用相同方式安装么？

@GreatV
Copy link
Contributor

GreatV commented Jul 26, 2023

@luotao1 这个是通过 apt-get 安装的。

@GreatV
Copy link
Contributor

GreatV commented Jul 26, 2023 •
edited
Loading

镜像里默认的是clang-tidy 3，是和llvm一起安装上的，对c++17的支持好像不太行。

@ccsuzzh ccsuzzh requested review from luotao1 and GreatV July 27, 2023 06:26
luotao1
luotao1 reviewed Jul 31, 2023
View reviewed changes
Copy link
Contributor

@luotao1 luotao1 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
  1. 可以在脚本里加上clang-tidy是否安装以及安装的版本检查么？有的在pre_commit脚本里加的，有的在xxx.hook里加，clang-tidy是要加在clang-tidy.py里么？
  2. 可以测一下修改不同数量文件比如10/50/100/100，pre-commit clang-tidy 运行时间么？以及是否能拦截住
  3. Dockerfile中的安装不只这一处，可以多个Dockerfile中都安装上

@GreatV
Copy link
Contributor

GreatV commented Aug 1, 2023 •
edited
Loading

@luotao1

  1. 我们到时候另外提个PR，可以在clang-tidy.py里加一下。
  2. 等我在这个PR的基础上提几个测试PR，看看效果。等测试完，再合这个PR。

@GreatV GreatV mentioned this pull request Aug 2, 2023
Closed
@luotao1 luotao1 mentioned this pull request Aug 3, 2023
Merged
@luotao1 luotao1 added HappyOpenSource Pro 进阶版快乐开源活动，更具挑战性的任务 and removed HappyOpenSource 快乐开源活动issue与PR labels Aug 7, 2023
@paddle-ci-bot
Copy link

paddle-ci-bot bot commented Aug 12, 2023

Sorry to inform you that 822b9f5's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@paddle-ci-bot
Copy link

paddle-ci-bot bot commented Aug 27, 2023

Sorry to inform you that 57ec34f's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@ccsuzzh ccsuzzh requested a review from luotao1 December 2, 2023 12:52
@luotao1
Copy link
Contributor

luotao1 commented Dec 4, 2023

@ccsuzzh @GreatV

可以测一下修改不同数量文件比如10/50/100/100，pre-commit clang-tidy 运行时间么？以及是否能拦截住

能否补充下这个运行时间么

@GreatV
Copy link
Contributor

GreatV commented Dec 4, 2023

@luotao1
好的，好的，收到。

@ccsuzzh ccsuzzh closed this Dec 4, 2023
@ccsuzzh ccsuzzh reopened this Dec 4, 2023
@ccsuzzh
Copy link
Contributor Author

ccsuzzh commented Dec 4, 2023

@ccsuzzh @GreatV

可以测一下修改不同数量文件比如10/50/100/100，pre-commit clang-tidy 运行时间么？以及是否能拦截住

能否补充下这个运行时间么

好的.

@ccsuzzh ccsuzzh changed the title [clang-tidy] add clang-tidy check in the end of PR-CI-Build [WIP][clang-tidy] add clang-tidy check in the end of PR-CI-Build Dec 12, 2023
@ccsuzzh
Copy link
Contributor Author

ccsuzzh commented Dec 12, 2023

参考@GreatV大佬的PR #55923，在测试时间时把所有检查rules打开。

@ccsuzzh
Copy link
Contributor Author

ccsuzzh commented Dec 12, 2023 •
edited
Loading

待补充CI测试时间：

Commit ID 流水线链接 文件修改数量 运行时间 是否拦截成功
40d36e8 24783755 10 image ✅
50
100

@ccsuzzh
Copy link
Contributor Author

ccsuzzh commented Dec 15, 2023

待补充CI测试时间：

Commit ID 流水线链接 文件修改数量 运行时间 是否拦截成功
40d36e8 24783755 10 image ✅
50
100

@luotao1 @GreatV 跟二位确认一下目前这样的测试方法是否可行，是否合理？可以的话，我就继续往下测试，得一个文件一个文件手动修改。

@GreatV
Copy link
Contributor

GreatV commented Dec 15, 2023 •
edited
Loading

测试方法应该是可以的， 但这个PR好像还有点问题

  • omp.h会提示找不到 /paddle/third_party/eigen3/unsupported/Eigen/CXX11/../../../Eigen/Core:70:10: error: 'omp.h' file not found [clang-diagnostic-error]
  • 重复安装了 clang-tidy 2023-12-14 22:07:48 clang-tidy not found, attempting auto-install...
  • 2023-12-14 22:07:48 Error: no input files specified.

@ccsuzzh
Copy link
Contributor Author

ccsuzzh commented Dec 19, 2023

测试方法应该是可以的， 但这个PR好像还有点问题

  • omp.h会提示找不到 /paddle/third_party/eigen3/unsupported/Eigen/CXX11/../../../Eigen/Core:70:10: error: 'omp.h' file not found [clang-diagnostic-error]
  • 重复安装了 clang-tidy 2023-12-14 22:07:48 clang-tidy not found, attempting auto-install...
  • 2023-12-14 22:07:48 Error: no input files specified.

我在本地环境也存在这个两个问题。问题一，我本地环境已修复，是缺少libomp5,libomp-dev这两个包，但是在ci中安装这个两个包后仍然存在该问题。问题二，已修复。

@ccsuzzh
Copy link
Contributor Author

ccsuzzh commented Dec 21, 2023

测试方法应该是可以的， 但这个PR好像还有点问题

  • omp.h会提示找不到 /paddle/third_party/eigen3/unsupported/Eigen/CXX11/../../../Eigen/Core:70:10: error: 'omp.h' file not found [clang-diagnostic-error]
  • 重复安装了 clang-tidy 2023-12-14 22:07:48 clang-tidy not found, attempting auto-install...
  • 2023-12-14 22:07:48 Error: no input files specified.

大佬，请教一下clang-tidy脚本能加-fopenmp这个参数吗？具体加在哪里？

@GreatV
Copy link
Contributor

GreatV commented Dec 21, 2023

这里是只用安装 libomp-dev

sudo apt-get install libomp-dev

@ccsuzzh
Copy link
Contributor Author

ccsuzzh commented Dec 21, 2023

是只用安装 libomp-dev

是的，我在本地安装libomp-dev的时候需要依赖libomp5. 我在脚本里面把这两个都安装了，但是还是会有这个报错error: 'omp.h' file not found [clang-diagnostic-error]

@ccsuzzh ccsuzzh mentioned this pull request Dec 22, 2023
Closed
26 tasks
@luotao1
Copy link
Contributor

luotao1 commented Feb 19, 2024

@ccsuzzh @GreatV @enkilee 三位大佬一起看看这个PR的后续？包括下面这个PR

@luotao1 luotao1 mentioned this pull request May 16, 2024
Open
@luotao1 luotao1 removed the HappyOpenSource Pro 进阶版快乐开源活动，更具挑战性的任务 label Jul 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GreatV GreatV Awaiting requested review from GreatV

@luotao1 luotao1 Awaiting requested review from luotao1

At least 1 approving review is required to merge this pull request.

Assignees

@luotao1 luotao1

@GreatV GreatV

@zhangbo9674 zhangbo9674

Labels
contributor External developers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ccsuzzh @luotao1 @GreatV @zhangbo9674