-
Notifications
You must be signed in to change notification settings - Fork 137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New attributes for license tag #347
Open
ralph-lange
wants to merge
10
commits into
ros-infrastructure:master
Choose a base branch
from
boschresearch:0149_new_license_attributes
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
b753414
Added attribute source-files in license tag.
ralph-lange ce3a38b
Added attribute copyright in license tag.
ralph-lange 0efefb3
Added note on the use of the new license tag attributes.
ralph-lange 7fab8c0
Removed newly proposed copyright attribute in license tag.
ralph-lange 2af4e80
Introduced type attribute for license tag.
ralph-lange 1bd1c77
Added example for use of license type attribute.
ralph-lange a9b3579
Added paragraph on limitation to one filename pattern.
ralph-lange 040c7eb
Added note on spaces in filename patterns.
ralph-lange 007d8b2
Added section on important design decisions.
ralph-lange 49c1b3a
Fixed typo in new design decisions section.
ralph-lange File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ Status: Final | |
Type: Standards Track | ||
Content-Type: text/x-rst | ||
Created: 11-Oct-2017 | ||
Post-History: 02-Jan-2018, 31-Aug-2020 | ||
Post-History: 02-Jan-2018, 31-Aug-2020, 14-Apr-2022 | ||
|
||
Outline | ||
======= | ||
|
@@ -346,8 +346,8 @@ Example | |
</description> | ||
<maintainer email="[email protected]">Someone</maintainer> | ||
|
||
<license>BSD</license> | ||
<license file="LICENSE">LGPL</license> | ||
<license file="LICENSE">BSD-3-Clause</license> | ||
<license source-files="include/my_package/linear_math/*">Zlib</license> | ||
ralph-lange marked this conversation as resolved.
Show resolved
Hide resolved
ralph-lange marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
<url type="website">http://wiki.ros.org/my_package</url> | ||
<url type="repository">http://www.github.com/my_org/my_package</url> | ||
|
@@ -465,32 +465,18 @@ Example | |
<license> (multiple, but at least one) | ||
-------------------------------------- | ||
|
||
Name of license for this package, e.g. BSD, GPL, LGPL. In order to | ||
assist machine readability, only include the license name in this tag. | ||
For multiple licenses multiple separate tags must be used. A package | ||
will have multiple licenses if different source files have different | ||
licenses. Every license occurring in the source files should have | ||
a corresponding ``<license>`` tag. For any explanatory text about | ||
licensing caveats, please use the ``<description>`` tag. | ||
|
||
Most common open-source licenses are described on the | ||
`OSI website <http://www.opensource.org/licenses/alphabetical>`_. | ||
|
||
Commonly used license strings: | ||
|
||
- Apache-2.0 | ||
- BSD | ||
- Boost Software License | ||
- GPLv2 | ||
- GPLv3 | ||
- LGPLv2.1 | ||
- LGPLv3 | ||
- MIT | ||
- Mozilla Public License Version 1.1 | ||
Name of license for this package or selected files of this package, | ||
e.g. BSD-3-Clause, GPL-3.0-or-later, Apache-2.0. In order to assist | ||
machine readability, only include the `SPDX license identifier | ||
<https://spdx.org/licenses/>`_ in this tag. In the rare case that | ||
a package (or selected source files of the package) are licensed under | ||
multiple alternative licenses, the identifiers can be combined by | ||
``or`` as described in Section 7.2 of the `Machine-readable | ||
debian/copyright file specification V1.0 | ||
<https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/>`_. | ||
|
||
Attributes | ||
'''''''''' | ||
|
||
ralph-lange marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.. raw:: html | ||
|
||
<font color="blue"> | ||
|
@@ -505,10 +491,40 @@ Attributes | |
|
||
"You must give any other recipients of the Work or Derivative Works a copy of this License" | ||
|
||
``source-files="FILENAME-PATTERN"`` *(optional)* | ||
|
||
A filename pattern using the simplified shell glob syntax specified in Section 6.9 of the `Machine-readable | ||
debian/copyright file specification V1.0 <https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/>`_ | ||
and relative to the ``package.xml`` file. | ||
|
||
The filename pattern specifies the source files this license information refers to. The value | ||
``source-files="*"`` refers to all source files of the package, including source files that are downloaded automatically | ||
during the build process - for example in the case of so-called *vendor packages*. If the attribute is not specified, | ||
the tag again refers to all source files of the package, including downloaded source files. | ||
|
||
If the filename patterns of multiple license tags match a particular file, the last tag applies to it - following | ||
the logic described in Section 6.9 of the `Machine-readable | ||
debian/copyright file specification V1.0 <https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/>`_. | ||
Consequently, more general tags should be given first. | ||
|
||
.. raw:: html | ||
|
||
</font> | ||
|
||
Notes | ||
''''' | ||
|
||
The license information given in the license tags has to be consistent | ||
with the information given in the license headers of the source files. | ||
This may be checked by suitable linting tools. | ||
|
||
Furthermore, by the license tags in the ``package.xml`` file and the | ||
copyright information obtained from the license headers of the source files | ||
(e.g., using ``licensecheck --copyright``) | ||
a copyright file according to the `Machine-readable debian/copyright file | ||
specification V1.0 <https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/>`_ | ||
for binary versions of this package can be created automatically. | ||
|
||
<url> (multiple) | ||
---------------- | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One suggestion I have here is to add a
type
attribute to the license tag. Thetype
attribute would describe what kind of license tag is being used, either anSPDX
identifier, or afreeform
identifier. If notype
attribute is specified, we would assumefreeform
.Thus you could have:
<license>BSD</license>
- a common license tag today, which can be assumed to be a "freeform" license tag (and thus nothing can mechanically be determined about it)<license type='freeform'>BSD</license>
- which is the same as above but more explicit<license type='spdx'>BSD-3-Clause</license>
- which is very explicitly using the SPDX identifier, and thus can be mechanically verifiedWith this in place, we could put in linters (for instance) that would complain when the "type" is SPDX, but the actual text doesn't match a known SPDX identifier. We could also have linters that would complain if any packages in the workspace don't use the "spdx" type (though we'd have to do a lot of work to enable that by default).
Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it not be simpler to require the use of SPDX identifiers?
Other efforts to standardise licensing information (like REUSE) have gone that way and it makes parsing and validation much more straightforward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, yes, we would require SPDX identifiers.
The issue is backwards compatibility. There are thousands of ROS (1 & 2) packages out there in the wild, and almost none of the package.xml files in them follow SPDX. Explicitly putting a type field in will make it easier for tools (and humans) to determine whether the field is expected to be SPDX or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of introducing a type attribute. There are many ROS 1 packages that simply specify
BSD
as license identifier, which is not enough information to infer the exact license.I would be a bit more optimistic for the linter and in particular for generating a debian/copyright file:
<license file="path_to_license.txt">BSD</license>
: This is enough information to generate a Debian copyright file. The generator (and the linter) should check whetherpath_to_license.txt
really contains a BSD license text.<license>BSD</license>
(or<license source-files="*">BSD</license>
) with a fileLICENSE
in the root of the repo: Again, this is enough information to generate a Debian copyright file in my opinion.<license>Some SPDX full name or SPDX identifier</license>
: Again enough information to generate a Debian copyright file (and to validate it against a potential LICENSE file in the package or repo root).<license file="custom_license_text.txt">MyCustomLicenseIdentifier</license>
: Also this would allow the generation of a debian/copyright file according to https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/. ("If there are licenses present in the package without a standard short name, an arbitrary short name may be assigned for these licenses. These arbitrary names are only guaranteed to be unique within a single copyright file.")Even the case that a debian/copyright file is generated where the license short name does not match the license text I do not consider dramatic: It simply repeats the inconsistent license names and texts from the source code and should be discovered by relevant tools applied to the debian/copyright file.
As a consequence, the
type="spdx"
attribute is not really necessary but its use would introduce a safety layer that prevents that a typo in an SPDX license identifier leads to the interpretation as a custom license identifier (i.e., as case 4).What we must prevent in any case is that the future debian/copyright file generator refines the license specification for which there is no evidence - which is the case for some ROS 1 packages that specify
<license>BSD</license>
without any further license headers orLICENSE
file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but don't we have the same problem with the attributes and elements proposed here? All the packages in existence today would also not have those.
if the package format version would be increased, tools could just assume "v4 manifest->spdx".
Anything below that does not (have to) conform to that.
If the concern is "there may be licenses in use for which there are no SPDX identifiers", then SPDX has a standard approach for that in place IIRC.
Edit: absence of the type attribute could of course also be used, but it seems much cleaner to me to be able to say "format 4 manifests must use SPDX identifiers" than "if the
license
tag doesn't have thetype
attribute then it's OK to not use SPDX identifiers".There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for the
type
attribute.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for completeness: The SPDX Specification also describes a solution for naming custom licenses (cf. https://spdx.dev/spdx-specification-21-web-version/#h.1v1yuxt), but I still prefer the explicit
type
attribute to distinguish between licenses from the SPDX License List and custom ones.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type
attribution with default looks good to me.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Introduced the
type
attribute as proposed by @clalancette. (Sorry, I should have updated the PR before yesterday's discussion in the TSC meeting.)Is the description of the new attribute sufficient or should I give explicit explanations of the two possible values
freeform
andspdx
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@clalancette, are you fine if I mark the discussion on the type attribute as resolved, cf. my last question on the explanation of the attribute in the REP?