From 24b5e2fbc0023d20d2e8e0eaae64a42a070a36fe Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Mon, 27 May 2024 13:47:18 +0800 Subject: [PATCH] Minor tweaks to warning and error messages. --- changes/1812.feature.rst | 2 +- changes/1812.removal.rst | 1 + src/briefcase/config.py | 8 +- src/briefcase/platforms/linux/system.py | 23 +++-- .../convert/test_migrate_necessary_files.py | 28 +++++++ tests/config/test_parse_config.py | 83 ++++++++++++++----- tests/platforms/linux/system/test_build.py | 13 +-- 7 files changed, 120 insertions(+), 38 deletions(-) create mode 100644 changes/1812.removal.rst diff --git a/changes/1812.feature.rst b/changes/1812.feature.rst index 0950eb030..8b08b18a3 100644 --- a/changes/1812.feature.rst +++ b/changes/1812.feature.rst @@ -1 +1 @@ -The name of the LICENSE file can now be specified in the PEP621-section of the pyproject.toml file +The name of the license file can now be specified using a PEP 621-compliant format for the ``license`` setting. diff --git a/changes/1812.removal.rst b/changes/1812.removal.rst new file mode 100644 index 000000000..b10c6b780 --- /dev/null +++ b/changes/1812.removal.rst @@ -0,0 +1 @@ +The format for the ``license`` field has been converted to PEP 621 format. Existing projects that specify ``license`` as a string should update their configurations to point at the generated license file using ``license.file = "LICENSE"``. diff --git a/src/briefcase/config.py b/src/briefcase/config.py index e9e08997a..179f23505 100644 --- a/src/briefcase/config.py +++ b/src/briefcase/config.py @@ -560,12 +560,12 @@ def parse_config(config_file, platform, output_format, logger): if isinstance(config.get("license"), str): config["license"] = {"file": "LICENSE"} old_license_format = True + if old_license_format: logger.warning( - "The license was specified using a string, however Briefcase prefers a PEP621 " - "compatible dictionary on the form {'file': '{filename}'} or {'text': '{license_text}'}.\n" - "Therefore, when given the license as a string, Briefcase ignores the license string and " - "defaults to {'file': 'LICENSE'}." + "Your app configuration has a `license` field that is specified as a string. " + "Briefcase now uses PEP 621 format for license definitions. To silence this " + 'warning, replace the `license` declaration with `license.file = "LICENSE".' ) return global_config, app_configs diff --git a/src/briefcase/platforms/linux/system.py b/src/briefcase/platforms/linux/system.py index 3deba3c0e..8f236905e 100644 --- a/src/briefcase/platforms/linux/system.py +++ b/src/briefcase/platforms/linux/system.py @@ -711,9 +711,11 @@ def build_app(self, app: AppConfig, **kwargs): else: raise BriefcaseCommandError( f"""\ -You specified that the license file is {license_file}. However, this file does not exist. +Your `pyproject.toml` specifies a license file of {str(license_file.relative_to(self.base_path))!r}. +However, this file does not exist. + +Ensure you have correctly spelled the filename in your `license.file` setting. -Make sure you spelled the name of the license file correctly in `pyproject.toml`. """ ) elif license_text := app.license.get("text"): @@ -721,18 +723,19 @@ def build_app(self, app: AppConfig, **kwargs): if len(license_text.splitlines()) <= 1: self.logger.warning( """ -You specified the license using a text string. However, this string is only one line. - -Make sure that the license text is a full license (or specify a license file). +Your app specifies a license using `license.text`, but the value doesn't appear to be a +full license. Briefcase will generate a `copyright` file for your project; you should +ensure that the contents of this file is adequate. """ ) else: raise BriefcaseCommandError( """\ -Your project does not contain a LICENSE file. +Your project does not contain a LICENSE definition. Create a file named `LICENSE` in the same directory as your `pyproject.toml` -with your app's licensing terms. +with your app's licensing terms, and set `license.file = 'LICENSE'` in your +app's configuration. """ ) @@ -811,7 +814,11 @@ class LinuxSystemRunCommand(LinuxSystemMixin, RunCommand): supported_host_os_reason = "Linux system projects can only be executed on Linux." def run_app( - self, app: AppConfig, test_mode: bool, passthrough: list[str], **kwargs + self, + app: AppConfig, + test_mode: bool, + passthrough: list[str], + **kwargs, ): """Start the application. diff --git a/tests/commands/convert/test_migrate_necessary_files.py b/tests/commands/convert/test_migrate_necessary_files.py index 02a478eca..9853cd764 100644 --- a/tests/commands/convert/test_migrate_necessary_files.py +++ b/tests/commands/convert/test_migrate_necessary_files.py @@ -143,6 +143,34 @@ def test_pep621_specified_license_filename( assert not (convert_command.base_path / "LICENSE").exists() +@pytest.mark.parametrize("test_source_dir", ["tests"]) +def test_pep621_specified_license_text( + convert_command, + project_dir_with_files, + dummy_app_name, + test_source_dir, +): + """A license file is copied if the license is specified as text and no LICENSE file + exists.""" + convert_command.logger.warning = mock.MagicMock() + create_file( + convert_command.base_path / "pyproject.toml", + '[project]\nlicense = { text = "New BSD" }', + ) + create_file(convert_command.base_path / "CHANGELOG", "") + convert_command.migrate_necessary_files( + project_dir_with_files, + test_source_dir, + dummy_app_name, + ) + assert (convert_command.base_path / "LICENSE").exists() + + convert_command.logger.warning.assert_called_once_with( + f"\nLicense file not found in '{convert_command.base_path}'. " + "Briefcase will create a template 'LICENSE' file." + ) + + @pytest.mark.parametrize("test_source_dir", ["tests"]) def test_warning_without_changelog_file( convert_command, diff --git a/tests/config/test_parse_config.py b/tests/config/test_parse_config.py index 1cd15363b..45b0bae9d 100644 --- a/tests/config/test_parse_config.py +++ b/tests/config/test_parse_config.py @@ -13,7 +13,10 @@ def test_invalid_toml(): with pytest.raises(BriefcaseConfigError, match="Invalid pyproject.toml"): parse_config( - config_file, platform="macOS", output_format="Xcode", logger=Mock() + config_file, + platform="macOS", + output_format="Xcode", + logger=Mock(), ) @@ -29,7 +32,10 @@ def test_no_briefcase_section(): with pytest.raises(BriefcaseConfigError, match="No tool.briefcase section"): parse_config( - config_file, platform="macOS", output_format="Xcode", logger=Mock() + config_file, + platform="macOS", + output_format="Xcode", + logger=Mock(), ) @@ -45,7 +51,10 @@ def test_no_apps(): with pytest.raises(BriefcaseConfigError, match="No Briefcase apps defined"): parse_config( - config_file, platform="macOS", output_format="Xcode", logger=Mock() + config_file, + platform="macOS", + output_format="Xcode", + logger=Mock(), ) @@ -62,7 +71,10 @@ def test_single_minimal_app(): ) global_options, apps = parse_config( - config_file, platform="macOS", output_format="Xcode", logger=Mock() + config_file, + platform="macOS", + output_format="Xcode", + logger=Mock(), ) # There's a single global option @@ -71,7 +83,11 @@ def test_single_minimal_app(): # The app gets the name from its header line. # It inherits the value from the base definition. assert apps == { - "my_app": {"app_name": "my_app", "value": 42, "license": {"file": "LICENSE"}} + "my_app": { + "app_name": "my_app", + "value": 42, + "license": {"file": "LICENSE"}, + } } @@ -91,7 +107,10 @@ def test_multiple_minimal_apps(): ) global_options, apps = parse_config( - config_file, platform="macOS", output_format="Xcode", logger=Mock() + config_file, + platform="macOS", + output_format="Xcode", + logger=Mock(), ) # There are no global options @@ -137,7 +156,10 @@ def test_platform_override(): ) global_options, apps = parse_config( - config_file, platform="macOS", output_format="Xcode", logger=Mock() + config_file, + platform="macOS", + output_format="Xcode", + logger=Mock(), ) # The global options are exactly as specified @@ -200,7 +222,10 @@ def test_platform_override_ordering(): ) global_options, apps = parse_config( - config_file, platform="macOS", output_format="Xcode", logger=Mock() + config_file, + platform="macOS", + output_format="Xcode", + logger=Mock(), ) # The global options are exactly as specified @@ -279,7 +304,10 @@ def test_format_override(): ) global_options, apps = parse_config( - config_file, platform="macOS", output_format="app", logger=Mock() + config_file, + platform="macOS", + output_format="app", + logger=Mock(), ) # The global options are exactly as specified @@ -359,7 +387,10 @@ def test_format_override_ordering(): ) global_options, apps = parse_config( - config_file, platform="macOS", output_format="app", logger=Mock() + config_file, + platform="macOS", + output_format="app", + logger=Mock(), ) # The global options are exactly as specified @@ -427,7 +458,10 @@ def test_requires(): # Request a macOS app global_options, apps = parse_config( - config_file, platform="macOS", output_format="app", logger=Mock() + config_file, + platform="macOS", + output_format="app", + logger=Mock(), ) # The global options are exactly as specified @@ -464,7 +498,10 @@ def test_requires(): # Request a macOS xcode project config_file.seek(0) global_options, apps = parse_config( - config_file, platform="macOS", output_format="Xcode", logger=Mock() + config_file, + platform="macOS", + output_format="Xcode", + logger=Mock(), ) # The global options are exactly as specified @@ -500,7 +537,10 @@ def test_requires(): config_file.seek(0) global_options, apps = parse_config( - config_file, platform="linux", output_format="appimage", logger=Mock() + config_file, + platform="linux", + output_format="appimage", + logger=Mock(), ) # The global options are exactly as specified @@ -561,7 +601,10 @@ def test_document_types(): # Request a macOS app global_options, apps = parse_config( - config_file, platform="macOS", output_format="Xcode", logger=Mock() + config_file, + platform="macOS", + output_format="Xcode", + logger=Mock(), ) # The macOS my_app app specifies a full inherited chain. @@ -632,7 +675,10 @@ def test_pep621_defaults(): ) global_options, apps = parse_config( - config_file, platform="macOS", output_format="app", logger=Mock() + config_file, + platform="macOS", + output_format="app", + logger=Mock(), ) awesome = apps["awesome"] @@ -683,8 +729,7 @@ def test_license_is_string(): "license": {"file": "LICENSE"}, } logger.warning.assert_called_once_with( - "The license was specified using a string, however Briefcase prefers a PEP621 " - "compatible dictionary on the form {'file': '{filename}'} or {'text': '{license_text}'}.\n" - "Therefore, when given the license as a string, Briefcase ignores the license string and " - "defaults to {'file': 'LICENSE'}." + "Your app configuration has a `license` field that is specified as a string. " + "Briefcase now uses PEP 621 format for license definitions. To silence this " + 'warning, replace the `license` declaration with `license.file = "LICENSE".' ) diff --git a/tests/platforms/linux/system/test_build.py b/tests/platforms/linux/system/test_build.py index 1e315ec63..7a0114663 100644 --- a/tests/platforms/linux/system/test_build.py +++ b/tests/platforms/linux/system/test_build.py @@ -118,7 +118,7 @@ def test_missing_license(build_command, first_app, tmp_path): # Build the app; it will fail with pytest.raises( BriefcaseCommandError, - match=r"You specified that the license file is ", + match=r"Your `pyproject.toml` specifies a license file of 'LICENSE'", ): build_command.build_app(first_app) @@ -183,22 +183,23 @@ def test_license_text_warns_with_single_line_license(build_command, first_app): build_command.logger.warning.assert_called_once_with( """ -You specified the license using a text string. However, this string is only one line. - -Make sure that the license text is a full license (or specify a license file). +Your app specifies a license using `license.text`, but the value doesn't appear to be a +full license. Briefcase will generate a `copyright` file for your project; you should +ensure that the contents of this file is adequate. """ ) def test_exception_with_no_license(build_command, first_app): - """An exception is raised if there is no license.""" + """An exception is raised if there is no license defined.""" first_app.license = {} build_command.logger.warning = mock.MagicMock() # Build the app with pytest.raises( - BriefcaseCommandError, match="Your project does not contain a LICENSE file." + BriefcaseCommandError, + match="Your project does not contain a LICENSE definition.", ): build_command.build_app(first_app)