parts: allow debug shell after the lifecycle (#3895)

Shell into the instance if --debug is used and a packing error happens
after the execution of the parts lifecycle.

Signed-off-by: Claudio Matsuoka <[email protected]>

Additional details to snap pack error declared in SnapcraftError's
`details` field.

Signed-off-by: Claudio Matsuoka <[email protected]>

pyproject.toml

@@ -38,6 +38,7 @@ disable = "too-few-public-methods,fixme,use-implicit-booleaness-not-comparison,d
 max-attributes = 15
 max-args = 6
 max-locals = 20
+max-branches = 16
 good-names = "id"
 
 [tool.pylint.MASTER]

snapcraft/parts/lifecycle.py

@@ -45,7 +45,7 @@
 )
 
 from . import grammar, yaml_utils
-from .parts import PartsLifecycle
+from .parts import PartsLifecycle, launch_shell
 from .project_check import run_project_checks
 from .setup_assets import setup_assets
 from .update_metadata import update_project_metadata
@@ -197,18 +197,15 @@ def run(command_name: str, parsed_args: "argparse.Namespace") -> None:
         )
         project = Project.unmarshal(yaml_data_for_arch)
 
-        try:
-            _run_command(
-                command_name,
-                project=project,
-                parse_info=parse_info,
-                parallel_build_count=build_count,
-                assets_dir=snap_project.assets_dir,
-                start_time=start_time,
-                parsed_args=parsed_args,
-            )
-        except PermissionError as err:
-            raise errors.FilePermissionError(err.filename, reason=err.strerror)
+        _run_command(
+            command_name,
+            project=project,
+            parse_info=parse_info,
+            parallel_build_count=build_count,
+            assets_dir=snap_project.assets_dir,
+            start_time=start_time,
+            parsed_args=parsed_args,
+        )
 
 
 def _run_command(
@@ -279,15 +276,59 @@ def _run_command(
         lifecycle.clean(part_names=part_names)
         return
 
+    try:
+        _run_lifecycle_and_pack(
+            lifecycle,
+            command_name=command_name,
+            step_name=step_name,
+            project=project,
+            project_dir=project_dir,
+            assets_dir=assets_dir,
+            start_time=start_time,
+            parsed_args=parsed_args,
+        )
+    except PermissionError as err:
+        if parsed_args.debug:
+            emit.progress(str(err), permanent=True)
+            launch_shell()
+        raise errors.FilePermissionError(err.filename, reason=err.strerror)
+    except OSError as err:
+        msg = err.strerror
+        if err.filename:
+            msg = f"{err.filename}: {msg}"
+        if parsed_args.debug:
+            emit.progress(msg, permanent=True)
+            launch_shell()
+        raise errors.SnapcraftError(msg) from err
+    except Exception as err:
+        if parsed_args.debug:
+            emit.progress(str(err), permanent=True)
+            launch_shell()
+        raise errors.SnapcraftError(str(err)) from err
+
+
+def _run_lifecycle_and_pack(
+    lifecycle: PartsLifecycle,
+    *,
+    command_name: str,
+    step_name: str,
+    project: Project,
+    project_dir: Path,
+    assets_dir: Path,
+    start_time: datetime,
+    parsed_args: "argparse.Namespace",
+) -> None:
+    """Execute the parts lifecycle, generate metadata, and create the snap."""
     lifecycle.run(
         step_name,
-        debug=parsed_args.debug,
         shell=getattr(parsed_args, "shell", False),
         shell_after=getattr(parsed_args, "shell_after", False),
     )
 
     # Extract metadata and generate snap.yaml
     project_vars = lifecycle.project_vars
+    part_names = getattr(parsed_args, "part_names", None)
+
     if step_name == "prime" and not part_names:
         emit.progress("Extracting and updating metadata...")
         metadata_list = lifecycle.extract_metadata()
@@ -449,7 +490,11 @@ def _run_in_provider(
         except subprocess.CalledProcessError as err:
             capture_logs_from_instance(instance)
             raise errors.SnapcraftError(
-                f"Failed to execute {command_name} in instance."
+                f"Failed to execute {command_name} in instance.",
+                details=(
+                    "Run the same command again with --debug to shell into "
+                    "the environment if you wish to introspect this failure."
+                ),
             ) from err
 
 

snapcraft/parts/parts.py

@@ -131,7 +131,6 @@ def run(
         self,
         step_name: str,
         *,
-        debug: bool = False,
         shell: bool = False,
         shell_after: bool = False,
     ) -> None:
@@ -171,21 +170,17 @@ def run(
                     emit.progress(f"Executed: {message}", permanent=True)
 
             if shell_after:
-                _launch_shell()
+                launch_shell()
 
             emit.progress("Executed parts lifecycle", permanent=True)
         except RuntimeError as err:
             raise RuntimeError(f"Parts processing internal error: {err}") from err
         except OSError as err:
-            if debug:
-                _launch_shell()
             msg = err.strerror
             if err.filename:
                 msg = f"{err.filename}: {msg}"
             raise errors.PartsLifecycleError(msg) from err
         except Exception as err:
-            if debug:
-                _launch_shell()
             raise errors.PartsLifecycleError(str(err)) from err
 
     def _install_package_repositories(self) -> None:
@@ -271,7 +266,7 @@ def get_part_pull_assets(self, *, part_name: str) -> Optional[Dict[str, Any]]:
         return self._lcm.get_pull_assets(part_name=part_name)
 
 
-def _launch_shell(*, cwd: Optional[pathlib.Path] = None) -> None:
+def launch_shell(*, cwd: Optional[pathlib.Path] = None) -> None:
     """Launch a user shell for debugging environment.
 
     :param cwd: Working directory to start user in.

tests/unit/parts/test_lifecycle.py

@@ -192,14 +192,14 @@ def test_lifecycle_legacy_run_provider(cmd, snapcraft_yaml, new_dir, mocker):
         ("prime", "prime"),
     ],
 )
-@pytest.mark.parametrize("debug_shell", [None, "debug", "shell", "shell_after"])
+@pytest.mark.parametrize("debug_shell", [None, "shell", "shell_after"])
 def test_lifecycle_run_command_step(
     cmd, step, debug_shell, 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")
-    pack_mock = mocker.patch("snapcraft.pack.pack_snap")
+    mocker.patch("snapcraft.pack.pack_snap")
 
     parsed_args = argparse.Namespace(
         debug=False,
@@ -224,12 +224,11 @@ def test_lifecycle_run_command_step(
         parsed_args=parsed_args,
     )
 
-    call_args = {"debug": False, "shell": False, "shell_after": False}
+    call_args = {"shell": False, "shell_after": False}
     if debug_shell:
         call_args[debug_shell] = True
 
     assert run_mock.mock_calls == [call(step, **call_args)]
-    assert pack_mock.mock_calls == []
 
 
 @pytest.mark.parametrize("cmd", ["pack", "snap"])
@@ -259,9 +258,7 @@ def test_lifecycle_run_command_pack(cmd, snapcraft_yaml, project_vars, new_dir,
         ),
     )
 
-    assert run_mock.mock_calls == [
-        call("prime", debug=False, shell=False, shell_after=False)
-    ]
+    assert run_mock.mock_calls == [call("prime", shell=False, shell_after=False)]
     assert pack_mock.mock_calls[:1] == [
         call(
             new_dir / "prime",
@@ -310,9 +307,7 @@ def test_lifecycle_pack_destructive_mode(
     )
 
     assert run_in_provider_mock.mock_calls == []
-    assert run_mock.mock_calls == [
-        call("prime", debug=False, shell=False, shell_after=False)
-    ]
+    assert run_mock.mock_calls == [call("prime", shell=False, shell_after=False)]
     assert pack_mock.mock_calls[:1] == [
         call(
             new_dir / "home/prime",
@@ -362,9 +357,7 @@ 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", debug=False, shell=False, shell_after=False)
-    ]
+    assert run_mock.mock_calls == [call("prime", shell=False, shell_after=False)]
     assert pack_mock.mock_calls[:1] == [
         call(
             new_dir / "home/prime",
@@ -456,9 +449,7 @@ 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", debug=False, shell=False, shell_after=False)
-    ]
+    assert run_mock.mock_calls == [call("prime", shell=False, shell_after=False)]
     assert pack_mock.mock_calls == []
 
 
@@ -637,10 +628,10 @@ def test_lifecycle_clean_managed(snapcraft_yaml, project_vars, new_dir, mocker):
 def test_lifecycle_debug_shell(snapcraft_yaml, cmd, new_dir, mocker):
     """Adoptable fields shouldn't be empty after adoption."""
     mocker.patch("craft_parts.executor.Executor.execute", side_effect=Exception)
-    mock_shell = mocker.patch("subprocess.run")
+    mock_shell = mocker.patch("snapcraft.parts.lifecycle.launch_shell")
     project = Project.unmarshal(snapcraft_yaml(base="core22"))
 
-    with pytest.raises(errors.PartsLifecycleError):
+    with pytest.raises(errors.SnapcraftError):
         parts_lifecycle._run_command(
             cmd,
             project=project,
@@ -656,11 +647,43 @@ def test_lifecycle_debug_shell(snapcraft_yaml, cmd, new_dir, mocker):
                 shell=False,
                 shell_after=False,
                 use_lxd=False,
+                enable_manifest=False,
                 parts=["part1"],
             ),
         )
 
-    assert mock_shell.mock_calls == [call(["bash"], check=False, cwd=None)]
+    assert mock_shell.mock_calls == [call()]
+
+
+def test_lifecycle_post_lifecycle_debug_shell(snapcraft_yaml, new_dir, mocker):
+    """Adoptable fields shouldn't be empty after adoption."""
+    mocker.patch("snapcraft.pack.pack_snap", side_effect=Exception)
+    mocker.patch("snapcraft.meta.snap_yaml.write")
+    mock_shell = mocker.patch("snapcraft.parts.lifecycle.launch_shell")
+    project = Project.unmarshal(snapcraft_yaml(base="core22"))
+
+    with pytest.raises(errors.SnapcraftError):
+        parts_lifecycle._run_command(
+            "pack",
+            project=project,
+            parse_info={},
+            assets_dir=Path(),
+            start_time=datetime.now(),
+            parallel_build_count=8,
+            parsed_args=argparse.Namespace(
+                directory=None,
+                output=None,
+                debug=True,
+                destructive_mode=True,
+                shell=False,
+                shell_after=False,
+                use_lxd=False,
+                enable_manifest=False,
+                parts=["part1"],
+            ),
+        )
+
+    assert mock_shell.mock_calls == [()]
 
 
 @pytest.mark.parametrize("cmd", ["pull", "build", "stage", "prime"])
@@ -691,6 +714,7 @@ def _fake_execute(_, action: Action, **kwargs):  # pylint: disable=unused-argume
             shell=True,
             shell_after=False,
             use_lxd=False,
+            enable_manifest=False,
             parts=["part1"],
         ),
     )
@@ -735,6 +759,7 @@ def _fake_execute(_, action: Action, **kwargs):  # pylint: disable=unused-argume
             shell=False,
             shell_after=True,
             use_lxd=False,
+            enable_manifest=False,
             parts=["part1"],
         ),
     )