Add checking for license formats to third_party/PRESUBMIT

Add checking for license formats to third_party/PRESUBMIT

Since we expect a certain format for the "License:" field, it's better to
check it earlier -- during the presubmit stage, so people won't get
surprises later.

Also, updated the license checking code to accept multiple licenses if
one of them is compatible with Android.

BUG=168324
R=dannyb@google.com, mkosiba@chromium.org
TBR=darin@chromium.org

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

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

PTAL!

mkosiba@: the code

dannyb@: I would like to double-check that in case of multiple licenses, it is
fine when one of them isn't compatible with Android, e.g. "BSD and GPL v3" -- as
I understand, in this case we are just using BSD and ignoring the presence of
others, right?

[mkosiba (inactive), 2014-01-31 17:47:38]

code LGTM

https://codereview.chromium.org/146803006/diff/50001/android_webview/tools/we...
File android_webview/tools/webview_licenses.py (right):

https://codereview.chromium.org/146803006/diff/50001/android_webview/tools/we...
android_webview/tools/webview_licenses.py:45: self.re = re
could you explain why this is needed? PRESUBMIT.py already imports re so I'd
expect it would "just work"..

https://codereview.chromium.org/146803006/diff/50001/third_party/PRESUBMIT.py
File third_party/PRESUBMIT.py (right):

https://codereview.chromium.org/146803006/diff/50001/third_party/PRESUBMIT.py...
third_party/PRESUBMIT.py:111: 'The license specified does not seem to be allowed
in\n' +
I guess this should be something like

'Cannot determine whether specified license is compatible with the Android
licensing requirements. Please check...'

right?
The way I understand it, this check says 'the license isn't on the whitelist and
the README.chromium doesn't explicitly specify whether it's compatible, so we
don't know'

[mnaganov (inactive), 2014-02-03 10:00:26]

Thanks, Marcin!

https://codereview.chromium.org/146803006/diff/50001/android_webview/tools/we...
File android_webview/tools/webview_licenses.py (right):

https://codereview.chromium.org/146803006/diff/50001/android_webview/tools/we...
android_webview/tools/webview_licenses.py:45: self.re = re
On 2014/01/31 17:47:38, mkosiba wrote:
> could you explain why this is needed? PRESUBMIT.py already imports re so I'd
> expect it would "just work"..

No -- in fact, PRESUBMIT scripts don't import anything in a "classic" way.
Instead, they get all the packages via 'input_api' and 'output_api' objects
passed to their functions. See
https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/pres...

https://codereview.chromium.org/146803006/diff/50001/third_party/PRESUBMIT.py
File third_party/PRESUBMIT.py (right):

https://codereview.chromium.org/146803006/diff/50001/third_party/PRESUBMIT.py...
third_party/PRESUBMIT.py:111: 'The license specified does not seem to be allowed
in\n' +
On 2014/01/31 17:47:38, mkosiba wrote:
> I guess this should be something like
> 
> 'Cannot determine whether specified license is compatible with the Android
> licensing requirements. Please check...'
> 
> right?
> The way I understand it, this check says 'the license isn't on the whitelist
and
> the README.chromium doesn't explicitly specify whether it's compatible, so we
> don't know'

Yes, you are right. Fixed the text.

Also, I have realized that making this issue an error actually prevents anyone
from using a non-compatible license, unless she also states 'License Android
Compatible: no', which is kind of silly. So I reclassified this into a warning.

[mkosiba (inactive), 2014-02-03 10:30:55]

LGTM

https://codereview.chromium.org/146803006/diff/50001/android_webview/tools/we...
File android_webview/tools/webview_licenses.py (right):

https://codereview.chromium.org/146803006/diff/50001/android_webview/tools/we...
android_webview/tools/webview_licenses.py:45: self.re = re
On 2014/02/03 10:00:26, Mikhail Naganov (Cr) wrote:
> On 2014/01/31 17:47:38, mkosiba wrote:
> > could you explain why this is needed? PRESUBMIT.py already imports re so I'd
> > expect it would "just work"..
> 
> No -- in fact, PRESUBMIT scripts don't import anything in a "classic" way.
> Instead, they get all the packages via 'input_api' and 'output_api' objects
> passed to their functions. See
>
https://code.google.com/p/chromium/codesearch#chromium/tools/depot_tools/pres...

ah, thanks for that. I got confused by the import statements at the beginning of
the file.

https://codereview.chromium.org/146803006/diff/50001/third_party/PRESUBMIT.py
File third_party/PRESUBMIT.py (right):

https://codereview.chromium.org/146803006/diff/50001/third_party/PRESUBMIT.py...
third_party/PRESUBMIT.py:111: 'The license specified does not seem to be allowed
in\n' +
On 2014/02/03 10:00:26, Mikhail Naganov (Cr) wrote:
> On 2014/01/31 17:47:38, mkosiba wrote:
> > I guess this should be something like
> > 
> > 'Cannot determine whether specified license is compatible with the Android
> > licensing requirements. Please check...'
> > 
> > right?
> > The way I understand it, this check says 'the license isn't on the whitelist
> and
> > the README.chromium doesn't explicitly specify whether it's compatible, so
we
> > don't know'
> 
> Yes, you are right. Fixed the text.
> 
> Also, I have realized that making this issue an error actually prevents anyone
> from using a non-compatible license, unless she also states 'License Android
> Compatible: no', which is kind of silly. So I reclassified this into a
warning.

