Add footer license check to licensecheck.pl

Some files, notably CPAN Perl modules, have the license in the footer.
This adds a check to the footer if no license is found in the header.
(Reading the footer is *much* slower than the header, and can lead
 to spurious results if there is a legit header license, so do only
 if necessary.)

Minor changes:
* refactor "remove_comments" so common header/footer processing;
* change "program" to "(program|library)" in Perl license regex.

BUG=239771
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=205104

[Nils Barth (inactive), 2013-06-05 06:41:46]

Hi Paweł and Lei,
Here's a CL that adds a footer check so we catch the Perl licenses in the
footers of CPAN module, as discussed over at:
Issue 15736030: Add JSON.pm to third_party
https://chromiumcodereview.appspot.com/15736030/

Dunno about the details (should "Perl" license be white-listed or case-by-case
(build-only, say)?), but how is the approach?

Best,
Nils

[Paweł Hajdan Jr., 2013-06-05 17:37:38]

Thank you for the patch, just some minor comments.

https://codereview.chromium.org/16426003/diff/1/third_party/devscripts/licens...
File third_party/devscripts/licensecheck.pl (right):

https://codereview.chromium.org/16426003/diff/1/third_party/devscripts/licens...
third_party/devscripts/licensecheck.pl:311: if ($license eq "UNKNOWN") {
Add a comment why we do this - that Perl files often have license at the end of
the file.

https://codereview.chromium.org/16426003/diff/1/third_party/devscripts/licens...
third_party/devscripts/licensecheck.pl:456: } elsif ($licensetext =~ /is free
software.? you (can|may) redistribute it and\/or modify it under the terms of
(?:version [^ ]+ (?:\(?only\)? )?of )?the GNU General Public License/i) {
Is this change needed?

https://codereview.chromium.org/16426003/diff/1/tools/checklicenses/checklice...
File tools/checklicenses/checklicenses.py (left):

https://codereview.chromium.org/16426003/diff/1/tools/checklicenses/checklice...
tools/checklicenses/checklicenses.py:74: 'MPL (v1.1,) BSD (3 clause) GPL
(unversioned/unknown version) '
Just checking why these changes are here?

https://codereview.chromium.org/16426003/diff/1/tools/checklicenses/checklice...
File tools/checklicenses/checklicenses.py (right):

https://codereview.chromium.org/16426003/diff/1/tools/checklicenses/checklice...
tools/checklicenses/checklicenses.py:84: 'Perl',
Let's only allow it for specific directories.

[Nils Barth (inactive), 2013-06-06 06:28:26]

Hi Paweł,
Updated CL per comments
(moved LGPL fix to other CL, will post shortly).
Cheers, Nils

https://codereview.chromium.org/16426003/diff/1/third_party/devscripts/licens...
File third_party/devscripts/licensecheck.pl (right):

https://codereview.chromium.org/16426003/diff/1/third_party/devscripts/licens...
third_party/devscripts/licensecheck.pl:311: if ($license eq "UNKNOWN") {
On 2013/06/05 17:37:38, Paweł Hajdan Jr. wrote:
> Add a comment why we do this - that Perl files often have license at the end
of
> the file.

Done!

https://codereview.chromium.org/16426003/diff/1/third_party/devscripts/licens...
third_party/devscripts/licensecheck.pl:456: } elsif ($licensetext =~ /is free
software.? you (can|may) redistribute it and\/or modify it under the terms of
(?:version [^ ]+ (?:\(?only\)? )?of )?the GNU General Public License/i) {
On 2013/06/05 17:37:38, Paweł Hajdan Jr. wrote:
> Is this change needed?

This is an unrelated fix that I noticed on the way.
Will post in separate CL.

https://codereview.chromium.org/16426003/diff/1/tools/checklicenses/checklice...
File tools/checklicenses/checklicenses.py (left):

https://codereview.chromium.org/16426003/diff/1/tools/checklicenses/checklice...
tools/checklicenses/checklicenses.py:74: 'MPL (v1.1,) BSD (3 clause) GPL
(unversioned/unknown version) '
On 2013/06/05 17:37:38, Paweł Hajdan Jr. wrote:
> Just checking why these changes are here?

As above: unrelated fix which I noticed on the way;
will post in separate CL.

https://codereview.chromium.org/16426003/diff/1/tools/checklicenses/checklice...
File tools/checklicenses/checklicenses.py (right):

https://codereview.chromium.org/16426003/diff/1/tools/checklicenses/checklice...
tools/checklicenses/checklicenses.py:84: 'Perl',
On 2013/06/05 17:37:38, Paweł Hajdan Jr. wrote:
> Let's only allow it for specific directories.

Got it, done.

[Paweł Hajdan Jr., 2013-06-06 17:58:05]

LGTM

[commit-bot: I haz the power, 2013-06-06 18:00:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nbarth@chromium.org/16426003/8001

[commit-bot: I haz the power, 2013-06-06 18:13:48]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Nils Barth (inactive), 2013-06-07 06:09:14]

On 2013/06/06 18:13:48, I haz the power (commit-bot) wrote:
> Retried try job too often on chromium_presubmit for step(s) presubmit
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

Thanks Paweł!
Looks like you're not listed as an owner here, so need an approval from
third_party/OWNERS.
Sending a request, and opened a CL to fix this:
Issue 16621002: Add OWNERS to third_party/devscripts
https://codereview.chromium.org/16621002/

[Nils Barth (inactive), 2013-06-07 06:12:18]

Hi Carlos & Darin,

This CL needs an ok from third_party/OWNERS b/c there's no OWNERS in the
directory.
* Paweł ok'ed it (he's the expert), but he's not an owner
* I've posted a CL to add Paweł and Lei as owners so you won't need to review
these in future; if that lands, no need to ok this separately

Issue 16621002: Add OWNERS to third_party/devscripts
https://codereview.chromium.org/16621002/

Thanks!

[cpu_(ooo_6.6-7.5), 2013-06-07 17:58:40]

lgtm

[commit-bot: I haz the power, 2013-06-08 10:43:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nbarth@chromium.org/16426003/8001

[commit-bot: I haz the power, 2013-06-08 17:30:26]

Retried try job too often on win_rel for step(s) chrome_frame_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-06-09 01:46:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nbarth@chromium.org/16426003/8001

[commit-bot: I haz the power, 2013-06-09 06:04:54]

Change committed as 205104