OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

[Rebecca Cran]

I've finished the work to move BhyvePkg into OvmfPkg/Bhyve, and the 
changes are available at 
https://github.com/bcran/edk2/commit/09cfc08b50992ed3f76459611e8a928b74ed8c8a 
.

Since it's such a large patch (over 400KB, but the vast majority of it 
is existing changes from previously-reviewed patches), I'm posting a 
link to the changes here in case there are any fundamental issues with 
it before I post it for review.

I've run PatchCheck.py and the only issue it flagged is that the 
'license' on VbeShim.h is wrong, and that's because the header is 
non-standard:

//
// THIS FILE WAS GENERATED BY "VbeShim.sh". DO NOT EDIT.
//


-- 

Rebecca Cran

Re: OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

[Laszlo Ersek]

Hi Rebecca,

On 06/26/20 17:38, Rebecca Cran wrote:
> I've finished the work to move BhyvePkg into OvmfPkg/Bhyve, and the
> changes are available at
> https://github.com/bcran/edk2/commit/09cfc08b50992ed3f76459611e8a928b74ed8c8a
> .
> 
> Since it's such a large patch (over 400KB, but the vast majority of it
> is existing changes from previously-reviewed patches), I'm posting a
> link to the changes here in case there are any fundamental issues with
> it before I post it for review.

I'll look at the patches on the list. (I don't believe in reviewing
before reviewing. In case I need to make comments for what I find in
your repo, I could only make them without any context to quote.)

> I've run PatchCheck.py and the only issue it flagged is that the
> 'license' on VbeShim.h is wrong, and that's because the header is
> non-standard:
> 
> //
> // THIS FILE WAS GENERATED BY "VbeShim.sh". DO NOT EDIT.
> //
> 
> 

This likely comes from BaseTools commit a4cfb842fca9
("BaseTools/PatchCheck.py: Add LicenseCheck", 2020-06-12).

One approach would be to remove "VbeShim.h" from the tracked files under
OvmfPkg, replacing it with a PREBUILD command in the OVMF DSC files.
(Then Bhyve could do the same.)

However, the generator, namely "VbeShim.sh", is not written in Python,
but in (POSIX) shell, and so it can't be called from PREBUILD (I think
it would break OVMF builds on Windows).

I don't know what to tell you, other than the blanket license
enforcement from commit a4cfb842fca9 is likely wrong.

The generated include file *must* be a ".h" file, otherwise the INF file
reference won't be able to trigger an incremental build, if I understand
correctly. So replacing the ".h" suffix with something else, such as
".genh" (for "generated header") won't work, I believe.

Modifying the printf invocations in the generator script to also output
a license header would not be right either, IMO. A license tag makes no
sense (I think) without a copyright (C) statement. And what copyright
(C) notice should we put on a generated file?

Furthermore, although "ReadMe.rst" in the project root states

"""
Contributions of code put into the public domain can also be accepted.
"""

I don't see how the license check implemented in commit a4cfb842fca9
would accommodate a public domain contribution. (I think it would be
fine to place the the generated header file in the public domain, *if*
(a) we could express that somehow (is there an SPDX tag for that?),
*and* (b) if that would eliminate the need for a (C) notice / authorship
mark.)

Note that this generator use case is not unique to QemuVideoDxe; see for
example commit 1e9d6b0f98b5 ("OvmfPkg/OvmfXen: Creating an ELF header",
2019-08-21).

I've now filed a bug for BaseTools:

https://bugzilla.tianocore.org/show_bug.cgi?id=2833

Once that bug is solved -- that is, once we standardize a tag for
marking generated source files as such --, we can update "VbeShim.sh" to
produce the tag, in "VbeShim.h". Then OvmfPkg/Bhyve can do the same.

Thanks
Laszlo

Re: License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

[Leif Lindholm]

