Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
127213003:
Whitelist third_party/fontconfig in licensecheck (Closed)
|
Created:
6 years, 11 months ago by spang Modified:
6 years, 11 months ago CC:
chromium-reviews Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionWhitelist third_party/fontconfig in licensecheck
It's MIT licensed. We'll put it at third_party/fontconfig.
See https://chromium.googlesource.com/external/fontconfig/+/master/COPYING
R=phajdan.jr@chromium.org
BUG=332244
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=243941
Patch Set 1 #
Total comments: 3
Patch Set 2 : add link to freedesktop.org bug #
Total comments: 1
Patch Set 3 : move upstream bug link & remove crbug link #
Messages
Total messages: 11 (0 generated)
+Daniel could you advise on what license it is? https://codereview.chromium.org/127213003/diff/1/third_party/devscripts/licen... File third_party/devscripts/licensecheck.pl (right): https://codereview.chromium.org/127213003/diff/1/third_party/devscripts/licen... third_party/devscripts/licensecheck.pl:495: $license = "MIT/X11 (BSD like) $license"; It's not obvious to me whether this is really MIT, especially the "and that the name of the author(s) not be used in advertising or publicity pertaining to distribution of the software without specific, written prior permission." part (see https://chromium.googlesource.com/external/fontconfig/+/master/COPYING). https://codereview.chromium.org/127213003/diff/1/tools/checklicenses/checklic... File tools/checklicenses/checklicenses.py (right): https://codereview.chromium.org/127213003/diff/1/tools/checklicenses/checklic... tools/checklicenses/checklicenses.py:189: 'third_party/fontconfig': [ # http://crbug.com/332244 Please file an upstream bug for files that do not have license headers and link to it here. Make sure the license headers really are missing or very ambiguous - otherwise we should fix licensecheck.pl to recognize them. I don't see many files that the bug refers to at https://chromium.googlesource.com/external/fontconfig/+/master/ .
On 2014/01/08 13:50:06, Paweł Hajdan Jr. wrote: > +Daniel could you advise on what license it is? > > https://codereview.chromium.org/127213003/diff/1/third_party/devscripts/licen... > File third_party/devscripts/licensecheck.pl (right): > > https://codereview.chromium.org/127213003/diff/1/third_party/devscripts/licen... > third_party/devscripts/licensecheck.pl:495: $license = "MIT/X11 (BSD like) > $license"; > It's not obvious to me whether this is really MIT, especially the "and that the > name of the author(s) not be used in advertising or publicity pertaining to > distribution of the software without specific, written prior permission." part > (see https://chromium.googlesource.com/external/fontconfig/+/master/COPYING). > I referred to https://fedoraproject.org/wiki/Licensing:MIT?rd=Licensing/MIT > https://codereview.chromium.org/127213003/diff/1/tools/checklicenses/checklic... > File tools/checklicenses/checklicenses.py (right): > > https://codereview.chromium.org/127213003/diff/1/tools/checklicenses/checklic... > tools/checklicenses/checklicenses.py:189: 'third_party/fontconfig': [ # > http://crbug.com/332244 > Please file an upstream bug for files that do not have license headers and link > to it here. > Alright. > Make sure the license headers really are missing or very ambiguous - otherwise > we should fix licensecheck.pl to recognize them. > > I don't see many files that the bug refers to at > https://chromium.googlesource.com/external/fontconfig/+/master/ . Some of them are generated files I checked into svn. For those, I guess the bug should be against the program generating them (to include a note at the top saying the file is generated).
MIT/BSD like is a good description. It's essentially a mix and match hybrid :) On Wed, Jan 8, 2014 at 7:35 AM, <spang@chromium.org> wrote: > Reviewers: Paweł Hajdan Jr., Daniel Berlin, > > Message: > > On 2014/01/08 13:50:06, Paweł Hajdan Jr. wrote: >> >> +Daniel could you advise on what license it is? > > > > https://codereview.chromium.org/127213003/diff/1/third_party/devscripts/licen... >> >> File third_party/devscripts/licensecheck.pl (right): > > > > https://codereview.chromium.org/127213003/diff/1/third_party/devscripts/licen... >> >> third_party/devscripts/licensecheck.pl:495: $license = "MIT/X11 (BSD like) >> $license"; >> It's not obvious to me whether this is really MIT, especially the "and >> that > > the >> >> name of the author(s) not be used in advertising or publicity pertaining >> to >> distribution of the software without specific, written prior permission." >> part >> (see >> https://chromium.googlesource.com/external/fontconfig/+/master/COPYING). > > > > I referred to https://fedoraproject.org/wiki/Licensing:MIT?rd=Licensing/MIT > > > > https://codereview.chromium.org/127213003/diff/1/tools/checklicenses/checklic... >> >> File tools/checklicenses/checklicenses.py (right): > > > > https://codereview.chromium.org/127213003/diff/1/tools/checklicenses/checklic... >> >> tools/checklicenses/checklicenses.py:189: 'third_party/fontconfig': [ # >> http://crbug.com/332244 >> Please file an upstream bug for files that do not have license headers and > > link >> >> to it here. > > > > Alright. > > >> Make sure the license headers really are missing or very ambiguous - >> otherwise >> we should fix licensecheck.pl to recognize them. > > >> I don't see many files that the bug refers to at >> https://chromium.googlesource.com/external/fontconfig/+/master/ . > > > Some of them are generated files I checked into svn. For those, I guess the > bug > should be against the program generating them (to include a note at the top > saying the file is generated). > > Description: > Whitelist third_party/fontconfig in licensecheck > > It's MIT licensed. We'll put it at third_party/fontconfig. > > See https://chromium.googlesource.com/external/fontconfig/+/master/COPYING > > R=phajdan.jr@chromium.org > > BUG=332244 > > Please review this at https://codereview.chromium.org/127213003/ > > SVN Base: svn://svn.chromium.org/chrome/trunk/src > > Affected files (+5, -0 lines): > M third_party/devscripts/licensecheck.pl > M tools/checklicenses/checklicenses.py > > > Index: third_party/devscripts/licensecheck.pl > diff --git a/third_party/devscripts/licensecheck.pl > b/third_party/devscripts/licensecheck.pl > index > f4260dde12f6bd9008a5aeb3e3d97441c4ea14d5..0227ad1d02a3332bf6acb50235f9a1c9d5d67b74 > 100755 > --- a/third_party/devscripts/licensecheck.pl > +++ b/third_party/devscripts/licensecheck.pl > @@ -491,6 +491,8 @@ sub parselicense($) { > > $license = "MIT/X11 (BSD like) $license"; > } elsif ($licensetext =~ /Permission is hereby granted, without written > agreement and without license or royalty fees, to use, copy, modify, and > distribute this software and its documentation for any purpose/) { > > $license = "MIT/X11 (BSD like) $license"; > + } elsif ($licensetext =~ /Permission to use, copy, modify, distribute, > and sell this software and its documentation for any purpose is hereby > granted without fee/) { > > + $license = "MIT/X11 (BSD like) $license"; > } elsif ($licensetext =~ /MIT .* License/) { > > $license = "MIT/X11 (BSD like) $license"; > } > Index: tools/checklicenses/checklicenses.py > diff --git a/tools/checklicenses/checklicenses.py > b/tools/checklicenses/checklicenses.py > index > ea935559a702d9743b7bdf4937b315f245279fb6..ef9f82a0cfad5a5002efbdd441222487f4ac31fb > 100755 > --- a/tools/checklicenses/checklicenses.py > +++ b/tools/checklicenses/checklicenses.py > @@ -186,6 +186,9 @@ PATH_SPECIFIC_WHITELISTED_LICENSES = { > 'GPL (v2 or later)', > 'UNKNOWN', # http://crbug.com/98123 > ], > + 'third_party/fontconfig': [ # http://crbug.com/332244 > + 'UNKNOWN', > + ], > 'third_party/freetype2': [ # http://crbug.com/177319 > 'UNKNOWN', > ], > > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
https://codereview.chromium.org/127213003/diff/1/tools/checklicenses/checklic... File tools/checklicenses/checklicenses.py (right): https://codereview.chromium.org/127213003/diff/1/tools/checklicenses/checklic... tools/checklicenses/checklicenses.py:189: 'third_party/fontconfig': [ # http://crbug.com/332244 On 2014/01/08 13:50:06, Paweł Hajdan Jr. wrote: > Please file an upstream bug for files that do not have license headers and link > to it here. > > Make sure the license headers really are missing or very ambiguous - otherwise > we should fix licensecheck.pl to recognize them. > > I don't see many files that the bug refers to at > https://chromium.googlesource.com/external/fontconfig/+/master/ . Please do this. I'm fine with the licensecheck.pl change.
On 2014/01/09 11:02:18, Paweł Hajdan Jr. wrote: > https://codereview.chromium.org/127213003/diff/1/tools/checklicenses/checklic... > File tools/checklicenses/checklicenses.py (right): > > https://codereview.chromium.org/127213003/diff/1/tools/checklicenses/checklic... > tools/checklicenses/checklicenses.py:189: 'third_party/fontconfig': [ # > http://crbug.com/332244 > On 2014/01/08 13:50:06, Paweł Hajdan Jr. wrote: > > Please file an upstream bug for files that do not have license headers and > link > > to it here. > > > > Make sure the license headers really are missing or very ambiguous - otherwise > > we should fix licensecheck.pl to recognize them. > > > > I don't see many files that the bug refers to at > > https://chromium.googlesource.com/external/fontconfig/+/master/ . > > Please do this. I'm fine with the licensecheck.pl change. I've checked them and they are all indeed missing. Here's the upstream bug I filed: https://bugs.freedesktop.org/show_bug.cgi?id=73401
Could you just update the CL to link to the upstream bug instead?
On 2014/01/09 15:19:32, Paweł Hajdan Jr. wrote: > Could you just update the CL to link to the upstream bug instead? Done
LGTM with a comment https://codereview.chromium.org/127213003/diff/160001/tools/checklicenses/che... File tools/checklicenses/checklicenses.py (right): https://codereview.chromium.org/127213003/diff/160001/tools/checklicenses/che... tools/checklicenses/checklicenses.py:189: # http://crbug.com/332244 nit: Please put these just above 'UNKNOWN' so it's obvious to what they're applying (scope). Feel free to remove the crbug link, in fact I'd recommend removing it.
On 2014/01/09 16:07:22, Paweł Hajdan Jr. wrote: > LGTM with a comment > > https://codereview.chromium.org/127213003/diff/160001/tools/checklicenses/che... > File tools/checklicenses/checklicenses.py (right): > > https://codereview.chromium.org/127213003/diff/160001/tools/checklicenses/che... > tools/checklicenses/checklicenses.py:189: # http://crbug.com/332244 > nit: Please put these just above 'UNKNOWN' so it's obvious to what they're > applying (scope). Feel free to remove the crbug link, in fact I'd recommend > removing it. Done
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/spang@chromium.org/127213003/210001
Message was sent while issue was closed.
Change committed as 243941 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
