diff mbox series

[XEN,for-4.17,2/4] tools/include/xen-foreign: Capture licences from the input headers

Message ID 20221102112854.49020-3-anthony.perard@citrix.com (mailing list archive)
State Superseded
Headers show
Series Fixing some licences issue in public headers | expand

Commit Message

Anthony PERARD Nov. 2, 2022, 11:28 a.m. UTC
The headers install in "/usr/include/xen/foreign/" are missing a
licence.

While we could probably just add the MIT licence to the generated
file, this patch instead try to grab the licence from the original
input file.

Since licences are in the first multiline C comments, we just look for
that. Also with this patch, the possible licences will not be in the
"input" variable anymore, but it should be unnecessary to generate the
foreign header.

With this change, the licence will be copied 2 or 3 time in the
install headers depending on the number of input headers.

Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    Maybe instead of this, we should just stamp this on the generated header
        /* SPDX-License-Identifier: MIT */
    
    but we would be missing the "Copyright" informations. I guess we could
    look for those line with Copyright and copy them.
    
    Or, we could replace the licence in the input header by a SPDX and have
    the script parse that. (Probably still need to grab the Copyright lines)
    
    CC: Andrew Cooper <andrew.cooper3@citrix.com>
    CC: George Dunlap <george.dunlap@citrix.com>
    CC: Jan Beulich <jbeulich@suse.com>
    CC: Julien Grall <julien@xen.org>
    CC: Stefano Stabellini <sstabellini@kernel.org>
    CC: Wei Liu <wl@xen.org>

 tools/include/xen-foreign/mkheader.py | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Andrew Cooper Nov. 2, 2022, 12:24 p.m. UTC | #1
On 02/11/2022 11:28, Anthony PERARD wrote:
> The headers install in "/usr/include/xen/foreign/" are missing a
> licence.
>
> While we could probably just add the MIT licence to the generated
> file, this patch instead try to grab the licence from the original
> input file.
>
> Since licences are in the first multiline C comments, we just look for
> that. Also with this patch, the possible licences will not be in the
> "input" variable anymore, but it should be unnecessary to generate the
> foreign header.
>
> With this change, the licence will be copied 2 or 3 time in the
> install headers depending on the number of input headers.
>
> Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>
> Notes:
>     Maybe instead of this, we should just stamp this on the generated header
>         /* SPDX-License-Identifier: MIT */
>     
>     but we would be missing the "Copyright" informations. I guess we could
>     look for those line with Copyright and copy them.
>     
>     Or, we could replace the licence in the input header by a SPDX and have
>     the script parse that. (Probably still need to grab the Copyright lines)

All public headers (except one :-( ) are MIT.

We should SPDX the lot, not least because that removes all the
guestimation from this script; we can require that the SPDX line is the
first line, and sanity check it as we process it.

The headers also ought to say "automatically generated from $OTHER", and
include no other information.  That's now most autogen headers work.

So the generated files ought to end up looking like:

/* SPDX-License-Identifier: MIT */
/* Automatically generated from $FILE */

#ifndef $BLAH
...

~Andrew
Anthony PERARD Nov. 2, 2022, 3:14 p.m. UTC | #2
On Wed, Nov 02, 2022 at 12:24:21PM +0000, Andrew Cooper wrote:
> On 02/11/2022 11:28, Anthony PERARD wrote:
> > Notes:
> >     Maybe instead of this, we should just stamp this on the generated header
> >         /* SPDX-License-Identifier: MIT */
> >     
> >     but we would be missing the "Copyright" informations. I guess we could
> >     look for those line with Copyright and copy them.
> >     
> >     Or, we could replace the licence in the input header by a SPDX and have
> >     the script parse that. (Probably still need to grab the Copyright lines)
> 
> All public headers (except one :-( ) are MIT.
>
> We should SPDX the lot, not least because that removes all the
> guestimation from this script; we can require that the SPDX line is the
> first line, and sanity check it as we process it.

Yes, but I'm not sure I want to do the SPDX change when the tree is
supposed to be frozen.

> The headers also ought to say "automatically generated from $OTHER", and
> include no other information.  That's now most autogen headers work.
> 
> So the generated files ought to end up looking like:
> 
> /* SPDX-License-Identifier: MIT */
> /* Automatically generated from $FILE */

So the headers already have:
    /*
     * public xen defines and struct for arm32
     * generated by mkheader.py -- DO NOT EDIT
     */

So we just miss the source files used as input by mkheader.py. I can fix
that.

Thanks,
Andrew Cooper Nov. 2, 2022, 3:57 p.m. UTC | #3
On 02/11/2022 15:14, Anthony PERARD wrote:
> On Wed, Nov 02, 2022 at 12:24:21PM +0000, Andrew Cooper wrote:
>> On 02/11/2022 11:28, Anthony PERARD wrote:
>>> Notes:
>>>     Maybe instead of this, we should just stamp this on the generated header
>>>         /* SPDX-License-Identifier: MIT */
>>>     
>>>     but we would be missing the "Copyright" informations. I guess we could
>>>     look for those line with Copyright and copy them.
>>>     
>>>     Or, we could replace the licence in the input header by a SPDX and have
>>>     the script parse that. (Probably still need to grab the Copyright lines)
>> All public headers (except one :-( ) are MIT.
>>
>> We should SPDX the lot, not least because that removes all the
>> guestimation from this script; we can require that the SPDX line is the
>> first line, and sanity check it as we process it.
> Yes, but I'm not sure I want to do the SPDX change when the tree is
> supposed to be frozen.

The licensing corrections are a release blocker.

The frozenness of the tree has no bearing on the acceptableness of the
fix.  Especially when all we're talking about is adjustment of some
(legally-relevant) comments.

In this case, switching to a more machine-parsable form, in order to get
the autogeneration correct, is IMO fully acceptable even at this point
in the release.

~Andrew
diff mbox series

Patch

diff --git a/tools/include/xen-foreign/mkheader.py b/tools/include/xen-foreign/mkheader.py
index fb268f0dce..07a9bcbd01 100644
--- a/tools/include/xen-foreign/mkheader.py
+++ b/tools/include/xen-foreign/mkheader.py
@@ -114,9 +114,19 @@  input  = "";
 output = "";
 fileid = re.sub("[-.]", "_", "__FOREIGN_%s__" % outfile.upper());
 
-# read input header files
+# Try to captures licences headers from original files.
+# heuristic: just look for the end of the first multiline comment.
+licence_headers = "";
+
 for name in infiles:
     f = open(name, "r");
+    while True:
+        line = f.readline()
+        if not line:
+            break
+        licence_headers += line
+        if line == " */\n":
+            break
     input += f.read();
     f.close();
 
@@ -126,11 +136,12 @@  output += """
  * public xen defines and struct for %s
  * generated by %s -- DO NOT EDIT
  */
+%s
 
 #ifndef %s
 #define %s 1
 
-""" % (arch, sys.argv[0], fileid, fileid)
+""" % (arch, sys.argv[0], licence_headers, fileid, fileid)
 
 if arch in header:
     output += header[arch];