Merge pull request #623 from ferrocene/pa-spdx-concluded-license

Populate the `LicenseConcluded` field in the SPDX report

CHANGELOG.md

@@ -48,6 +48,8 @@ The versions follow [semantic versioning](https://semver.org).
   - Clang format (`.clang-format`) (#632)
 - Added loglevel argument to pytest and skip one test if loglevel is too high
   (#645).
+- `--add-license-concluded`, `--creator-person`, and `--creator-organization`
+  added to `reuse spdx`. (#623)
 
 ### Changed
 

src/reuse/lint.py

@@ -326,7 +326,7 @@ def lint_summary(report: ProjectReport, out=sys.stdout) -> None:
 def add_arguments(parser):
     """Add arguments to parser."""
     parser.add_argument(
-        "-q", "--quiet", action="store_true", help="Prevents output"
+        "-q", "--quiet", action="store_true", help=_("prevents output")
     )
 
 

src/reuse/report.py

@@ -1,5 +1,6 @@
 # SPDX-FileCopyrightText: 2017 Free Software Foundation Europe e.V. <https://fsfe.org>
 # SPDX-FileCopyrightText: 2022 Florian Snow <florian@familysnow.net>
+# SPDX-FileCopyrightText: 2022 Pietro Albini <pietro.albini@ferrous-systems.com>
 #
 # SPDX-License-Identifier: GPL-3.0-or-later
 
@@ -27,17 +28,21 @@
 class _MultiprocessingContainer:
     """Container that remembers some data in order to generate a FileReport."""
 
-    def __init__(self, project, do_checksum):
+    def __init__(self, project, do_checksum, add_license_concluded):
         self.project = project
         self.do_checksum = do_checksum
+        self.add_license_concluded = add_license_concluded
 
     def __call__(self, file_):
         # pylint: disable=broad-except
         try:
             return _MultiprocessingResult(
                 file_,
                 FileReport.generate(
-                    self.project, file_, do_checksum=self.do_checksum
+                    self.project,
+                    file_,
+                    do_checksum=self.do_checksum,
+                    add_license_concluded=self.add_license_concluded,
                 ),
                 None,
             )
@@ -98,7 +103,11 @@ def to_dict(self):
             "file_reports": [report.to_dict() for report in self.file_reports],
         }
 
-    def bill_of_materials(self) -> str:
+    def bill_of_materials(
+        self,
+        creator_person: Optional[str] = None,
+        creator_organization: Optional[str] = None,
+    ) -> str:
         """Generate a bill of materials from the project.
 
         See https://spdx.org/specifications.
@@ -118,9 +127,10 @@ def bill_of_materials(self) -> str:
         )
 
         # Author
-        # TODO: Fix Person and Organization
-        out.write("Creator: Person: Anonymous ()\n")
-        out.write("Creator: Organization: Anonymous ()\n")
+        out.write(f"Creator: Person: {format_creator(creator_person)}\n")
+        out.write(
+            f"Creator: Organization: {format_creator(creator_organization)}\n"
+        )
         out.write(f"Creator: Tool: reuse-{__version__}\n")
 
         now = datetime.datetime.utcnow()
@@ -145,9 +155,9 @@ def bill_of_materials(self) -> str:
             out.write(f"FileName: {report.spdxfile.name}\n")
             out.write(f"SPDXID: {report.spdxfile.spdx_id}\n")
             out.write(f"FileChecksum: SHA1: {report.spdxfile.chk_sum}\n")
-            # IMPORTANT: Make no assertion about concluded license. This tool
-            # cannot, with full certainty, determine the license of a file.
-            out.write("LicenseConcluded: NOASSERTION\n")
+            out.write(
+                f"LicenseConcluded: {report.spdxfile.license_concluded}\n"
+            )
 
             for lic in sorted(report.spdxfile.licenses_in_file):
                 out.write(f"LicenseInfoInFile: {lic}\n")
@@ -177,6 +187,7 @@ def generate(
         project: Project,
         do_checksum: bool = True,
         multiprocessing: bool = cpu_count() > 1,
+        add_license_concluded: bool = False,
     ) -> "ProjectReport":
         """Generate a ProjectReport from a Project."""
         project_report = cls(do_checksum=do_checksum)
@@ -186,7 +197,9 @@ def generate(
             project.licenses_without_extension
         )
 
-        container = _MultiprocessingContainer(project, do_checksum)
+        container = _MultiprocessingContainer(
+            project, do_checksum, add_license_concluded
+        )
 
         if multiprocessing:
             with mp.Pool() as pool:
@@ -306,6 +319,7 @@ def __init__(self, name, spdx_id=None, chk_sum=None):
         self.spdx_id: str = spdx_id
         self.chk_sum: str = chk_sum
         self.licenses_in_file: List[str] = []
+        self.license_concluded: str = None
         self.copyright: str = None
 
 
@@ -337,7 +351,11 @@ def to_dict(self):
 
     @classmethod
     def generate(
-        cls, project: Project, path: PathLike, do_checksum: bool = True
+        cls,
+        project: Project,
+        path: PathLike,
+        do_checksum: bool = True,
+        add_license_concluded: bool = False,
     ) -> "FileReport":
         """Generate a FileReport from a path in a Project."""
         path = Path(path)
@@ -378,6 +396,26 @@ def generate(
                 # Add license to report.
                 report.spdxfile.licenses_in_file.append(identifier)
 
+        if not add_license_concluded:
+            report.spdxfile.license_concluded = "NOASSERTION"
+        elif not spdx_info.spdx_expressions:
+            report.spdxfile.license_concluded = "NONE"
+        else:
+            # Merge all the license expressions together, wrapping them in
+            # parentheses to make sure an expression doesn't spill into another
+            # one. The extra parentheses will be removed by the roundtrip
+            # through parse() -> simplify() -> render().
+            report.spdxfile.license_concluded = (
+                _LICENSING.parse(
+                    " AND ".join(
+                        f"({expression})"
+                        for expression in spdx_info.spdx_expressions
+                    ),
+                )
+                .simplify()
+                .render()
+            )
+
         # Copyright text
         report.spdxfile.copyright = "\n".join(sorted(spdx_info.copyright_lines))
 
@@ -387,3 +425,13 @@ def __hash__(self):
         if self.spdxfile.chk_sum is not None:
             return hash(self.spdxfile.name + self.spdxfile.chk_sum)
         return super().__hash__(self)
+
+
+def format_creator(creator: str) -> str:
+    """Render the creator field based on the provided flag"""
+    if creator is None:
+        return "Anonymous ()"
+    if "(" in creator and creator.endswith(")"):
+        # The creator field already contains an email address
+        return creator
+    return creator + " ()"

src/reuse/spdx.py

@@ -1,4 +1,5 @@
 # SPDX-FileCopyrightText: 2017 Free Software Foundation Europe e.V. <https://fsfe.org>
+# SPDX-FileCopyrightText: 2022 Pietro Albini <pietro.albini@ferrous-systems.com>
 #
 # SPDX-License-Identifier: GPL-3.0-or-later
 
@@ -22,10 +23,43 @@ def add_arguments(parser) -> None:
     parser.add_argument(
         "--output", "-o", dest="file", action="store", type=PathType("w")
     )
+    parser.add_argument(
+        "--add-license-concluded",
+        action="store_true",
+        help=_(
+            "populate the LicenseConcluded field; note that reuse cannot "
+            "guarantee the field is accurate"
+        ),
+    )
+    parser.add_argument(
+        "--creator-person",
+        metavar="NAME",
+        help=_("name of the person signing off on the SPDX report"),
+    )
+    parser.add_argument(
+        "--creator-organization",
+        metavar="NAME",
+        help=_("name of the organization signing off on the SPDX report"),
+    )
 
 
 def run(args, project: Project, out=sys.stdout) -> int:
     """Print the project's bill of materials."""
+    # The SPDX spec mandates that a creator must be specified when a license
+    # conclusion is made, so here we enforce that. More context:
+    # https://github.com/fsfe/reuse-tool/issues/586#issuecomment-1310425706
+    if (
+        args.add_license_concluded
+        and args.creator_person is None
+        and args.creator_organization is None
+    ):
+        args.parser.error(
+            _(
+                "error: --creator-person=NAME or --creator-organization=NAME"
+                " required when --add-license-concluded is provided"
+            ),
+        )
+
     with contextlib.ExitStack() as stack:
         if args.file:
             out = stack.enter_context(args.file.open("w", encoding="utf-8"))
@@ -43,9 +77,16 @@ def run(args, project: Project, out=sys.stdout) -> int:
                 )
 
         report = ProjectReport.generate(
-            project, multiprocessing=not args.no_multiprocessing
+            project,
+            multiprocessing=not args.no_multiprocessing,
+            add_license_concluded=args.add_license_concluded,
         )
 
-        out.write(report.bill_of_materials())
+        out.write(
+            report.bill_of_materials(
+                creator_person=args.creator_person,
+                creator_organization=args.creator_organization,
+            )
+        )
 
     return 0

tests/conftest.py

@@ -91,6 +91,11 @@ def multiprocessing(request, monkeypatch) -> bool:
     yield request.param
 
 
+@pytest.fixture(params=[True, False])
+def add_license_concluded(request) -> bool:
+    yield request
+
+
 @pytest.fixture()
 def empty_directory(tmpdir_factory) -> Path:
     """Create a temporary empty directory."""
@@ -124,6 +129,13 @@ def fake_repository(tmpdir_factory) -> Path:
         "SPDX-License-Identifier: LicenseRef-custom",
         encoding="utf-8",
     )
+    (directory / "src/multiple_licenses.rs").write_text(
+        "SPDX-FileCopyrightText: 2022 Jane Doe\n"
+        "SPDX-License-Identifier: GPL-3.0-or-later\n"
+        "SPDX-License-Identifier: Apache-2.0 OR CC0-1.0"
+        " WITH Autoconf-exception-3.0\n",
+        encoding="utf-8",
+    )
 
     os.chdir(directory)
     return directory
@@ -162,11 +174,13 @@ def git_repository(fake_repository: Path, git_exe: Optional[str]) -> Path:
     os.chdir(fake_repository)
     _repo_contents(fake_repository)
 
+    # TODO: To speed this up, maybe directly write to '.gitconfig' instead.
     subprocess.run([git_exe, "init", str(fake_repository)], check=True)
     subprocess.run([git_exe, "config", "user.name", "Example"], check=True)
     subprocess.run(
         [git_exe, "config", "user.email", "example@example.com"], check=True
     )
+    subprocess.run([git_exe, "config", "commit.gpgSign", "false"], check=True)
 
     subprocess.run([git_exe, "add", str(fake_repository)], check=True)
     subprocess.run(

tests/resources/fake_repository/LICENSES/Apache-2.0.txt

@@ -0,0 +1 @@
+License text.

tests/resources/fake_repository/src/multiple_licenses.rs

@@ -0,0 +1 @@
+// This file is overridden by the fake_repository fixture.

tests/test_main.py

@@ -2,6 +2,7 @@
 # SPDX-FileCopyrightText: 2019 Stefan Bakker <s.bakker777@gmail.com>
 # SPDX-FileCopyrightText: © 2020 Liferay, Inc. <https://liferay.com>
 # SPDX-FileCopyrightText: 2022 Florian Snow <florian@familysnow.net>
+# SPDX-FileCopyrightText: 2022 Pietro Albini <pietro.albini@ferrous-systems.com>
 #
 # SPDX-License-Identifier: GPL-3.0-or-later
 
@@ -277,10 +278,66 @@ def test_spdx(fake_repository, stringio):
     """Compile to an SPDX document."""
     os.chdir(str(fake_repository))
     result = main(["spdx"], out=stringio)
+    output = stringio.getvalue()
+
+    # Ensure no LicenseConcluded is included without the flag
+    assert "\nLicenseConcluded: NOASSERTION\n" in output
+    assert "\nLicenseConcluded: GPL-3.0-or-later\n" not in output
+    assert "\nCreator: Person: Anonymous ()\n" in output
+    assert "\nCreator: Organization: Anonymous ()\n" in output
 
     # TODO: This test is rubbish.
     assert result == 0
-    assert stringio.getvalue()
+    assert output
+
+
+def test_spdx_creator_info(fake_repository, stringio):
+    """Ensure the --creator-* flags are properly formatted"""
+    os.chdir(str(fake_repository))
+    result = main(
+        [
+            "spdx",
+            "--creator-person=Jane Doe (jane.doe@example.org)",
+            "--creator-organization=FSFE",
+        ],
+        out=stringio,
+    )
+    output = stringio.getvalue()
+
+    assert result == 0
+    assert "\nCreator: Person: Jane Doe (jane.doe@example.org)\n" in output
+    assert "\nCreator: Organization: FSFE ()\n" in output
+
+
+def test_spdx_add_license_concluded(fake_repository, stringio):
+    """Compile to an SPDX document with the LicenseConcluded field."""
+    os.chdir(str(fake_repository))
+    result = main(
+        [
+            "spdx",
+            "--add-license-concluded",
+            "--creator-person=Jane Doe",
+            "--creator-organization=FSFE",
+        ],
+        out=stringio,
+    )
+    output = stringio.getvalue()
+
+    # Ensure no LicenseConcluded is included without the flag
+    assert result == 0
+    assert "\nLicenseConcluded: NOASSERTION\n" not in output
+    assert "\nLicenseConcluded: GPL-3.0-or-later\n" in output
+    assert "\nCreator: Person: Jane Doe ()\n" in output
+    assert "\nCreator: Organization: FSFE ()\n" in output
+
+
+def test_spdx_add_license_concluded_without_creator_info(
+    fake_repository, stringio
+):
+    """Adding LicenseConcluded should require creator information"""
+    os.chdir(str(fake_repository))
+    with pytest.raises(SystemExit):
+        main(["spdx", "--add-license-concluded"], out=stringio)
 
 
 def test_spdx_no_multiprocessing(fake_repository, stringio, multiprocessing):