On Thu, Jul 02, 2020 at 11:27:25 +0200, Laszlo Ersek wrote:
> This likely comes from BaseTools commit a4cfb842fca9
> ("BaseTools/PatchCheck.py: Add LicenseCheck", 2020-06-12).
> 
> One approach would be to remove "VbeShim.h" from the tracked files under
> OvmfPkg, replacing it with a PREBUILD command in the OVMF DSC files.
> (Then Bhyve could do the same.)
> 
> However, the generator, namely "VbeShim.sh", is not written in Python,
> but in (POSIX) shell, and so it can't be called from PREBUILD (I think
> it would break OVMF builds on Windows).
> 
> I don't know what to tell you, other than the blanket license
> enforcement from commit a4cfb842fca9 is likely wrong.

*Reads patch*
*Figuratively spits coffee all over keyboard*

No, this is not OK.

We *STILL* have no agreed process for accepting non bsd+patent content
since we dropped the contribution agreement. I have tried to raise
this issue several times in the past, and there has never been any
outcome from resulting discussions.

So now I'm going to send out a two-patch set consisting of:
- Reverting a4cfb842fca9. (Doing nothing is better than implying that
  anything !bsd+patent can currently be added to the tree.)
- Deleting the statement in ReadmMe.rst erroneously claiming that the
  includion of these other licenses are acceptable until such a point
  an active decision has been taken, approved by the community, that
  this is permitted.

> The generated include file *must* be a ".h" file, otherwise the INF file
> reference won't be able to trigger an incremental build, if I understand
> correctly. So replacing the ".h" suffix with something else, such as
> ".genh" (for "generated header") won't work, I believe.
> 
> Modifying the printf invocations in the generator script to also output
> a license header would not be right either, IMO. A license tag makes no
> sense (I think) without a copyright (C) statement. And what copyright
> (C) notice should we put on a generated file?
> 
> Furthermore, although "ReadMe.rst" in the project root states
> 
> """
> Contributions of code put into the public domain can also be accepted.
> """
> 
> I don't see how the license check implemented in commit a4cfb842fca9
> would accommodate a public domain contribution. (I think it would be
> fine to place the the generated header file in the public domain, *if*
> (a) we could express that somehow (is there an SPDX tag for that?),
> *and* (b) if that would eliminate the need for a (C) notice / authorship
> mark.)

Public domain is not an OSI-compatible license:
https://opensource.org/node/878

The public domain statement is also one that needs to be re-evaluated
in light of the dropped contribution agreement.

/
    Leif

> Note that this generator use case is not unique to QemuVideoDxe; see for
> example commit 1e9d6b0f98b5 ("OvmfPkg/OvmfXen: Creating an ELF header",
> 2019-08-21).
> 
> I've now filed a bug for BaseTools:
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2833
> 
> Once that bug is solved -- that is, once we standardize a tag for
> marking generated source files as such --, we can update "VbeShim.sh" to
> produce the tag, in "VbeShim.h". Then OvmfPkg/Bhyve can do the same.
> 
> Thanks
> Laszlo
> 

Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

[Liming Gao]

Leif:

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif Lindholm
> Sent: Thursday, July 2, 2020 6:54 PM
> To: Laszlo Ersek <lersek@redhat.com>
> Cc: Rebecca Cran <rebecca@bsdio.com>; edk2-devel-groups-io <devel@edk2.groups.io>; Ard Biesheuvel <ard.biesheuvel@arm.com>;
> Andrew Fish <afish@apple.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve
> 
> On Thu, Jul 02, 2020 at 11:27:25 +0200, Laszlo Ersek wrote:
> > This likely comes from BaseTools commit a4cfb842fca9
> > ("BaseTools/PatchCheck.py: Add LicenseCheck", 2020-06-12).
> >
> > One approach would be to remove "VbeShim.h" from the tracked files under
> > OvmfPkg, replacing it with a PREBUILD command in the OVMF DSC files.
> > (Then Bhyve could do the same.)
> >
> > However, the generator, namely "VbeShim.sh", is not written in Python,
> > but in (POSIX) shell, and so it can't be called from PREBUILD (I think
> > it would break OVMF builds on Windows).
> >
> > I don't know what to tell you, other than the blanket license
> > enforcement from commit a4cfb842fca9 is likely wrong.
> 
> *Reads patch*
> *Figuratively spits coffee all over keyboard*
> 
> No, this is not OK.
> 
> We *STILL* have no agreed process for accepting non bsd+patent content
> since we dropped the contribution agreement. I have tried to raise
> this issue several times in the past, and there has never been any
> outcome from resulting discussions.
> 
> So now I'm going to send out a two-patch set consisting of:
> - Reverting a4cfb842fca9. (Doing nothing is better than implying that
>   anything !bsd+patent can currently be added to the tree.)
> - Deleting the statement in ReadmMe.rst erroneously claiming that the
>   includion of these other licenses are acceptable until such a point
>   an active decision has been taken, approved by the community, that
>   this is permitted.
> 

