Re: [edk2-devel] [PATCH] BaseTools/PatchCheck.py: Add LicenseCheck

["Laszlo Ersek" <lersek@redhat.com>]

On 04/22/20 18:01, Liming Gao wrote:
> Mike:
>   The checker purpose is to make sure the correct license be used for new added file. If the file has the different license, it should be reviewed carefully. 
>   
>   I remember we still have one open for third party non bsd+patent code (the detail can refer to https://edk2.groups.io/g/devel/message/41639). Now, there is no non bsd+patent license files to be added in edk2 after edk2 switches to bsd+patent license. 

Some files introduced by Rebecca's BhyvePkg patch series come under the
2-clause BSD License, and not the 2-clause BSD + Patent License. And
Rebecca cannot relicense them because she's not the (sole) copyright holder.

Readme.md states:

4. It is preferred that contributions are submitted using the same
   copyright license as the base project. When that is not possible,
   then contributions using the following licenses can be accepted:
   * BSD (2-clause): http://opensource.org/licenses/BSD-2-Clause
   * BSD (3-clause): http://opensource.org/licenses/BSD-3-Clause
   * MIT: http://opensource.org/licenses/MIT
   * Python-2.0: http://opensource.org/licenses/Python-2.0
   * Zlib: http://opensource.org/licenses/Zlib
[...]
   Contributions using other licenses might be accepted, but further
   review will be required.

This seems to imply that the "normal" 2-clause BSDL does not require
"further review".

Thanks
Laszlo

>> -----Original Message-----
>> From: Kinney, Michael D <michael.d.kinney@intel.com>
>> Sent: Wednesday, April 22, 2020 11:41 PM
>> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>; Zhang, Shenglei <shenglei.zhang@intel.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>
>> Cc: Feng, Bob C <bob.c.feng@intel.com>
>> Subject: RE: [edk2-devel] [PATCH] BaseTools/PatchCheck.py: Add LicenseCheck
>>
>> Hi Liming,
>>
>> I do not see this change checking that the license is in a proper
>> SPDX Identifier statement?
>>
>> 	https://spdx.org/ids-how
>>
>> Only checking the for a license name is not sufficient.
>>
>> A file may be covered by more than one license.  What is the behavior in
>> this case?
>>
>> The EDK II project has BSD-2-Clause-Patent as the preferred license, but
>> other licenses are allowed.  We use PatchCheck.py in EDK II CI.  Will this
>> change block file added with a different allowed license?
>>
>> I think these questions should be addressed in the file header of this
>> source file, so the behavior of PatchCheck.py is clearly defined.
>>
>> Thanks,
>>
>> Mike
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On
>>> Behalf Of Liming Gao
>>> Sent: Wednesday, April 22, 2020 12:02 AM
>>> To: Zhang, Shenglei <shenglei.zhang@intel.com>;
>>> devel@edk2.groups.io
>>> Cc: Feng, Bob C <bob.c.feng@intel.com>
>>> Subject: Re: [edk2-devel] [PATCH]
>>> BaseTools/PatchCheck.py: Add LicenseCheck
>>>
>>> Shenglei:
>>>   Please submit BZ to describe it. The license should
>>> be BSD-2-Clause-Patent.
>>>
>>> Thanks
>>> Liming
>>>> -----Original Message-----
>>>> From: Zhang, Shenglei <shenglei.zhang@intel.com>
>>>> Sent: Wednesday, April 22, 2020 2:57 PM
>>>> To: devel@edk2.groups.io
>>>> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
>>> <liming.gao@intel.com>
>>>> Subject: [PATCH] BaseTools/PatchCheck.py: Add
>>> LicenseCheck
>>>>
>>>> For files to be added to the tree, this feature will
>>> check
>>>> whether it has BSD license.
>>>>
>>>> Cc: Bob Feng <bob.c.feng@intel.com>
>>>> Cc: Liming Gao <liming.gao@intel.com>
>>>> Signed-off-by: Shenglei Zhang
>>> <shenglei.zhang@intel.com>
>>>> ---
>>>>  BaseTools/Scripts/PatchCheck.py | 52
>>> ++++++++++++++++++++++++++++++++-
>>>>  1 file changed, 51 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/BaseTools/Scripts/PatchCheck.py
>>> b/BaseTools/Scripts/PatchCheck.py
>>>> index 13da6967785d..15663d02a3c0 100755
>>>> --- a/BaseTools/Scripts/PatchCheck.py
>>>> +++ b/BaseTools/Scripts/PatchCheck.py
>>>> @@ -491,6 +491,53 @@ class GitDiffCheck:
>>>>              print(prefix, line)
>>>>              count += 1
>>>>
>>>> +class LicenseCheck():
>>>> +
>>>> +    def __init__(self, diff):
>>>> +        self.ok = True
>>>> +        self.startcheck = False
>>>> +        self.license = True
>>>> +        lines = diff.splitlines(True)
>>>> +        count = len(lines)
>>>> +        line_index = 0
>>>> +        for line in lines:
>>>> +            if line.startswith('--- /dev/null'):
>>>> +                nextline = lines[line_index + 1]
>>>> +                added_file =
>>> self.Readdedfileformat.search(nextline).group(1)
>>>> +                added_file_extension =
>>> os.path.splitext(added_file)[1]
>>>> +                if added_file_extension in
>>> self.file_extension_list:
>>>> +                    self.startcheck = True
>>>> +                    self.license = False
>>>> +            if self.startcheck and self.license_name
>>> in line:
>>>> +                self.license = True
>>>> +            if line_index + 1 == count or
>>> lines[line_index + 1].startswith('diff --') and
>>> self.startcheck:
>>>> +                if not self.license:
>>>> +                    self.error(added_file)
>>>> +                self.startcheck = False
>>>> +                self.license = True
>>>> +            line_index = line_index + 1
>>>> +
>>>> +    def error(self, *err):
>>>> +        if self.ok and Verbose.level >
>>> Verbose.ONELINE:
>>>> +            print('License is missing!')
>>>> +        self.ok = False
>>>> +        if Verbose.level < Verbose.NORMAL:
>>>> +            return
>>>> +        count = 0
>>>> +        for line in err:
>>>> +            prefix = (' *', '  ')[count > 0]
>>>> +            error_format = 'Missing license in:'
>>>> +            print(prefix, error_format, line)
>>>> +            count += 1
>>>> +
>>>> +
>>>> +    license_name = 'BSD-2-Clause-Patent'
>>>> +
>>>> +    Readdedfileformat = re.compile(r'\+\+\+
>>> b\/(.*)\n')
>>>> +
>>>> +    file_extension_list = [".c", ".h", ".inf",
>>> ".dsc", ".dec", ".py", ".bat", ".sh", ".uni", ".yaml",
>>> ".fdf", ".inc"]
>>>> +
>>>> +
>>>>  class CheckOnePatch:
>>>>      """Checks the contents of a git email formatted
>>> patch.
>>>>
>>>> @@ -508,12 +555,15 @@ class CheckOnePatch:
>>>>          msg_check =
>>> CommitMessageCheck(self.commit_subject,
>>> self.commit_msg)
>>>>          msg_ok = msg_check.ok
>>>>
>>>> +        license_check = LicenseCheck(self.diff)
>>>> +        license_ok = license_check.ok
>>>> +
>>>>          diff_ok = True
>>>>          if self.diff is not None:
>>>>              diff_check = GitDiffCheck(self.diff)
>>>>              diff_ok = diff_check.ok
>>>>
>>>> -        self.ok = email_ok and msg_ok and diff_ok
>>>> +        self.ok = email_ok and msg_ok and diff_ok
>>> and license_ok
>>>>
>>>>          if Verbose.level == Verbose.ONELINE:
>>>>              if self.ok:
>>>> --
>>>> 2.18.0.windows.1
>>>
>>>
>>>
> 
> 
> 
>