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.

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

read stdout_stderr.log from build steps to extract CMake / compiler warnings #477

Merged
merged 3 commits into from
Jun 17, 2020

Conversation

dirk-thomas
Copy link
Member

@dirk-thomas dirk-thomas commented Jun 16, 2020

Instead of using the whole console output which includes output from other steps of the process, e.g. testing as well as repeated stderr output from colcon.

Addresses #316 on ci.ros2.org. Fixes #473.

CI builds without this change (and a custom branch to produce a compiler warning):

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

CI build with this change (the jobs will go away once this PR has been merged):

  • Linux Build Status (still extracting the compiler warnings, but without the duplicate GCC warning)
  • macOS: Build Status (still extracting the compiler warnings)
  • Windows Build Status (still extracting the compiler warnings) (no compiler warning anymore)

False positive resolved by this patch:
Before: Build Status (test output was classified as a compiler warning)
After (the jobs will go away once this PR has been merged): Build Status

@dirk-thomas dirk-thomas added the bug Something isn't working label Jun 16, 2020
@dirk-thomas dirk-thomas self-assigned this Jun 16, 2020
@dirk-thomas dirk-thomas changed the title read stdout_stderr.log from latest build to extract CMake / compiler warnings read stdout_stderr.log from build steps to extract CMake / compiler warnings Jun 16, 2020
@nuclearsandwich
Copy link
Member

These all hard-code the path ws. Do any builds randomly name the workspace path workspace or work space anymore?

When I was working on #447 I had to handle the elapsed time output (eg [0.554s] ...). Do any of the other warning parsers in use have an anchor to the start of the line (^)?

Signed-off-by: Dirk Thomas <[email protected]>
@dirk-thomas
Copy link
Member Author

These all hard-code the path ws. Do any builds randomly name the workspace path workspace or work space anymore?

Good point, I already forgot we have that parameter: d34af48.

Do any of the other warning parsers in use have an anchor to the start of the line (^)?

I don't have an answer for this specific questions. But the above jobs show that compiler warnings are correctly being extracted - for GNU C, Clang, Clang-tidy, and MSBuild.

I have triggered another build to check if CMake warnings are captures too:
Build Status (one CMake warning as expected)

So I would conclude it is good as is.

@dirk-thomas dirk-thomas merged commit 1827044 into master Jun 17, 2020
@delete-merged-branch delete-merged-branch bot deleted the dirk-thomas/compiler-warning-from-file branch June 17, 2020 15:21
@azeey
Copy link

azeey commented Jun 17, 2020

https://ci.ros2.org/view/nightly/job/nightly_linux_repeated/1905/ and https://ci.ros2.org/view/nightly/job/nightly_osx_debug/1666/ failed I think because it was looking for log files in work space/..., but the files were in workspace/.... @dirk-thomas, can you verify?

@dirk-thomas
Copy link
Member Author

When I was working on #447 I had to handle the elapsed time output (eg [0.554s] ...). Do any of the other warning parsers in use have an anchor to the start of the line (^)?

While this patch still extracts the warnings correctly the grouping contains the timestamp prefix from the log and as a consequence doesn't deduplicate same warnings (see https://ci.ros2.org/view/packaging/job/packaging_linux-centos/446/clang/#folderContent).

I don't think it is a feasible option to customize the Jenkins warnings handling. So the only viable option I can think of is generating the log files we need to parse without timestamp...

@dirk-thomas
Copy link
Member Author

See colcon/colcon-output#33.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False positive Clang warnings on Linux from openssl output.
3 participants