You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2018/03/08 03:52:44 UTC
[kudu-CR] iwyu: workaround missing IWYU results when using Ninja
Todd Lipcon has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9554
Change subject: iwyu: workaround missing IWYU results when using Ninja
......................................................................
iwyu: workaround missing IWYU results when using Ninja
This adds a workaround so that IWYU reports the same results whether
cmake is invoked with the Makefile generator or the Ninja generator.
Tested by verifying that IWYU on my laptop now reports the same issues
that Jenkins was reporting precommit on a recent in-flight patch.
Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
---
M build-support/iwyu/iwyu_tool.py
1 file changed, 27 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/9554/1
--
To view, visit http://gerrit.cloudera.org:8080/9554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
[kudu-CR] iwyu: workaround missing IWYU results when using Ninja
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9554 )
Change subject: iwyu: workaround missing IWYU results when using Ninja
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9554/1/build-support/iwyu/iwyu_tool.py
File build-support/iwyu/iwyu_tool.py:
http://gerrit.cloudera.org:8080/#/c/9554/1/build-support/iwyu/iwyu_tool.py@190
PS1, Line 190: )
maybe, make it possible to have spaces after the '-I' option but before the path?
Also, maybe the better criterion for an absolute path is that it's first character is not '/'?
--
To view, visit http://gerrit.cloudera.org:8080/9554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 08 Mar 2018 04:16:23 +0000
Gerrit-HasComments: Yes
[kudu-CR] iwyu: workaround missing IWYU results when using Ninja
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9554 )
Change subject: iwyu: workaround missing IWYU results when using Ninja
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 24 Apr 2018 03:50:21 +0000
Gerrit-HasComments: No
[kudu-CR] iwyu: workaround missing IWYU results when using Ninja
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9554 )
Change subject: iwyu: workaround missing IWYU results when using Ninja
......................................................................
iwyu: workaround missing IWYU results when using Ninja
This adds a workaround so that IWYU reports the same results whether
cmake is invoked with the Makefile generator or the Ninja generator.
Tested by verifying that IWYU on my laptop now reports the same issues
that Jenkins was reporting precommit on a recent in-flight patch.
Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Reviewed-on: http://gerrit.cloudera.org:8080/9554
Tested-by: Todd Lipcon <to...@apache.org>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M build-support/iwyu/iwyu_tool.py
1 file changed, 43 insertions(+), 0 deletions(-)
Approvals:
Todd Lipcon: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/9554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] iwyu: workaround missing IWYU results when using Ninja
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9554
to look at the new patch set (#3).
Change subject: iwyu: workaround missing IWYU results when using Ninja
......................................................................
iwyu: workaround missing IWYU results when using Ninja
This adds a workaround so that IWYU reports the same results whether
cmake is invoked with the Makefile generator or the Ninja generator.
Tested by verifying that IWYU on my laptop now reports the same issues
that Jenkins was reporting precommit on a recent in-flight patch.
Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
---
M build-support/iwyu/iwyu_tool.py
1 file changed, 43 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/9554/3
--
To view, visit http://gerrit.cloudera.org:8080/9554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] iwyu: workaround missing IWYU results when using Ninja
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9554 )
Change subject: iwyu: workaround missing IWYU results when using Ninja
......................................................................
Patch Set 2:
> Patch Set 2:
>
> (1 comment)
oh indeed, good catch
--
To view, visit http://gerrit.cloudera.org:8080/9554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 13 Mar 2018 01:39:13 +0000
Gerrit-HasComments: No
[kudu-CR] iwyu: workaround missing IWYU results when using Ninja
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9554
to look at the new patch set (#2).
Change subject: iwyu: workaround missing IWYU results when using Ninja
......................................................................
iwyu: workaround missing IWYU results when using Ninja
This adds a workaround so that IWYU reports the same results whether
cmake is invoked with the Makefile generator or the Ninja generator.
Tested by verifying that IWYU on my laptop now reports the same issues
that Jenkins was reporting precommit on a recent in-flight patch.
Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
---
M build-support/iwyu/iwyu_tool.py
1 file changed, 27 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/9554/2
--
To view, visit http://gerrit.cloudera.org:8080/9554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] iwyu: workaround missing IWYU results when using Ninja
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9554 )
Change subject: iwyu: workaround missing IWYU results when using Ninja
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9554/2/build-support/iwyu/iwyu_tool.py
File build-support/iwyu/iwyu_tool.py:
http://gerrit.cloudera.org:8080/#/c/9554/2/build-support/iwyu/iwyu_tool.py@190
PS2, Line 190: '(-isystem |-I\s*)([^/]\S+)'
I don't think this pattern works properly with include path specs like '-I /usr/include'.
--
To view, visit http://gerrit.cloudera.org:8080/9554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 13 Mar 2018 00:28:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] iwyu: workaround missing IWYU results when using Ninja
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has removed a vote on this change.
Change subject: iwyu: workaround missing IWYU results when using Ninja
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/9554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] iwyu: workaround missing IWYU results when using Ninja
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9554 )
Change subject: iwyu: workaround missing IWYU results when using Ninja
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9554/2/build-support/iwyu/iwyu_tool.py
File build-support/iwyu/iwyu_tool.py:
http://gerrit.cloudera.org:8080/#/c/9554/2/build-support/iwyu/iwyu_tool.py@190
PS2, Line 190: '(-isystem |-I\s*)([^/]\S+)'
> but that's not a relative path, so it shouldn't match, right?
Maybe I'm missing something, but I don't see how it's guaranteed that the command doesn't include absolute paths for '-I' directives. And if so, then I think the result is not what we want:
>>> import os
>>> import re
>>>
>>> def subber(m):
... return m.group(1) + os.path.abspath(os.path.join('/tmp') + m.group(2))
...
>>> re.sub(r'(-isystem |-I\s*)([^/]\S+)', subber, '-I /usr/include')
'-I/tmp /usr/include'
>>>
So, that's not what we want, right?
--
To view, visit http://gerrit.cloudera.org:8080/9554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 13 Mar 2018 00:42:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] iwyu: workaround missing IWYU results when using Ninja
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9554 )
Change subject: iwyu: workaround missing IWYU results when using Ninja
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9554/1/build-support/iwyu/iwyu_tool.py
File build-support/iwyu/iwyu_tool.py:
http://gerrit.cloudera.org:8080/#/c/9554/1/build-support/iwyu/iwyu_tool.py@190
PS1, Line 190: )
> maybe, make it possible to have spaces after the '-I' option but before the
basically, I think about the following pattern:
'(-isystem |-I\s*)([^\/\s]\S+)'
--
To view, visit http://gerrit.cloudera.org:8080/9554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 08 Mar 2018 04:40:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] iwyu: workaround missing IWYU results when using Ninja
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9554 )
Change subject: iwyu: workaround missing IWYU results when using Ninja
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9554/2/build-support/iwyu/iwyu_tool.py
File build-support/iwyu/iwyu_tool.py:
http://gerrit.cloudera.org:8080/#/c/9554/2/build-support/iwyu/iwyu_tool.py@190
PS2, Line 190: '(-isystem |-I\s*)([^/]\S+)'
> I don't think this pattern works properly with include path specs like '-I
but that's not a relative path, so it shouldn't match, right?
--
To view, visit http://gerrit.cloudera.org:8080/9554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 13 Mar 2018 00:29:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] iwyu: workaround missing IWYU results when using Ninja
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9554 )
Change subject: iwyu: workaround missing IWYU results when using Ninja
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/9554
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I092cab794fd20a39fcfbbbe52f4c325fac2d149c
Gerrit-Change-Number: 9554
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 23 Apr 2018 20:53:47 +0000
Gerrit-HasComments: No