project: method to return snaps to install (#3816)

- moved it away from lifecycle - added support to not install content snaps if a build-snap provides it Signed-off-by: Sergio Schvezov <[email protected]>
canonical · Jun 29, 2022 · 372fbd0 · 372fbd0
1 parent 6e04061
commit 372fbd0
Show file tree

Hide file tree

Showing 5 changed files with 165 additions and 116 deletions.
diff --git a/snapcraft/parts/lifecycle.py b/snapcraft/parts/lifecycle.py
@@ -20,7 +20,7 @@
 import subprocess
 from dataclasses import dataclass
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, Dict, List, Optional
+from typing import TYPE_CHECKING, Any, Dict, List
 
 import craft_parts
 from craft_cli import EmitterMode, emit
@@ -232,7 +232,7 @@ def _run_command(
             "version": project.version or "",
             "grade": project.grade or "",
         },
-        extra_build_snaps=_get_extra_build_snaps(project),
+        extra_build_snaps=project.get_extra_build_snaps(),
     )
 
     if command_name == "clean":
@@ -359,17 +359,6 @@ def _get_arch() -> str:
     return infos._ARCH_TRANSLATIONS[machine]["deb"]  # pylint: disable=protected-access
 
 
-def _get_extra_build_snaps(project: Project) -> Optional[List[str]]:
-    """Get list of extra snaps required to build."""
-    extra_build_snaps = project.get_content_snaps()
-    if project.base is not None:
-        if extra_build_snaps is None:
-            extra_build_snaps = [project.base]
-        else:
-            extra_build_snaps.append(project.base)
-    return extra_build_snaps
-
-
 def _set_global_environment(info: ProjectInfo) -> None:
     """Set global environment variables."""
     info.global_environment.update(

diff --git a/snapcraft/projects.py b/snapcraft/projects.py
@@ -421,15 +421,34 @@ def _get_content_plugs(self) -> List[ContentPlug]:
             ]
         return []
 
-    def get_content_snaps(self) -> Optional[List[str]]:
+    def get_content_snaps(self) -> List[str]:
         """Get list of snaps from ContentPlug `default-provider` fields."""
