Adding PRESUBMIT check on orphan headers files.

Adding PRESUBMIT check on orphan headers files.

We want all .h files to be tracked by a GN target. This CL adds a
presubmit check to ensure that newly added targets are tracked by
GN.

In this CL we add a directory called 'presubmit_checks_lib' so we
can avoid to put everything in PRESUBMIT.py (especially if we want
to test some checks). I tried to use 'tools-webrtc' but it is not
easy to include a python module from a directory with '-' in the
name.

BUG=webrtc:7514
Review-Url: https://codereview.webrtc.org/2872493002
Cr-Commit-Position: refs/heads/master@{#18069}
Committed: https://chromium.googlesource.com/external/webrtc/+/74973ed847fb594fb8b80b809236eeef13b7338c

[mbonadei, 2017-05-08 06:53:55]

mbonadei@webrtc.org changed reviewers:
+ kjellander@webrtc.org

[kjellander_webrtc, 2017-05-08 18:52:00]

https://codereview.webrtc.org/2872493002/diff/1/PRESUBMIT.py
File PRESUBMIT.py (right):

https://codereview.webrtc.org/2872493002/diff/1/PRESUBMIT.py#newcode608
PRESUBMIT.py:608: def CheckOrphanHeaders(input_api, output_api):
Prefix this with _ to show it's an internal function.

https://codereview.webrtc.org/2872493002/diff/1/PRESUBMIT.py#newcode610
PRESUBMIT.py:610: # roundabout construct to import prebubmit_checks_lis because
this file is
prebubmit_checks_lis -> prebubmit_checks_lib

https://codereview.webrtc.org/2872493002/diff/1/PRESUBMIT.py#newcode634
PRESUBMIT.py:634: file_path, gn_file_path)))
+2 spaces indent to get 4-spaces hanging indent.

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
File presubmit_checks_lib/check_orphan_headers.py (right):

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers.py:67: os.pardir))
-5 spaces indent

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers.py:73: """Returns True if the header
is listed in the BUILD.gn file
nit: end with punctation.

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers.py:104: source_file_re =
re.compile(r'.*\"(?P<source_file>.*)\"')
Let's make this regex a constant like the other ones, for consistency.

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
File presubmit_checks_lib/check_orphan_headers_test.py (right):

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers_test.py:67: # Some comments
indenting this much looks kind of weird to me. Let's use the 4-space indent
style here.

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers_test.py:73: sources = ["foo.h"]
Can you add another header with the same name in a subdirectory? e.g.

source_set("baz_foo") {
  sources = ["baz/foo.h"]
}

And verify it works as expected.

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers_test.py:87: '/a/b'))
alight with previous line's parenthesis (-1 space)

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers_test.py:88: 
+1 blank line

[kjellander_webrtc, 2017-05-09 06:37:38]

Now that https://codereview.webrtc.org/2864213004/ is landed you can put this in
tools_webrtc

[mbonadei, 2017-05-09 07:31:08]

On 2017/05/09 06:37:38, kjellander_webrtc wrote:
> Now that https://codereview.webrtc.org/2864213004/ is landed you can put this
in
> tools_webrtc

Done. I have a strange presubmit warning though...

** Presubmit Warnings **
src/tools_webrtc/presubmit_checks_lib/check_orphan_headers_test.py exec failure
(0.01s)
   Execution failed with error: [Errno 8] Exec format error.
Check that src or
src/tools_webrtc/presubmit_checks_lib/check_orphan_headers_test.py 
exist and have execution permission.

And yes, the file is executable with permissions -rwxr-x---.

Do you have any idea?

[mbonadei, 2017-05-09 07:31:22]

https://codereview.webrtc.org/2872493002/diff/1/PRESUBMIT.py
File PRESUBMIT.py (right):

https://codereview.webrtc.org/2872493002/diff/1/PRESUBMIT.py#newcode608
PRESUBMIT.py:608: def CheckOrphanHeaders(input_api, output_api):
On 2017/05/08 18:51:59, kjellander_webrtc wrote:
> Prefix this with _ to show it's an internal function.

Done.

https://codereview.webrtc.org/2872493002/diff/1/PRESUBMIT.py#newcode610
PRESUBMIT.py:610: # roundabout construct to import prebubmit_checks_lis because
this file is
On 2017/05/08 18:51:59, kjellander_webrtc wrote:
> prebubmit_checks_lis -> prebubmit_checks_lib

Done.

https://codereview.webrtc.org/2872493002/diff/1/PRESUBMIT.py#newcode634
PRESUBMIT.py:634: file_path, gn_file_path)))
On 2017/05/08 18:51:59, kjellander_webrtc wrote:
> +2 spaces indent to get 4-spaces hanging indent.

Done.

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
File presubmit_checks_lib/check_orphan_headers.py (right):

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers.py:67: os.pardir))
On 2017/05/08 18:52:00, kjellander_webrtc wrote:
> -5 spaces indent

Done.

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers.py:73: """Returns True if the header
is listed in the BUILD.gn file
On 2017/05/08 18:52:00, kjellander_webrtc wrote:
> nit: end with punctation.

