public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael Kubacki" <mikuback@linux.microsoft.com>
To: Laszlo Ersek <lersek@redhat.com>, devel@edk2.groups.io
Cc: Andrew Fish <afish@apple.com>,
	Leif Lindholm <quic_llindhol@quicinc.com>,
	Michael D Kinney <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH v4 8/8] ReadMe.rst: Add CodeQL/analyze directory under other licenses
Date: Fri, 3 Nov 2023 11:19:07 -0400	[thread overview]
Message-ID: <1a73433a-ba01-4320-b7dd-fa05f0b58c3e@linux.microsoft.com> (raw)
In-Reply-To: <c832442b-5ac3-cbd8-5dc0-1c5097928771@redhat.com>

On 11/3/2023 10:46 AM, Laszlo Ersek wrote:
> On 11/3/23 15:16, Michael Kubacki wrote:
>> On 11/3/2023 9:06 AM, Laszlo Ersek wrote:
>>> On 11/2/23 21:03, Michael Kubacki wrote:
>>>> From: Michael Kubacki <michael.kubacki@microsoft.com>
>>>>
>>>> The code in this directory is licensed under Apache License, Version
>>>> 2.0. Therefore, the directory is listed under paths with licenses
>>>> other than BSD-2-Clause Plus Patent. The directory link points to the
>>>> complete Apache License, Version 2.0 on apache.org.
>>>>
>>>> Cc: Andrew Fish <afish@apple.com>
>>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>>> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
>>>> ---
>>>>    ReadMe.rst | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/ReadMe.rst b/ReadMe.rst
>>>> index 06fb122ef382..808ccd37af50 100644
>>>> --- a/ReadMe.rst
>>>> +++ b/ReadMe.rst
>>>> @@ -73,6 +73,7 @@ The majority of the content in the EDK II open
>>>> source project uses a
>>>>    source project contains the following components that are covered
>>>> by additional
>>>>    licenses:
>>>>    +-  `BaseTools/Plugin/CodeQL/analyze
>>>> <https://www.apache.org/licenses/LICENSE-2.0>`__
>>>>    -  `BaseTools/Source/C/LzmaCompress
>>>> <BaseTools/Source/C/LzmaCompress/LZMA-SDK-README.txt>`__
>>>>    -  `BaseTools/Source/C/VfrCompile/Pccts
>>>> <BaseTools/Source/C/VfrCompile/Pccts/RIGHTS>`__
>>>>    -  `CryptoPkg\Library\BaseCryptLib\SysCall\inet_pton.c
>>>> <CryptoPkg\Library\BaseCryptLib\SysCall\inet_pton.c>`__
>>>
>>> I've carefully read through the cover letter now (impressive work!). I
>>> have some questions, with reference to Leif's comment at
>>> <https://edk2.groups.io/g/devel/message/110475> as well:
>>>
>>> - Is the BaseTools/Plugin/CodeQL/analyze subdirectory not supposed to
>>> contain a standalone "COPYING" or similar file?
>>>
>>> If not, then the current patch seems fine:
>>>
>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>>
>> I wasn't aware of anything further needed for the Apache License 2.0.
>> I'm familiar with COPYING in the context of GNU licensing
>> (https://www.gnu.org/licenses/gpl-howto.html). I don't see it applying
>> directly to the Apache licensing process as I understand it.
> 
> Apologies, I was unclear.
> 
> My point was only that, if the copyright notices were included inside the local subdir, then we should point this reference too to that local file. And, I thought that any project would include such a separate file (which we'd now inherit).
> 
> Given that that is not the case, just apply my R-b. :)
> 
>>
>>> - I'd like to understand where the BaseTools/Plugin/CodeQL/analyze/
>>> contents (three files) originate from. If it was authored by Microsoft,
>>> then I don't understand (per v4 series changelog in the cover letter)
>>> why the Microsoft copyright notice had to be removed. And if it is not
>>> original work by Microsoft, but work derived by Microsoft from other
>>> original work, then it should contain both the original copyright
>>> notices, and Microsofts.
>>>
>> Because these are only a couple files, I tried to follow the guidance in
>> "To apply the Apache License to specific files in your work..." in "How
>> To Apply the Apache License to Your Work" in
>> https://www.apache.org/licenses/LICENSE-2.0.
>>
>> For those files I:
>>
>> 1. Made the upper text clearly state Apache License Version 2.0 with a
>> link to apache.org/licenses.
>>
>> 2. Included the boilerplate text as given in the above link for
>> "licensing specific files in your work".
>>
>> 3. Preserved any existing copyrights.
>>
>>     - globber.py had a pre-existing copyright preserved
> 
> Ah, indeed! Sorry, I totally missed that. Mea culpa!
> 
>>     - analyze_filter.py did not have one in the source Python file or
>>       its LICENSE file
> 
> OK!
> 
> 
> Finally, I'm just noticing that "BaseTools/Plugin/CodeQL/analyze/__init__.py" is actually an empty file. This looks like a python trick:
> 
>    https://old.reddit.com/r/learnpython/comments/fuxv57/can_init_py_actually_be_empty/
>    https://stackoverflow.com/questions/448271/what-is-init-py-for
> 
> So I now understand this empty __init__.py is not derived from <https://github.com/advanced-security/filter-sarif> -- it is a genuine addition under edk2, right?
> 
> But, because it is zero size (intentionally), adding a Microsoft copyright notice to it was deemed overkill. Is that correct?
> 
> We have a bunch of other, similarly empty __init__.py files:
> 
>    BaseTools/Plugin/DebugMacroCheck/tests/__init__.py
>    BaseTools/Source/C/BrotliCompress/brotli/python/tests/__init__.py
>    BaseTools/Source/Python/Ecc/CParser3/__init__.py
>    BaseTools/Source/Python/Ecc/CParser4/__init__.py
>    BaseTools/Source/Python/Eot/CParser3/__init__.py
>    BaseTools/Source/Python/Eot/CParser4/__init__.py
>    MdeModulePkg/Library/BrotliCustomDecompressLib/brotli/python/tests/__init__.py
> 
That's correct and my reasoning. If a copyright notice must be added, 
I'm happy to do so.

>>
>> 4. Appended text stating the source of the files and a brief summary of
>> the changes in this copy relative to the original.
>>
>>> The file-top comments in those three files reference
>>>
>>>     https://github.com/advanced-security/filter-sarif
>>>
>>> as the origin. Do the original files in that repository contain
>>> copyright notices? (Or does their containing project come with a COPYING
>>> or similar file?) I'm not looking for a license specification (SPDX or
>>> natural language), but specifically for copyright notices on the
>>> original work.
>>>
>> All copyright notices from original files are preserved.
> 
> Indeed -- I'm sorry for missing that previously.
> 
>>
>> https://github.com/advanced-security itself actually includes a local
>> copy of globber.py
>> https://github.com/advanced-security/filter-sarif/blob/main/globber.py.
>>
>> I dropped the Microsoft copyright in those specific files because my
>> contributions the those files were not significant. If there are other
>> factors to consider, please let me know and I will reconsider.
> 
> I think the only other factor here may be that you are creating the file in the edk2 tree.
> 
> Whenever I create a new file in edk2 (for example by copying an existent library instance, and customizing the code in the new instance, however minimally), I add a Red Hat copyright notice.
> 
> But I don't insist at all, I was just curious of the reasoning!
> 
I defaulted to that initially. But after we dived deeper into licensing 
and reevaluating the changes, I concluded to remove based on the 
triviality of those particular changes to the source file.

