fix(remotebuild): error early for multiple artifacts per build-on (#5005

)

Launchpad cannot build multiple artifacts on the same build-on
architecture and will fail with an esoteric error.

Snapcraft now checks for the scenario and fails with an informative
error before the build begins.

Signed-off-by: Callahan Kovacs <callahan.kovacs@canonical.com>

requirements-devel.txt

@@ -117,6 +117,7 @@ pyramid==2.0.2
 pyRFC3339==1.1
 pyspelling==2.10
 pytest==8.3.2
+pytest-check==2.4.1
 pytest-cov==5.0.0
 pytest-mock==3.14.0
 pytest-subprocess==1.5.2

setup.py

@@ -80,6 +80,7 @@ def recursive_data_files(directory, install_directory):
     "pyramid",
     "pytest",
     "pytest-cov",
+    "pytest-check",
     "pytest-mock",
     "pytest-subprocess",
     "tox>=4.5",

snapcraft/commands/remote.py

@@ -201,6 +201,41 @@ def _validate(self, parsed_args: argparse.Namespace) -> None:
                 retcode=os.EX_CONFIG,
             )
 
+        self._validate_single_artifact_per_build_on()
+
+    def _validate_single_artifact_per_build_on(self) -> None:
+        """Validate that only one artifact will be created for each build-on.
+
+        :raise RemoteBuildError: If multiple artifacts will be created for the same build-on.
+        """
+        # mapping of `build-on` to `build-for` architectures
+        build_map: dict[str, list[str]] = {}
+        for build_info in self.build_plan:
+            build_map.setdefault(build_info.build_on, []).append(build_info.build_for)
+
+        # assemble a list so all errors are shown at once
+        build_on_errors = list()
+        for build_on, build_fors in build_map.items():
+            if len(build_fors) > 1:
+                build_on_errors.append(
+                    f"\n  - Building on {build_on!r} will create snaps for "
+                    f"{humanize_list(build_fors, 'and')}."
+                )
+
+        if build_on_errors:
+            raise errors.RemoteBuildError(
+                message=(
+                    "Remote build does not support building multiple snaps on the "
+                    f"same architecture:{''.join(build_on_errors)}"
+                ),
+                resolution=(
+                    "Ensure that only one snap will be created for each build-on "
+                    "architecture."
+                ),
+                doc_slug="/explanation/remote-build.html",
+                retcode=os.EX_CONFIG,
+            )
+
     def _run(  # noqa: PLR0915 [too-many-statements]
         self, parsed_args: argparse.Namespace, **kwargs: Any
     ) -> int | None:
@@ -220,6 +255,12 @@ def _run(  # noqa: PLR0915 [too-many-statements]
 
         builder = self._services.remote_build
         self.project = cast(models.Project, self._services.project)
+
+        self.build_plan = self._app.BuildPlannerClass.unmarshal(
+            self.project.marshal()
+        ).get_build_plan()
+        emit.debug(f"Build plan: {self.build_plan}")
+
         config = cast(dict[str, Any], self.config)
         project_dir = (
             Path(config.get("global_args", {}).get("project_dir") or ".")
@@ -365,12 +406,7 @@ def _get_project_build_fors(self) -> list[str]:
 
         :returns: A list of architectures.
         """
-        build_plan = self._app.BuildPlannerClass.unmarshal(
-            self.project.marshal()
-        ).get_build_plan()
-        emit.debug(f"Build plan: {build_plan}")
-
-        build_fors = set({build_info.build_for for build_info in build_plan})
+        build_fors = set({build_info.build_for for build_info in self.build_plan})
         emit.debug(f"Parsed build-for architectures from build plan: {build_fors}")
 
         return list(build_fors)

tests/spread/core24/remote-build/snaps/platforms/snapcraft.yaml

@@ -8,7 +8,6 @@ grade: stable
 confinement: strict
 
 # This does not test:
-# - multiple artefacts on the same build-on (#4995)
 # - cross-compiling (#4996)
 
 platforms:

tests/unit/commands/test_remote.py

@@ -769,6 +769,125 @@ def test_platform_core22_error(
     assert "Use '--build-for' instead." in err
 
 
+@pytest.mark.parametrize(
+    ("base", "build_info", "error_messages"),
+    [
+        pytest.param(
+            "core22",
+            {
+                "architectures": [
+                    {"build-on": "amd64", "build-for": "amd64"},
+                    {"build-on": ["amd64", "riscv64"], "build-for": "riscv64"},
+                ],
+            },
+            ["Building on 'amd64' will create snaps for 'amd64' and 'riscv64'."],
+            id="core22-simple",
+        ),
+        pytest.param(
+            "core22",
+            {
+                "architectures": [
+                    {"build-on": "amd64", "build-for": "amd64"},
+                    {"build-on": ["amd64", "riscv64"], "build-for": "riscv64"},
+                    {"build-on": "s390x", "build-for": "s390x"},
+                    {"build-on": "s390x", "build-for": "ppc64el"},
+                ],
+            },
+            [
+                "Building on 'amd64' will create snaps for 'amd64' and 'riscv64'.",
+                "Building on 's390x' will create snaps for 'ppc64el' and 's390x'.",
+            ],
+            id="core22-complex",
+        ),
+        pytest.param(
+            "core24",
+            {
+                "platforms": {
+                    "platform1": {"build-on": "amd64", "build-for": "amd64"},
+                    "platform2": {
+                        "build-on": ["amd64", "riscv64"],
+                        "build-for": "riscv64",
+                    },
+                },
+            },
+            ["Building on 'amd64' will create snaps for 'amd64' and 'riscv64'."],
+            id="core24-simple",
+        ),
+        pytest.param(
+            "core24",
+            {
+                "platforms": {
+                    "platform1": {"build-on": "amd64", "build-for": "riscv64"},
+                    "identical-platform": {"build-on": "amd64", "build-for": "riscv64"},
+                },
+            },
+            # this will show 'riscv64' twice because the platform name is different
+            ["Building on 'amd64' will create snaps for 'riscv64' and 'riscv64'."],
+            id="core24-same-architectures-different-platform-name",
+        ),
+        pytest.param(
+            "core24",
+            {
+                "platforms": {
+                    "amd64": None,
+                    "riscv64": {"build-on": "amd64", "build-for": "riscv64"},
+                },
+            },
+            ["Building on 'amd64' will create snaps for 'amd64' and 'riscv64'."],
+            id="core24-shorthand",
+        ),
+        pytest.param(
+            "core24",
+            {
+                "platforms": {
+                    "platform1": {"build-on": "amd64", "build-for": "amd64"},
+                    "platform2": {
+                        "build-on": ["amd64", "s390x"],
+                        "build-for": "riscv64",
+                    },
+                    "s390x": None,
+                    "platform4": {"build-on": "s390x", "build-for": "riscv64"},
+                },
+            },
+            [
+                "Building on 'amd64' will create snaps for 'amd64' and 'riscv64'.",
+                "Building on 's390x' will create snaps for 'riscv64', 'riscv64', and 's390x'.",
+            ],
+            id="core24-complex",
+        ),
+    ],
+)
+def test_multiple_artifacts_per_build_on(
+    check,
+    base,
+    build_info,
+    error_messages,
+    capsys,
+    mocker,
+    snapcraft_yaml,
+    fake_services,
+    mock_confirm,
+    mock_remote_builder_fake_build_process,
+):
+    """Error when multiple artifacts will be produced on one build-on architecture."""
+    snapcraft_yaml(**{"base": base, **build_info})
+    mocker.patch.object(sys, "argv", ["snapcraft", "remote-build"])
+    mocker.patch(
+        "craft_application.services.remotebuild.RemoteBuildService.start_builds"
+    )
+    app = application.create_app()
+    assert app.run() == os.EX_CONFIG
+
+    _, err = capsys.readouterr()
+
+    check.is_in(
+        "Remote build does not support building multiple snaps on the same architecture",
+        err,
+    )
+    for message in error_messages:
+        check.is_in(message, err)
+
+
 ########################
 # Remote builder tests #
 ########################