diff --git a/CHANGELOG.md b/CHANGELOG.md index 95b197f31..edeff6340 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,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 diff --git a/src/reuse/lint.py b/src/reuse/lint.py index 083b6be74..8ff9657b1 100644 --- a/src/reuse/lint.py +++ b/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") ) diff --git a/src/reuse/report.py b/src/reuse/report.py index 458316d76..8426d306c 100644 --- a/src/reuse/report.py +++ b/src/reuse/report.py @@ -1,5 +1,6 @@ # SPDX-FileCopyrightText: 2017 Free Software Foundation Europe e.V. # SPDX-FileCopyrightText: 2022 Florian Snow +# SPDX-FileCopyrightText: 2022 Pietro Albini # # SPDX-License-Identifier: GPL-3.0-or-later @@ -27,9 +28,10 @@ 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 @@ -37,7 +39,10 @@ def __call__(self, file_): 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 + " ()" diff --git a/src/reuse/spdx.py b/src/reuse/spdx.py index 6a512ae44..e94bddc21 100644 --- a/src/reuse/spdx.py +++ b/src/reuse/spdx.py @@ -1,4 +1,5 @@ # SPDX-FileCopyrightText: 2017 Free Software Foundation Europe e.V. +# SPDX-FileCopyrightText: 2022 Pietro Albini # # 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 diff --git a/tests/conftest.py b/tests/conftest.py index 138dde820..58934a544 100644 --- a/tests/conftest.py +++ b/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( diff --git a/tests/resources/fake_repository/LICENSES/Apache-2.0.txt b/tests/resources/fake_repository/LICENSES/Apache-2.0.txt new file mode 100644 index 000000000..78b0bb56e --- /dev/null +++ b/tests/resources/fake_repository/LICENSES/Apache-2.0.txt @@ -0,0 +1 @@ +License text. diff --git a/tests/resources/fake_repository/src/multiple_licenses.rs b/tests/resources/fake_repository/src/multiple_licenses.rs new file mode 100644 index 000000000..9768b946c --- /dev/null +++ b/tests/resources/fake_repository/src/multiple_licenses.rs @@ -0,0 +1 @@ +// This file is overridden by the fake_repository fixture. diff --git a/tests/test_main.py b/tests/test_main.py index d789c37ac..519491594 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -2,6 +2,7 @@ # SPDX-FileCopyrightText: 2019 Stefan Bakker # SPDX-FileCopyrightText: © 2020 Liferay, Inc. # SPDX-FileCopyrightText: 2022 Florian Snow +# SPDX-FileCopyrightText: 2022 Pietro Albini # # 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): diff --git a/tests/test_report.py b/tests/test_report.py index 7ae17ff3d..1ff0da58c 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -1,5 +1,6 @@ # SPDX-FileCopyrightText: 2017 Free Software Foundation Europe e.V. # SPDX-FileCopyrightText: 2022 Florian Snow +# SPDX-FileCopyrightText: 2022 Pietro Albini # # SPDX-License-Identifier: GPL-3.0-or-later @@ -28,58 +29,101 @@ # REUSE-IgnoreStart -def test_generate_file_report_file_simple(fake_repository): +def test_generate_file_report_file_simple( + fake_repository, add_license_concluded +): """An extremely simple generate test, just to see if the function doesn't crash. """ project = Project(fake_repository) - result = FileReport.generate(project, "src/source_code.py") + result = FileReport.generate( + project, + "src/source_code.py", + add_license_concluded=add_license_concluded, + ) + assert result.spdxfile.licenses_in_file == ["GPL-3.0-or-later"] + assert ( + result.spdxfile.license_concluded == "GPL-3.0-or-later" + if add_license_concluded + else "NOASSERTION" + ) assert result.spdxfile.copyright == "SPDX-FileCopyrightText: 2017 Jane Doe" assert not result.bad_licenses assert not result.missing_licenses -def test_generate_file_report_file_from_different_cwd(fake_repository): +def test_generate_file_report_file_from_different_cwd( + fake_repository, add_license_concluded +): """Another simple generate test, but from a different CWD.""" os.chdir("/") project = Project(fake_repository) result = FileReport.generate( - project, fake_repository / "src/source_code.py" + project, + fake_repository / "src/source_code.py", + add_license_concluded=add_license_concluded, ) assert result.spdxfile.licenses_in_file == ["GPL-3.0-or-later"] + assert ( + result.spdxfile.license_concluded == "GPL-3.0-or-later" + if add_license_concluded + else "NOASSERTION" + ) assert result.spdxfile.copyright == "SPDX-FileCopyrightText: 2017 Jane Doe" assert not result.bad_licenses assert not result.missing_licenses -def test_generate_file_report_file_missing_license(fake_repository): +def test_generate_file_report_file_missing_license( + fake_repository, add_license_concluded +): """Simple generate test with a missing license.""" (fake_repository / "foo.py").write_text( "SPDX-License-Identifier: BSD-3-Clause" ) project = Project(fake_repository) - result = FileReport.generate(project, "foo.py") + result = FileReport.generate( + project, "foo.py", add_license_concluded=add_license_concluded + ) assert result.spdxfile.copyright == "" + assert result.spdxfile.licenses_in_file == ["BSD-3-Clause"] + assert ( + result.spdxfile.license_concluded == "BSD-3-Clause" + if add_license_concluded + else "NOASSERTION" + ) assert result.missing_licenses == {"BSD-3-Clause"} assert not result.bad_licenses -def test_generate_file_report_file_bad_license(fake_repository): +def test_generate_file_report_file_bad_license( + fake_repository, add_license_concluded +): """Simple generate test with a bad license.""" (fake_repository / "foo.py").write_text( "SPDX-License-Identifier: fakelicense" ) project = Project(fake_repository) - result = FileReport.generate(project, "foo.py") + result = FileReport.generate( + project, "foo.py", add_license_concluded=add_license_concluded + ) assert result.spdxfile.copyright == "" + assert result.spdxfile.licenses_in_file == ["fakelicense"] + assert ( + result.spdxfile.license_concluded == "fakelicense" + if add_license_concluded + else "NOASSERTION" + ) assert result.bad_licenses == {"fakelicense"} assert result.missing_licenses == {"fakelicense"} -def test_generate_file_report_license_contains_plus(fake_repository): +def test_generate_file_report_license_contains_plus( + fake_repository, add_license_concluded +): """Given a license expression akin to Apache-1.0+, LICENSES/Apache-1.0.txt should be an appropriate license file. """ @@ -88,26 +132,93 @@ def test_generate_file_report_license_contains_plus(fake_repository): ) (fake_repository / "LICENSES/Apache-1.0.txt").touch() project = Project(fake_repository) - result = FileReport.generate(project, "foo.py") + result = FileReport.generate( + project, "foo.py", add_license_concluded=add_license_concluded + ) assert result.spdxfile.copyright == "" + assert result.spdxfile.licenses_in_file == ["Apache-1.0+"] + assert ( + result.spdxfile.license_concluded == "Apache-1.0+" + if add_license_concluded + else "NOASSERTION" + ) assert not result.bad_licenses assert not result.missing_licenses -def test_generate_file_report_exception(fake_repository): +def test_generate_file_report_exception(fake_repository, add_license_concluded): """Simple generate test to test if the exception is detected.""" project = Project(fake_repository) - result = FileReport.generate(project, "src/exception.py") + result = FileReport.generate( + project, "src/exception.py", add_license_concluded=add_license_concluded + ) + assert set(result.spdxfile.licenses_in_file) == { "GPL-3.0-or-later", "Autoconf-exception-3.0", } + assert ( + result.spdxfile.license_concluded + == "GPL-3.0-or-later WITH Autoconf-exception-3.0" + if add_license_concluded + else "NOASSERTION" + ) assert result.spdxfile.copyright == "SPDX-FileCopyrightText: 2017 Jane Doe" assert not result.bad_licenses assert not result.missing_licenses +def test_generate_file_report_no_licenses( + fake_repository, add_license_concluded +): + """Test behavior when no license information is present in the file""" + (fake_repository / "foo.py").write_text("") + project = Project(fake_repository) + result = FileReport.generate( + project, "foo.py", add_license_concluded=add_license_concluded + ) + + assert result.spdxfile.copyright == "" + assert not result.spdxfile.licenses_in_file + assert ( + result.spdxfile.license_concluded == "NONE" + if add_license_concluded + else "NOASSERTION" + ) + assert not result.bad_licenses + assert not result.missing_licenses + + +def test_generate_file_report_multiple_licenses( + fake_repository, add_license_concluded +): + """Test that all licenses are included in LicenseConcluded""" + project = Project(fake_repository) + result = FileReport.generate( + project, + "src/multiple_licenses.rs", + add_license_concluded=add_license_concluded, + ) + + assert result.spdxfile.copyright == "SPDX-FileCopyrightText: 2022 Jane Doe" + assert set(result.spdxfile.licenses_in_file) == { + "GPL-3.0-or-later", + "Apache-2.0", + "CC0-1.0", + "Autoconf-exception-3.0", + } + assert ( + result.spdxfile.license_concluded + == "GPL-3.0-or-later AND (Apache-2.0 OR CC0-1.0" + " WITH Autoconf-exception-3.0)" + if add_license_concluded + else "NOASSERTION" + ) + assert not result.bad_licenses + assert not result.missing_licenses + + def test_generate_project_report_simple(fake_repository, multiprocessing): """Simple generate test, just to see if it sort of works.""" project = Project(fake_repository)