codegen: drop config.h and use short license header for Wireshark

Submitted by Peter Wu on Aug. 31, 2018, 11:43 p.m.

Details

Message ID 20180831234311.16483-1-peter@lekensteyn.nl
State New
Headers show
Series "codegen: drop config.h and use short license header for Wireshark" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Peter Wu Aug. 31, 2018, 11:43 p.m.
Shorten the generated dissector, drop unused config.h inclusion and
abbreviate the license blurb. No functional change, cosmetics only.

Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
For the updated output, see https://code.wireshark.org/review/29380
I considered capitalizing names such as SPICE_VSC_MESSAGE_TYPE_FLUSHCOMPLETE
(instead of SPICE_VSC_MESSAGE_TYPE_FlushComplete), but that breaks the public
spice-protocol interface which has been there since at least 2010, so nope.
---
 spice_codegen.py | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/spice_codegen.py b/spice_codegen.py
index 76d7c5e..d791064 100755
--- a/spice_codegen.py
+++ b/spice_codegen.py
@@ -176,7 +176,15 @@  writer.header.set_option("dest_file", dest_file)
 writer.set_option("source", os.path.basename(proto_file))
 
 if options.license == "LGPL":
-    license = """/*
+    if options.generate_dissector:
+        license = """/*
+ * Copyright (C) 2013 Red Hat, Inc.
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+"""
+    else:
+        license = """/*
   Copyright (C) 2013 Red Hat, Inc.
 
   This library is free software; you can redistribute it and/or
@@ -236,7 +244,7 @@  writer.writeln("/* this is a file autogenerated by spice_codegen.py */")
 writer.write(license)
 writer.header.writeln("/* this is a file autogenerated by spice_codegen.py */")
 writer.header.write(license)
-if not options.header and not options.generate_enums:
+if not (options.header or options.generate_enums or options.generate_dissector):
     writer.writeln("#ifdef HAVE_CONFIG_H")
     writer.writeln("#include <config.h>")
     writer.writeln("#endif")

Comments

> 
> Shorten the generated dissector, drop unused config.h inclusion and
> abbreviate the license blurb. No functional change, cosmetics only.
> 
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>

I'm fine with the config.h, the license I would retain the full version

Frediano