If only bsd+patent is allowed, the checker can be enhanced to check this license only. 
I don't understand why remove this checker.

Thanks
Liming
> > The generated include file *must* be a ".h" file, otherwise the INF file
> > reference won't be able to trigger an incremental build, if I understand
> > correctly. So replacing the ".h" suffix with something else, such as
> > ".genh" (for "generated header") won't work, I believe.
> >
> > Modifying the printf invocations in the generator script to also output
> > a license header would not be right either, IMO. A license tag makes no
> > sense (I think) without a copyright (C) statement. And what copyright
> > (C) notice should we put on a generated file?
> >
> > Furthermore, although "ReadMe.rst" in the project root states
> >
> > """
> > Contributions of code put into the public domain can also be accepted.
> > """
> >
> > I don't see how the license check implemented in commit a4cfb842fca9
> > would accommodate a public domain contribution. (I think it would be
> > fine to place the the generated header file in the public domain, *if*
> > (a) we could express that somehow (is there an SPDX tag for that?),
> > *and* (b) if that would eliminate the need for a (C) notice / authorship
> > mark.)
> 
> Public domain is not an OSI-compatible license:
> https://opensource.org/node/878
> 
> The public domain statement is also one that needs to be re-evaluated
> in light of the dropped contribution agreement.
> 
> /
>     Leif
> 
> > Note that this generator use case is not unique to QemuVideoDxe; see for
> > example commit 1e9d6b0f98b5 ("OvmfPkg/OvmfXen: Creating an ELF header",
> > 2019-08-21).
> >
> > I've now filed a bug for BaseTools:
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2833
> >
> > Once that bug is solved -- that is, once we standardize a tag for
> > marking generated source files as such --, we can update "VbeShim.sh" to
> > produce the tag, in "VbeShim.h". Then OvmfPkg/Bhyve can do the same.
> >
> > Thanks
> > Laszlo
> >
> 
> 

Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

[Leif Lindholm]

On Thu, Jul 02, 2020 at 13:49:45 +0000, Gao, Liming wrote:
> Leif:
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif Lindholm
> > Sent: Thursday, July 2, 2020 6:54 PM
> > To: Laszlo Ersek <lersek@redhat.com>
> > Cc: Rebecca Cran <rebecca@bsdio.com>; edk2-devel-groups-io <devel@edk2.groups.io>; Ard Biesheuvel <ard.biesheuvel@arm.com>;
> > Andrew Fish <afish@apple.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve
> > 
> > On Thu, Jul 02, 2020 at 11:27:25 +0200, Laszlo Ersek wrote:
> > > This likely comes from BaseTools commit a4cfb842fca9
> > > ("BaseTools/PatchCheck.py: Add LicenseCheck", 2020-06-12).
> > >
> > > One approach would be to remove "VbeShim.h" from the tracked files under
> > > OvmfPkg, replacing it with a PREBUILD command in the OVMF DSC files.
> > > (Then Bhyve could do the same.)
> > >
> > > However, the generator, namely "VbeShim.sh", is not written in Python,
> > > but in (POSIX) shell, and so it can't be called from PREBUILD (I think
> > > it would break OVMF builds on Windows).
> > >
> > > I don't know what to tell you, other than the blanket license
> > > enforcement from commit a4cfb842fca9 is likely wrong.
> > 
> > *Reads patch*
> > *Figuratively spits coffee all over keyboard*
> > 
> > No, this is not OK.
> > 
> > We *STILL* have no agreed process for accepting non bsd+patent content
> > since we dropped the contribution agreement. I have tried to raise
> > this issue several times in the past, and there has never been any
> > outcome from resulting discussions.
> > 
> > So now I'm going to send out a two-patch set consisting of:
> > - Reverting a4cfb842fca9. (Doing nothing is better than implying that
> >   anything !bsd+patent can currently be added to the tree.)
> > - Deleting the statement in ReadmMe.rst erroneously claiming that the
> >   includion of these other licenses are acceptable until such a point
> >   an active decision has been taken, approved by the community, that
> >   this is permitted.
> > 
> 
> If only bsd+patent is allowed, the checker can be enhanced to check this license only. 
> I don't understand why remove this checker.  

