public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: Michael Kubacki <mikuback@linux.microsoft.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 15:46:19 +0100	[thread overview]
Message-ID: <c832442b-5ac3-cbd8-5dc0-1c5097928771@redhat.com> (raw)
In-Reply-To: <b96efe81-8758-40a5-b134-70479712875a@linux.microsoft.com>

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


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

>> 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 (#110630): https://edk2.groups.io/g/devel/message/110630
Mute This Topic: https://groups.io/mt/102350800/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2023-11-03 14:46 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 [this message]
2023-11-03 14:48         ` Laszlo Ersek
2023-11-03 15:19         ` Michael Kubacki
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=c832442b-5ac3-cbd8-5dc0-1c5097928771@redhat.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