Merge ~jugmac00/lpci:add-license-key-to-configuration into lpci:main
- Git
- lp:~jugmac00/lpci
- add-license-key-to-configuration
- Merge into main
Proposed by
Jürgen Gmach
Status: | Merged |
---|---|
Merged at revision: | cbb8798e803f0bd87801c579fe5c433998569a61 |
Proposed branch: | ~jugmac00/lpci:add-license-key-to-configuration |
Merge into: | lpci:main |
Diff against target: |
494 lines (+401/-3) 6 files modified
NEWS.rst (+2/-1) docs/configuration.rst (+21/-0) lpcraft/commands/run.py (+11/-0) lpcraft/commands/tests/test_run.py (+270/-0) lpcraft/config.py (+29/-2) lpcraft/tests/test_config.py (+68/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Approve | ||
Review via email: mp+427838@code.launchpad.net |
Commit message
Enable adding license information
... via the `.launchpad.yaml` configuration file.
Description of the change
To post a comment you must log in.
Revision history for this message
Jürgen Gmach (jugmac00) : | # |
Revision history for this message
Jürgen Gmach (jugmac00) wrote : | # |
Revision history for this message
Jürgen Gmach (jugmac00) wrote : | # |
Ready to review.
Revision history for this message
Colin Watson (cjwatson) : | # |
review:
Approve
Revision history for this message
Jürgen Gmach (jugmac00) : | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/NEWS.rst b/NEWS.rst |
2 | index e1b73fd..8461350 100644 |
3 | --- a/NEWS.rst |
4 | +++ b/NEWS.rst |
5 | @@ -5,7 +5,8 @@ Version history |
6 | 0.0.24 (unreleased) |
7 | =================== |
8 | |
9 | -- Nothing yet. |
10 | +- Enable adding license information via the `.launchpad.yaml` configuration |
11 | + file. |
12 | |
13 | 0.0.23 (2022-08-03) |
14 | =================== |
15 | diff --git a/docs/configuration.rst b/docs/configuration.rst |
16 | index a020b07..ea5164c 100644 |
17 | --- a/docs/configuration.rst |
18 | +++ b/docs/configuration.rst |
19 | @@ -33,6 +33,12 @@ Top-level configuration |
20 | Mapping of job names (:ref:`identifiers <identifiers>`) to job |
21 | definitions. |
22 | |
23 | +``license`` (optional) |
24 | + The :ref:`license <license-properties>` info for the given repository can |
25 | + be configured either via an |
26 | + `spdx identifier <https://spdx.org/licenses/>`_ |
27 | + or a relative path to the license file. |
28 | + |
29 | Job definitions |
30 | --------------- |
31 | |
32 | @@ -178,3 +184,18 @@ More properties can be implemented on demand. |
33 | which do not pass authentication checks. ``false`` does the opposite. |
34 | By default APT decides whether a source is considered trusted. This third |
35 | option cannot be set explicitly. |
36 | + |
37 | + |
38 | +.. _license-properties: |
39 | + |
40 | +License properties |
41 | +------------------ |
42 | + |
43 | +Please note that either `spdx` or `path` is required. |
44 | + |
45 | +``spdx`` (optional) |
46 | + A string representing a license, |
47 | + see `spdx identifier <https://spdx.org/licenses/>`_. |
48 | + |
49 | +``path`` (optional) |
50 | + A string with the relative path to the license file. |
51 | diff --git a/lpcraft/commands/run.py b/lpcraft/commands/run.py |
52 | index 131811f..cab092b 100644 |
53 | --- a/lpcraft/commands/run.py |
54 | +++ b/lpcraft/commands/run.py |
55 | @@ -453,6 +453,17 @@ def _run_job( |
56 | remote_cwd=remote_cwd, |
57 | environment=environment, |
58 | ) |
59 | + if config.license: |
60 | + if not job.output: |
61 | + job.output = Output() |
62 | + job.output.properties = dict() |
63 | + values = config.license.dict() |
64 | + # workaround necessary to please mypy |
65 | + assert isinstance(job.output.properties, dict) |
66 | + for key, value in values.items(): |
67 | + if "license" not in job.output.properties: |
68 | + job.output.properties["license"] = dict() |
69 | + job.output.properties["license"][key] = value |
70 | |
71 | if job.output is not None and output is not None: |
72 | target_path = output / job_name / str(job_index) |
73 | diff --git a/lpcraft/commands/tests/test_run.py b/lpcraft/commands/tests/test_run.py |
74 | index 3dc6bbc..b2d7f37 100644 |
75 | --- a/lpcraft/commands/tests/test_run.py |
76 | +++ b/lpcraft/commands/tests/test_run.py |
77 | @@ -2148,6 +2148,141 @@ class TestRun(RunBaseTestCase): |
78 | file_contents, |
79 | ) |
80 | |
81 | + @patch("lpcraft.env.get_managed_environment_project_path") |
82 | + @patch("lpcraft.commands.run.get_provider") |
83 | + @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64") |
84 | + def test_license_field_spdx_gets_written_to_properties( |
85 | + self, |
86 | + mock_get_host_architecture, |
87 | + mock_get_provider, |
88 | + mock_get_project_path, |
89 | + ): |
90 | + target_path = Path(self.useFixture(TempDir()).path) |
91 | + launcher = Mock(spec=launch) |
92 | + provider = makeLXDProvider(lxd_launcher=launcher) |
93 | + mock_get_provider.return_value = provider |
94 | + execute_run = LocalExecuteRun(self.tmp_project_path) |
95 | + launcher.return_value.execute_run = execute_run |
96 | + mock_get_project_path.return_value = self.tmp_project_path |
97 | + config = dedent( |
98 | + """ |
99 | + pipeline: |
100 | + - build |
101 | + |
102 | + jobs: |
103 | + build: |
104 | + series: focal |
105 | + architectures: amd64 |
106 | + run: | |
107 | + true |
108 | + license: |
109 | + spdx: MIT |
110 | + """ |
111 | + ) |
112 | + Path(".launchpad.yaml").write_text(config) |
113 | + |
114 | + result = self.run_command( |
115 | + "run", "--output-directory", str(target_path) |
116 | + ) |
117 | + |
118 | + self.assertEqual(0, result.exit_code) |
119 | + job_output = target_path / "build" / "0" |
120 | + self.assertEqual( |
121 | + {"license": {"spdx": "MIT", "path": None}}, |
122 | + json.loads((job_output / "properties").read_text()), |
123 | + ) |
124 | + |
125 | + @patch("lpcraft.env.get_managed_environment_project_path") |
126 | + @patch("lpcraft.commands.run.get_provider") |
127 | + @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64") |
128 | + def test_license_field_path_gets_written_to_properties( |
129 | + self, |
130 | + mock_get_host_architecture, |
131 | + mock_get_provider, |
132 | + mock_get_project_path, |
133 | + ): |
134 | + target_path = Path(self.useFixture(TempDir()).path) |
135 | + launcher = Mock(spec=launch) |
136 | + provider = makeLXDProvider(lxd_launcher=launcher) |
137 | + mock_get_provider.return_value = provider |
138 | + execute_run = LocalExecuteRun(self.tmp_project_path) |
139 | + launcher.return_value.execute_run = execute_run |
140 | + mock_get_project_path.return_value = self.tmp_project_path |
141 | + config = dedent( |
142 | + """ |
143 | + pipeline: |
144 | + - build |
145 | + |
146 | + jobs: |
147 | + build: |
148 | + series: focal |
149 | + architectures: amd64 |
150 | + run: | |
151 | + true |
152 | + license: |
153 | + path: LICENSE.txt |
154 | + """ |
155 | + ) |
156 | + Path(".launchpad.yaml").write_text(config) |
157 | + |
158 | + result = self.run_command( |
159 | + "run", "--output-directory", str(target_path) |
160 | + ) |
161 | + |
162 | + self.assertEqual(0, result.exit_code) |
163 | + job_output = target_path / "build" / "0" |
164 | + self.assertEqual( |
165 | + {"license": {"path": "LICENSE.txt", "spdx": None}}, |
166 | + json.loads((job_output / "properties").read_text()), |
167 | + ) |
168 | + |
169 | + @patch("lpcraft.env.get_managed_environment_project_path") |
170 | + @patch("lpcraft.commands.run.get_provider") |
171 | + @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64") |
172 | + def test_license_field_works_also_with_other_properties( |
173 | + self, |
174 | + mock_get_host_architecture, |
175 | + mock_get_provider, |
176 | + mock_get_project_path, |
177 | + ): |
178 | + target_path = Path(self.useFixture(TempDir()).path) |
179 | + launcher = Mock(spec=launch) |
180 | + provider = makeLXDProvider(lxd_launcher=launcher) |
181 | + mock_get_provider.return_value = provider |
182 | + execute_run = LocalExecuteRun(self.tmp_project_path) |
183 | + launcher.return_value.execute_run = execute_run |
184 | + mock_get_project_path.return_value = self.tmp_project_path |
185 | + config = dedent( |
186 | + """ |
187 | + pipeline: |
188 | + - build |
189 | + |
190 | + jobs: |
191 | + build: |
192 | + series: focal |
193 | + architectures: amd64 |
194 | + run: | |
195 | + true |
196 | + output: |
197 | + properties: |
198 | + foo: bar |
199 | + license: |
200 | + path: LICENSE.txt |
201 | + """ |
202 | + ) |
203 | + Path(".launchpad.yaml").write_text(config) |
204 | + |
205 | + result = self.run_command( |
206 | + "run", "--output-directory", str(target_path) |
207 | + ) |
208 | + |
209 | + self.assertEqual(0, result.exit_code) |
210 | + job_output = target_path / "build" / "0" |
211 | + self.assertEqual( |
212 | + {"foo": "bar", "license": {"path": "LICENSE.txt", "spdx": None}}, |
213 | + json.loads((job_output / "properties").read_text()), |
214 | + ) |
215 | + |
216 | |
217 | class TestRunOne(RunBaseTestCase): |
218 | def test_config_file_not_under_project_directory(self): |
219 | @@ -3031,3 +3166,138 @@ class TestRunOne(RunBaseTestCase): |
220 | ), |
221 | file_contents, |
222 | ) |
223 | + |
224 | + @patch("lpcraft.env.get_managed_environment_project_path") |
225 | + @patch("lpcraft.commands.run.get_provider") |
226 | + @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64") |
227 | + def test_license_field_spdx_gets_written_to_properties( |
228 | + self, |
229 | + mock_get_host_architecture, |
230 | + mock_get_provider, |
231 | + mock_get_project_path, |
232 | + ): |
233 | + target_path = Path(self.useFixture(TempDir()).path) |
234 | + launcher = Mock(spec=launch) |
235 | + provider = makeLXDProvider(lxd_launcher=launcher) |
236 | + mock_get_provider.return_value = provider |
237 | + execute_run = LocalExecuteRun(self.tmp_project_path) |
238 | + launcher.return_value.execute_run = execute_run |
239 | + mock_get_project_path.return_value = self.tmp_project_path |
240 | + config = dedent( |
241 | + """ |
242 | + pipeline: |
243 | + - build |
244 | + |
245 | + jobs: |
246 | + build: |
247 | + series: focal |
248 | + architectures: amd64 |
249 | + run: | |
250 | + true |
251 | + license: |
252 | + spdx: MIT |
253 | + """ |
254 | + ) |
255 | + Path(".launchpad.yaml").write_text(config) |
256 | + |
257 | + result = self.run_command( |
258 | + "run-one", "--output-directory", str(target_path), "build", "0" |
259 | + ) |
260 | + |
261 | + self.assertEqual(0, result.exit_code) |
262 | + job_output = target_path / "build" / "0" |
263 | + self.assertEqual( |
264 | + {"license": {"spdx": "MIT", "path": None}}, |
265 | + json.loads((job_output / "properties").read_text()), |
266 | + ) |
267 | + |
268 | + @patch("lpcraft.env.get_managed_environment_project_path") |
269 | + @patch("lpcraft.commands.run.get_provider") |
270 | + @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64") |
271 | + def test_license_field_path_gets_written_to_properties( |
272 | + self, |
273 | + mock_get_host_architecture, |
274 | + mock_get_provider, |
275 | + mock_get_project_path, |
276 | + ): |
277 | + target_path = Path(self.useFixture(TempDir()).path) |
278 | + launcher = Mock(spec=launch) |
279 | + provider = makeLXDProvider(lxd_launcher=launcher) |
280 | + mock_get_provider.return_value = provider |
281 | + execute_run = LocalExecuteRun(self.tmp_project_path) |
282 | + launcher.return_value.execute_run = execute_run |
283 | + mock_get_project_path.return_value = self.tmp_project_path |
284 | + config = dedent( |
285 | + """ |
286 | + pipeline: |
287 | + - build |
288 | + |
289 | + jobs: |
290 | + build: |
291 | + series: focal |
292 | + architectures: amd64 |
293 | + run: | |
294 | + true |
295 | + license: |
296 | + path: LICENSE.txt |
297 | + """ |
298 | + ) |
299 | + Path(".launchpad.yaml").write_text(config) |
300 | + |
301 | + result = self.run_command( |
302 | + "run-one", "--output-directory", str(target_path), "build", "0" |
303 | + ) |
304 | + |
305 | + self.assertEqual(0, result.exit_code) |
306 | + job_output = target_path / "build" / "0" |
307 | + self.assertEqual( |
308 | + {"license": {"path": "LICENSE.txt", "spdx": None}}, |
309 | + json.loads((job_output / "properties").read_text()), |
310 | + ) |
311 | + |
312 | + @patch("lpcraft.env.get_managed_environment_project_path") |
313 | + @patch("lpcraft.commands.run.get_provider") |
314 | + @patch("lpcraft.commands.run.get_host_architecture", return_value="amd64") |
315 | + def test_license_field_works_also_with_other_properties( |
316 | + self, |
317 | + mock_get_host_architecture, |
318 | + mock_get_provider, |
319 | + mock_get_project_path, |
320 | + ): |
321 | + target_path = Path(self.useFixture(TempDir()).path) |
322 | + launcher = Mock(spec=launch) |
323 | + provider = makeLXDProvider(lxd_launcher=launcher) |
324 | + mock_get_provider.return_value = provider |
325 | + execute_run = LocalExecuteRun(self.tmp_project_path) |
326 | + launcher.return_value.execute_run = execute_run |
327 | + mock_get_project_path.return_value = self.tmp_project_path |
328 | + config = dedent( |
329 | + """ |
330 | + pipeline: |
331 | + - build |
332 | + |
333 | + jobs: |
334 | + build: |
335 | + series: focal |
336 | + architectures: amd64 |
337 | + run: | |
338 | + true |
339 | + output: |
340 | + properties: |
341 | + foo: bar |
342 | + license: |
343 | + path: LICENSE.txt |
344 | + """ |
345 | + ) |
346 | + Path(".launchpad.yaml").write_text(config) |
347 | + |
348 | + result = self.run_command( |
349 | + "run-one", "--output-directory", str(target_path), "build", "0" |
350 | + ) |
351 | + |
352 | + self.assertEqual(0, result.exit_code) |
353 | + job_output = target_path / "build" / "0" |
354 | + self.assertEqual( |
355 | + {"foo": "bar", "license": {"path": "LICENSE.txt", "spdx": None}}, |
356 | + json.loads((job_output / "properties").read_text()), |
357 | + ) |
358 | diff --git a/lpcraft/config.py b/lpcraft/config.py |
359 | index 328cd56..6ce9ae7 100644 |
360 | --- a/lpcraft/config.py |
361 | +++ b/lpcraft/config.py |
362 | @@ -30,7 +30,6 @@ class _Identifier(pydantic.ConstrainedStr): |
363 | class ModelConfigDefaults( |
364 | pydantic.BaseModel, |
365 | extra=pydantic.Extra.forbid, |
366 | - frozen=True, |
367 | alias_generator=lambda s: s.replace("_", "-"), |
368 | underscore_attrs_are_private=True, |
369 | ): |
370 | @@ -49,7 +48,8 @@ class Output(ModelConfigDefaults): |
371 | paths: Optional[List[StrictStr]] |
372 | distribute: Optional[OutputDistributeEnum] |
373 | channels: Optional[List[StrictStr]] |
374 | - properties: Optional[Dict[StrictStr, StrictStr]] |
375 | + # instead of `Any` this should be something like `JSONSerializable` |
376 | + properties: Optional[Dict[StrictStr, Any]] |
377 | dynamic_properties: Optional[Path] |
378 | expires: Optional[timedelta] |
379 | |
380 | @@ -205,11 +205,38 @@ def _expand_job_values( |
381 | return expanded_values |
382 | |
383 | |
384 | +class License(ModelConfigDefaults): |
385 | + """A representation of a license.""" |
386 | + |
387 | + # We do not need to check that at least one value is set, as currently |
388 | + # there are only these two values, no others. That means not setting any of |
389 | + # them will not result in the creation of a `License` object. |
390 | + # Once we have more fields, we need to add e.g. a root validator, see |
391 | + # https://stackoverflow.com/questions/58958970 |
392 | + |
393 | + # XXX jugmac00 2022-08-03: add validator for spdx identifier |
394 | + # XXX jugmac00 2022-08-04: add validator for path |
395 | + |
396 | + spdx: Optional[StrictStr] = None |
397 | + path: Optional[StrictStr] = None |
398 | + |
399 | + @validator("path", always=True) |
400 | + def disallow_setting_both_sources( |
401 | + cls, path: str, values: Dict[str, str] |
402 | + ) -> str: |
403 | + if values.get("spdx") and path: |
404 | + raise ValueError( |
405 | + "You cannot set `spdx` and `path` at the same time." |
406 | + ) |
407 | + return path |
408 | + |
409 | + |
410 | class Config(ModelConfigDefaults): |
411 | """A .launchpad.yaml configuration file.""" |
412 | |
413 | pipeline: List[List[_Identifier]] |
414 | jobs: Dict[StrictStr, List[Job]] |
415 | + license: Optional[License] |
416 | |
417 | @pydantic.validator("pipeline", pre=True) |
418 | def validate_pipeline( |
419 | diff --git a/lpcraft/tests/test_config.py b/lpcraft/tests/test_config.py |
420 | index 575f284..2ac30e9 100644 |
421 | --- a/lpcraft/tests/test_config.py |
422 | +++ b/lpcraft/tests/test_config.py |
423 | @@ -526,3 +526,71 @@ class TestConfig(TestCase): |
424 | ], # noqa: E501 |
425 | list(repositories[2].sources_list_lines()), |
426 | ) |
427 | + |
428 | + def test_specify_license_via_spdx(self): |
429 | + path = self.create_config( |
430 | + dedent( |
431 | + """ |
432 | + pipeline: |
433 | + - test |
434 | + |
435 | + jobs: |
436 | + test: |
437 | + series: focal |
438 | + architectures: amd64 |
439 | + license: |
440 | + spdx: "MIT" |
441 | + """ # noqa: E501 |
442 | + ) |
443 | + ) |
444 | + config = Config.load(path) |
445 | + |
446 | + # workaround necessary to please mypy |
447 | + assert config.license is not None |
448 | + self.assertEqual("MIT", config.license.spdx) |
449 | + |
450 | + def test_specify_license_via_path(self): |
451 | + path = self.create_config( |
452 | + dedent( |
453 | + """ |
454 | + pipeline: |
455 | + - test |
456 | + |
457 | + jobs: |
458 | + test: |
459 | + series: focal |
460 | + architectures: amd64 |
461 | + license: |
462 | + path: LICENSE.txt |
463 | + """ # noqa: E501 |
464 | + ) |
465 | + ) |
466 | + config = Config.load(path) |
467 | + |
468 | + # workaround necessary to please mypy |
469 | + assert config.license is not None |
470 | + self.assertEqual("LICENSE.txt", config.license.path) |
471 | + |
472 | + def test_license_setting_both_sources_not_allowed(self): |
473 | + path = self.create_config( |
474 | + dedent( |
475 | + """ |
476 | + pipeline: |
477 | + - test |
478 | + |
479 | + jobs: |
480 | + test: |
481 | + series: focal |
482 | + architectures: amd64 |
483 | + license: |
484 | + spdx: MIT |
485 | + path: LICENSE.txt |
486 | + """ # noqa: E501 |
487 | + ) |
488 | + ) |
489 | + self.assertRaisesRegex( |
490 | + ValidationError, |
491 | + "You cannot set `spdx` and `path` at the same time.", |
492 | + Config.load, |
493 | + path, |
494 | + ) |
I need to copy/paste the three tests from run to run-one also... this duplication effort gets a bit annoying over time... I wonder whether there would be a way to escape that.