Make webview_licenses.py script to use licensecheck.pl

third_party/devscripts/licensecheck.pl

Index: third_party/devscripts/licensecheck.pl
diff --git a/third_party/devscripts/licensecheck.pl b/third_party/devscripts/licensecheck.pl
index 3be28de6cd40dad97ac2a3d045f5e654106f22b1..a17ac8bceef7a4f7ad307f1aa200e8e73948e45e 100755
--- a/third_party/devscripts/licensecheck.pl
+++ b/third_party/devscripts/licensecheck.pl
@@ -288,12 +288,26 @@ while (@files) {
     my $copyright_match;
     my $copyright = '';
     my $license = '';
+    my $line;
     my %copyrights;
 
     open (F, "<$file") or die "Unable to access $file\n";
     while (<F>) {
-        last if ($. > $opt_lines);
-        $content .= $_;
+        unless ($. > $opt_lines) {

[Paweł Hajdan Jr., 2013/01/25 17:54:33]

This change seems pretty invasive compared to just adding a regex. What are you
trying to do?

[mnaganov (inactive), 2013/01/28 14:51:54]

I'm feeding all the lines (not only up to $opt_lines) to the copyrights
detector. $opt_lines still restricts the number of lines used for license
detection. If $opt_copyright isn't set, the behavior is unchanged compared to
your modified version.

We need to look for copyrights in the whole file, since it can contain a
copyrighted function somewhere in the middle. But remember, that we only need to
look for copyrights outside of third_party dirs of the script.

[Paweł Hajdan Jr., 2013/01/28 16:28:29]

If that's the case, could you rather implement that in your Android-specific
script instead of here? It seems that this change goes beyond what
licensecheck.pl was supposed to do.

[mnaganov (inactive), 2013/01/28 17:12:38]

Finding copyrighted code isn't Android-specific (I believe, we had agreed on
this before). The original script was also capable of doing this (with
--copyright flag in place). My changes are:

 - to scan the whole file instead of the number of '--lines'. Perhaps, we can
generalize that by introducing '--copyright-lines' argument that will default to
'--lines', so the original behavior could be preserved;

 - to avoid some false positives by cutting out C/C++ string contents, as they
can contain the 'copyright' word.

[Paweł Hajdan Jr., 2013/01/29 09:29:38]

And this is way beyond what the original licensecheck.pl was doing. I still
think this belongs in an Android-specific file, even though licensecheck.pl has
some *basic* capability of printing copyright info.

+            $content .= $_;
+            next unless ($opt_copyright);
+        } else {
+            last unless ($opt_copyright);
+        }
+        $line = $_;
+        # Remove C / C++ strings to avoid false positives.
+        if (index($line, '"') != -1) {
+            $line =~ s/"[^"\\]*(?:\\.[^"\\]*)*"//g;
+        }
+        $copyright_match = parse_copyright($line);
+        if ($copyright_match) {
+            $copyrights{lc("$copyright_match")} = "$copyright_match";
+        }
     }
     close(F);
 
@@ -326,6 +340,16 @@ while (@files) {
 }
 
 sub parse_copyright($) {
+
+    my $line = $_[0];

[Paweł Hajdan Jr., 2013/01/25 17:54:33]

This seems pretty invasive too - what are you trying to do?

[mnaganov (inactive), 2013/01/28 14:51:54]

Just speeding things up. Since for Chromium, about 44M lines of code are passing
through this check, matching becomes a bottleneck (found that using NYTDprof).
Using index for early bailout significantly reduces the processing time.

[Paweł Hajdan Jr., 2013/01/28 16:28:29]

Please include difference in run time when speed is the reason.

I think this would be better done in Android-specific script.

[mnaganov (inactive), 2013/01/28 17:12:38]

Not sure, what do you mean by "Android-specific script". My intention is to
reuse code as much as possible. There is nothing Android-specific in this
optimization.

The optimization reduces copyrights scan time from ~22 sec to ~12 sec with
populated file cache (Z620). To me, this looks like a significant win--almost
twice as fast.

[Paweł Hajdan Jr., 2013/01/29 09:29:38]

Changes should be upstreamable though.
IMHO not worth it. If you go the Android-specific file path as I suggested, feel
free to do it the way you do here, saving the time. I'm fine with that approach,
just not in licensecheck.pl .

+    my $uc_line = uc($line);
+    # Fast bailout, uses the same patterns as the regexp.
+    return '' if (index($uc_line, 'COPYRIGHT') == -1 &&
+                  index($uc_line, 'COPR.') == -1 &&
+                  index($uc_line, '\x{00a9}') == -1 &&
+                  index($uc_line, '\xc2\xa9') == -1 &&
+                  index($uc_line, '(C)') == -1);
+
     my $copyright = '';
     my $match;
 
@@ -342,7 +366,7 @@ sub parse_copyright($) {
 	|and|or                 # Part of a sentence
 	)\b';
 
-    if (m%$copyright_indicator_regex(?::\s*|\s+)(\S.*)$%ix) {
+    if ($line =~ m%\W$copyright_indicator_regex(?::\s*|\s+)(\w.*)$%ix) {
 	$match = $1;
 
 	# Ignore lines matching "see foo for copyright information" etc.
@@ -404,6 +428,7 @@ EOF
 
 sub parselicense($) {
     my ($licensetext) = @_;
+    my $upcase_licensetext = uc($licensetext);
 
     my $gplver = "";
     my $extrainfo = "";
@@ -423,8 +448,14 @@ sub parselicense($) {
 	$extrainfo = " (with Qt exception)$extrainfo"
     }
 
-    if ($licensetext =~ /(All changes made in this file will be lost|DO NOT (EDIT|delete this file)|Generated (automatically|by|from)|generated.*file)/i) {
-	$license = "GENERATED FILE";
+    # The regexp is slow. First, do a quick check using index.

[Paweł Hajdan Jr., 2013/01/25 17:54:33]

Is this really needed?

[mnaganov (inactive), 2013/01/28 14:51:54]

Yes. According to the profiler, this expression matches really slow (not sure
why, though). Using index, the bottleneck was removed.

[Paweł Hajdan Jr., 2013/01/28 16:28:29]

What is the difference in run time?

[mnaganov (inactive), 2013/01/28 17:12:38]

For the Android use case (scan excluding third-party dirs) this reduces scan
time from ~19 sec to ~12 sec (-1/3 of time), with populated cache on Z620.

For the Chromium use case (scan everything) this reduces scan time from ~46 sec
to ~24 sec with the same conditions.

I have started to optimize things out, because the original method we were using
(with plain grep) was taking about 3 sec on a hot cache (although, as you can
see, it had unacceptably more false positives).

[Paweł Hajdan Jr., 2013/01/29 09:29:38]

Again, not worth it. Each of these modifications has a maintenance cost, and a
minute-long build step is already fine. Taking a few seconds away is a
negligible win, even if it looks like a high percentage. The base value is
pretty low anyway.

+    if (index($upcase_licensetext, 'ALL CHANGES MADE IN THIS FILE WILL BE LOST') != -1 ||
+        index($upcase_licensetext, 'DO NOT EDIT') != -1 ||
+        index($upcase_licensetext, 'DO NOT DELETE') != -1 ||
+        index($upcase_licensetext, 'GENERATED') != -1) {
+        if ($licensetext =~ /(All changes made in this file will be lost|DO NOT (EDIT|delete this file)|Generated (at|automatically|data|by|from)|Automatically generated|\Wgenerated\s+(?:\w+\s+)*file\W)/i) {
+            $license = "GENERATED FILE";
+        }
     }
 
     if ($licensetext =~ /is (free software.? you can redistribute it and\/or modify it|licensed) under the terms of (version [^ ]+ of )?the (GNU (Library |Lesser )General Public License|LGPL)/i) {