public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] BaseTools/PatchCheck.py: Add LicenseCheck
@ 2020-04-22  6:56 Zhang, Shenglei
  2020-04-22  7:01 ` Liming Gao
  0 siblings, 1 reply; 7+ messages in thread
From: Zhang, Shenglei @ 2020-04-22  6:56 UTC (permalink / raw)
  To: devel; +Cc: Bob Feng, Liming Gao

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] BaseTools/PatchCheck.py: Add LicenseCheck
  2020-04-22  6:56 [PATCH] BaseTools/PatchCheck.py: Add LicenseCheck Zhang, Shenglei
@ 2020-04-22  7:01 ` Liming Gao
  2020-04-22 15:41   ` [edk2-devel] " Michael D Kinney
  0 siblings, 1 reply; 7+ messages in thread
From: Liming Gao @ 2020-04-22  7:01 UTC (permalink / raw)
  To: Zhang, Shenglei, devel@edk2.groups.io; +Cc: Feng, Bob C

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [PATCH] BaseTools/PatchCheck.py: Add LicenseCheck
  2020-04-22  7:01 ` Liming Gao
@ 2020-04-22 15:41   ` Michael D Kinney
  2020-04-22 16:01     ` Liming Gao
  0 siblings, 1 reply; 7+ messages in thread
From: Michael D Kinney @ 2020-04-22 15:41 UTC (permalink / raw)
  To: devel@edk2.groups.io, Gao, Liming, Zhang, Shenglei,
	Kinney, Michael D
  Cc: Feng, Bob C

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
> 
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [PATCH] BaseTools/PatchCheck.py: Add LicenseCheck
  2020-04-22 15:41   ` [edk2-devel] " Michael D Kinney
@ 2020-04-22 16:01     ` Liming Gao
  2020-04-24 16:13       ` Laszlo Ersek
  0 siblings, 1 reply; 7+ messages in thread
From: Liming Gao @ 2020-04-22 16:01 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, Zhang, Shenglei
  Cc: Feng, Bob C, Gao, Liming

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. 

Thanks
Liming
> -----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
> >
> >
> > 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [PATCH] BaseTools/PatchCheck.py: Add LicenseCheck
  2020-04-22 16:01     ` Liming Gao
@ 2020-04-24 16:13       ` Laszlo Ersek
  2020-04-24 16:25         ` Leif Lindholm
  0 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2020-04-24 16:13 UTC (permalink / raw)
  To: devel, liming.gao, Kinney, Michael D, Zhang, Shenglei
  Cc: Feng, Bob C, Leif Lindholm (Nuvia address), Rebecca Cran

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
>>>
>>>
>>>
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [PATCH] BaseTools/PatchCheck.py: Add LicenseCheck
  2020-04-24 16:13       ` Laszlo Ersek
@ 2020-04-24 16:25         ` Leif Lindholm
       [not found]           ` <dcfe88b6-f4cf-cea0-ca93-3d5b15548c4f@redhat.com>
  0 siblings, 1 reply; 7+ messages in thread
From: Leif Lindholm @ 2020-04-24 16:25 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: devel, liming.gao, Kinney, Michael D, Zhang, Shenglei,
	Feng, Bob C, Rebecca Cran

On Fri, Apr 24, 2020 at 18:13:58 +0200, Laszlo Ersek wrote:
> 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.

I disagree.
BSD+Patent is a pure superset of BSD - this was the logic by which the
whole EDK2 project was relicensed in the first place. Rebecca can
definitely add the explicit patent grant as part of the contribution.

The explicit patent grant of course affects only the contributor, and
users of the contributed code, not the original source (and the
originating project would not be able to take modifications back
without accepting the amended license).

> 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".

And I still hold the opinion that I held when I posted the message
referenced above - we do not today have any real policy here.

Now, as per my comment above, I don't think that applies in this
situation, but it is still somethihg we must resolve (i.e. take an
active decision about) before we accept any non BSD+Patent content
into any of our BSD+Patent trees.

/
    Leif

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [edk2-devel] [PATCH] BaseTools/PatchCheck.py: Add LicenseCheck
       [not found]           ` <dcfe88b6-f4cf-cea0-ca93-3d5b15548c4f@redhat.com>