Done.

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers.py:104: source_file_re =
re.compile(r'.*\"(?P<source_file>.*)\"')
On 2017/05/08 18:51:59, kjellander_webrtc wrote:
> Let's make this regex a constant like the other ones, for consistency.

Done.

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
File presubmit_checks_lib/check_orphan_headers_test.py (right):

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers_test.py:67: # Some comments
On 2017/05/08 18:52:00, kjellander_webrtc wrote:
> indenting this much looks kind of weird to me. Let's use the 4-space indent
> style here.

Done.

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers_test.py:73: sources = ["foo.h"]
On 2017/05/08 18:52:00, kjellander_webrtc wrote:
> Can you add another header with the same name in a subdirectory? e.g.
> 
> source_set("baz_foo") {
>   sources = ["baz/foo.h"]
> }
> 
> And verify it works as expected.

Done.

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers_test.py:87: '/a/b'))
On 2017/05/08 18:52:00, kjellander_webrtc wrote:
> alight with previous line's parenthesis (-1 space)

Done.

https://codereview.webrtc.org/2872493002/diff/1/presubmit_checks_lib/check_or...
presubmit_checks_lib/check_orphan_headers_test.py:88: 
On 2017/05/08 18:52:00, kjellander_webrtc wrote:
> +1 blank line

Done.

[kjellander_webrtc, 2017-05-09 09:17:35]

lgtm with comment fixed (probably solves the presubmit error).

https://codereview.webrtc.org/2872493002/diff/40001/tools_webrtc/presubmit_ch...
File tools_webrtc/presubmit_checks_lib/check_orphan_headers.py (right):

https://codereview.webrtc.org/2872493002/diff/40001/tools_webrtc/presubmit_ch...
tools_webrtc/presubmit_checks_lib/check_orphan_headers.py:1: # Copyright (c)
2017 The WebRTC project authors. All Rights Reserved.
You're missing the Python shebang here:
https://google.github.io/styleguide/pyguide.html?showone=Shebang_Line#Shebang...

I think that may be causing the presubmit error. Please add
#!/usr/bin/env python

[mbonadei, 2017-05-09 11:16:40]

PTAL. :)

https://codereview.webrtc.org/2872493002/diff/40001/tools_webrtc/presubmit_ch...
File tools_webrtc/presubmit_checks_lib/check_orphan_headers.py (right):

https://codereview.webrtc.org/2872493002/diff/40001/tools_webrtc/presubmit_ch...
tools_webrtc/presubmit_checks_lib/check_orphan_headers.py:1: # Copyright (c)
2017 The WebRTC project authors. All Rights Reserved.
On 2017/05/09 09:17:35, kjellander_webrtc wrote:
> You're missing the Python shebang here:
>
https://google.github.io/styleguide/pyguide.html?showone=Shebang_Line#Shebang...
> 
> I think that may be causing the presubmit error. Please add
> #!/usr/bin/env python

It does. Thanks a lot!

[kjellander_webrtc, 2017-05-09 11:33:10]

still lgtm

[mbonadei, 2017-05-09 14:35:29]

The CQ bit was checked by mbonadei@webrtc.org

[commit-bot: I haz the power, 2017-05-09 14:35:34]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-05-09 14:58:06]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494340529386080,
"parent_rev": "77b376c094aa13220c31a44a5032d19d15fbef7e", "commit_rev":
"74973ed847fb594fb8b80b809236eeef13b7338c"}

[commit-bot: I haz the power, 2017-05-09 14:58:08]

Description was changed from

==========
Adding PRESUBMIT check on orphan headers files.

We want all .h files to be tracked by a GN target. This CL adds a
presubmit check to ensure that newly added targets are tracked by
GN.

In this CL we add a directory called 'presubmit_checks_lib' so we
can avoid to put everything in PRESUBMIT.py (especially if we want
to test some checks). I tried to use 'tools-webrtc' but it is not
easy to include a python module from a directory with '-' in the
name.

BUG=webrtc:7514
==========

to

==========
Adding PRESUBMIT check on orphan headers files.

We want all .h files to be tracked by a GN target. This CL adds a
presubmit check to ensure that newly added targets are tracked by
GN.

In this CL we add a directory called 'presubmit_checks_lib' so we
can avoid to put everything in PRESUBMIT.py (especially if we want
to test some checks). I tried to use 'tools-webrtc' but it is not
easy to include a python module from a directory with '-' in the
name.

BUG=webrtc:7514

Review-Url: https://codereview.webrtc.org/2872493002
Cr-Commit-Position: refs/heads/master@{#18069}
Committed:
https://chromium.googlesource.com/external/webrtc/+/74973ed847fb594fb8b80b809...
==========

[commit-bot: I haz the power, 2017-05-09 14:58:09]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/external/webrtc/+/74973ed847fb594fb8b80b809...