launchpad-reviewers team mailing list archive

[Colin Watson <mp+428099@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/lpcraft:fix-input-properties into lpcraft:main.

Commit message:
Fix input handling of jobs that produce subdirectories of artifacts

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/lpcraft/+git/lpcraft/+merge/428099

It's quite common for e.g. Python jobs to use something like `paths: ["dist/*"]`, which produces a subdirectory under `files` in the output directory.  The `input` property crashed when faced with this.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lpcraft:fix-input-properties into lpcraft:main.

diff --git a/lpcraft/commands/run.py b/lpcraft/commands/run.py
index 99babf1..f485601 100644
--- a/lpcraft/commands/run.py
+++ b/lpcraft/commands/run.py
@@ -143,17 +143,30 @@ def _copy_input_paths(
         instance, [remote_cwd / input.target_directory]
     )
     _check_relative_path(target_path, remote_cwd)
-    instance.execute_run(
-        ["mkdir", "-p", str(target_path / "files")], check=True
-    )
+
+    paths = []
+    for dirpath, _, filenames in os.walk(source_path / "files"):
+        paths.extend(
+            [
+                Path(dirpath).relative_to(source_path / "files") / filename
+                for filename in filenames
+            ]
+        )
+    paths = sorted(paths)
+    parent_paths = sorted(set(path.parent for path in paths) | {Path(".")})
 
     try:
-        for path in sorted((source_path / "files").iterdir()):
+        instance.execute_run(
+            ["mkdir", "-p"]
+            + [str(target_path / "files" / path) for path in parent_paths],
+            check=True,
+        )
+        for path in paths:
             instance.push_file(
-                source=path,
+                source=source_path / "files" / path,
                 # Path() here works around
                 # https://github.com/canonical/craft-providers/pull/135.
-                destination=Path(target_path / "files" / path.name),
+                destination=Path(target_path / "files" / path),
             )
         instance.push_file(
             source=source_path / "properties",
diff --git a/lpcraft/commands/tests/test_run.py b/lpcraft/commands/tests/test_run.py
index 2d71247..c60e0d7 100644
--- a/lpcraft/commands/tests/test_run.py
+++ b/lpcraft/commands/tests/test_run.py
@@ -1445,7 +1445,9 @@ class TestRun(RunBaseTestCase):
                     architectures: [amd64]
                     run: "true"
                     output:
-                        paths: [binary]
+                        paths:
+                            - binary
+                            - dist/*
 
                 test:
                     series: focal
@@ -1458,6 +1460,8 @@ class TestRun(RunBaseTestCase):
         )
         Path(".launchpad.yaml").write_text(config)
         Path("binary").write_bytes(b"binary")
+        Path("dist").mkdir()
+        Path("dist/empty").touch()
         result = self.run_command(
             "run", "--output-directory", str(target_path)
         )
@@ -1470,7 +1474,11 @@ class TestRun(RunBaseTestCase):
                 call(
                     source=self.tmp_project_path / "binary",
                     destination=build_job_output / "files" / "binary",
-                )
+                ),
+                call(
+                    source=self.tmp_project_path / "dist" / "empty",
+                    destination=build_job_output / "files" / "dist" / "empty",
+                ),
             ],
             launcher.return_value.pull_file.call_args_list,
         )
@@ -1481,6 +1489,10 @@ class TestRun(RunBaseTestCase):
                     destination=artifacts_path / "files" / "binary",
                 ),
                 call(
+                    source=build_job_output / "files" / "dist" / "empty",
+                    destination=artifacts_path / "files" / "dist" / "empty",
+                ),
+                call(
                     source=build_job_output / "properties",
                     destination=artifacts_path / "properties",
                 ),
@@ -1492,12 +1504,22 @@ class TestRun(RunBaseTestCase):
             sorted(path.name for path in artifacts_path.iterdir()),
         )
         self.assertEqual(
-            ["binary"],
+            ["binary", "dist"],
             sorted(path.name for path in (artifacts_path / "files").iterdir()),
         )
         self.assertEqual(
+            ["empty"],
+            sorted(
+                path.name
+                for path in (artifacts_path / "files" / "dist").iterdir()
+            ),
+        )
+        self.assertEqual(
             b"binary", (artifacts_path / "files" / "binary").read_bytes()
         )
+        self.assertEqual(
+            b"", (artifacts_path / "files" / "dist" / "empty").read_bytes()
+        )
 
     @patch("lpcraft.env.get_managed_environment_project_path")
     @patch("lpcraft.commands.run.get_provider")