>>> Does the <https://github.com/advanced-security> organization perhaps use
>>> an over-arching copyright notice somewhere?
>>>
>> I couldn't find anything.
> 
> Thanks a lot for checking!
> 
> I don't object to any of the v4 patches getting merged as posted.
> 
> Cheers,
> Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110635): https://edk2.groups.io/g/devel/message/110635
Mute This Topic: https://groups.io/mt/102350800/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  parent reply	other threads:[~2023-11-03 15:19 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-02 20:03 [edk2-devel] [PATCH v4 0/8] Use CodeQL CLI Michael Kubacki
2023-11-02 20:03 ` [edk2-devel] [PATCH v4 1/8] Remove existing CodeQL infrastructure Michael Kubacki
2023-11-02 20:03 ` [edk2-devel] [PATCH v4 2/8] BaseTools/Plugin/CodeQL: Add CodeQL build plugin Michael Kubacki
2023-11-02 20:03 ` [edk2-devel] [PATCH v4 3/8] BaseTools/Plugin/CodeQL: Add integration helpers Michael Kubacki
2023-11-02 20:03 ` [edk2-devel] [PATCH v4 4/8] .pytool/CISettings.py: Integrate CodeQL Michael Kubacki
2023-11-02 20:03 ` [edk2-devel] [PATCH v4 5/8] .github/workflows/codeql.yml: Add CodeQL workflow Michael Kubacki
2023-11-02 20:03 ` [edk2-devel] [PATCH v4 6/8] .pytool/CISettings: Enable CodeQL audit mode Michael Kubacki
2023-11-07  0:57   ` Sean
2023-11-02 20:03 ` [edk2-devel] [PATCH v4 7/8] BaseTools/Plugin/CodeQL: Enable 30 queries Michael Kubacki
2023-11-07  0:55   ` Sean
2023-11-02 20:03 ` [edk2-devel] [PATCH v4 8/8] ReadMe.rst: Add CodeQL/analyze directory under other licenses Michael Kubacki
2023-11-03 13:06   ` Laszlo Ersek
2023-11-03 14:16     ` Michael Kubacki
2023-11-03 14:46       ` Laszlo Ersek
2023-11-03 14:48         ` Laszlo Ersek
2023-11-03 15:19         ` Michael Kubacki [this message]
2023-11-07  1:00 ` [edk2-devel] [PATCH v4 0/8] Use CodeQL CLI Sean

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1a73433a-ba01-4320-b7dd-fa05f0b58c3e@linux.microsoft.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox