You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2018/09/17 23:07:05 UTC
[kudu-CR] clang tidy gerrit.py: fix output when no changes found in first path
Hello Alexey Serbin, Andrew Wong,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/11457
to review the following change.
Change subject: clang_tidy_gerrit.py: fix output when no changes found in first path
......................................................................
clang_tidy_gerrit.py: fix output when no changes found in first path
I was perplexed as to why some patches weren't getting any clang-tidy
comments, so I went looking at the clang-tidy job output and found this:
Clang output
No relevant changes found.
No relevant changes found.
... <actual clang-tidy changes>
Traceback (most recent call last):
File "build-support/clang_tidy_gerrit.py", line 209, in <module>
parsed = parse_clang_output(clang_output)
File "build-support/clang_tidy_gerrit.py", line 103, in parse_clang_output
raise Exception("bad warning: " + w)
Exception: bad warning: No relevant changes found.
No relevant changes found.
Turns out that although clang_tidy_gerrit.py never handled "No relevant..."
properly, commit a9271b05d significantly increased the chances that the very
first line of output would be this string. That's a necessary condition for
split_warnings() to generate a warning starting with a non-warning string,
which led to the exception and no posted comments.
Change-Id: I111dff7508f841489ba1625a4ca4b7af92f3d8d0
---
M build-support/clang_tidy_gerrit.py
1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/11457/1
--
To view, visit http://gerrit.cloudera.org:8080/11457
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I111dff7508f841489ba1625a4ca4b7af92f3d8d0
Gerrit-Change-Number: 11457
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
[kudu-CR] clang tidy gerrit.py: fix output when no changes found in first path
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11457
to look at the new patch set (#2).
Change subject: clang_tidy_gerrit.py: fix output when no changes found in first path
......................................................................
clang_tidy_gerrit.py: fix output when no changes found in first path
For a while now some patches weren't getting any Tidy Bot comments. These
comments originate in a standalone Jenkins job that runs clang-tidy and
converts its output into comments that are posted back to gerrit. Here's
what I saw in the console output of one such failed job run:
Clang output
No relevant changes found.
No relevant changes found.
... <actual clang-tidy changes>
Traceback (most recent call last):
File "build-support/clang_tidy_gerrit.py", line 209, in <module>
parsed = parse_clang_output(clang_output)
File "build-support/clang_tidy_gerrit.py", line 103, in parse_clang_output
raise Exception("bad warning: " + w)
Exception: bad warning: No relevant changes found.
No relevant changes found.
The "No relevant changes found." line is what clang-tidy prints when it has
no recommendations. That won't happen when clang-tidy's input includes at
least one "dirty" file, but as of commit a9271b05d we run clang-tidy in
parallel on a per-file basis, which makes it quite likely that a given patch
will include at least one completely tidy file.
Turns out that when the very first line of output parsed by this script is
that "No relevant..." line, split_warnings() generates a warning with a
non-warning string, which causes parse_clang_output() to raise an exception
and for the job to fail silently.
Change-Id: I111dff7508f841489ba1625a4ca4b7af92f3d8d0
---
M build-support/clang_tidy_gerrit.py
1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/11457/2
--
To view, visit http://gerrit.cloudera.org:8080/11457
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I111dff7508f841489ba1625a4ca4b7af92f3d8d0
Gerrit-Change-Number: 11457
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] clang tidy gerrit.py: fix output when no changes found in first path
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11457 )
Change subject: clang_tidy_gerrit.py: fix output when no changes found in first path
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11457/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11457/1//COMMIT_MSG@12
PS1, Line 12: Clang output
: No relevant changes found.
: No relevant changes found.
: ... <actual clang-tidy changes>
: Traceback (most recent call last):
: File "build-support/clang_tidy_gerrit.py", line 209, in <module>
: parsed = parse_clang_output(clang_output)
: File "build-support/clang_tidy_gerrit.py", line 103, in parse_clang_output
: raise Exception("bad warning: " + w)
: Exception: bad warning: No relevant changes found.
: No relevant changes found.
Without digging into the clang tidy script, I'm not sure why certain patches would yield "No relevant..." errors. What patches would the job fail on before?
--
To view, visit http://gerrit.cloudera.org:8080/11457
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I111dff7508f841489ba1625a4ca4b7af92f3d8d0
Gerrit-Change-Number: 11457
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 18 Sep 2018 04:57:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] clang tidy gerrit.py: fix output when no changes found in first path
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11457 )
Change subject: clang_tidy_gerrit.py: fix output when no changes found in first path
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
> Patch Set 2:
>
> (1 comment)
http://gerrit.cloudera.org:8080/#/c/11457/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11457/1//COMMIT_MSG@12
PS1, Line 12: what I saw in the console output of one such failed job run:
:
: Clang output
: No relevant changes found.
: No relevant changes found.
: ... <actual clang-tidy changes>
: Traceback (most recent call last):
: File "build-support/clang_tidy_gerrit.py", line 209, in <module>
: parsed = parse_clang_output(clang_output)
: File "build-support/clang_tidy_gerrit.py", line 103, in parse_clang_output
: raise Exception("bad w
> "No relevant..." is what clang-tidy returns when it has nothing to recommen
Got it. Thanks for the explanation (and the update)!
--
To view, visit http://gerrit.cloudera.org:8080/11457
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I111dff7508f841489ba1625a4ca4b7af92f3d8d0
Gerrit-Change-Number: 11457
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 18 Sep 2018 18:26:08 +0000
Gerrit-HasComments: Yes
[kudu-CR] clang tidy gerrit.py: fix output when no changes found in first path
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11457 )
Change subject: clang_tidy_gerrit.py: fix output when no changes found in first path
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11457/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11457/1//COMMIT_MSG@12
PS1, Line 12: what I saw in the console output of one such failed job run:
:
: Clang output
: No relevant changes found.
: No relevant changes found.
: ... <actual clang-tidy changes>
: Traceback (most recent call last):
: File "build-support/clang_tidy_gerrit.py", line 209, in <module>
: parsed = parse_clang_output(clang_output)
: File "build-support/clang_tidy_gerrit.py", line 103, in parse_clang_output
: raise Exception("bad w
> Without digging into the clang tidy script, I'm not sure why certain patche
"No relevant..." is what clang-tidy returns when it has nothing to recommend. That's very unlikely when run on an entire patch (as we used to do), but after commit a9271b05d, we actually run it once per changed file, so now it's possible to see it as some files (especially new ones) are completely tidy.
To trigger the bug, you need the _very first_ file processed to produce this message, at which point split_warnings() gets messed up and returns a warning containing just the "No relevant..." line, which causes parse_clang_output() to raise an exception, the script to abort, and no clang-tidy comments to materialize in the gerrit.
--
To view, visit http://gerrit.cloudera.org:8080/11457
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I111dff7508f841489ba1625a4ca4b7af92f3d8d0
Gerrit-Change-Number: 11457
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 18 Sep 2018 17:18:20 +0000
Gerrit-HasComments: Yes
[kudu-CR] clang tidy gerrit.py: fix output when no changes found in first path
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11457 )
Change subject: clang_tidy_gerrit.py: fix output when no changes found in first path
......................................................................
clang_tidy_gerrit.py: fix output when no changes found in first path
For a while now some patches weren't getting any Tidy Bot comments. These
comments originate in a standalone Jenkins job that runs clang-tidy and
converts its output into comments that are posted back to gerrit. Here's
what I saw in the console output of one such failed job run:
Clang output
No relevant changes found.
No relevant changes found.
... <actual clang-tidy changes>
Traceback (most recent call last):
File "build-support/clang_tidy_gerrit.py", line 209, in <module>
parsed = parse_clang_output(clang_output)
File "build-support/clang_tidy_gerrit.py", line 103, in parse_clang_output
raise Exception("bad warning: " + w)
Exception: bad warning: No relevant changes found.
No relevant changes found.
The "No relevant changes found." line is what clang-tidy prints when it has
no recommendations. That won't happen when clang-tidy's input includes at
least one "dirty" file, but as of commit a9271b05d we run clang-tidy in
parallel on a per-file basis, which makes it quite likely that a given patch
will include at least one completely tidy file.
Turns out that when the very first line of output parsed by this script is
that "No relevant..." line, split_warnings() generates a warning with a
non-warning string, which causes parse_clang_output() to raise an exception
and for the job to fail silently.
Change-Id: I111dff7508f841489ba1625a4ca4b7af92f3d8d0
Reviewed-on: http://gerrit.cloudera.org:8080/11457
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M build-support/clang_tidy_gerrit.py
1 file changed, 5 insertions(+), 1 deletion(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
Andrew Wong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/11457
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I111dff7508f841489ba1625a4ca4b7af92f3d8d0
Gerrit-Change-Number: 11457
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] clang tidy gerrit.py: fix output when no changes found in first path
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11457 )
Change subject: clang_tidy_gerrit.py: fix output when no changes found in first path
......................................................................
Patch Set 1: Code-Review+2
Good catch!
--
To view, visit http://gerrit.cloudera.org:8080/11457
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I111dff7508f841489ba1625a4ca4b7af92f3d8d0
Gerrit-Change-Number: 11457
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 17 Sep 2018 23:25:56 +0000
Gerrit-HasComments: No
[kudu-CR] clang tidy gerrit.py: fix output when no changes found in first path
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11457 )
Change subject: clang_tidy_gerrit.py: fix output when no changes found in first path
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11457
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I111dff7508f841489ba1625a4ca4b7af92f3d8d0
Gerrit-Change-Number: 11457
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 18 Sep 2018 18:19:56 +0000
Gerrit-HasComments: No