launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+427838@xxxxxxxxxxxxxxxxxx>]

Jürgen Gmach has proposed merging ~jugmac00/lpcraft:add-license-key-to-configuration into lpcraft:main.

Commit message:
Enable adding license information

... via the `.launchpad.yaml` configuration file.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jugmac00/lpcraft/+git/lpcraft/+merge/427838
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/lpcraft:add-license-key-to-configuration into lpcraft:main.

diff --git a/NEWS.rst b/NEWS.rst
index e1b73fd..8461350 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -5,7 +5,8 @@ Version history
 0.0.24 (unreleased)
 ===================
 
-- Nothing yet.
+- Enable adding license information via the `.launchpad.yaml` configuration
+  file.
 
 0.0.23 (2022-08-03)
 ===================
diff --git a/docs/configuration.rst b/docs/configuration.rst
index a020b07..ea5164c 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -33,6 +33,12 @@ Top-level configuration
      Mapping of job names (:ref:`identifiers <identifiers>`) to job
      definitions.
 
+``license`` (optional)
+     The :ref:`license <license-properties>` info for the given repository can
+     be configured either via an
+     `spdx identifier <https://spdx.org/licenses/>`_
+     or a relative path to the license file.
+
 Job definitions
 ---------------
 
@@ -178,3 +184,18 @@ More properties can be implemented on demand.
     which do not pass authentication checks. ``false`` does the opposite.
     By default APT decides whether a source is considered trusted. This third
     option cannot be set explicitly.
+
+
+.. _license-properties:
+
+License properties
+------------------
+
+Please note that either `spdx` or `path` is required.
+
+``spdx`` (optional)
+     A string representing a license,
+     see `spdx identifier <https://spdx.org/licenses/>`_.
+
+``path`` (optional)
+    A string with the relative path to the license file.
diff --git a/lpcraft/commands/run.py b/lpcraft/commands/run.py
index 131811f..3216d05 100644
--- a/lpcraft/commands/run.py
+++ b/lpcraft/commands/run.py
@@ -453,6 +453,19 @@ def _run_job(
                     remote_cwd=remote_cwd,
                     environment=environment,
                 )
+        if config.license:
+            if not job.output:
+                job.output = Output()
+                job.output.properties = dict()
+            values = config.license.dict()
+            # workaround necessary to please mypy
+            assert isinstance(job.output.properties, dict)
+            for key, value in values.items():
+                if "license" in job.output.properties:
+                    job.output.properties["license"][key] = value
+                else:
+                    job.output.properties["license"] = dict()
+                    job.output.properties["license"][key] = value
 
         if job.output is not None and output is not None:
             target_path = output / job_name / str(job_index)
diff --git a/lpcraft/commands/tests/test_run.py b/lpcraft/commands/tests/test_run.py
index 3dc6bbc..02b791a 100644
--- a/lpcraft/commands/tests/test_run.py
+++ b/lpcraft/commands/tests/test_run.py
@@ -2148,6 +2148,141 @@ class TestRun(RunBaseTestCase):
             file_contents,
         )
 
+    @patch("lpcraft.env.get_managed_environment_project_path")
+    @patch("lpcraft.commands.run.get_provider")
+    @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+    def test_license_field_spdx_gets_written_to_properties(
+        self,
+        mock_get_host_architecture,
+        mock_get_provider,
+        mock_get_project_path,
+    ):
+        target_path = Path(self.useFixture(TempDir()).path)
+        launcher = Mock(spec=launch)
+        provider = makeLXDProvider(lxd_launcher=launcher)
+        mock_get_provider.return_value = provider
+        execute_run = LocalExecuteRun(self.tmp_project_path)
+        launcher.return_value.execute_run = execute_run
+        mock_get_project_path.return_value = self.tmp_project_path
+        config = dedent(
+            """
+            pipeline:
+                - build
+
+            jobs:
+                build:
+                    series: focal
+                    architectures: amd64
+                    run: |
+                        true
+            license:
+                spdx: MIT
+            """
+        )
+        Path(".launchpad.yaml").write_text(config)
+
+        result = self.run_command(
+            "run", "--output-directory", str(target_path)
+        )
+
+        self.assertEqual(0, result.exit_code)
+        job_output = target_path / "build" / "0"
+        self.assertEqual(
+            {"license": {"spdx": "MIT", "path": None}},
+            json.loads((job_output / "properties").read_text()),
+        )
+
+    @patch("lpcraft.env.get_managed_environment_project_path")
+    @patch("lpcraft.commands.run.get_provider")
+    @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+    def test_license_field_path_gets_written_to_properties(
+        self,
+        mock_get_host_architecture,
+        mock_get_provider,
+        mock_get_project_path,
+    ):
+        target_path = Path(self.useFixture(TempDir()).path)
+        launcher = Mock(spec=launch)
+        provider = makeLXDProvider(lxd_launcher=launcher)
+        mock_get_provider.return_value = provider
+        execute_run = LocalExecuteRun(self.tmp_project_path)
+        launcher.return_value.execute_run = execute_run
+        mock_get_project_path.return_value = self.tmp_project_path
+        config = dedent(
+            """
+            pipeline:
+                - build
+
+            jobs:
+                build:
+                    series: focal
+                    architectures: amd64
+                    run: |
+                        true
+            license:
+                path: LICENSE.txt
+            """
+        )
+        Path(".launchpad.yaml").write_text(config)
+
+        result = self.run_command(
+            "run", "--output-directory", str(target_path)
+        )
+
+        self.assertEqual(0, result.exit_code)
+        job_output = target_path / "build" / "0"
+        self.assertEqual(
+            {"license": {"path": "LICENSE.txt", "spdx": None}},
+            json.loads((job_output / "properties").read_text()),
+        )
+
+    @patch("lpcraft.env.get_managed_environment_project_path")
+    @patch("lpcraft.commands.run.get_provider")
+    @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64")
+    def test_license_field_works_also_with_other_properties(
+        self,
+        mock_get_host_architecture,
+        mock_get_provider,
+        mock_get_project_path,
+    ):
+        target_path = Path(self.useFixture(TempDir()).path)
+        launcher = Mock(spec=launch)
+        provider = makeLXDProvider(lxd_launcher=launcher)
+        mock_get_provider.return_value = provider
+        execute_run = LocalExecuteRun(self.tmp_project_path)
+        launcher.return_value.execute_run = execute_run
+        mock_get_project_path.return_value = self.tmp_project_path
+        config = dedent(
+            """
+            pipeline:
+                - build
+
+            jobs:
+                build:
+                    series: focal
+                    architectures: amd64
+                    run: |
+                        true
+                    output:
+                        properties:
+                            foo: bar
+            license:
+                path: LICENSE.txt
+            """
+        )
+        Path(".launchpad.yaml").write_text(config)
+
+        result = self.run_command(
+            "run", "--output-directory", str(target_path)
+        )
+
+        self.assertEqual(0, result.exit_code)
+        job_output = target_path / "build" / "0"
+        self.assertEqual(
+            {"foo": "bar", "license": {"path": "LICENSE.txt", "spdx": None}},
+            json.loads((job_output / "properties").read_text()),
+        )
+
 
 class TestRunOne(RunBaseTestCase):
     def test_config_file_not_under_project_directory(self):