Mainly because that was the easiest thing to do :)

But also because:
- The thread that spawned this also raised the problem of
  machine-generated files.
- I am somewhat unhappy the checker got merged in the first place
  without wider community feedback. BaseTools and its contents are
  used for many repositories (even within TianoCore), and this added
  unconditional check breaks the use for some of those.

Regards,

Leif

> 
> Thanks
> Liming
> > > The generated include file *must* be a ".h" file, otherwise the INF file
> > > reference won't be able to trigger an incremental build, if I understand
> > > correctly. So replacing the ".h" suffix with something else, such as
> > > ".genh" (for "generated header") won't work, I believe.
> > >
> > > Modifying the printf invocations in the generator script to also output
> > > a license header would not be right either, IMO. A license tag makes no
> > > sense (I think) without a copyright (C) statement. And what copyright
> > > (C) notice should we put on a generated file?
> > >
> > > Furthermore, although "ReadMe.rst" in the project root states
> > >
> > > """
> > > Contributions of code put into the public domain can also be accepted.
> > > """
> > >
> > > I don't see how the license check implemented in commit a4cfb842fca9
> > > would accommodate a public domain contribution. (I think it would be
> > > fine to place the the generated header file in the public domain, *if*
> > > (a) we could express that somehow (is there an SPDX tag for that?),
> > > *and* (b) if that would eliminate the need for a (C) notice / authorship
> > > mark.)
> > 
> > Public domain is not an OSI-compatible license:
> > https://opensource.org/node/878
> > 
> > The public domain statement is also one that needs to be re-evaluated
> > in light of the dropped contribution agreement.
> > 
> > /
> >     Leif
> > 
> > > Note that this generator use case is not unique to QemuVideoDxe; see for
> > > example commit 1e9d6b0f98b5 ("OvmfPkg/OvmfXen: Creating an ELF header",
> > > 2019-08-21).
> > >
> > > I've now filed a bug for BaseTools:
> > >
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=2833
> > >
> > > Once that bug is solved -- that is, once we standardize a tag for
> > > marking generated source files as such --, we can update "VbeShim.sh" to
> > > produce the tag, in "VbeShim.h". Then OvmfPkg/Bhyve can do the same.
> > >
> > > Thanks
> > > Laszlo
> > >
> > 
> > 
> 

Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

[Ard Biesheuvel]

On 7/2/20 4:13 PM, Leif Lindholm via groups.io wrote:
> On Thu, Jul 02, 2020 at 13:49:45 +0000, Gao, Liming wrote:
>> Leif:
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif Lindholm
>>> Sent: Thursday, July 2, 2020 6:54 PM
>>> To: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Rebecca Cran <rebecca@bsdio.com>; edk2-devel-groups-io <devel@edk2.groups.io>; Ard Biesheuvel <ard.biesheuvel@arm.com>;
>>> Andrew Fish <afish@apple.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
>>> Subject: Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve
>>>
>>> On Thu, Jul 02, 2020 at 11:27:25 +0200, Laszlo Ersek wrote:
>>>> This likely comes from BaseTools commit a4cfb842fca9
>>>> ("BaseTools/PatchCheck.py: Add LicenseCheck", 2020-06-12).
>>>>
>>>> One approach would be to remove "VbeShim.h" from the tracked files under
>>>> OvmfPkg, replacing it with a PREBUILD command in the OVMF DSC files.
>>>> (Then Bhyve could do the same.)
>>>>
>>>> However, the generator, namely "VbeShim.sh", is not written in Python,
>>>> but in (POSIX) shell, and so it can't be called from PREBUILD (I think
>>>> it would break OVMF builds on Windows).
>>>>
>>>> I don't know what to tell you, other than the blanket license
>>>> enforcement from commit a4cfb842fca9 is likely wrong.
>>>
>>> *Reads patch*
>>> *Figuratively spits coffee all over keyboard*
>>>
>>> No, this is not OK.
>>>
>>> We *STILL* have no agreed process for accepting non bsd+patent content
>>> since we dropped the contribution agreement. I have tried to raise
>>> this issue several times in the past, and there has never been any
>>> outcome from resulting discussions.
>>>
>>> So now I'm going to send out a two-patch set consisting of:
>>> - Reverting a4cfb842fca9. (Doing nothing is better than implying that
>>>    anything !bsd+patent can currently be added to the tree.)
>>> - Deleting the statement in ReadmMe.rst erroneously claiming that the
>>>    includion of these other licenses are acceptable until such a point
>>>    an active decision has been taken, approved by the community, that
>>>    this is permitted.
>>>
>>
>> If only bsd+patent is allowed, the checker can be enhanced to check this license only.
>> I don't understand why remove this checker.
> 
> Mainly because that was the easiest thing to do :)
> 
> But also because:
> - The thread that spawned this also raised the problem of
>    machine-generated files.
> - I am somewhat unhappy the checker got merged in the first place
>    without wider community feedback. BaseTools and its contents are
>    used for many repositories (even within TianoCore), and this added
>    unconditional check breaks the use for some of those.
> 

I think the fundamental problem is that contributing code under a 
contribution agreement that includes a patent grant is not the same as 
contributing it under a patent grant license, given that the latter can 
only be done by the author of the code, while the former could be done 
by anyone.

This means our current licensing policy is actually more restrictive 
that the old one, making it more difficult to incorporate 'second hand' 
code.

I don't think we can fix this with a patch though :-(

Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

[Liming Gao]

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
> Sent: Thursday, July 2, 2020 10:32 PM
> To: devel@edk2.groups.io; leif@nuviainc.com; Gao, Liming <liming.gao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>; Rebecca Cran <rebecca@bsdio.com>; Andrew Fish <afish@apple.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve
> 
> On 7/2/20 4:13 PM, Leif Lindholm via groups.io wrote:
> > On Thu, Jul 02, 2020 at 13:49:45 +0000, Gao, Liming wrote:
> >> Leif:
> >>
> >>> -----Original Message-----
> >>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif Lindholm
> >>> Sent: Thursday, July 2, 2020 6:54 PM
> >>> To: Laszlo Ersek <lersek@redhat.com>
> >>> Cc: Rebecca Cran <rebecca@bsdio.com>; edk2-devel-groups-io <devel@edk2.groups.io>; Ard Biesheuvel
> <ard.biesheuvel@arm.com>;
> >>> Andrew Fish <afish@apple.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> >>> Subject: Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve
> >>>
> >>> On Thu, Jul 02, 2020 at 11:27:25 +0200, Laszlo Ersek wrote:
> >>>> This likely comes from BaseTools commit a4cfb842fca9
> >>>> ("BaseTools/PatchCheck.py: Add LicenseCheck", 2020-06-12).
> >>>>
> >>>> One approach would be to remove "VbeShim.h" from the tracked files under
> >>>> OvmfPkg, replacing it with a PREBUILD command in the OVMF DSC files.
> >>>> (Then Bhyve could do the same.)
> >>>>
> >>>> However, the generator, namely "VbeShim.sh", is not written in Python,
> >>>> but in (POSIX) shell, and so it can't be called from PREBUILD (I think
> >>>> it would break OVMF builds on Windows).
> >>>>
> >>>> I don't know what to tell you, other than the blanket license
> >>>> enforcement from commit a4cfb842fca9 is likely wrong.
> >>>
> >>> *Reads patch*
> >>> *Figuratively spits coffee all over keyboard*
> >>>
> >>> No, this is not OK.
> >>>
> >>> We *STILL* have no agreed process for accepting non bsd+patent content
> >>> since we dropped the contribution agreement. I have tried to raise
> >>> this issue several times in the past, and there has never been any
> >>> outcome from resulting discussions.
> >>>
> >>> So now I'm going to send out a two-patch set consisting of:
> >>> - Reverting a4cfb842fca9. (Doing nothing is better than implying that
> >>>    anything !bsd+patent can currently be added to the tree.)
> >>> - Deleting the statement in ReadmMe.rst erroneously claiming that the
> >>>    includion of these other licenses are acceptable until such a point
> >>>    an active decision has been taken, approved by the community, that
> >>>    this is permitted.
> >>>
> >>
> >> If only bsd+patent is allowed, the checker can be enhanced to check this license only.
> >> I don't understand why remove this checker.
> >
> > Mainly because that was the easiest thing to do :)