-        content_snaps = [
+        return [
             x.default_provider
             for x in self._get_content_plugs()
             if x.default_provider is not None
         ]
 
-        return content_snaps if content_snaps else None
+    def get_extra_build_snaps(self) -> List[str]:
+        """Get list of extra snaps required to build."""
+        # Build snaps defined by the user with channel stripped
+        build_snaps: List[str] = []
+        for part in self.parts.values():
+            build_snaps.extend(part.get("build-snaps", []))
+        part_build_snaps = {p.split("/")[0] for p in build_snaps}
+
+        # Content snaps the project uses
+        content_snaps = set(self.get_content_snaps())
+
+        # Do not add the content snaps if provided by the user
+        extra_build_snaps = list(content_snaps - part_build_snaps)
+
+        # Always add the base as an extra build snap
+        if self.base is not None:
+            extra_build_snaps.append(self.base)
+        extra_build_snaps.sort()
+
+        return extra_build_snaps
 
     def get_effective_base(self) -> str:
         """Return the base to use to create the snap."""

diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py
@@ -14,13 +14,75 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import textwrap
+from pathlib import Path
 from typing import Any, Dict, Optional, Tuple
 
 import pytest
 
 from snapcraft.extensions import extension, register, unregister
 
 
+@pytest.fixture
+def snapcraft_yaml(new_dir):
+    """Return a fixture that can write a snapcraft.yaml."""
+
+    def write_file(
+        *, base: str, filename: str = "snap/snapcraft.yaml"
+    ) -> Dict[str, Any]:
+        content = textwrap.dedent(
+            f"""
+            name: mytest
+            version: '0.1'
+            base: {base}
+            summary: Just some test data
+            description: This is just some test data.
+            grade: stable
+            confinement: strict
+
+            parts:
+              part1:
+                plugin: nil
+            """
+        )
+        yaml_path = Path(filename)
+        yaml_path.parent.mkdir(parents=True, exist_ok=True)
+        yaml_path.write_text(content)
+
+        return {
+            "name": "mytest",
+            "title": None,
+            "base": base,
+            "compression": "xz",
+            "version": "0.1",
+            "contact": None,
+            "donation": None,
+            "issues": None,
+            "source-code": None,
+            "website": None,
+            "summary": "Just some test data",
+            "description": "This is just some test data.",
+            "type": None,
+            "confinement": "strict",
+            "icon": None,
+            "layout": None,
+            "license": None,
+            "grade": "stable",
+            "architectures": [],
+            "package-repositories": [],
+            "assumes": [],
+            "hooks": None,
+            "passthrough": None,
+            "apps": None,
+            "plugs": None,
+            "slots": None,
+            "parts": {"part1": {"plugin": "nil"}},
+            "epoch": None,
+        }
+
+    yield write_file
+
+
 @pytest.fixture
 def fake_extension():
     """Basic extension."""

diff --git a/tests/unit/parts/test_lifecycle.py b/tests/unit/parts/test_lifecycle.py
@@ -17,7 +17,6 @@
 import argparse
 import textwrap
 from pathlib import Path
-from typing import Any, Dict
 from unittest.mock import PropertyMock, call
 
 import pytest
@@ -47,64 +46,6 @@ def unregister_callbacks(mocker):
     callbacks.unregister_all()
 
 
-@pytest.fixture
-def snapcraft_yaml(new_dir):
-    def write_file(
-        *, base: str, filename: str = "snap/snapcraft.yaml"
-    ) -> Dict[str, Any]:
-        content = textwrap.dedent(
-            f"""
-            name: mytest
-            version: '0.1'
-            base: {base}
-            summary: Just some test data
-            description: This is just some test data.
-            grade: stable
-            confinement: strict
-
-            parts:
-              part1:
-                plugin: nil
-            """
-        )
-        yaml_path = Path(filename)
-        yaml_path.parent.mkdir(parents=True, exist_ok=True)
-        yaml_path.write_text(content)
-
-        return {
-            "name": "mytest",
-            "title": None,
-            "base": base,
-            "compression": "xz",
-            "version": "0.1",
-            "contact": None,
-            "donation": None,
-            "issues": None,
-            "source-code": None,
-            "website": None,
-            "summary": "Just some test data",
-            "description": "This is just some test data.",
-            "type": None,
-            "confinement": "strict",
-            "icon": None,
-            "layout": None,
-            "license": None,
-            "grade": "stable",
-            "architectures": [],
-            "package-repositories": [],
-            "assumes": [],
-            "hooks": None,
-            "passthrough": None,
-            "apps": None,
-            "plugs": None,
-            "slots": None,
-            "parts": {"part1": {"plugin": "nil"}},
-            "epoch": None,
-        }
-
-    yield write_file
-
-
 @pytest.fixture
 def project_vars(mocker):
     yield mocker.patch(
@@ -771,47 +712,6 @@ def test_get_snap_project_no_base(snapcraft_yaml, new_dir):
     )
 
 
-def test_get_snap_project_with_base(snapcraft_yaml):
-    project = Project.unmarshal(snapcraft_yaml(base="core22"))
-
-    assert parts_lifecycle._get_extra_build_snaps(project) == ["core22"]
-
-
-def test_get_snap_project_with_content_plugs(snapcraft_yaml, new_dir):
-    yaml_data = {
-        "name": "mytest",
-        "version": "0.1",
-        "base": "core22",
-        "summary": "Just some test data",
-        "description": "This is just some test data.",
-        "grade": "stable",
-        "confinement": "strict",
-        "parts": {"part1": {"plugin": "nil"}},
-        "plugs": {
-            "test-plug-1": {
-                "content": "content-interface",
-                "interface": "content",
-                "target": "$SNAP/content",
-                "default-provider": "test-snap-1",
-            },
-            "test-plug-2": {
-                "content": "content-interface",
-                "interface": "content",
-                "target": "$SNAP/content",
-                "default-provider": "test-snap-2",
-            },
-        },
-    }
-
-    project = Project(**yaml_data)
-
-    assert parts_lifecycle._get_extra_build_snaps(project) == [
-        "test-snap-1",
-        "test-snap-2",
-        "core22",
-    ]
-
-
 def test_expand_environment(new_dir, mocker):
     mocker.patch("platform.machine", return_value="aarch64")
 

diff --git a/tests/unit/test_projects.py b/tests/unit/test_projects.py
@@ -1163,3 +1163,82 @@ def test_project_system_usernames_invalid(self, system_username, project_yaml_da
         error = "- value is not a valid dict"
         with pytest.raises(errors.ProjectValidationError, match=error):
             Project.unmarshal(project_yaml_data(system_usernames=system_username))
+
+
+def test_get_snap_project_with_base(snapcraft_yaml):
+    project = Project.unmarshal(snapcraft_yaml(base="core22"))
+
+    assert project.get_extra_build_snaps() == ["core22"]
+
+
+def test_get_snap_project_with_content_plugs(snapcraft_yaml, new_dir):
+    yaml_data = {
+        "name": "mytest",
+        "version": "0.1",
+        "base": "core22",
+        "summary": "Just some test data",
+        "description": "This is just some test data.",
+        "grade": "stable",
+        "confinement": "strict",
+        "parts": {"part1": {"plugin": "nil"}},
+        "plugs": {
+            "test-plug-1": {
+                "content": "content-interface",
+                "interface": "content",
+                "target": "$SNAP/content",
+                "default-provider": "test-snap-1",
+            },
+            "test-plug-2": {
+                "content": "content-interface",
+                "interface": "content",
+                "target": "$SNAP/content",
+                "default-provider": "test-snap-2",
+            },
+        },
+    }
+
+    project = Project(**yaml_data)
+
+    assert project.get_extra_build_snaps() == [
+        "core22",
+        "test-snap-1",
+        "test-snap-2",
+    ]
+
+
+def test_get_snap_project_with_content_plugs_does_not_add_extension(
+    snapcraft_yaml, new_dir
+):
+    yaml_data = {
+        "name": "mytest",
+        "version": "0.1",
+        "base": "core22",
+        "summary": "Just some test data",
+        "description": "This is just some test data.",
+        "grade": "stable",
+        "confinement": "strict",
+        "plugs": {
+            "test-plug-1": {
+                "content": "content-interface",
+                "interface": "content",
+                "target": "$SNAP/content",
+                "default-provider": "test-snap-1",
+            },
+            "test-plug-2": {
+                "content": "content-interface",
+                "interface": "content",
+                "target": "$SNAP/content",
+                "default-provider": "test-snap-2",
+            },
+        },
+        "parts": {
+            "part1": {"plugin": "nil", "build-snaps": ["test-snap-2", "test-snap-3"]}
+        },
+    }
+
+    project = Project(**yaml_data)
+
+    assert project.get_extra_build_snaps() == [
+        "core22",
+        "test-snap-1",
+    ]