diff --git a/lpcraft/config.py b/lpcraft/config.py
index 328cd56..2615cbb 100644
--- a/lpcraft/config.py
+++ b/lpcraft/config.py
@@ -30,7 +30,7 @@ class _Identifier(pydantic.ConstrainedStr):
 class ModelConfigDefaults(
     pydantic.BaseModel,
     extra=pydantic.Extra.forbid,
-    frozen=True,
+    # frozen=True,
     alias_generator=lambda s: s.replace("_", "-"),
     underscore_attrs_are_private=True,
 ):
@@ -49,7 +49,7 @@ class Output(ModelConfigDefaults):
     paths: Optional[List[StrictStr]]
     distribute: Optional[OutputDistributeEnum]
     channels: Optional[List[StrictStr]]
-    properties: Optional[Dict[StrictStr, StrictStr]]
+    properties: Optional[Dict[StrictStr, Any]]
     dynamic_properties: Optional[Path]
     expires: Optional[timedelta]
 
@@ -205,11 +205,38 @@ def _expand_job_values(
     return expanded_values
 
 
+class License(ModelConfigDefaults):
+    """A representation of a license."""
+
+    # We do not need to check that at least one value is set, as currently
+    # there are only these two values, no others. That means not setting any of
+    # them will not result in the creation of a `License` object.
+    # Once we have more fields, we need to add e.g. a root validator, see
+    # https://stackoverflow.com/questions/58958970
+
+    # XXX jugmac00 2022-08-03: add validator for spdx identifier
+    # XXX jugmac00 2022-08-04: add validator for path
+
+    spdx: Optional[StrictStr] = None
+    path: Optional[StrictStr] = None
+
+    @validator("path", always=True)
+    def disallow_setting_both_sources(
+        cls, path: str, values: Dict[str, str]
+    ) -> str:
+        if values.get("spdx") and path:
+            raise ValueError(
+                "You cannot set `spdx` and `path` at the same time."
+            )
+        return path
+
+
 class Config(ModelConfigDefaults):
     """A .launchpad.yaml configuration file."""
 
     pipeline: List[List[_Identifier]]
     jobs: Dict[StrictStr, List[Job]]
+    license: Optional[License]
 
     @pydantic.validator("pipeline", pre=True)
     def validate_pipeline(
diff --git a/lpcraft/tests/test_config.py b/lpcraft/tests/test_config.py
index 575f284..2ac30e9 100644
--- a/lpcraft/tests/test_config.py
+++ b/lpcraft/tests/test_config.py
@@ -526,3 +526,71 @@ class TestConfig(TestCase):
             ],  # noqa: E501
             list(repositories[2].sources_list_lines()),
         )
+
+    def test_specify_license_via_spdx(self):
+        path = self.create_config(
+            dedent(
+                """
+                pipeline:
+                    - test
+
+                jobs:
+                    test:
+                        series: focal
+                        architectures: amd64
+                license:
+                    spdx: "MIT"
+                """  # noqa: E501
+            )
+        )
+        config = Config.load(path)
+
+        # workaround necessary to please mypy
+        assert config.license is not None
+        self.assertEqual("MIT", config.license.spdx)
+
+    def test_specify_license_via_path(self):
+        path = self.create_config(
+            dedent(
+                """
+                pipeline:
+                    - test
+
+                jobs:
+                    test:
+                        series: focal
+                        architectures: amd64
+                license:
+                    path: LICENSE.txt
+                """  # noqa: E501
+            )
+        )
+        config = Config.load(path)
+
+        # workaround necessary to please mypy
+        assert config.license is not None
+        self.assertEqual("LICENSE.txt", config.license.path)
+
+    def test_license_setting_both_sources_not_allowed(self):
+        path = self.create_config(
+            dedent(
+                """
+                pipeline:
+                    - test
+
+                jobs:
+                    test:
+                        series: focal
+                        architectures: amd64
+                license:
+                    spdx: MIT
+                    path: LICENSE.txt
+                """  # noqa: E501
+            )
+        )
+        self.assertRaisesRegex(
+            ValidationError,
+            "You cannot set `spdx` and `path` at the same time.",
+            Config.load,
+            path,
+        )