People may miss it. So, the checker is helpful to detect the issue. 

> >
> > But also because:
> > - The thread that spawned this also raised the problem of
> >    machine-generated files.
This is a gap. We have no rule for the generated file. 

> > - I am somewhat unhappy the checker got merged in the first place
> >    without wider community feedback. BaseTools and its contents are
> >    used for many repositories (even within TianoCore), and this added
> >    unconditional check breaks the use for some of those.
> >
The patch to add the license checker is reviewed in edk2 mail list for several weeks. 
I don't get other comments. Can you give the suggestion on how to improve the communication in edk2 community? 

Besides, there is another new checker of ECC to check coding style for each patch. Can you give your comment?
https://edk2.groups.io/g/devel/message/61966

> 
> I think the fundamental problem is that contributing code under a
> contribution agreement that includes a patent grant is not the same as
> contributing it under a patent grant license, given that the latter can
> only be done by the author of the code, while the former could be done
> by anyone.
> 
> This means our current licensing policy is actually more restrictive
> that the old one, making it more difficult to incorporate 'second hand'
> code.
> 
> I don't think we can fix this with a patch though :-(

Yes. This checker is for current allowed license. It doesn't resolve this issue. 

Thanks
Liming
> 
> 

Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

[Leif Lindholm]

On Fri, Jul 03, 2020 at 01:40:26 +0000, Gao, Liming wrote:
> > >>> *Reads patch*
> > >>> *Figuratively spits coffee all over keyboard*
> > >>>
> > >>> No, this is not OK.
> > >>>
> > >>> We *STILL* have no agreed process for accepting non bsd+patent content
> > >>> since we dropped the contribution agreement. I have tried to raise
> > >>> this issue several times in the past, and there has never been any
> > >>> outcome from resulting discussions.
> > >>>
> > >>> So now I'm going to send out a two-patch set consisting of:
> > >>> - Reverting a4cfb842fca9. (Doing nothing is better than implying that
> > >>>    anything !bsd+patent can currently be added to the tree.)
> > >>> - Deleting the statement in ReadmMe.rst erroneously claiming that the
> > >>>    includion of these other licenses are acceptable until such a point
> > >>>    an active decision has been taken, approved by the community, that
> > >>>    this is permitted.
> > >>>
> > >>
> > >> If only bsd+patent is allowed, the checker can be enhanced to check this license only.
> > >> I don't understand why remove this checker.
> > >
> > > Mainly because that was the easiest thing to do :)
> 
> People may miss it. So, the checker is helpful to detect the issue. 

The feature is useful, but enabling it by default is not the correct
decision for all TianoCore repos, and the situation for non-bsd+patent
contributions is less than ideal.

> > >
> > > But also because:
> > > - The thread that spawned this also raised the problem of
> > >    machine-generated files.
>
> This is a gap. We have no rule for the generated file. 
> 
> > > - I am somewhat unhappy the checker got merged in the first place
> > >    without wider community feedback. BaseTools and its contents are
> > >    used for many repositories (even within TianoCore), and this added
> > >    unconditional check breaks the use for some of those.
> > >
> The patch to add the license checker is reviewed in edk2 mail list
> for several weeks.
> I don't get other comments. Can you give the suggestion on how to
> improve the communication in edk2 community?

I think that for something as fundamental as this, we need to actively
chase feedback. I know that I will never manage to always read all
emails to the lists, so there is always a risk I will miss something
I'm not cc:d on.
For something with as big an impact as a tightening of requirements in
PatchCheck.py, if sufficient feedback (like at least 2-3 maintainers
outside of BaseTools) has not been received, then it would make sense
to ping *all* maintainers, alternatively ping the stewards and ask us
to go gather feedback.

