[Android WebView] Rewrite copyrights scanner in Python

[Android WebView] Rewrite copyrights scanner in Python

This is to prepare for running the scanner as a presubmit check.

I have added some manual tests to make sure that the new scanner is
compatible with the old one. I plan to replace them with automatic
unittests once I start using presubmit-style input API objects that
are easily mockable.

BUG=343104
Committed: https://crrev.com/f8af6334c60ec5b39355e414e587b0190d8db86c
Cr-Commit-Position: refs/heads/master@{#297828}

[mnaganov (inactive), 2014-10-01 16:16:31]

mnaganov@chromium.org changed reviewers:
+ mkosiba@chromium.org

[mkosiba (inactive), 2014-10-02 13:14:47]

lgtm

https://codereview.chromium.org/622493004/diff/1/android_webview/tools/copyri...
File android_webview/tools/copyright_scanner.py (right):

https://codereview.chromium.org/622493004/diff/1/android_webview/tools/copyri...
android_webview/tools/copyright_scanner.py:48: dirnames.remove(item)
the os.walk docs say:

> When topdown is True, the caller can modify the dirnames list in-place
(perhaps using del or slice assignment), and walk() will only recurse into the
subdirectories whose names remain in dirnames [...] Modifying dirnames when
topdown is False is ineffective, because in bottom-up mode the directories in
dirnames are generated before dirpath itself is generated.

so I guess you'll want to os.walk(full_path, topdown=True)

https://codereview.chromium.org/622493004/diff/1/android_webview/tools/copyri...
android_webview/tools/copyright_scanner.py:107: # Record '(a)' and '(b)' last
occurences in C++ comments.
Maybe move/copy the explanation from line 121 to here. It makes it easier to
understand why we're recording (a) and (b)

https://codereview.chromium.org/622493004/diff/1/android_webview/tools/copyri...
android_webview/tools/copyright_scanner.py:109: if not cpp_comment_idx == -1:
cpp_comment_idx != -1 or even '//' in upcase_line

https://codereview.chromium.org/622493004/diff/1/android_webview/tools/copyri...
android_webview/tools/copyright_scanner.py:159: for l in
open(os.path.join(root_dir, f), 'r').readlines():
umm.. you might run out of fd's if you don't close the file? maybe use 'with' ?

[mnaganov (inactive), 2014-10-02 13:51:30]

Thanks, Marcin!

https://codereview.chromium.org/622493004/diff/1/android_webview/tools/copyri...
File android_webview/tools/copyright_scanner.py (right):

https://codereview.chromium.org/622493004/diff/1/android_webview/tools/copyri...
android_webview/tools/copyright_scanner.py:48: dirnames.remove(item)
On 2014/10/02 13:14:46, mkosiba wrote:
> the os.walk docs say:
> 
> > When topdown is True, the caller can modify the dirnames list in-place
> (perhaps using del or slice assignment), and walk() will only recurse into the
> subdirectories whose names remain in dirnames [...] Modifying dirnames when
> topdown is False is ineffective, because in bottom-up mode the directories in
> dirnames are generated before dirpath itself is generated.
> 
> so I guess you'll want to os.walk(full_path, topdown=True)

Yeah, but the same doc also lists the defaults for this function:

os.walk(top, topdown=True, onerror=None, followlinks=False)

https://codereview.chromium.org/622493004/diff/1/android_webview/tools/copyri...
android_webview/tools/copyright_scanner.py:107: # Record '(a)' and '(b)' last
occurences in C++ comments.
On 2014/10/02 13:14:46, mkosiba wrote:
> Maybe move/copy the explanation from line 121 to here. It makes it easier to
> understand why we're recording (a) and (b)

Done.

https://codereview.chromium.org/622493004/diff/1/android_webview/tools/copyri...
android_webview/tools/copyright_scanner.py:109: if not cpp_comment_idx == -1:
On 2014/10/02 13:14:47, mkosiba wrote:
> cpp_comment_idx != -1 or even '//' in upcase_line

We use the value of cpp_comment_idx if it's not -1, so just changed to "!= -1".

https://codereview.chromium.org/622493004/diff/1/android_webview/tools/copyri...
android_webview/tools/copyright_scanner.py:159: for l in
open(os.path.join(root_dir, f), 'r').readlines():
On 2014/10/02 13:14:46, mkosiba wrote:
> umm.. you might run out of fd's if you don't close the file? maybe use 'with'
?

That's a really good catch, thanks! Fixed here and also in webview_licenses.py

[mnaganov (inactive), 2014-10-02 13:52:26]

The CQ bit was checked by mnaganov@chromium.org

[commit-bot: I haz the power, 2014-10-02 13:53:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/622493004/20001

[commit-bot: I haz the power, 2014-10-02 14:01:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-02 14:01:55]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[mnaganov (inactive), 2014-10-02 14:09:08]

The CQ bit was checked by mnaganov@chromium.org

[commit-bot: I haz the power, 2014-10-02 14:09:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/622493004/40001

[commit-bot: I haz the power, 2014-10-02 14:39:12]

Committed patchset #3 (id:40001) as 328a97854f592603c94c20359324714993219828

[commit-bot: I haz the power, 2014-10-02 14:39:53]

Patchset 3 (id:??) landed as
https://crrev.com/f8af6334c60ec5b39355e414e587b0190d8db86c
Cr-Commit-Position: refs/heads/master@{#297828}