Skip to content

Commit

Permalink
projects: warn on "global" assign of plugs and slots
Browse files Browse the repository at this point in the history
This commit adds a warning (via emit.message()) when a snapcraft.yaml
project has top-level plugs and/or slots that are used for assignment
and not per-plug/slot configuration. Thanks to lucyllewy for suggested
wording of the warning.
  • Loading branch information
tigarmo committed Apr 17, 2023
1 parent 38f6594 commit f11d2f9
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 1 deletion.
40 changes: 39 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 @@ -376,6 +377,14 @@ class ContentPlug(ProjectModel):

MANDATORY_ADOPTABLE_FIELDS = ("version", "summary", "description")

GLOBAL_PLUGS_SLOTS_WARNING = (
"Implicit {lowercase} assignment in {culprits}. "
"{uppercase}s should be assigned to the app that they apply, "
"and not implicitly assigned via the global '{lowercase}s:' "
"stanza which is intended for configuration only."
"\n(Reference: https://snapcraft.io/docs/snapcraft-interfaces)"
)


class Project(ProjectModel):
"""Snapcraft project definition.
Expand Down Expand Up @@ -429,6 +438,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 +452,36 @@ 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:
culprits = utils.humanize_list(empty_plugs, "and")
message = GLOBAL_PLUGS_SLOTS_WARNING.format(
lowercase="plug", uppercase="Plug", culprits=culprits
)
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:
culprits = utils.humanize_list(empty_slots, "and")
message = GLOBAL_PLUGS_SLOTS_WARNING.format(
lowercase="slot", uppercase="Slot", culprits=culprits
)
emit.message(message)

return slots

@pydantic.root_validator(pre=True)
@classmethod
def _validate_adoptable_fields(cls, values):
Expand Down
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 @@
Implicit plug assignment in 'desktop' and 'desktop-legacy'. Plugs should be assigned to the app that they apply, and not implicitly assigned via the global 'plugs:' stanza which is intended for configuration only.
(Reference: https://snapcraft.io/docs/snapcraft-interfaces)
Implicit slot assignment in 'network' and 'opengl'. Slots should be assigned to the app that 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=`cat 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 = (
"Implicit plug assignment in 'desktop' and 'desktop-legacy'. "
"Plugs should be assigned to the app that 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 = (
"Implicit slot assignment in 'home' and 'removable-media'. "
"Slots should be assigned to the app that 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

0 comments on commit f11d2f9

Please sign in to comment.