> Besides, there is another new checker of ECC to check coding style
> for each patch. Can you give your comment?
> https://edk2.groups.io/g/devel/message/61966

I have never managed to get ECC running in any of my setups.
Perhaps I should start trying to track down why, or at least raise a
bugzilla for someone else to investigate.

Regards,

Leif

> > I think the fundamental problem is that contributing code under a
> > contribution agreement that includes a patent grant is not the same as
> > contributing it under a patent grant license, given that the latter can
> > only be done by the author of the code, while the former could be done
> > by anyone.
> > 
> > This means our current licensing policy is actually more restrictive
> > that the old one, making it more difficult to incorporate 'second hand'
> > code.
> > 
> > I don't think we can fix this with a patch though :-(
> 
> Yes. This checker is for current allowed license. It doesn't resolve this issue. 
> 
> Thanks
> Liming
> > 
> > 
> 

Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

[Liming Gao]

> -----Original Message-----
> From: Leif Lindholm <leif@nuviainc.com>
> Sent: Friday, July 3, 2020 6:38 PM
> To: Gao, Liming <liming.gao@intel.com>
> Cc: devel@edk2.groups.io; ard.biesheuvel@arm.com; Laszlo Ersek <lersek@redhat.com>; Rebecca Cran <rebecca@bsdio.com>;
> Andrew Fish <afish@apple.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve
> 
> On Fri, Jul 03, 2020 at 01:40:26 +0000, Gao, Liming wrote:
> > > >>> *Reads patch*
> > > >>> *Figuratively spits coffee all over keyboard*
> > > >>>
> > > >>> No, this is not OK.
> > > >>>
> > > >>> We *STILL* have no agreed process for accepting non bsd+patent content
> > > >>> since we dropped the contribution agreement. I have tried to raise
> > > >>> this issue several times in the past, and there has never been any
> > > >>> outcome from resulting discussions.
> > > >>>
> > > >>> So now I'm going to send out a two-patch set consisting of:
> > > >>> - Reverting a4cfb842fca9. (Doing nothing is better than implying that
> > > >>>    anything !bsd+patent can currently be added to the tree.)
> > > >>> - Deleting the statement in ReadmMe.rst erroneously claiming that the
> > > >>>    includion of these other licenses are acceptable until such a point
> > > >>>    an active decision has been taken, approved by the community, that
> > > >>>    this is permitted.
> > > >>>
> > > >>
> > > >> If only bsd+patent is allowed, the checker can be enhanced to check this license only.
> > > >> I don't understand why remove this checker.
> > > >
> > > > Mainly because that was the easiest thing to do :)
> >
> > People may miss it. So, the checker is helpful to detect the issue.
> 
> The feature is useful, but enabling it by default is not the correct
> decision for all TianoCore repos, and the situation for non-bsd+patent
> contributions is less than ideal.

It can be added in open CI for Edk2 project now. 

> 
> > > >
> > > > But also because:
> > > > - The thread that spawned this also raised the problem of
> > > >    machine-generated files.
> >
> > This is a gap. We have no rule for the generated file.

Laszlo gives one proposal to use the specific tag in file header if this file is auto generated. 
Then, the license is not required. 

> >
> > > > - I am somewhat unhappy the checker got merged in the first place
> > > >    without wider community feedback. BaseTools and its contents are
> > > >    used for many repositories (even within TianoCore), and this added
> > > >    unconditional check breaks the use for some of those.
> > > >
> > The patch to add the license checker is reviewed in edk2 mail list
> > for several weeks.
> > I don't get other comments. Can you give the suggestion on how to
> > improve the communication in edk2 community?
> 
> I think that for something as fundamental as this, we need to actively
> chase feedback. I know that I will never manage to always read all
> emails to the lists, so there is always a risk I will miss something
> I'm not cc:d on.
> For something with as big an impact as a tightening of requirements in
> PatchCheck.py, if sufficient feedback (like at least 2-3 maintainers
> outside of BaseTools) has not been received, then it would make sense
> to ping *all* maintainers, alternatively ping the stewards and ask us
> to go gather feedback.

