From 24e430d05e540a1548bfd6ab0f3a5a838e059980 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Wed, 9 Aug 2023 17:09:29 -0300 Subject: [PATCH 1/4] requirements: update craft-parts to 1.19.7 (#4305) Craft Parts 1.19.7 addresses issues related to version being set twice during rebuilds. Signed-off-by: Claudio Matsuoka Co-authored-by: Callahan --- requirements-devel.txt | 2 +- requirements.txt | 2 +- .../linter-ros2-humble-mixed/task.yaml | 14 ++++++++- .../set-version-twice/modified-snapcraft.yaml | 24 +++++++++++++++ .../set-version-twice/original-snapcraft.yaml | 23 +++++++++++++++ .../spread/core22/set-version-twice/task.yaml | 29 +++++++++++++++++++ 6 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 tests/spread/core22/set-version-twice/modified-snapcraft.yaml create mode 100644 tests/spread/core22/set-version-twice/original-snapcraft.yaml create mode 100644 tests/spread/core22/set-version-twice/task.yaml diff --git a/requirements-devel.txt b/requirements-devel.txt index aabaa76a1c..4837e9470a 100644 --- a/requirements-devel.txt +++ b/requirements-devel.txt @@ -14,7 +14,7 @@ coverage==7.2.5 craft-archives==1.1.3 craft-cli==1.2.0 craft-grammar==1.1.1 -craft-parts==1.19.6 +craft-parts==1.19.7 craft-providers==1.10.1 craft-store==2.4.0 cryptography==40.0.2 diff --git a/requirements.txt b/requirements.txt index 5231f45b01..eb60778d80 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,7 +8,7 @@ click==8.1.3 craft-archives==1.1.3 craft-cli==1.2.0 craft-grammar==1.1.1 -craft-parts==1.19.6 +craft-parts==1.19.7 craft-providers==1.10.1 craft-store==2.4.0 cryptography==40.0.2 diff --git a/tests/spread/core22/linters/linter-ros2-humble-mixed/task.yaml b/tests/spread/core22/linters/linter-ros2-humble-mixed/task.yaml index 9076af8790..7db84e9943 100644 --- a/tests/spread/core22/linters/linter-ros2-humble-mixed/task.yaml +++ b/tests/spread/core22/linters/linter-ros2-humble-mixed/task.yaml @@ -10,4 +10,16 @@ execute: | test -f linter-ros2-humble-mixed_1.0_*.snap sed -n '/^Running linters/,/^Creating snap/p' < output.txt > linter_output.txt - diff -u linter_output.txt expected_linter_output.txt + + # diff everything except the list of libraries + diff --ignore-matching-lines "^- library: lib" expected_linter_output.txt linter_output.txt + + # `|| true` is used because grep will error if there is no output from the `diff` call + NUM_DIFFERENCES=$(diff expected_linter_output.txt linter_output.txt | grep --count "[<>]" || true) + + # the list of unused libraries may change from time to time, so check the output is similar but not identical + if [[ $NUM_DIFFERENCES -gt 10 ]]; then + echo "Error: linter warnings are significantly different:" + diff --unified expected_linter_output.txt linter_output.txt + exit 1 + fi diff --git a/tests/spread/core22/set-version-twice/modified-snapcraft.yaml b/tests/spread/core22/set-version-twice/modified-snapcraft.yaml new file mode 100644 index 0000000000..c002a7f939 --- /dev/null +++ b/tests/spread/core22/set-version-twice/modified-snapcraft.yaml @@ -0,0 +1,24 @@ +name: test-set-version-twice +base: core22 +version: '0.1' +summary: Test fix for set version and out of order step execution +description: | + As described in https://bugs.launchpad.net/snapcraft/+bug/1831135/comments/10, + a bug in craft-parts caused unexpected double setting of project variables + such as `version`. Make sure this scenario builds correctly. +adopt-info: part1 + +grade: devel +confinement: devmode + +parts: + part1: + plugin: nil + override-pull: | + craftctl default + craftctl set version=xx + echo + + part2: + plugin: nil + after: [part1] diff --git a/tests/spread/core22/set-version-twice/original-snapcraft.yaml b/tests/spread/core22/set-version-twice/original-snapcraft.yaml new file mode 100644 index 0000000000..48af83e031 --- /dev/null +++ b/tests/spread/core22/set-version-twice/original-snapcraft.yaml @@ -0,0 +1,23 @@ +name: test-set-version-twice +base: core22 +version: '0.1' +summary: Test fix for set version and out of order step execution +description: | + As described in https://bugs.launchpad.net/snapcraft/+bug/1831135/comments/10, + a bug in craft-parts caused unexpected double setting of project variables + such as `version`. Make sure this scenario builds correctly. +adopt-info: part1 + +grade: devel +confinement: devmode + +parts: + part1: + plugin: nil + override-pull: | + craftctl default + craftctl set version=xx + + part2: + plugin: nil + after: [part1] diff --git a/tests/spread/core22/set-version-twice/task.yaml b/tests/spread/core22/set-version-twice/task.yaml new file mode 100644 index 0000000000..bf756d072b --- /dev/null +++ b/tests/spread/core22/set-version-twice/task.yaml @@ -0,0 +1,29 @@ +summary: Test fix for version setting corner case + +prepare: | + #shellcheck source=tests/spread/tools/snapcraft-yaml.sh + . "$TOOLS_DIR/snapcraft-yaml.sh" + +restore: | + snapcraft clean + rm -Rf subdir ./*.snap + rm -f snap/*.yaml + + #shellcheck source=tests/spread/tools/snapcraft-yaml.sh + . "$TOOLS_DIR/snapcraft-yaml.sh" + +execute: | + mkdir -p snap + cp original-snapcraft.yaml snap/snapcraft.yaml + + snapcraft prime + + cp modified-snapcraft.yaml snap/snapcraft.yaml + + snapcraft build part2 + snapcraft prime + + cp original-snapcraft.yaml snap/snapcraft.yaml + + snapcraft build part2 + snapcraft prime From 15011660ae4d8e12a02bf55d629fc5162fddf5d5 Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Fri, 1 Sep 2023 13:35:28 -0300 Subject: [PATCH 2/4] fix(legacy): export sources method to local plugins (#4337) Support legacy local plugins that invoke `snapcraft.sources.get` by redirecting imports to `snapcraft_legacy`. This is similar to the strategy used for V1 and V2 plugins. Fixes #4285 Signed-off-by: Claudio Matsuoka --- snapcraft/__init__.py | 3 ++ snapcraft/sources.py | 26 +++++++++++ .../plugins/v1/x-local/local-plugin/task.yaml | 1 + .../source-get/snap/plugins/x_local_plugin.py | 43 +++++++++++++++++++ .../snaps/source-get/snap/snapcraft.yaml | 12 ++++++ 5 files changed, 85 insertions(+) create mode 100644 snapcraft/sources.py create mode 100644 tests/spread/plugins/v1/x-local/snaps/source-get/snap/plugins/x_local_plugin.py create mode 100644 tests/spread/plugins/v1/x-local/snaps/source-get/snap/snapcraft.yaml diff --git a/snapcraft/__init__.py b/snapcraft/__init__.py index 6c9a0a516e..a3da6b7da8 100644 --- a/snapcraft/__init__.py +++ b/snapcraft/__init__.py @@ -20,6 +20,9 @@ import pkg_resources +# For legacy compatibility +import snapcraft.sources # noqa: F401 + def _get_version(): if os.environ.get("SNAP_NAME") == "snapcraft": diff --git a/snapcraft/sources.py b/snapcraft/sources.py new file mode 100644 index 0000000000..455aa612a4 --- /dev/null +++ b/snapcraft/sources.py @@ -0,0 +1,26 @@ +# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- +# +# Copyright 2023 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 3 as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +"""Legacy support for local plugins.""" + +import sys as _sys + +if _sys.platform == "linux": + from snapcraft_legacy.sources import get + + __all__ = [ + "get", + ] diff --git a/tests/spread/plugins/v1/x-local/local-plugin/task.yaml b/tests/spread/plugins/v1/x-local/local-plugin/task.yaml index 8e036f8547..ddacb900e2 100644 --- a/tests/spread/plugins/v1/x-local/local-plugin/task.yaml +++ b/tests/spread/plugins/v1/x-local/local-plugin/task.yaml @@ -4,6 +4,7 @@ environment: SNAP_DIR/baseplugin: ../snaps/from-baseplugin SNAP_DIR/nilplugin: ../snaps/from-nilplugin SNAP_DIR/pluginv1: ../snaps/from-pluginv1 + SNAP_DIR/sourceget: ../snaps/source-get prepare: | #shellcheck source=tests/spread/tools/snapcraft-yaml.sh diff --git a/tests/spread/plugins/v1/x-local/snaps/source-get/snap/plugins/x_local_plugin.py b/tests/spread/plugins/v1/x-local/snaps/source-get/snap/plugins/x_local_plugin.py new file mode 100644 index 0000000000..1da6ecbeb0 --- /dev/null +++ b/tests/spread/plugins/v1/x-local/snaps/source-get/snap/plugins/x_local_plugin.py @@ -0,0 +1,43 @@ +# -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- +# +# Copyright 2023 Canonical Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 3 as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +import snapcraft +from snapcraft.plugins.v1 import PluginV1 + + +class LocalPlugin(PluginV1): + @classmethod + def schema(cls): + schema = super().schema() + + schema["properties"]["foo"] = {"type": "string"} + + return schema + + @classmethod + def get_pull_properties(cls): + return ["foo", "stage-packages"] + + @classmethod + def get_build_properties(cls): + return ["foo", "stage-packages"] + + def pull(self): + super().pull() + print(snapcraft.sources.get) + + def build(self): + return self.run(["touch", "build-stamp"], self.installdir) diff --git a/tests/spread/plugins/v1/x-local/snaps/source-get/snap/snapcraft.yaml b/tests/spread/plugins/v1/x-local/snaps/source-get/snap/snapcraft.yaml new file mode 100644 index 0000000000..66ea5d7b9c --- /dev/null +++ b/tests/spread/plugins/v1/x-local/snaps/source-get/snap/snapcraft.yaml @@ -0,0 +1,12 @@ +name: test-local-plugins +base: core18 +version: "0.1" +summary: local plugin using snapcraft.sources.get +description: Tests if local plugins load and can build +confinement: strict +grade: devel + +parts: + x-local-plugin: + plugin: x-local-plugin + source: . From 89fb02a1986df52af3dec00b6b625af3d707a34c Mon Sep 17 00:00:00 2001 From: Sergio Schvezov Date: Wed, 6 Sep 2023 10:22:17 -0300 Subject: [PATCH 3/4] fix: reprime for try (#4347) This solves the case where prime was run before try, where a subsequent run of try would mask the contents of the existing prime directory with a newly mounted one from the host. In legacy we had the concept of `clean --unprime` to solve this. Signed-off-by: Sergio Schvezov --- snapcraft/parts/lifecycle.py | 3 ++ snapcraft/parts/parts.py | 14 +++++++++ tests/spread/core22/try/task.yaml | 5 ++- tests/unit/parts/test_lifecycle.py | 50 +++++++++++++++++++++++++++--- 4 files changed, 66 insertions(+), 6 deletions(-) diff --git a/snapcraft/parts/lifecycle.py b/snapcraft/parts/lifecycle.py index c096963238..4c52e4ab1c 100644 --- a/snapcraft/parts/lifecycle.py +++ b/snapcraft/parts/lifecycle.py @@ -346,6 +346,9 @@ def _run_lifecycle_and_pack( step_name, shell=getattr(parsed_args, "shell", False), shell_after=getattr(parsed_args, "shell_after", False), + # Repriming needs to happen to take into account any changes to + # the actual target directory. + rerun_step=command_name == "try", ) # Extract metadata and generate snap.yaml diff --git a/snapcraft/parts/parts.py b/snapcraft/parts/parts.py index be2d95072b..24be7f1baf 100644 --- a/snapcraft/parts/parts.py +++ b/snapcraft/parts/parts.py @@ -141,10 +141,14 @@ def run( *, shell: bool = False, shell_after: bool = False, + rerun_step: bool = False, ) -> None: """Run the parts lifecycle. :param target_step: The final step to execute. + :param shell: Enter a shell instead of running step_name. + :param shell_after: Enter a shell after running step_name. + :param rerun_step: Force running step_name. :raises PartsLifecycleError: On error during lifecycle. :raises RuntimeError: On unexpected error. @@ -171,6 +175,16 @@ def run( with self._lcm.action_executor() as aex: for action in actions: + # Workaround until canonical/craft-parts#540 is fixed + if action.step == target_step and rerun_step: + action = craft_parts.Action( + part_name=action.part_name, + step=action.step, + action_type=ActionType.RERUN, + reason="forced rerun", + project_vars=action.project_vars, + properties=action.properties, + ) message = _action_message(action) emit.progress(f"Executing parts lifecycle: {message}") with emit.open_stream("Executing action") as stream: diff --git a/tests/spread/core22/try/task.yaml b/tests/spread/core22/try/task.yaml index c2e9bb2418..5dff360e1c 100644 --- a/tests/spread/core22/try/task.yaml +++ b/tests/spread/core22/try/task.yaml @@ -7,6 +7,9 @@ execute: | chmod a+w prime unset SNAPCRAFT_BUILD_ENVIRONMENT + # Prime first to regression test snapcore/snapcraft#4219 + snapcraft prime --use-lxd + # Followed by the actual try snapcraft try --use-lxd find prime/meta/snap.yaml @@ -14,4 +17,4 @@ execute: | snap try prime hello-try | MATCH "Hello, world" - snap remove hello-try \ No newline at end of file + snap remove hello-try diff --git a/tests/unit/parts/test_lifecycle.py b/tests/unit/parts/test_lifecycle.py index 369dbf88b3..f41a629d98 100644 --- a/tests/unit/parts/test_lifecycle.py +++ b/tests/unit/parts/test_lifecycle.py @@ -299,13 +299,45 @@ def test_lifecycle_run_command_step( parsed_args=parsed_args, ) - call_args = {"shell": False, "shell_after": False} + call_args = {"shell": False, "shell_after": False, "rerun_step": False} if debug_shell: call_args[debug_shell] = True assert run_mock.mock_calls == [call(step, **call_args)] +def test_lifecycle_run_try_command(snapcraft_yaml, project_vars, new_dir, mocker): + project = Project.unmarshal(snapcraft_yaml(base="core22")) + run_mock = mocker.patch("snapcraft.parts.PartsLifecycle.run") + mocker.patch("snapcraft.meta.snap_yaml.write") + mocker.patch("snapcraft.pack.pack_snap") + + parsed_args = argparse.Namespace( + debug=False, + destructive_mode=True, + enable_manifest=False, + shell=False, + shell_after=False, + use_lxd=False, + ua_token=None, + parts=[], + ) + + parts_lifecycle._run_command( + "try", + project=project, + parse_info={}, + assets_dir=Path(), + start_time=datetime.now(), + parallel_build_count=8, + parsed_args=parsed_args, + ) + + assert run_mock.mock_calls == [ + call("prime", shell=False, shell_after=False, rerun_step=True) + ] + + @pytest.mark.parametrize("cmd", ["pack", "snap"]) def test_lifecycle_run_command_pack(cmd, snapcraft_yaml, project_vars, new_dir, mocker): project = Project.unmarshal(snapcraft_yaml(base="core22")) @@ -333,7 +365,9 @@ def test_lifecycle_run_command_pack(cmd, snapcraft_yaml, project_vars, new_dir, ), ) - assert run_mock.mock_calls == [call("prime", shell=False, shell_after=False)] + assert run_mock.mock_calls == [ + call("prime", shell=False, shell_after=False, rerun_step=False) + ] assert pack_mock.mock_calls[:1] == [ call( new_dir / "prime", @@ -382,7 +416,9 @@ def test_lifecycle_pack_destructive_mode( ) assert run_in_provider_mock.mock_calls == [] - assert run_mock.mock_calls == [call("prime", shell=False, shell_after=False)] + assert run_mock.mock_calls == [ + call("prime", shell=False, shell_after=False, rerun_step=False) + ] assert pack_mock.mock_calls[:1] == [ call( new_dir / "home/prime", @@ -432,7 +468,9 @@ def test_lifecycle_pack_managed(cmd, snapcraft_yaml, project_vars, new_dir, mock ) assert run_in_provider_mock.mock_calls == [] - assert run_mock.mock_calls == [call("prime", shell=False, shell_after=False)] + assert run_mock.mock_calls == [ + call("prime", shell=False, shell_after=False, rerun_step=False) + ] assert pack_mock.mock_calls[:1] == [ call( new_dir / "home/prime", @@ -525,7 +563,9 @@ def test_lifecycle_pack_metadata_error(cmd, snapcraft_yaml, new_dir, mocker): assert str(raised.value) == ( "error setting grade: unexpected value; permitted: 'stable', 'devel'" ) - assert run_mock.mock_calls == [call("prime", shell=False, shell_after=False)] + assert run_mock.mock_calls == [ + call("prime", shell=False, shell_after=False, rerun_step=False) + ] assert pack_mock.mock_calls == [] From 061b4f0f8b0afc6fcf66057e9a2001901207fbb5 Mon Sep 17 00:00:00 2001 From: Sergio Schvezov Date: Wed, 6 Sep 2023 18:52:39 -0300 Subject: [PATCH 4/4] fix: support abstract socket listen-streams Rudimentary support for validating abstract sockets in listen-streams. The complete check will take place at `snap pack --check-skeleton` at the end of the process. Signed-off-by: Sergio Schvezov --- snapcraft/projects.py | 4 +++- tests/unit/test_projects.py | 18 ++++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/snapcraft/projects.py b/snapcraft/projects.py index c9a441efbf..3ca81a055e 100644 --- a/snapcraft/projects.py +++ b/snapcraft/projects.py @@ -187,7 +187,9 @@ def _validate_list_stream(cls, listen_stream): f"{listen_stream!r} is not an integer between 1 and 65535 (inclusive)." ) elif isinstance(listen_stream, str): - if not re.match(r"^[A-Za-z0-9/._#:$-]*$", listen_stream): + if not listen_stream.startswith("@snap.") and not re.match( + r"^[A-Za-z0-9/._#:$-]*$", listen_stream + ): raise ValueError( f"{listen_stream!r} is not a valid socket path (e.g. /tmp/mysocket.sock)." ) diff --git a/tests/unit/test_projects.py b/tests/unit/test_projects.py index 28164cc928..5f9546be0f 100644 --- a/tests/unit/test_projects.py +++ b/tests/unit/test_projects.py @@ -1053,7 +1053,9 @@ def test_app_command_chain(self, command_chain, app_yaml_data): assert project.apps is not None assert project.apps["app1"].command_chain == command_chain - @pytest.mark.parametrize("listen_stream", [1, 100, 65535, "/tmp/mysocket.sock"]) + @pytest.mark.parametrize( + "listen_stream", [1, 100, 65535, "/tmp/mysocket.sock", "@snap.foo"] + ) def test_app_sockets_valid_listen_stream(self, listen_stream, socket_yaml_data): data = socket_yaml_data(listen_stream=listen_stream) @@ -1063,13 +1065,25 @@ def test_app_sockets_valid_listen_stream(self, listen_stream, socket_yaml_data): assert project.apps["app1"].sockets["socket1"].listen_stream == listen_stream @pytest.mark.parametrize("listen_stream", [-1, 0, 65536]) - def test_app_sockets_invalid_listen_stream(self, listen_stream, socket_yaml_data): + def test_app_sockets_invalid_int_listen_stream( + self, listen_stream, socket_yaml_data + ): data = socket_yaml_data(listen_stream=listen_stream) error = f".*{listen_stream} is not an integer between 1 and 65535" with pytest.raises(errors.ProjectValidationError, match=error): Project.unmarshal(data) + @pytest.mark.parametrize("listen_stream", ["@foo"]) + def test_app_sockets_invalid_socket_listen_stream( + self, listen_stream, socket_yaml_data + ): + data = socket_yaml_data(listen_stream=listen_stream) + + error = f".*{listen_stream!r} is not a valid socket path.*" + with pytest.raises(errors.ProjectValidationError, match=error): + Project.unmarshal(data) + def test_app_sockets_missing_listen_stream(self, socket_yaml_data): data = socket_yaml_data()