@ 2020-04-28 15:01             ` Leif Lindholm
  0 siblings, 0 replies; 7+ messages in thread
From: Leif Lindholm @ 2020-04-28 15:01 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: devel, liming.gao, Kinney, Michael D, Zhang, Shenglei,
	Feng, Bob C, Rebecca Cran

On Tue, Apr 28, 2020 at 15:59:48 +0200, Laszlo Ersek wrote:
> On 04/24/20 18:25, Leif Lindholm wrote:
> > On Fri, Apr 24, 2020 at 18:13:58 +0200, Laszlo Ersek wrote:
> >> 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.
> > 
> > I disagree.
> > BSD+Patent is a pure superset of BSD - this was the logic by which the
> > whole EDK2 project was relicensed in the first place. Rebecca can
> > definitely add the explicit patent grant as part of the contribution.
> > 
> > The explicit patent grant of course affects only the contributor, and
> > users of the contributed code, not the original source (and the
> > originating project would not be able to take modifications back
> > without accepting the amended license).
> > 
> >> 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".
> > 
> > And I still hold the opinion that I held when I posted the message
> > referenced above - we do not today have any real policy here.
> > 
> > Now, as per my comment above, I don't think that applies in this
> > situation, but it is still somethihg we must resolve (i.e. take an
> > active decision about) before we accept any non BSD+Patent content
> > into any of our BSD+Patent trees.
> 
> I couldn't be happier to *share* the burden of verification with others,
> of licensing questions in new contributions. But for that, others --
> more versed in licensing questions than I am -- have to be willing to
> *look* at those contributions.
> 
> Based on the above, I now have no idea what to ask of Rebecca, regarding
> the 2-clause BSDL on some of the files she's contributing.

Apologies if I was unclear. My suggestion (and opinion) was that this
contribution can be completely switched over to
SPDX-License-Identifier: BSD-2-Clause-Patent
without any conflict with the originating code's
SPDX-License-Identifier: BSD-2-Clause
since the former is simply a superset of the latter.

Had it been (for example) BSD-3-Clause, my suggestion would have been
that it should be contributed as
SPDX-License-Identifier: BSD-3-Clause AND BSD-2-Clause-Patent
as per the "Representing Multiple Licenses" section in appendix V of
https://spdx.org/spdx-specification-21-web-version.

But we have not as a project explicitly approved the use of SPDX OR
and AND operators, and I have heard opinions to the extent that that
would be desirable before we start using them. This has however no
bearing on the change of BSD-2-Clause to BSD-2-Clause-Patent.

> Here's what I know:
> 
> - Microsoft does not want BhyvePkg to be in edk2, because "platform! boo!",

This is an open source project, in which companies do not have
assigned votes on individual patches or approaches. Hence, inasmuch as
an opinion from Microsoft is to be paid attention to beyond that of
any other entity, Michael Kubacki is by some margin the employee most
involved in core EDK2 development. I have not heard his opinion on the
matter.

> - I do want BhyvePkg to be in edk2, minimally because that's only fair
> and consistent with ArmVirtPkg and OvmfPkg (BhyvePkg is virtual),

I agree.

> - I as a steward am especially responsible for reviewing new top-level
> package additions, and that entails licensing bits too,

Sure.

> - I'm not particularly good with licensing bits, but thus far noone else
> from the stewards has chimed in on the series, at all. AFAIR.

Apart from on this patch, I have responded on another thread ... but
now that you mention it I realise that thread was off-list.

I confess to mostly having ignored this set as 1) I have no experience
with bhyve and 2) some quick searching gives me the impression that is
is (currently) x86 only.
But as per the above: I am of the opinion that based on the rule we
applied for keeping ArmVirtPkg, OvmfPkg, and to some extent
EmbeddedPkg in edk2, this is also the appropriate location for bhyve.

While I understand that dealing with components supplied from many
different directions is something that has been "just the way it is"
in the BIOS industry since before there *was* a UEFI, I (like you) am
not convinced by the arguments I have seen in threads on this topic
for keeping things out of EDK2.

If anything, they have strenghthened my opinion, since several of them
seem to boil down to wanting to push *more* responsibility for
updating platform code to adhere to changing core APIs onto the
platform maintainers. I feel that tips the balance the wrong way.

/
    Leif

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-04-28 15:01 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-22  6:56 [PATCH] BaseTools/PatchCheck.py: Add LicenseCheck Zhang, Shenglei
2020-04-22  7:01 ` Liming Gao
2020-04-22 15:41   ` [edk2-devel] " Michael D Kinney
2020-04-22 16:01     ` Liming Gao
2020-04-24 16:13       ` Laszlo Ersek
2020-04-24 16:25         ` Leif Lindholm
     [not found]           ` <dcfe88b6-f4cf-cea0-ca93-3d5b15548c4f@redhat.com>
2020-04-28 15:01             ` Leif Lindholm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox