Re: [dpdk-dev] [PATCH v3 1/4] Introducing SPDX License Identifiers

[Thomas Monjalon <thomas@monjalon.net>]

Hi Hemant,

Some comments below

08/12/2017 08:41, Hemant Agrawal:
> --- /dev/null
> +++ b/Licenses/Exceptions.txt

Please use lowercase for file and directory.
By the way, the text is referring to exceptions.txt.

> @@ -0,0 +1,12 @@
> +This file will record any exceptions in DPDK Project with respect to DPDK
> +IP License policy as defined in DPDK Charter available at:
> +
> +http://dpdk.org/about/charter#ip

This link might be indented.

I think we should make clear that
	- BSD-3-Clause
	- GPL-2.0
	- dual BSD-3-Clause/GPL-2.0
	- dual BSD-3-Clause/LGPL-2.1
are not exceptions.

> +----------------------------------------------------------------------------------------------
> +License Name                 SPDX Identifier     TB Approval Date  GB Approval Date  File name
> +----------------------------------------------------------------------------------------------

The table is large, and file names will be long.
Can we remove "License Name" as it is redundant with SPDX id?

> --- /dev/null
> +++ b/Licenses/README

Good idea to add a README here.

> @@ -0,0 +1,82 @@
> +The DPDK uses the Open Source BSD-3-Clause license for the core libraries and
> +drivers. The kernel components are naturally GPLv2 licensed.

You should use SPDX GPL-2.0

> +Including big blocks of License headers in all files blows up the
> +source code with mostly redundant information.  An additional problem
> +is that even the same licenses are referred to by a number of
> +slightly varying text blocks (full, abbreviated, different
> +indentation, line wrapping and/or white space, with obsolete address
> +information, ...) which makes validation and automatic processing a nightmare.
> +
> +To make this easier, DPDK is adpoting the use of a single line reference to

Please do not use this tense in the README.
We could say "DPDK uses" instead of "DPDK is adpoting the use".

> +Unique License Identifiers in source files as defined by the Linux Foundation's
> +SPDX project [1].

My preference is to insert URLs inline to make reading flow easier.

> +Adding license information in this fashion, rather than adding full license
> +text, can be more efficient for developers; decreases errors; and improves
> +automated detection of licenses. The current set of valid, predefined SPDX
> +identifiers is set forth on the SPDX License List[2]
> +at https://spdx.org/licenses/.

Here you are mixing inline and reference :)

> +For example, to label a file as subject to the BSD-3-Clause license,
> +the following text would be used:
> +
> +Copyright (C) [YEAR] NAME-OF-COPYRIGHT-HOLDER

I think (C) is useless.
About the YEAR, we should explicit what it is.
I think it is only the first year, and we do not need to update
the last year of update.
We should also explicit why it is there and why it is not required
to add more copyrights.
The copyright is required to express who is allowed to declare the
license of the code.
It is a common practice to add a Copyright line when doing a big update.
I think it is fair, but for small changes, it is really not required
as we implicitly comply with the current copyright holder and license.

> +SPDX-License-Identifier:        BSD-3-Clause

Why adding these spaces in the middle of the line?

[...]
> +Note: DPDK currently adhere to it's IP policies[3]. Any exception to this shall

This sentence is strange.
To me, it is obvious that DPDK adheres to its policies.
Suggested rewording:
Any exception to the DPDK IP policies shall...

> +be approved by DPDK tech board and DPDK Governing Board. Steps for any exception
> +approval:

[...]
> +Projects like U-boot have been been using SPDX License Idenfiers successfully
> +[2]. They have been referered in implementing SPDX based guidelines in DPDK.

I think you can remove this paragraph from the README.

[...]
> +DPDK project supported licenses are:
> +
> +Full name				        SPDX Identifier	OSI Approved	File name		URI
> +=======================================================================================================================================

This table is large.
I suggest to format it as a list:

SPDX identifier
	full name
	file name
	URL

> +GNU General Public License v2.0 only		GPL-2.0		Y		gpl-2.0.txt		http://spdx.org/licenses/GPL-2.0.html#licenseText
> +GNU Lesser General Public License v2.1 or later	LGPL-2.1+	Y		lgpl-2.1.txt		http://spdx.org/licenses/LGPL-2.1.html#licenseText

Later than LGPL-2.1 is not specified in the charter.
Better to remove the "+".

> +BSD 3-clause "New" or "Revised" License		BSD-3-Clause	Y		bsd-3-clause.txt	http://spdx.org/licenses/BSD-3-Clause#licenseText

[...]
> --- a/README
> +++ b/README
> @@ -1,8 +1,9 @@
>  DPDK is a set of libraries and drivers for fast packet processing.
>  It supports many processor architectures and both FreeBSD and Linux.
>  
> -The DPDK uses the Open Source BSD license for the core libraries and
> -drivers. The kernel components are GPLv2 licensed.
> +The DPDK uses the Open Source BSD-3-Clause license for the core libraries
> +and drivers. The kernel components are GPLv2 licensed. DPDK usages

s/GPLv2/GPL-2.0/
s/usages/uses/

> +SPDX Licenese identifiers instead of full license text in source files.

s/Licenese/license/

I think it is not needed to mention SPDX in this README.

[...]
> --- a/doc/guides/contributing/patches.rst
> +++ b/doc/guides/contributing/patches.rst
> @@ -32,6 +32,42 @@ It is also worth registering for the DPDK `Patchwork <http://dpdk.org/dev/patchw
>  The development process requires some familiarity with the ``git`` version control system.
>  Refer to the `Pro Git Book <http://www.git-scm.com/book/>`_ for further information.
>  
> +Source License
> +--------------
> +
> +The DPDK uses the Open Source BSD-3-Clause license for the core libraries and
> +drivers. The kernel components are GPLv2 licensed. DPDK usaes single line

s/GPLv2/GPL-2.0/
s/usaes/uses/

> +reference to Unique License Identifiers in source files as defined by the Linux
> +Foundation's `SPDX project <http://spdx.org/>`_.
> +
> +For example, to label a file as subject to the BSD-3-Clause license,
> +the following text would be used:
> +
> +Copyright (C) [YEAR] NAME-OF-COPYRIGHT-HOLDER
> +``SPDX-License-Identifier:        BSD-3-Clause``
> +
> +To label a file as dual-licensed with BSD-3-Clause and GPL-2.0 (e.g., for code
> +that is shared between the kernel and userspace), the following text would be
> +used:
> +
> +Copyright (C) [YEAR] NAME-OF-COPYRIGHT-HOLDER
> +``SPDX-License-Identifier:        BSD-3-Clause OR GPL-2.0``
> +
> +To label a file as dual-licensed with BSD-3-Clause and LGPL-2.1 (e.g., for code
> +that is shared between the kernel and userspace), the following text would be
> +used:
> +
> +Copyright (C) [YEAR] NAME-OF-COPYRIGHT-HOLDER
> +``SPDX-License-Identifier:        BSD-3-Clause OR LGPL-2.1``
> +
> +To label a file as GPL-2.0 (e.g., for code that runs in the kernel), the
> +following text would be used:
> +
> +Copyright (C) [YEAR] NAME-OF-COPYRIGHT-HOLDER
> +``SPDX-License-Identifier:        GPL-2.0``
> +
> +Any new file contributions in DPDK shall adhere to the above scheme.
> +It is also being recommended to replace the existing license.

No need to repeat everything in the guide.
You can refer to licenses/README.

+Cc John for doc review