Good suggestion to include more maintainers and stewards.

> 
> > Besides, there is another new checker of ECC to check coding style
> > for each patch. Can you give your comment?
> > https://edk2.groups.io/g/devel/message/61966
> 
> I have never managed to get ECC running in any of my setups.
> Perhaps I should start trying to track down why, or at least raise a
> bugzilla for someone else to investigate.
> 

For ECC checker, I will include more people in ECC checker mail list. 

Thanks
Liming
> Regards,
> 
> Leif
> 
> > > I think the fundamental problem is that contributing code under a
> > > contribution agreement that includes a patent grant is not the same as
> > > contributing it under a patent grant license, given that the latter can
> > > only be done by the author of the code, while the former could be done
> > > by anyone.
> > >
> > > This means our current licensing policy is actually more restrictive
> > > that the old one, making it more difficult to incorporate 'second hand'
> > > code.
> > >
> > > I don't think we can fix this with a patch though :-(
> >
> > Yes. This checker is for current allowed license. It doesn't resolve this issue.
> >
> > Thanks
> > Liming
> > >
> > > 
> >

Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

[Laszlo Ersek]

On 07/03/20 03:40, Gao, Liming wrote:

> Besides, there is another new checker of ECC to check coding style for each patch. Can you give your comment?
> https://edk2.groups.io/g/devel/message/61966

I've seen that feature.

Importantly, it gives package maintainers package-level exception lists.
So if ECC rejects something, and the package maintainers disagree with
the rejection, they can create an exception inside the package, and
suppress the ECC report.

We don't have the same option with PatchCheck.

(For generated files anyway, that's what I'm suggesting in fact: if
PatchCheck sees the string "@file: generated" in a source file, it
should omit the license check altogether, for that file.)

Thanks!
Laszlo

Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

[Liming Gao]

Laszlo:

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Friday, July 3, 2020 11:49 PM
> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; ard.biesheuvel@arm.com; leif@nuviainc.com
> Cc: Rebecca Cran <rebecca@bsdio.com>; Andrew Fish <afish@apple.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] License Check - was OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve
> 
> On 07/03/20 03:40, Gao, Liming wrote:
> 
> > Besides, there is another new checker of ECC to check coding style for each patch. Can you give your comment?
> > https://edk2.groups.io/g/devel/message/61966
> 
> I've seen that feature.
> 
> Importantly, it gives package maintainers package-level exception lists.
> So if ECC rejects something, and the package maintainers disagree with
> the rejection, they can create an exception inside the package, and
> suppress the ECC report.
> 
> We don't have the same option with PatchCheck.
> 
> (For generated files anyway, that's what I'm suggesting in fact: if
> PatchCheck sees the string "@file: generated" in a source file, it
> should omit the license check altogether, for that file.)

License checker can be added as the same way to ECC checker. 
If so, the package maintainers can add the generated file as the exception. 

Thanks
Liming
> 
> Thanks!
> Laszlo

Re: OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

[Rebecca Cran]

On 7/2/20 3:27 AM, Laszlo Ersek wrote:

> I'll look at the patches on the list. (I don't believe in reviewing
> before reviewing. In case I need to make comments for what I find in
> your repo, I could only make them without any context to quote.)

Thanks, I'll wait for the changes to PatchCheck.py to land, then send 
the bhyve patch to the list. I think at this point I've updated all the 
licenses to be BSD+Patent with the exception of the generated file.


-- 
Rebecca Cran

Re: OvmfPkg: Adding support for bhyve as OvmfPkg/Bhyve

[Laszlo Ersek]

On 07/03/20 05:13, Rebecca Cran wrote:
> On 7/2/20 3:27 AM, Laszlo Ersek wrote:
> 
>> I'll look at the patches on the list. (I don't believe in reviewing
>> before reviewing. In case I need to make comments for what I find in
>> your repo, I could only make them without any context to quote.)
> 
> Thanks, I'll wait for the changes to PatchCheck.py to land, then send
> the bhyve patch to the list. I think at this point I've updated all the
> licenses to be BSD+Patent with the exception of the generated file.

Sounds great!
Laszlo