From 474b6259198e902711b7aa07ff687ae01ded080a Mon Sep 17 00:00:00 2001 From: Sheng Yu Date: Fri, 12 Apr 2024 16:18:47 -0400 Subject: [PATCH 1/3] feat(lint): migrate lint command --- snapcraft/application.py | 23 +++++- snapcraft/cli.py | 1 - snapcraft/commands/__init__.py | 2 + snapcraft/commands/core22/__init__.py | 2 - snapcraft/commands/{core22 => }/lint.py | 19 +++-- snapcraft/commands/unimplemented.py | 6 -- tests/spread/core24/linters-file/task.yaml | 2 - tests/unit/commands/test_lint.py | 82 ++++++++++------------ 8 files changed, 72 insertions(+), 65 deletions(-) rename snapcraft/commands/{core22 => }/lint.py (96%) diff --git a/snapcraft/application.py b/snapcraft/application.py index 75b96e65d6..8a5f40f549 100644 --- a/snapcraft/application.py +++ b/snapcraft/application.py @@ -171,12 +171,27 @@ def _extra_yaml_transform( self._parse_info = extract_parse_info(new_yaml_data) return apply_root_packages(new_yaml_data) + @staticmethod + def _get_argv_command() -> str | None: + """Return the first non-option argument.""" + for arg in sys.argv[1:]: + if arg.startswith("-"): + continue + return arg + + return None + @override def _get_dispatcher(self) -> craft_cli.Dispatcher: # Handle "multiplexing" of Snapcraft "codebases" depending on the # project's base (if any). Here, we handle the case where there *is* # a project and it's core24, which means it should definitely fall into # the craft-application-based flow. + argv_command = self._get_argv_command() + if argv_command == "lint": + # We don't need to check for core24 if we're just linting + return super()._get_dispatcher() + if self._snapcraft_yaml_path: with self._snapcraft_yaml_path.open() as file: yaml_data = util.safe_yaml_load(file) @@ -186,9 +201,11 @@ def _get_dispatcher(self) -> craft_cli.Dispatcher: if "core24" in (base, build_base) or build_base == "devel": # We know for sure that we're handling a core24 project self._known_core24 = True - elif any(arg in ("version", "--version", "-V") for arg in sys.argv): + elif ( + argv_command == "version" or "--version" in sys.argv or "-V" in sys.argv + ): pass - elif "remote-build" in sys.argv and any( + elif argv_command == "remote-build" and any( b in ("core20", "core22") for b in (base, build_base) ): build_strategy = os.environ.get("SNAPCRAFT_REMOTE_BUILD_STRATEGY", None) @@ -327,7 +344,7 @@ def create_app() -> Snapcraft: "Other", list(craft_app_commands.get_other_command_group().commands) + [ - unimplemented.Lint, + commands.LintCommand, unimplemented.Init, ], ) diff --git a/snapcraft/cli.py b/snapcraft/cli.py index e1b6f330b1..adda705eca 100644 --- a/snapcraft/cli.py +++ b/snapcraft/cli.py @@ -120,7 +120,6 @@ craft_cli.CommandGroup( "Other", [ - commands.core22.LintCommand, commands.core22.InitCommand, ], ), diff --git a/snapcraft/commands/__init__.py b/snapcraft/commands/__init__.py index a8549e867c..ffb82f2768 100644 --- a/snapcraft/commands/__init__.py +++ b/snapcraft/commands/__init__.py @@ -38,6 +38,7 @@ StoreLegacyValidateCommand, ) from .lifecycle import SnapCommand +from .lint import LintCommand from .manage import StoreCloseCommand, StoreReleaseCommand from .names import ( StoreLegacyListCommand, @@ -58,6 +59,7 @@ __all__ = [ "ExpandExtensions", + "LintCommand", "ListExtensions", "RemoteBuildCommand", "SnapCommand", diff --git a/snapcraft/commands/core22/__init__.py b/snapcraft/commands/core22/__init__.py index 4168fed89b..47ab16c4ca 100644 --- a/snapcraft/commands/core22/__init__.py +++ b/snapcraft/commands/core22/__init__.py @@ -33,7 +33,6 @@ StageCommand, TryCommand, ) -from .lint import LintCommand __all__ = [ "BuildCommand", @@ -41,7 +40,6 @@ "ExpandExtensionsCommand", "ExtensionsCommand", "InitCommand", - "LintCommand", "ListExtensionsCommand", "ListPluginsCommand", "PackCommand", diff --git a/snapcraft/commands/core22/lint.py b/snapcraft/commands/lint.py similarity index 96% rename from snapcraft/commands/core22/lint.py rename to snapcraft/commands/lint.py index 5c2f761e6e..af10c441d5 100644 --- a/snapcraft/commands/core22/lint.py +++ b/snapcraft/commands/lint.py @@ -1,6 +1,6 @@ # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- # -# Copyright 2023 Canonical Ltd. +# Copyright 2023-2024 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 @@ -24,9 +24,10 @@ import textwrap from contextlib import contextmanager from pathlib import Path, PurePosixPath -from typing import Iterator, Optional +from typing import Any, Iterator, Optional -from craft_cli import BaseCommand, emit +from craft_application.commands import AppCommand +from craft_cli import emit from craft_cli.errors import ArgumentParsingError from craft_providers.multipass import MultipassProvider from craft_providers.util import snap_cmd @@ -42,9 +43,10 @@ ) -class LintCommand(BaseCommand): +class LintCommand(AppCommand): """Lint-related commands.""" + always_load_project = False name = "lint" help_msg = "Lint a snap file" overview = textwrap.dedent( @@ -58,7 +60,7 @@ class LintCommand(BaseCommand): ) @overrides - def fill_parser(self, parser: "argparse.ArgumentParser") -> None: + def fill_parser(self, parser: argparse.ArgumentParser) -> None: parser.add_argument( "snap_file", metavar="snap-file", @@ -78,8 +80,11 @@ def fill_parser(self, parser: "argparse.ArgumentParser") -> None: help="Set https proxy", ) - @overrides - def run(self, parsed_args: argparse.Namespace): + def run( # pylint: disable=unused-argument + self, + parsed_args: argparse.Namespace, + **kwargs: Any, + ) -> None: """Run the linter command. :param parsed_args: snapcraft's argument namespace diff --git a/snapcraft/commands/unimplemented.py b/snapcraft/commands/unimplemented.py index 69d1b164a8..fef4ce8b11 100644 --- a/snapcraft/commands/unimplemented.py +++ b/snapcraft/commands/unimplemented.py @@ -105,9 +105,3 @@ class Try( UnimplementedMixin, commands.core22.TryCommand ): # noqa: D101 (missing docstring) pass - - -class Lint( - UnimplementedMixin, commands.core22.LintCommand -): # noqa: D101 (missing docstring) - pass diff --git a/tests/spread/core24/linters-file/task.yaml b/tests/spread/core24/linters-file/task.yaml index 155e905bf1..eda4cd685a 100644 --- a/tests/spread/core24/linters-file/task.yaml +++ b/tests/spread/core24/linters-file/task.yaml @@ -1,6 +1,4 @@ summary: Run snapcraft lint on a snap file. -# Disabled from regular CI runs until we have "snapcraft lint" in core24 -manual: true environment: SNAP: lint-file diff --git a/tests/unit/commands/test_lint.py b/tests/unit/commands/test_lint.py index cee9df46d9..666ff31cab 100644 --- a/tests/unit/commands/test_lint.py +++ b/tests/unit/commands/test_lint.py @@ -25,8 +25,8 @@ from craft_providers.bases import BuilddBaseAlias from craft_providers.multipass import MultipassProvider -from snapcraft import cli, models -from snapcraft.commands.core22.lint import LintCommand +from snapcraft import application, models +from snapcraft.commands.lint import LintCommand from snapcraft.errors import SnapcraftError from snapcraft.meta.snap_yaml import SnapMetadata @@ -81,9 +81,7 @@ def mock_argv(mocker, fake_snap_file): @pytest.fixture def mock_capture_logs_from_instance(mocker): - return mocker.patch( - "snapcraft.commands.core22.lint.providers.capture_logs_from_instance" - ) + return mocker.patch("snapcraft.commands.lint.providers.capture_logs_from_instance") @pytest.fixture @@ -100,16 +98,14 @@ def mock_get_base_configuration(mocker): @pytest.fixture def mock_is_managed_mode(mocker): - return mocker.patch( - "snapcraft.commands.core22.lint.is_managed_mode", return_value=False - ) + return mocker.patch("snapcraft.commands.lint.is_managed_mode", return_value=False) @pytest.fixture def mock_provider(mocker, mock_instance, fake_provider): _mock_provider = Mock(wraps=fake_provider) mocker.patch( - "snapcraft.commands.core22.lint.providers.get_provider", + "snapcraft.commands.lint.providers.get_provider", return_value=_mock_provider, ) return _mock_provider @@ -118,13 +114,13 @@ def mock_provider(mocker, mock_instance, fake_provider): @pytest.fixture def mock_run_linters(mocker): return mocker.patch( - "snapcraft.commands.core22.lint.linters.run_linters", return_value=Mock() + "snapcraft.commands.lint.linters.run_linters", return_value=Mock() ) @pytest.fixture def mock_report(mocker): - return mocker.patch("snapcraft.commands.core22.lint.linters.report") + return mocker.patch("snapcraft.commands.lint.linters.report") def test_lint_default( @@ -144,7 +140,7 @@ def test_lint_default( fake_snap_file.touch() fake_assert_file.touch() - cli.run() + application.main() mock_ensure_provider_is_available.assert_called_once() mock_get_base_configuration.assert_called_once_with( @@ -202,7 +198,7 @@ def test_lint_http_https_proxy( ], ) - cli.run() + application.main() mock_get_base_configuration.assert_called_once_with( alias=BuilddBaseAlias.JAMMY, @@ -228,7 +224,7 @@ def test_lint_assert_file_missing( # create a snap file fake_snap_file.touch() - cli.run() + application.main() mock_instance.push_file.assert_called_once_with( source=fake_snap_file, @@ -259,7 +255,7 @@ def test_lint_assert_file_not_valid( # make the assertion filepath a directory fake_assert_file.mkdir() - cli.run() + application.main() mock_instance.push_file.assert_called_once_with( source=fake_snap_file, @@ -285,14 +281,14 @@ def test_lint_multipass_not_supported( """Raise an error if Multipass is used as the build provider.""" _mock_provider = Mock(wraps=fake_provider, spec=MultipassProvider) mocker.patch( - "snapcraft.commands.core22.lint.providers.get_provider", + "snapcraft.commands.lint.providers.get_provider", return_value=_mock_provider, ) # create a snap file fake_snap_file.touch() - cli.run() + application.main() out, err = capsys.readouterr() assert not out @@ -303,7 +299,7 @@ def test_lint_multipass_not_supported( def test_lint_default_snap_file_missing(capsys, fake_snap_file, mock_argv): """Raise an error if the snap file does not exist.""" - cli.run() + application.main() out, err = capsys.readouterr() assert not out @@ -315,7 +311,7 @@ def test_lint_default_snap_file_not_valid(capsys, fake_snap_file, mock_argv): # make the snap filepath a directory fake_snap_file.mkdir() - cli.run() + application.main() out, err = capsys.readouterr() assert not out @@ -340,7 +336,7 @@ def test_lint_execute_run_error( mock_instance.execute_run.side_effect = CalledProcessError(cmd="err", returncode=1) - cli.run() + application.main() mock_capture_logs_from_instance.assert_called_once() out, err = capsys.readouterr() @@ -389,14 +385,14 @@ def test_lint_managed_mode( fake_snap_metadata.confinement = confinement fake_snap_metadata.grade = grade mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=fake_snapcraft_project, ) - cli.run() + application.main() mock_run_linters.assert_called_once_with( lint=models.Lint(ignore=["classic"]), @@ -445,14 +441,14 @@ def test_lint_managed_mode_without_snapcraft_yaml( # mock data from the unsquashed snap mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=None, ) - cli.run() + application.main() mock_run_linters.assert_called_once_with( lint=models.Lint(ignore=["classic"]), @@ -507,14 +503,14 @@ def test_lint_managed_mode_unsquash_error( # mock data from the unsquashed snap mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=fake_snapcraft_project, ) - cli.run() + application.main() out, err = capsys.readouterr() assert not out @@ -550,14 +546,14 @@ def test_lint_managed_mode_snap_install_error( # mock data from the unsquashed snap mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=fake_snapcraft_project, ) - cli.run() + application.main() out, err = capsys.readouterr() assert not out @@ -593,14 +589,14 @@ def test_lint_managed_mode_assert( # mock data from the unsquashed snap mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=fake_snapcraft_project, ) - cli.run() + application.main() mock_run_linters.assert_called_once_with( lint=models.Lint(ignore=["classic"]), @@ -657,14 +653,14 @@ def test_lint_managed_mode_assert_error( # mock data from the unsquashed snap mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=fake_snapcraft_project, ) - cli.run() + application.main() mock_run_linters.assert_called_once_with( lint=models.Lint(ignore=["classic"]), @@ -749,17 +745,17 @@ def test_lint_managed_mode_with_lint_config( # mock data from the unsquashed snap mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) # add a lint config to the project fake_snapcraft_project.lint = project_lint mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=fake_snapcraft_project, ) - cli.run() + application.main() # lint config from project should be passed to `run_linter()` mock_run_linters.assert_called_once_with( @@ -812,9 +808,7 @@ def test_load_project_complex(mocker, tmp_path): This includes lint, parse-info, architectures, and advanced grammar. """ # mock for advanced grammar parsing (i.e. `on amd64:`) - mocker.patch( - "snapcraft.commands.core22.lint.get_host_architecture", return_value="amd64" - ) + mocker.patch("snapcraft.commands.lint.get_host_architecture", return_value="amd64") # create a snap file (tmp_path / "snap").mkdir() From 236ff4afc0df76d5a0bdbd446be405e79aab18e4 Mon Sep 17 00:00:00 2001 From: Sheng Yu Date: Tue, 16 Apr 2024 12:36:17 -0400 Subject: [PATCH 2/3] fix: handle platforms in expand extensions --- snapcraft/commands/core22/extensions.py | 3 ++- snapcraft/parts/yaml_utils.py | 19 +++++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/snapcraft/commands/core22/extensions.py b/snapcraft/commands/core22/extensions.py index 48d19da778..fb6071dbb5 100644 --- a/snapcraft/commands/core22/extensions.py +++ b/snapcraft/commands/core22/extensions.py @@ -128,7 +128,8 @@ def run(self, parsed_args): # `apply_yaml()` adds or replaces the architectures keyword with an Architecture # object, which does not easily dump to a yaml file - yaml_data_for_arch.pop("architectures") + yaml_data_for_arch.pop("architectures", None) + yaml_data_for_arch.pop("platforms", None) # `parse-info` keywords must be removed before unmarshalling, because they are # not part of the Project model diff --git a/snapcraft/parts/yaml_utils.py b/snapcraft/parts/yaml_utils.py index 7ca593515e..ff9ac3649c 100644 --- a/snapcraft/parts/yaml_utils.py +++ b/snapcraft/parts/yaml_utils.py @@ -196,8 +196,23 @@ def apply_yaml( parts_yaml_data=yaml_data["parts"], arch=build_on, target_arch=build_for ) - # replace all architectures with the architectures in the current build plan - yaml_data["architectures"] = [Architecture(build_on=build_on, build_for=build_for)] + if any( + b.startswith("core20") or b.startswith("core22") + for b in ( + yaml_data.get("base", ""), + yaml_data.get("build-base", ""), + yaml_data.get("name", ""), + ) + ): + # replace all architectures with the architectures in the current build plan + yaml_data["architectures"] = [ + Architecture(build_on=build_on, build_for=build_for) + ] + else: + # replace all platforms with the platform in the current build plan + yaml_data["platforms"] = { + build_for: {"build-on": build_on, "build-for": build_for} + } return yaml_data From 18f1fdd79f4835440102bcf524cc20603a2051a4 Mon Sep 17 00:00:00 2001 From: Sheng Yu Date: Wed, 17 Apr 2024 14:55:25 -0400 Subject: [PATCH 3/3] test(linter): update linter spread test for core24 --- .../lint-file/expected-linter-output.txt | 8 +++----- .../core24/linters-file/lint-file/snapcraft.yaml | 2 +- tests/spread/core24/linters-file/task.yaml | 16 +++++++++++++--- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/tests/spread/core24/linters-file/lint-file/expected-linter-output.txt b/tests/spread/core24/linters-file/lint-file/expected-linter-output.txt index 8335fda178..ab792e1a0f 100644 --- a/tests/spread/core24/linters-file/lint-file/expected-linter-output.txt +++ b/tests/spread/core24/linters-file/lint-file/expected-linter-output.txt @@ -1,5 +1,3 @@ -Running linters... -Running linter: library -Lint warnings: -- library: linter-test: missing dependency 'libcaca.so.0'. (https://snapcraft.io/docs/linters-library) -- library: libpng16.so.16: unused library 'usr/lib/x86_64-linux-gnu/libpng16.so.16.37.0'. (https://snapcraft.io/docs/linters-library) +Lint warnings: +- library: linter-test: missing dependency 'libcaca.so.0'. (https://snapcraft.io/docs/linters-library) +- library: libogg.so.0: unused library 'usr/lib/x86_64-linux-gnu/libogg.so.0.8.5'. (https://snapcraft.io/docs/linters-library) \ No newline at end of file diff --git a/tests/spread/core24/linters-file/lint-file/snapcraft.yaml b/tests/spread/core24/linters-file/lint-file/snapcraft.yaml index 096a69fdcb..7ad7d9850e 100644 --- a/tests/spread/core24/linters-file/lint-file/snapcraft.yaml +++ b/tests/spread/core24/linters-file/lint-file/snapcraft.yaml @@ -15,6 +15,6 @@ parts: - gcc - libcaca-dev stage-packages: - - libpng16-16 + - libogg0 override-build: gcc -o $CRAFT_PART_INSTALL/linter-test test.c -lcaca diff --git a/tests/spread/core24/linters-file/task.yaml b/tests/spread/core24/linters-file/task.yaml index eda4cd685a..118e3b2f34 100644 --- a/tests/spread/core24/linters-file/task.yaml +++ b/tests/spread/core24/linters-file/task.yaml @@ -13,9 +13,19 @@ execute: | # build the test snap destructively to save time snapcraft --destructive-mode - snapcraft lint lint-file_0.1_*.snap 2> output.txt + snapcraft lint lint-file_0.1_*.snap 2> output.txt || { + lxc --project snapcraft start snapcraft-linter && + sleep 10 && + lxc --project snapcraft exec snapcraft-linter -- snap install --edge core24 && + snapcraft lint lint-file_0.1_*.snap 2> output.txt + } + # TODO: restore this when core24 is stable + # core24 testing has undefined symbol warnings # get the lint warnings at end of the log file - sed -n '/Running linters.../,+4 p' < output.txt > linter-output.txt + #sed -n '/Running linters.../,+4 p' < output.txt > linter-output.txt - diff -u linter-output.txt expected-linter-output.txt + #diff -u linter-output.txt expected-linter-output.txt + + MATCH "library: linter-test: missing dependency 'libcaca.so.0'" < output.txt + MATCH "library: libogg.so.0: unused library" < output.txt