Skip to content
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

projects: warn on "global" assign of plugs and slots #4097

Merged
merged 2 commits into from
Apr 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion snapcraft/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@

import pydantic
from craft_archives import repo
from craft_cli import emit
from craft_grammar.models import GrammarSingleEntryDictList, GrammarStr, GrammarStrList
from pydantic import PrivateAttr, conlist, constr

from snapcraft import parts
from snapcraft import parts, utils
from snapcraft.errors import ProjectValidationError
from snapcraft.utils import get_effective_base, get_host_architecture

Expand Down Expand Up @@ -429,6 +430,7 @@ class Project(ProjectModel):
@pydantic.validator("plugs")
@classmethod
def _validate_plugs(cls, plugs):
empty_plugs = []
if plugs is not None:
for plug_name, plug in plugs.items():
if (
Expand All @@ -442,8 +444,30 @@ def _validate_plugs(cls, plugs):
if isinstance(plug, list):
raise ValueError(f"Plug '{plug_name}' cannot be a list.")

if plug is None:
empty_plugs.append(plug_name)

if empty_plugs:
message = _format_global_keyword_warning("plug", empty_plugs)
emit.message(message)

return plugs

@pydantic.validator("slots")
@classmethod
def _validate_slots(cls, slots):
empty_slots = []
if slots is not None:
for slot_name, slot in slots.items():
if slot is None:
empty_slots.append(slot_name)

if empty_slots:
message = _format_global_keyword_warning("slot", empty_slots)
emit.message(message)

return slots

@pydantic.root_validator(pre=True)
@classmethod
def _validate_adoptable_fields(cls, values):
Expand Down Expand Up @@ -824,3 +848,24 @@ def _printable_field_location_split(location: str) -> Tuple[str, str]:
return field_name, repr(".".join(loc_split))

return field_name, "top-level"


def _format_global_keyword_warning(keyword: str, empty_entries: List[str]) -> str:
"""Create a warning message about global assignment in the ``keyword`` field.
:param keyword:
The top-level keyword that contains empty entries (currently either
"plug" or "slot").
:param empty_entries:
The entries inside the ``keyword`` dict that are empty.
:return:
A properly-formatted warning message.
"""
culprits = utils.humanize_list(empty_entries, "and")
return (
f"Warning: implicit {keyword.lower()} assignment in {culprits}. "
f"{keyword.capitalize()}s should be assigned to the app to which they apply, "
f"and not implicitly assigned via the global '{keyword.lower()}s:' "
"stanza which is intended for configuration only."
"\n(Reference: https://snapcraft.io/docs/snapcraft-interfaces)"
)
5 changes: 5 additions & 0 deletions tests/spread/core22/plugs-warn/expected_output.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Warning: implicit plug assignment in 'desktop' and 'desktop-legacy'. Plugs should be assigned to the app to which they apply, and not implicitly assigned via the global 'plugs:' stanza which is intended for configuration only.
(Reference: https://snapcraft.io/docs/snapcraft-interfaces)
Warning: implicit slot assignment in 'network' and 'opengl'. Slots should be assigned to the app to which they apply, and not implicitly assigned via the global 'slots:' stanza which is intended for configuration only.
(Reference: https://snapcraft.io/docs/snapcraft-interfaces)
Initializing parts lifecycle
20 changes: 20 additions & 0 deletions tests/spread/core22/plugs-warn/snap/snapcraft.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: plugs-warn
base: core22
version: '0.1'
summary: Check warnings for top-level enabling of slots and plugs
description: Check warnings for top-level enabling of slots and plugs.

grade: stable
confinement: strict

plugs:
desktop:
desktop-legacy:

slots:
network:
opengl:

parts:
my-part:
plugin: nil
9 changes: 9 additions & 0 deletions tests/spread/core22/plugs-warn/task.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
summary: Check warnings for top-level enabling of slots and plugs

restore: |
snapcraft clean --destructive-mode
rm -f ./*.snap
execute: |
expected_output=$(< expected_output.txt)
snapcraft pull -v --destructive-mode | MATCH "$expected_output"
24 changes: 24 additions & 0 deletions tests/unit/test_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,30 @@ def test_project_build_base_devel_grade_stable_error(self, project_yaml_data):
with pytest.raises(errors.ProjectValidationError, match=error):
Project.unmarshal(project_yaml_data(build_base="devel", grade="stable"))

def test_project_global_plugs_warning(self, project_yaml_data, emitter):
data = project_yaml_data(plugs={"desktop": None, "desktop-legacy": None})
Project.unmarshal(data)
expected_message = (
"Warning: implicit plug assignment in 'desktop' and 'desktop-legacy'. "
"Plugs should be assigned to the app to which they apply, and not "
"implicitly assigned via the global 'plugs:' stanza "
"which is intended for configuration only."
"\n(Reference: https://snapcraft.io/docs/snapcraft-interfaces)"
)
emitter.assert_message(expected_message)

def test_project_global_slots_warning(self, project_yaml_data, emitter):
data = project_yaml_data(slots={"home": None, "removable-media": None})
Project.unmarshal(data)
expected_message = (
"Warning: implicit slot assignment in 'home' and 'removable-media'. "
"Slots should be assigned to the app to which they apply, and not "
"implicitly assigned via the global 'slots:' stanza "
"which is intended for configuration only."
"\n(Reference: https://snapcraft.io/docs/snapcraft-interfaces)"
)
emitter.assert_message(expected_message)


class TestHookValidation:
"""Validate hooks."""
Expand Down