Re: [edk2-devel] [PATCH 1/2] Revert "BaseTools/PatchCheck.py: Add LicenseCheck"

["Leif Lindholm" <leif@nuviainc.com>]

Hi Liming,

Thanks, this solves my concern.

/
    Leif

On Tue, Jul 14, 2020 at 02:28:59 +0000, Gao, Liming wrote:
> Leif:
>   Shenglei is working on new patch to add license checker as open CI
>   plug-in (BZ 2833). This way allows to add the exception. So, the
>   autogen file can be handled. Based on current statement, only
>   BSD-2-Clause-Patent license will be allowed. Other license can be
>   added as the exception case if necessary. With new license checker
>   in open CI, I agree to revert this checker in
>   BaseTools/PatchCheck.py.
> 
> Thanks
> Liming
> -----Original Message-----
> From: Leif Lindholm <leif@nuviainc.com> 
> Sent: 2020年7月7日 23:57
> To: devel@edk2.groups.io; lersek@redhat.com
> Cc: Andrew Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Zhang, Shenglei <shenglei.zhang@intel.com>; Chen, Yuwei <yuwei.chen@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2-devel] [PATCH 1/2] Revert "BaseTools/PatchCheck.py: Add LicenseCheck"
> 
> On Fri, Jul 03, 2020 at 18:20:26 +0200, Laszlo Ersek wrote:
> > On 07/02/20 13:24, Leif Lindholm wrote:
> > > This reverts commit a4cfb842fca9693a330cb5435284c1ee8bfbbace.
> > > This commit suggests inclusion of non-edk2+license content without
> > 
> > The expression "non-edk2+license" is a typo.
> > 
> > I think you meant "non-BSD-2-Clause-Patent".
> 
> Err, yes. Sorry.
> 
> > > a contribution agreement is something the community has made a 
> > > decision on, which is incorrect.
> > 
> > I'm OK with commit a4cfb842fca9 being reverted, as this solves the 
> > practical problem of adding generated files.
> > 
> > Acked-by: Laszlo Ersek <lersek@redhat.com>
> > 
> > That said, I *still* don't grasp the more general problem you refer to.
> > 
> > Can we express it as follows (using commits 6b621f958123 and 
> > 6f21d772aa6b as reference):
> > 
> > (1) Content under OvmfPkg/Include/IndustryStandard/Xen was introduced 
> > under the MIT license in commit 6b621f958123. That commit was marked with:
> > 
> > > License: This patch adds many files under the MIT licence.
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > 
> > and the header files also contained open-coded instances of the MIT 
> > license. (These would later be replaced with SPDX identifiers in 
> > commit
> > 6f21d772aa6b.)
> > 
> > As a result, these files effectively granted use and distribution 
> > rights under the MIT license, *plus* a patent grant (per TCA).
> 
> Correct.
> 
> > (2) If we did the same today (that is, add new MIT-licensed files, but 
> > no "Contributed-under: TCA" line on the commit message), then that 
> > would grant use and distribution rights under the "MIT license", and 
> > *no* patent grant.
> > 
> > Is this the issue you're thinking of?
> 
> Yes.
> 
> > So are we basically looking to replace (for example):
> > 
> >   SPDX-License-Identifier: MIT
> > 
> > with *something* like:
> > 
> >   SPDX-License-Identifier: MIT-Patent
> > 
> > ? (Assuming the latter exists -- which it doesn't, at the moment?)
> 
> It doesn't.
> 
> For BSD3/BSD4, we can do what the BSD distros do and use (for example)
> SPDX-License-Identifier: BSD-3-Clause AND BSD-2-Clause-Patent
> 
> It is not clear to me if (for example) "MIT AND BSD-2-Clause-Patent"
> is a valid expression. But what about "public domain"? Do we figure out a jurisdiction in which public domain content can clearly have arbitrary licenses applied to it and ask someone there to relicense it to something we can accept?
> 
> If we decide the explicit patent grant isn't important, why did we add it in the first place? If we decide it is important, we need to actively make a decision on whether we can ever sidestep it, and if so under what circumstances. (e.g.: "we permit it for tools" - but then the code generation aspect also needs to be covered)
> 
> Regards,
> 
> Leif
> 
> > Thanks
> > Laszlo
> > 
> > > 
> > > Cc: Shenglei Zhang <shenglei.zhang@intel.com>
> > > Cc: Yuwei Chen <yuwei.chen@intel.com>
> > > Cc: Liming Gao <liming.gao@intel.com>
> > > Signed-off-by: Leif Lindholm <leif@nuviainc.com>
> > > ---
> > >  BaseTools/Scripts/PatchCheck.py | 50 
> > > ---------------------------------
> > >  1 file changed, 50 deletions(-)
> > > 
> > > diff --git a/BaseTools/Scripts/PatchCheck.py 
> > > b/BaseTools/Scripts/PatchCheck.py index e38cf61f93da..6372f71592d3 
> > > 100755
> > > --- a/BaseTools/Scripts/PatchCheck.py
> > > +++ b/BaseTools/Scripts/PatchCheck.py
> > > @@ -305,49 +305,12 @@ class GitDiffCheck:
> > >          self.line_num = 0
> > >          self.state = START
> > >          self.new_bin = []
> > > -        self.LicenseCheck(self.lines, self.count)
> > >          while self.line_num < self.count and self.format_ok:
> > >              line_num = self.line_num
> > >              self.run()
> > >              assert(self.line_num > line_num)
> > >          self.report_message_result()
> > >  
> > > -    def LicenseCheck(self, lines, count):
> > > -        self.ok = True
> > > -        self.startcheck = False
> > > -        self.license = True
> > > -        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_format_preflix in line:
> > > -                if self.bsd2_patent in line or self.bsd3_patent in line:
> > > -                    self.license = True
> > > -                else:
> > > -                    for optional_license in self.license_optional_list:
> > > -                        if optional_license in line:
> > > -                            self.license = True
> > > -                            self.warning(added_file)
> > > -            if line_index + 1 == count or lines[line_index + 1].startswith('diff --') and self.startcheck:
> > > -                if not self.license:
> > > -                    error_message = "Invalid License in: " + added_file
> > > -                    self.error(error_message)
> > > -                self.startcheck = False
> > > -                self.license = True
> > > -            line_index = line_index + 1
> > > -
> > > -    def warning(self, *err):
> > > -        count = 0
> > > -        for line in err:
> > > -            warning_format = 'Warning: License accepted but not BSD plus patent license in'
> > > -            print(warning_format, line)
> > > -            count += 1
> > > -
> > >      def report_message_result(self):
> > >          if Verbose.level < Verbose.NORMAL:
> > >              return
> > > @@ -534,19 +497,6 @@ class GitDiffCheck:
> > >              print(prefix, line)
> > >              count += 1
> > >  
> > > -    license_format_preflix = 'SPDX-License-Identifier'
> > > -
> > > -    bsd2_patent = 'BSD-2-Clause-Patent'
> > > -
> > > -    bsd3_patent = 'BSD-3-Clause-Patent'
> > > -
> > > -    license_optional_list = ['BSD-2-Clause', 'BSD-3-Clause', 'MIT', 'Python-2.0', 'Zlib']
> > > -
> > > -    Readdedfileformat = re.compile(r'\+\+\+ b\/(.*)\n')
> > > -
> > > -    file_extension_list = [".c", ".h", ".inf", ".dsc", ".dec", ".py", ".bat", ".sh", ".uni", ".yaml", ".fdf", ".inc", "yml", ".asm", \
> > > -                          ".asm16", ".asl", ".vfr", ".s", ".S", ".aslc", ".nasm", ".nasmb", ".idf", ".Vfr", ".H"]
> > > -
> > >  class CheckOnePatch:
> > >      """Checks the contents of a git email formatted patch.
> > >  
> > > 
> > 
> > 
> > 
> >