Skip to content

Commit

Permalink
Minor tweaks to warning and error messages.
Browse files Browse the repository at this point in the history
  • Loading branch information
freakboy3742 committed May 27, 2024
1 parent 67575d4 commit 24b5e2f
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 38 deletions.
2 changes: 1 addition & 1 deletion changes/1812.feature.rst
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions changes/1812.removal.rst
Original file line number Diff line number Diff line change
@@ -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"``.
8 changes: 4 additions & 4 deletions src/briefcase/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
23 changes: 15 additions & 8 deletions src/briefcase/platforms/linux/system.py
Original file line number Diff line number Diff line change
Expand Up @@ -711,28 +711,31 @@ 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"):
(doc_folder / "copyright").write_text(license_text, encoding="utf-8")
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.
"""
)

Expand Down Expand Up @@ -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.
Expand Down
28 changes: 28 additions & 0 deletions tests/commands/convert/test_migrate_necessary_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
83 changes: 64 additions & 19 deletions tests/config/test_parse_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)


Expand All @@ -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(),
)


Expand All @@ -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(),
)


Expand All @@ -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
Expand All @@ -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"},
}
}


Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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"]
Expand Down Expand Up @@ -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".'
)
13 changes: 7 additions & 6 deletions tests/platforms/linux/system/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit 24b5e2f

Please sign in to comment.