diff --git a/docs/reference/changelog.rst b/docs/reference/changelog.rst index 8559a3c2ff..fbc344f8ac 100644 --- a/docs/reference/changelog.rst +++ b/docs/reference/changelog.rst @@ -68,23 +68,84 @@ Changelog For a complete list of commits, check out the `X.Y.Z`_ release on GitHub. -8.3.4 (2024-Sep-13) +8.4.1 (2024-Sep-20) ------------------- Core ==== +* Fix a regression where numeric entries in ``snapcraft.yaml`` couldn't be + parsed. + +Bases +##### + +core24 +"""""" + +* Fix a regression where ``build-for`` couldn't be omitted in a ``platforms`` + entry in a ``snapcraft.yaml`` file. + +* Fix a regression where ``--shell`` and ``--shell-after`` weren't supported + for the ``pack`` command (`#4963`_). + +* Fix a regression where ``--debug`` wouldn't open a shell into the build + environment if the packing step fails (`#4959`_). + Plugins ####### NPM """ +* Fix a bug where NPM parts fail to build if the ``pull`` and ``build`` steps + didn't occur in the same instance of Snapcraft. + +Command line +============ + +* Fix a regression where store errors would be raised as an internal error + (`#4930`_). + +* Add documentation links for error messages about using an `ESM base`_. + +Remote build +============ + +* Fix a regression where ``--build-for`` and ``--platform`` couldn't accept + comma-separated values (`#4990`_). + +* Fix a regression where remote build errors would be raised as an internal + error (`#4908`_). + +* Add documentation links and recommended resolutions to remote-build errors. + +Store +===== + +* Fix a regression where Ubuntu One macaroons couldn't be refreshed + (`#5048`_). + +For a complete list of changes, check out the `8.4.1`_ release on GitHub. + + +8.3.4 (2024-Sep-13) +------------------- + +Core +==== + +Plugins +####### + +NPM +""" * Fix a bug where NPM parts fail to build if the ``pull`` and ``build`` steps did not occur in the same execution of Snapcraft. For a complete list of commits, check out the `8.3.4`_ release on GitHub. + 8.4.0 (2024-Sep-10) ------------------- @@ -1156,11 +1217,17 @@ For a complete list of commits, check out the `8.0.0`_ release on GitHub. .. _#4886: https://github.com/canonical/snapcraft/issues/4886 .. _#4889: https://github.com/canonical/snapcraft/issues/4889 .. _#4890: https://github.com/canonical/snapcraft/issues/4890 +.. _#4908: https://github.com/canonical/snapcraft/issues/4908 .. _#4909: https://github.com/canonical/snapcraft/issues/4909 +.. _#4930: https://github.com/canonical/snapcraft/issues/4930 .. _#4941: https://github.com/canonical/snapcraft/issues/4941 .. _#4942: https://github.com/canonical/snapcraft/issues/4942 +.. _#4959: https://github.com/canonical/snapcraft/issues/4959 +.. _#4963: https://github.com/canonical/snapcraft/issues/4963 +.. _#4990: https://github.com/canonical/snapcraft/issues/4990 .. _#4995: https://github.com/canonical/snapcraft/issues/4995 .. _#5008: https://github.com/canonical/snapcraft/issues/5008 +.. _#5048: https://github.com/canonical/snapcraft/issues/5048 .. _7.5.6: https://github.com/canonical/snapcraft/releases/tag/7.5.6 .. _8.0.0: https://github.com/canonical/snapcraft/releases/tag/8.0.0 @@ -1189,3 +1256,4 @@ For a complete list of commits, check out the `8.0.0`_ release on GitHub. .. _8.3.3: https://github.com/canonical/snapcraft/releases/tag/8.3.3 .. _8.3.4: https://github.com/canonical/snapcraft/releases/tag/8.3.4 .. _8.4.0: https://github.com/canonical/snapcraft/releases/tag/8.4.0 +.. _8.4.1: https://github.com/canonical/snapcraft/releases/tag/8.4.1 diff --git a/requirements-devel.txt b/requirements-devel.txt index cdd42303c3..515ced5b6b 100644 --- a/requirements-devel.txt +++ b/requirements-devel.txt @@ -24,14 +24,14 @@ click==8.1.7 codespell==2.3.0 colorama==0.4.6 coverage==7.6.1 -craft-application==4.1.2 +craft-application==4.2.4 craft-archives==2.0.0 craft-cli==2.7.0 -craft-grammar==2.0.0 +craft-grammar==2.0.1 craft-parts==2.1.1 craft-platforms==0.1.1 craft-providers==2.0.1 -craft-store==3.0.0 +craft-store==3.0.1 cryptography==43.0.1 cssutils==2.11.1 dict2css==0.3.0.post1 diff --git a/requirements-docs.txt b/requirements-docs.txt index 61102c9523..665fa0369f 100644 --- a/requirements-docs.txt +++ b/requirements-docs.txt @@ -19,14 +19,14 @@ chardet==5.2.0 charset-normalizer==3.3.2 click==8.1.7 colorama==0.4.6 -craft-application==4.1.2 +craft-application==4.2.4 craft-archives==2.0.0 craft-cli==2.7.0 -craft-grammar==2.0.0 +craft-grammar==2.0.1 craft-parts==2.1.1 craft-platforms==0.1.1 craft-providers==2.0.1 -craft-store==3.0.0 +craft-store==3.0.1 cryptography==43.0.1 cssutils==2.11.1 dict2css==0.3.0.post1 diff --git a/requirements.txt b/requirements.txt index 75f6107ff1..45c38f3fc6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,14 +7,14 @@ cffi==1.17.1 chardet==5.2.0 charset-normalizer==3.3.2 click==8.1.7 -craft-application==4.1.2 +craft-application==4.2.4 craft-archives==2.0.0 craft-cli==2.7.0 -craft-grammar==2.0.0 +craft-grammar==2.0.1 craft-parts==2.1.1 craft-platforms==0.1.1 craft-providers==2.0.1 -craft-store==3.0.0 +craft-store==3.0.1 cryptography==43.0.1 distro==1.9.0 docutils==0.19 diff --git a/setup.py b/setup.py index 7b2a06c73e..78ee0a0b6a 100755 --- a/setup.py +++ b/setup.py @@ -98,14 +98,14 @@ def recursive_data_files(directory, install_directory): "attrs", "catkin-pkg; sys_platform == 'linux'", "click", - "craft-application~=4.1", + "craft-application>=4.2.4,<5.0.0", "craft-archives~=2.0", "craft-cli~=2.6", - "craft-grammar~=2.0", + "craft-grammar>=2.0.1,<3.0.0", "craft-parts~=2.1", "craft-platforms~=0.1", "craft-providers~=2.0", - "craft-store~=3.0", + "craft-store>=3.0.1,<4.0.0", "docutils<0.20", # Frozen until we can update sphinx dependencies. "gnupg", "jsonschema==2.5.1", diff --git a/snapcraft/application.py b/snapcraft/application.py index 706dd87b21..df84454fa8 100644 --- a/snapcraft/application.py +++ b/snapcraft/application.py @@ -27,7 +27,7 @@ import craft_cli import craft_parts import craft_store -from craft_application import Application, AppMetadata, util +from craft_application import Application, AppMetadata, remote, util from craft_application.commands import get_other_command_group from craft_cli import emit from craft_parts.plugins.plugins import PluginType @@ -82,6 +82,7 @@ def _get_esm_error_for_base(base: str) -> None: f"Use Snapcraft {version} from the {channel!r} channel of snapcraft where " f"{base!r} was last supported." ), + doc_slug="/reference/bases", ) @@ -209,13 +210,13 @@ def _pre_run(self, dispatcher: craft_cli.Dispatcher) -> None: super()._pre_run(dispatcher) @override - def run(self) -> int: + def _run_inner(self) -> int: try: - return_code = super().run() + return_code = super()._run_inner() except craft_store.errors.NoKeyringError as err: self._emit_error( craft_cli.errors.CraftError( - f"craft-store error: {err}", + f"{err}", resolution=( "Ensure the keyring is working or " f"{store.constants.ENVIRONMENT_STORE_CREDENTIALS} " @@ -227,12 +228,14 @@ def run(self) -> int: return_code = 1 except craft_store.errors.CraftStoreError as err: self._emit_error( - craft_cli.errors.CraftError( - f"craft-store error: {err}", resolution=err.resolution - ), + craft_cli.errors.CraftError(f"{err}", resolution=err.resolution), cause=err, ) return_code = 1 + except remote.RemoteBuildError as err: + err.doc_slug = "/explanation/remote-build" + self._emit_error(err) + return_code = err.retcode return return_code @@ -393,6 +396,7 @@ def _ensure_remote_build_supported(base: str) -> None: resolution=( "Valid values are 'disable-fallback' and 'force-fallback'." ), + doc_slug="/explanation/remote-build", ) # 2. core20 projects must use the legacy remote builder (#4885) @@ -405,6 +409,7 @@ def _ensure_remote_build_supported(base: str) -> None: resolution=( "Unset the environment variable or set it to 'force-fallback'." ), + doc_slug="/explanation/remote-build", ) # 3. core24 and newer projects must use the craft-application remote builder @@ -417,6 +422,7 @@ def _ensure_remote_build_supported(base: str) -> None: resolution=( "Unset the environment variable or set it to 'disable-fallback'." ), + doc_slug="/explanation/remote-build", ) @override diff --git a/tests/unit/commands/test_lifecycle.py b/tests/unit/commands/test_lifecycle.py index ee8ce3b23d..b4e5ab47a1 100644 --- a/tests/unit/commands/test_lifecycle.py +++ b/tests/unit/commands/test_lifecycle.py @@ -107,7 +107,8 @@ def test_snap_command_fallback(tmp_path, emitter, mocker, fake_services): ) -def test_core24_try_command(tmp_path, mocker, fake_services): +@pytest.mark.usefixtures("emitter") +def test_core24_try_command(tmp_path, fake_services): parsed_args = argparse.Namespace(parts=[], output=tmp_path) cmd = lifecycle.TryCommand({"app": APP_METADATA, "services": fake_services}) diff --git a/tests/unit/commands/test_remote.py b/tests/unit/commands/test_remote.py index c8e109ef91..6ebdc50633 100644 --- a/tests/unit/commands/test_remote.py +++ b/tests/unit/commands/test_remote.py @@ -36,7 +36,8 @@ # remote-build control logic may check if the working dir is a git repo, # so execute all tests inside a test directory -pytestmark = pytest.mark.usefixtures("new_dir") +# The service also emits +pytestmark = pytest.mark.usefixtures("new_dir", "emitter") @pytest.fixture() @@ -163,7 +164,7 @@ def test_no_confirmation_for_private_project( @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv") +@pytest.mark.usefixtures("mock_argv", "emitter") def test_command_user_confirms_upload( snapcraft_yaml, base, mock_confirm, fake_services ): @@ -183,7 +184,7 @@ def test_command_user_confirms_upload( @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv", "fake_services") +@pytest.mark.usefixtures("mock_argv", "emitter", "fake_services") def test_command_user_denies_upload( capsys, snapcraft_yaml, @@ -206,7 +207,7 @@ def test_command_user_denies_upload( @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv", "fake_services") +@pytest.mark.usefixtures("mock_argv", "emitter", "fake_services") def test_command_accept_upload( mocker, snapcraft_yaml, base, mock_confirm, mock_run_remote_build ): @@ -224,7 +225,9 @@ def test_command_accept_upload( @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv", "mock_confirm", "fake_services", "fake_sudo") +@pytest.mark.usefixtures( + "mock_argv", "mock_confirm", "emitter", "fake_services", "fake_sudo" +) def test_remote_build_sudo_warns(emitter, snapcraft_yaml, base, mock_run_remote_build): "Check if a warning is shown when snapcraft is run with sudo." snapcraft_yaml_dict = {"base": base, "build-base": "devel", "grade": "devel"} @@ -240,8 +243,8 @@ def test_remote_build_sudo_warns(emitter, snapcraft_yaml, base, mock_run_remote_ @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv", "mock_confirm", "fake_services") -def test_launchpad_timeout_default(mocker, snapcraft_yaml, base, fake_services): +@pytest.mark.usefixtures("mock_argv", "mock_confirm", "emitter", "fake_services") +def test_launchpad_timeout_default(mocker, snapcraft_yaml, base): """Check if no timeout is set by default.""" snapcraft_yaml_dict = {"base": base, "build-base": "devel", "grade": "devel"} snapcraft_yaml(**snapcraft_yaml_dict) @@ -256,8 +259,8 @@ def test_launchpad_timeout_default(mocker, snapcraft_yaml, base, fake_services): @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv", "mock_confirm", "fake_services") -def test_launchpad_timeout(mocker, snapcraft_yaml, base, fake_services): +@pytest.mark.usefixtures("mock_argv", "mock_confirm", "emitter", "fake_services") +def test_launchpad_timeout(mocker, snapcraft_yaml, base): """Set the timeout for the remote builder.""" mocker.patch.object( sys, "argv", ["snapcraft", "remote-build", "--launchpad-timeout", "100"] @@ -281,7 +284,7 @@ def test_launchpad_timeout(mocker, snapcraft_yaml, base, fake_services): @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv", "mock_confirm", "fake_services") +@pytest.mark.usefixtures("mock_argv", "mock_confirm", "emitter", "fake_services") def test_run_core22_and_later(snapcraft_yaml, base, mock_remote_build_run): """Bases that are core22 and later will use craft-application remote-build.""" snapcraft_yaml_dict = {"base": base, "build-base": "devel", "grade": "devel"} @@ -309,15 +312,8 @@ def test_run_core20( @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_confirm", "mock_argv") -def test_run_in_repo_newer_than_core22( - emitter, - mocker, - snapcraft_yaml, - base, - new_dir, - fake_services, -): +@pytest.mark.usefixtures("mock_confirm", "mock_argv", "emitter", "fake_services") +def test_run_in_repo_newer_than_core22(mocker, snapcraft_yaml, base, new_dir): """Bases newer than core22 run craft-application remote-build regardless of being in a repo.""" # initialize a git repo GitRepo(new_dir) @@ -337,15 +333,10 @@ def test_run_in_repo_newer_than_core22( @pytest.mark.usefixtures( "mock_confirm", "mock_argv", + "mock_remote_builder_start_builds", + "fake_services", ) -def test_run_in_shallow_repo_unsupported( - capsys, - new_dir, - snapcraft_yaml, - base, - mock_remote_builder_start_builds, - fake_services, -): +def test_run_in_shallow_repo_unsupported(capsys, new_dir, snapcraft_yaml, base): """devel / core24 and newer bases run new remote-build in a shallow git repo.""" root_path = Path(new_dir) snapcraft_yaml_dict = {"base": base, "build-base": "devel", "grade": "devel"} diff --git a/tests/unit/models/test_projects.py b/tests/unit/models/test_projects.py index 50b5b07adb..99ccc4c4ee 100644 --- a/tests/unit/models/test_projects.py +++ b/tests/unit/models/test_projects.py @@ -82,6 +82,44 @@ def _socket_yaml_data(**kwargs) -> Dict[str, Any]: yield _socket_yaml_data +@pytest.fixture +def fake_project_with_numbers(project_yaml_data): + """Returns a fake project with numbers in string fields. + + This includes numbers in fields that are validated by snapcraft and fields + validated by craft-parts. + """ + return project_yaml_data( + # string + version=1.0, + # string + icon=2, + # list[str] + website=[3.0, 4], + # dict[str, str] + environment={ + "float": 5.0, + "int": 6, + }, + parts={ + "p1": { + "plugin": "nil", + # string + "source-type": 7, + # string + "source-commit": 8.0, + # list[str] + "build-snaps": [9, 10.0], + # dict[str, str] + "build-environment": [ + {"float": 11.0}, + {"int": 12}, + ], + } + }, + ) + + class TestProjectDefaults: """Ensure unspecified items have the correct default value.""" @@ -701,6 +739,23 @@ def test_links_list(self, project_yaml_data): "https://github.com/NickvisionApps/Denaro", ] + def test_coerce_numbers(self, fake_project_with_numbers): + """Coerce numbers into strings.""" + project = Project.unmarshal(fake_project_with_numbers) + + assert project.version == "1.0" + assert project.icon == "2" + assert project.website == ["3.0", "4"] + assert project.environment == {"float": "5.0", "int": "6"} + # parts remain a dictionary with original types + assert project.parts["p1"]["source-type"] == 7 + assert project.parts["p1"]["source-commit"] == 8.0 + assert project.parts["p1"]["build-snaps"] == [9, 10.0] + assert project.parts["p1"]["build-environment"] == [ + {"float": 11.0}, + {"int": 12}, + ] + class TestHookValidation: """Validate hooks.""" @@ -1504,6 +1559,10 @@ def test_grammar_try(self, project_yaml_data): with pytest.raises(errors.ProjectValidationError, match=error): GrammarAwareProject.validate_grammar(data) + def test_grammar_number_coercion(self, fake_project_with_numbers): + """Ensure that grammar validation does not fail when coercing numbers into strings.""" + GrammarAwareProject.validate_grammar(fake_project_with_numbers) + def test_grammar_type_error(self, project_yaml_data): data = project_yaml_data( parts={ @@ -1516,7 +1575,7 @@ def test_grammar_type_error(self, project_yaml_data): } ) - error = r"value must be a str: \[25\]" + error = r"Input should be a valid string \(in field 'parts\.p1\.source\[0\]'\)" with pytest.raises(errors.ProjectValidationError, match=error): GrammarAwareProject.validate_grammar(data) diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index 3f507b8663..ad1dab8a0b 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -17,9 +17,11 @@ import json import os +import re import sys from textwrap import dedent +import craft_application.remote import craft_cli import craft_parts.plugins import craft_store @@ -398,7 +400,13 @@ def test_esm_error(snapcraft_yaml, base, monkeypatch, capsys): application.main() _, err = capsys.readouterr() - assert f"Base {base!r} is not supported by this version of Snapcraft" in err + + assert re.match( + rf"^Base {base!r} is not supported by this version of Snapcraft.\n" + rf"Recommended resolution: Use Snapcraft .* from the '.*' channel of snapcraft where {base!r} was last supported.\n" + r"For more information, check out: .*/reference/bases\n", + err, + ) @pytest.mark.parametrize("base", const.CURRENT_BASES) @@ -459,10 +467,12 @@ def test_run_remote_build_core24_error(monkeypatch, snapcraft_yaml, base, capsys application.main() _, err = capsys.readouterr() - assert ( - "'SNAPCRAFT_REMOTE_BUILD_STRATEGY=force-fallback' cannot be used " - "for core24 and newer snaps" - ) in err + assert re.match( + r"^'SNAPCRAFT_REMOTE_BUILD_STRATEGY=force-fallback' cannot be used for core24 and newer snaps\.\n" + r"Recommended resolution: Unset the environment variable or set it to 'disable-fallback'\.\n" + r"For more information, check out: .*/explanation/remote-build", + err, + ) @pytest.mark.parametrize("base", const.LEGACY_BASES) @@ -477,9 +487,11 @@ def test_run_envvar_disable_fallback_core20(snapcraft_yaml, base, monkeypatch, c application.main() _, err = capsys.readouterr() - assert ( - f"'SNAPCRAFT_REMOTE_BUILD_STRATEGY=disable-fallback' cannot be used for {base} snaps" - in err + assert re.match( + r"'SNAPCRAFT_REMOTE_BUILD_STRATEGY=disable-fallback' cannot be used for core20 snaps\.\n" + r"Recommended resolution: Unset the environment variable or set it to 'force-fallback'\.\n" + r"For more information, check out: .*/explanation/remote-build", + err, ) @@ -544,9 +556,11 @@ def test_run_envvar_invalid(snapcraft_yaml, base, monkeypatch, capsys): application.main() _, err = capsys.readouterr() - assert ( - "Unknown value 'badvalue' in environment variable 'SNAPCRAFT_REMOTE_BUILD_STRATEGY'" - in err + assert re.match( + r"Unknown value 'badvalue' in environment variable 'SNAPCRAFT_REMOTE_BUILD_STRATEGY'\.\n" + r"Recommended resolution: Valid values are 'disable-fallback' and 'force-fallback'\.\n" + r"For more information, check out: .*/explanation/remote-build", + err, ) @@ -665,7 +679,7 @@ def test_known_core24(snapcraft_yaml, base, build_base, is_known_core24): ) def test_store_error(mocker, capsys, message, resolution, expected_message): mocker.patch( - "snapcraft.application.Application.run", + "snapcraft.application.Application._run_inner", side_effect=craft_store.errors.CraftStoreError(message, resolution=resolution), ) @@ -673,12 +687,12 @@ def test_store_error(mocker, capsys, message, resolution, expected_message): assert return_code == 1 _, err = capsys.readouterr() - assert f"craft-store error: {expected_message}" in err + assert expected_message in err def test_store_key_error(mocker, capsys): mocker.patch( - "snapcraft.application.Application.run", + "snapcraft.application.Application._run_inner", side_effect=craft_store.errors.NoKeyringError(), ) @@ -692,7 +706,7 @@ def test_store_key_error(mocker, capsys): # pylint: disable=[line-too-long] dedent( """\ - craft-store error: No keyring found to store or retrieve credentials from. + No keyring found to store or retrieve credentials from. Recommended resolution: Ensure the keyring is working or SNAPCRAFT_STORE_CREDENTIALS is correctly exported into the environment For more information, check out: https://snapcraft.io/docs/snapcraft-authentication """ @@ -701,6 +715,25 @@ def test_store_key_error(mocker, capsys): ) +def test_remote_build_error(mocker, capsys): + """Catch remote build errors and include a documentation link.""" + mocker.patch( + "snapcraft.application.Application._run_inner", + side_effect=craft_application.remote.RemoteBuildError(message="test-error"), + ) + + return_code = application.main() + + assert return_code == 1 + _, err = capsys.readouterr() + + assert re.match( + r"^test-error\n" + r"For more information, check out: .*/explanation/remote-build\n", + err, + ) + + @pytest.mark.parametrize( "command", {