Makes sense. I guess that once I add the other bit of the license check to the
AOSP bot/trybot it should be harder to add a third_party library with an
incompatible.

[Daniel Berlin, 2014-02-07 21:08:35]

LGTM for open-source-third-party-reviews

[mnaganov (inactive), 2014-02-10 11:14:43]

The CQ bit was checked by mnaganov@chromium.org

[commit-bot: I haz the power, 2014-02-10 11:14:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mnaganov@chromium.org/146803006/190001

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

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

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

Failed to apply patch for third_party/README.chromium.template:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file third_party/README.chromium.template
  Hunk #1 FAILED at 4.
  1 out of 1 hunk FAILED -- saving rejects to file
third_party/README.chromium.template.rej

Patch:       third_party/README.chromium.template
Index: third_party/README.chromium.template
diff --git a/third_party/README.chromium.template
b/third_party/README.chromium.template
index
7294612b10b9da8ae5673743614b6d3b1d09de24..fb271662b43f0d75e6b05327e7aa72d985a0fd44
100644
--- a/third_party/README.chromium.template
+++ b/third_party/README.chromium.template
@@ -4,7 +4,7 @@ URL: The URL where the package lives
 Version: A searchable version number for the package (if the package does not
version or is versioned by date or revision this field should be "0" and the
revision, or date should be enumerated in the appropriate field)
 Date: (OPTIONAL if version is supplied) The date that the package was updated
 Revision: (OPTIONAL if version is supplied) The current revision of the package
-License: The license under which the package is distributed. Arbitrary text is
allowed, but prefer standard forms, eg MIT/X11/BSD/Apache 2.0/GPL/LGPL.
+License: The license under which the package is distributed. Standard forms are
only accepted, eg MIT/X11/BSD/Apache 2.0/GPL/LGPL. See
ANDROID_WHITELISTED_LICENSES in PRESUBMIT.py for allowed patterns.
 License File: (OPTIONAL) File that contains a copy of the package's license.
Use the special value NOT_SHIPPED to indicate that the package is not included
in the shipped product, so its license does not need to be included in
about:credits and no license file is required.
 Security Critical: Either yes or no depending on whether this package is
shipped in releases. For example openssl is critical where cygwin is not.
 License Android Compatible: (OPTIONAL) Whether the package uses a license
compatible with Android. Required only if the package is compatible and the
'License' field uses a non-standard value.

[mnaganov (inactive), 2014-02-10 12:43:46]

On 2014/02/10 11:14:54, I haz the power (commit-bot) wrote:
> Failed to apply patch for third_party/README.chromium.template:
> While running patch -p1 --forward --force --no-backup-if-mismatch;
>   patching file third_party/README.chromium.template
>   Hunk #1 FAILED at 4.
>   1 out of 1 hunk FAILED -- saving rejects to file
> third_party/README.chromium.template.rej
> 
> Patch:       third_party/README.chromium.template
> Index: third_party/README.chromium.template
> diff --git a/third_party/README.chromium.template
> b/third_party/README.chromium.template
> index
>
7294612b10b9da8ae5673743614b6d3b1d09de24..fb271662b43f0d75e6b05327e7aa72d985a0fd44
> 100644
> --- a/third_party/README.chromium.template
> +++ b/third_party/README.chromium.template
> @@ -4,7 +4,7 @@ URL: The URL where the package lives
>  Version: A searchable version number for the package (if the package does not
> version or is versioned by date or revision this field should be "0" and the
> revision, or date should be enumerated in the appropriate field)
>  Date: (OPTIONAL if version is supplied) The date that the package was updated
>  Revision: (OPTIONAL if version is supplied) The current revision of the
package
> -License: The license under which the package is distributed. Arbitrary text
is
> allowed, but prefer standard forms, eg MIT/X11/BSD/Apache 2.0/GPL/LGPL.
> +License: The license under which the package is distributed. Standard forms
are
> only accepted, eg MIT/X11/BSD/Apache 2.0/GPL/LGPL. See
> ANDROID_WHITELISTED_LICENSES in PRESUBMIT.py for allowed patterns.
>  License File: (OPTIONAL) File that contains a copy of the package's license.
> Use the special value NOT_SHIPPED to indicate that the package is not included
> in the shipped product, so its license does not need to be included in
> about:credits and no license file is required.
>  Security Critical: Either yes or no depending on whether this package is
> shipped in releases. For example openssl is critical where cygwin is not.
>  License Android Compatible: (OPTIONAL) Whether the package uses a license
> compatible with Android. Required only if the package is compatible and the
> 'License' field uses a non-standard value.

Committing manually. As this is a change in the existing code, committing with
TBR (as per third_party/OWNERS).

[mnaganov (inactive), 2014-02-10 12:44:13]

Committed patchset #6 manually as r250068 (presubmit successful).