> ---
> For the updated output, see https://code.wireshark.org/review/29380
> I considered capitalizing names such as SPICE_VSC_MESSAGE_TYPE_FLUSHCOMPLETE
> (instead of SPICE_VSC_MESSAGE_TYPE_FlushComplete), but that breaks the public
> spice-protocol interface which has been there since at least 2010, so nope.
> ---
>  spice_codegen.py | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/spice_codegen.py b/spice_codegen.py
> index 76d7c5e..d791064 100755
> --- a/spice_codegen.py
> +++ b/spice_codegen.py
> @@ -176,7 +176,15 @@ writer.header.set_option("dest_file", dest_file)
>  writer.set_option("source", os.path.basename(proto_file))
>  
>  if options.license == "LGPL":
> -    license = """/*
> +    if options.generate_dissector:
> +        license = """/*
> + * Copyright (C) 2013 Red Hat, Inc.
> + * SPDX-License-Identifier: LGPL-2.1-or-later
> + */
> +
> +"""
> +    else:
> +        license = """/*
>    Copyright (C) 2013 Red Hat, Inc.
>  
>    This library is free software; you can redistribute it and/or
> @@ -236,7 +244,7 @@ writer.writeln("/* this is a file autogenerated by
> spice_codegen.py */")
>  writer.write(license)
>  writer.header.writeln("/* this is a file autogenerated by spice_codegen.py
>  */")
>  writer.header.write(license)
> -if not options.header and not options.generate_enums:
> +if not (options.header or options.generate_enums or
> options.generate_dissector):
>      writer.writeln("#ifdef HAVE_CONFIG_H")
>      writer.writeln("#include <config.h>")
>      writer.writeln("#endif")
On Fri, Sep 21, 2018 at 09:50:35AM -0400, Frediano Ziglio wrote:
> > 
> > Shorten the generated dissector, drop unused config.h inclusion and
> > abbreviate the license blurb. No functional change, cosmetics only.
> > 
> > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> 
> I'm fine with the config.h, the license I would retain the full version

Wireshark uses the shorter SPDX identifiers to reduce boilerplate, this
convention is also being adopted by other projects (such as the Linux
kernel). See https://spdx.org/ids

If Wireshark is the intended consumer for the generated output, then I
would recommend using the shorter SPDX output. Otherwise we would have
to manually modify the output which is not ideal.

Kind regards,
Peter

> Frediano
> 
> > ---
> > For the updated output, see https://code.wireshark.org/review/29380
> > I considered capitalizing names such as SPICE_VSC_MESSAGE_TYPE_FLUSHCOMPLETE
> > (instead of SPICE_VSC_MESSAGE_TYPE_FlushComplete), but that breaks the public
> > spice-protocol interface which has been there since at least 2010, so nope.
> > ---
> >  spice_codegen.py | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/spice_codegen.py b/spice_codegen.py
> > index 76d7c5e..d791064 100755
> > --- a/spice_codegen.py
> > +++ b/spice_codegen.py
> > @@ -176,7 +176,15 @@ writer.header.set_option("dest_file", dest_file)
> >  writer.set_option("source", os.path.basename(proto_file))
> >  
> >  if options.license == "LGPL":
> > -    license = """/*
> > +    if options.generate_dissector:
> > +        license = """/*
> > + * Copyright (C) 2013 Red Hat, Inc.
> > + * SPDX-License-Identifier: LGPL-2.1-or-later
> > + */
> > +
> > +"""
> > +    else:
> > +        license = """/*
> >    Copyright (C) 2013 Red Hat, Inc.
> >  
> >    This library is free software; you can redistribute it and/or
> > @@ -236,7 +244,7 @@ writer.writeln("/* this is a file autogenerated by
> > spice_codegen.py */")
> >  writer.write(license)
> >  writer.header.writeln("/* this is a file autogenerated by spice_codegen.py
> >  */")
> >  writer.header.write(license)
> > -if not options.header and not options.generate_enums:
> > +if not (options.header or options.generate_enums or
> > options.generate_dissector):
> >      writer.writeln("#ifdef HAVE_CONFIG_H")
> >      writer.writeln("#include <config.h>")
> >      writer.writeln("#endif")
> 
> On Fri, Sep 21, 2018 at 09:50:35AM -0400, Frediano Ziglio wrote:
> > > 
> > > Shorten the generated dissector, drop unused config.h inclusion and
> > > abbreviate the license blurb. No functional change, cosmetics only.
> > > 
> > > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> > 
> > I'm fine with the config.h, the license I would retain the full version
> 
> Wireshark uses the shorter SPDX identifiers to reduce boilerplate, this
> convention is also being adopted by other projects (such as the Linux
> kernel). See https://spdx.org/ids
> 
> If Wireshark is the intended consumer for the generated output, then I
> would recommend using the shorter SPDX output. Otherwise we would have
> to manually modify the output which is not ideal.
> 
> Kind regards,
> Peter
> 

Can you split the patch?

I don't think other projects care about a format or another, maybe
we can change the LGPL header entirely for all or add a "LGPL-SPDX"
license option.

Frediano

> 
> > > ---
> > > For the updated output, see https://code.wireshark.org/review/29380
> > > I considered capitalizing names such as
> > > SPICE_VSC_MESSAGE_TYPE_FLUSHCOMPLETE
> > > (instead of SPICE_VSC_MESSAGE_TYPE_FlushComplete), but that breaks the
> > > public
> > > spice-protocol interface which has been there since at least 2010, so
> > > nope.
> > > ---
> > >  spice_codegen.py | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/spice_codegen.py b/spice_codegen.py
> > > index 76d7c5e..d791064 100755
> > > --- a/spice_codegen.py
> > > +++ b/spice_codegen.py
> > > @@ -176,7 +176,15 @@ writer.header.set_option("dest_file", dest_file)
> > >  writer.set_option("source", os.path.basename(proto_file))
> > >  
> > >  if options.license == "LGPL":
> > > -    license = """/*
> > > +    if options.generate_dissector:
> > > +        license = """/*
> > > + * Copyright (C) 2013 Red Hat, Inc.
> > > + * SPDX-License-Identifier: LGPL-2.1-or-later
> > > + */
> > > +
> > > +"""
> > > +    else:
> > > +        license = """/*
> > >    Copyright (C) 2013 Red Hat, Inc.
> > >  
> > >    This library is free software; you can redistribute it and/or
> > > @@ -236,7 +244,7 @@ writer.writeln("/* this is a file autogenerated by
> > > spice_codegen.py */")
> > >  writer.write(license)
> > >  writer.header.writeln("/* this is a file autogenerated by
> > >  spice_codegen.py
> > >  */")
> > >  writer.header.write(license)
> > > -if not options.header and not options.generate_enums:
> > > +if not (options.header or options.generate_enums or
> > > options.generate_dissector):
> > >      writer.writeln("#ifdef HAVE_CONFIG_H")
> > >      writer.writeln("#include <config.h>")
> > >      writer.writeln("#endif")
>
On Fri, Sep 21, 2018 at 12:06:00PM -0400, Frediano Ziglio wrote:
> > 
> > On Fri, Sep 21, 2018 at 09:50:35AM -0400, Frediano Ziglio wrote:
> > > > 
> > > > Shorten the generated dissector, drop unused config.h inclusion and
> > > > abbreviate the license blurb. No functional change, cosmetics only.
> > > > 
> > > > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> > > 
> > > I'm fine with the config.h, the license I would retain the full version
> > 
> > Wireshark uses the shorter SPDX identifiers to reduce boilerplate, this
> > convention is also being adopted by other projects (such as the Linux
> > kernel). See https://spdx.org/ids
> > 
> > If Wireshark is the intended consumer for the generated output, then I
> > would recommend using the shorter SPDX output. Otherwise we would have
> > to manually modify the output which is not ideal.
> > 
> > Kind regards,
> > Peter
> > 
> 
> Can you split the patch?

Sure, I'll send a new one based on feedback of the following.

> I don't think other projects care about a format or another, maybe
> we can change the LGPL header entirely for all or add a "LGPL-SPDX"
> license option.

The --license option was added in 1d527e2 ("codegen: Allows to specify
license of generated files"), but I don't see an in-tree user. Would it
make sense to unconditionally dual-license the header?

And while doing that, replace the whole license blurb by the shorter:

    SPDX-License-Identifier: (LGPL-2.1-or-later OR MIT)

Kind regards,
Peter

> > > > ---
> > > > For the updated output, see https://code.wireshark.org/review/29380
> > > > I considered capitalizing names such as
> > > > SPICE_VSC_MESSAGE_TYPE_FLUSHCOMPLETE
> > > > (instead of SPICE_VSC_MESSAGE_TYPE_FlushComplete), but that breaks the
> > > > public
> > > > spice-protocol interface which has been there since at least 2010, so
> > > > nope.
> > > > ---
> > > >  spice_codegen.py | 12 ++++++++++--
> > > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/spice_codegen.py b/spice_codegen.py
> > > > index 76d7c5e..d791064 100755
> > > > --- a/spice_codegen.py
> > > > +++ b/spice_codegen.py
> > > > @@ -176,7 +176,15 @@ writer.header.set_option("dest_file", dest_file)
> > > >  writer.set_option("source", os.path.basename(proto_file))
> > > >  
> > > >  if options.license == "LGPL":
> > > > -    license = """/*
> > > > +    if options.generate_dissector:
> > > > +        license = """/*
> > > > + * Copyright (C) 2013 Red Hat, Inc.
> > > > + * SPDX-License-Identifier: LGPL-2.1-or-later
> > > > + */
> > > > +
> > > > +"""
> > > > +    else:
> > > > +        license = """/*
> > > >    Copyright (C) 2013 Red Hat, Inc.
> > > >  
> > > >    This library is free software; you can redistribute it and/or
> > > > @@ -236,7 +244,7 @@ writer.writeln("/* this is a file autogenerated by
> > > > spice_codegen.py */")
> > > >  writer.write(license)
> > > >  writer.header.writeln("/* this is a file autogenerated by
> > > >  spice_codegen.py
> > > >  */")
> > > >  writer.header.write(license)
> > > > -if not options.header and not options.generate_enums:
> > > > +if not (options.header or options.generate_enums or
> > > > options.generate_dissector):
> > > >      writer.writeln("#ifdef HAVE_CONFIG_H")
> > > >      writer.writeln("#include <config.h>")
> > > >      writer.writeln("#endif")