← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] ~cjwatson/lpcraft:output-from-parent into lpcraft:main

 

Colin Watson has proposed merging ~cjwatson/lpcraft:output-from-parent into lpcraft:main.

Commit message:
Allow output.paths to reference parent directory

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

`dpkg-buildpackage` writes its output files to the parent directory of the build tree.  This is awkward to use with lpcraft, because `output.paths` isn't allowed to escape the build tree.

We have to work with the world as it is, not as we'd like it to be, and it doesn't seem unreasonable to allow gathering files from the parent directory (but not further up the tree) in order to make things easier for people integrating build systems such as this.  Move the project path inside the container one level down (to `/root/lpcraft/project`) to support this.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/lpcraft:output-from-parent into lpcraft:main.
diff --git a/NEWS.rst b/NEWS.rst
index e297bc4..90344ac 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -5,7 +5,9 @@ Version history
 0.0.9   (unreleased)
 ====================
 
-- nothing yet
+- Allow ``output.paths`` to reference the parent directory of the build
+  tree, in order to make life easier for build systems such as
+  ``dpkg-buildpackage`` that write output files to their parent directory.
 
 0.0.8   (2022-04-13)
 ====================
diff --git a/docs/configuration.rst b/docs/configuration.rst
index a350f7e..5d16337 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -84,7 +84,10 @@ Output properties
     <https://docs.python.org/3/library/pathlib.html#pathlib.Path.glob>`_
     patterns; any files matching these patterns at the end of a successful
     build will be gathered by the build manager and attached to the build in
-    Launchpad.  Paths may not escape the build tree.
+    Launchpad.  Paths may not escape the parent directory of the build tree.
+    (The parent directory is allowed in order to make life easier for build
+    systems such as ``dpkg-buildpackage`` that write output files to their
+    parent directory.)
 
 ``distribute`` (optional)
     If ``artifactory``, then these artifacts may be distributed via
diff --git a/lpcraft/commands/run.py b/lpcraft/commands/run.py
index 23e9a04..dfd2f27 100644
--- a/lpcraft/commands/run.py
+++ b/lpcraft/commands/run.py
@@ -99,15 +99,22 @@ def _copy_output_paths(
         # of normpath.
         _check_relative_path(
             PurePath(os.path.normpath(remote_cwd / path_pattern)),
-            remote_cwd,
+            remote_cwd.parent,
         )
 
-    remote_paths = sorted(_list_files(instance, remote_cwd))
+    remote_paths = sorted(_list_files(instance, remote_cwd.parent))
     output_files = target_path / "files"
 
     filtered_paths: Set[PurePath] = set()
     for path_pattern in output.paths:
-        paths = [str(path) for path in remote_paths]
+        # We listed the parent of the build tree in order to allow
+        # output.paths to reference the parent directory.  The patterns are
+        # still relative to the build tree, though, so make our paths
+        # relative to the build tree again so that they can be matched
+        # properly.
+        paths = [
+            os.path.relpath(path, remote_cwd.name) for path in remote_paths
+        ]
         result = fnmatch.filter(paths, path_pattern)
         if not result:
             raise CommandError(
@@ -122,7 +129,15 @@ def _copy_output_paths(
     )
 
     for path in sorted(resolved_paths):
-        relative_path = _check_relative_path(path, remote_cwd)
+        relative_path = _check_relative_path(path, remote_cwd.parent)
+        # Paths in the parent directory of the build tree can stay as they
+        # are, but everything else should be made relative to the build tree
+        # and preserve any subdirectory structure there within the output
+        # directory.
+        try:
+            relative_path = relative_path.relative_to(remote_cwd.name)
+        except ValueError:
+            pass
         destination = output_files / relative_path
         destination.parent.mkdir(parents=True, exist_ok=True)
         try:
diff --git a/lpcraft/commands/tests/test_run.py b/lpcraft/commands/tests/test_run.py
index 27baf8b..08e9803 100644
--- a/lpcraft/commands/tests/test_run.py
+++ b/lpcraft/commands/tests/test_run.py
@@ -162,7 +162,7 @@ class TestRun(RunBaseTestCase):
 
         execute_run.assert_called_once_with(
             ["bash", "--noprofile", "--norc", "-ec", "pyproject-build"],
-            cwd=Path("/root/project"),
+            cwd=Path("/root/lpcraft/project"),
             env={},
             stdout=ANY,
             stderr=ANY,
@@ -261,7 +261,7 @@ class TestRun(RunBaseTestCase):
             [
                 call(
                     ["bash", "--noprofile", "--norc", "-ec", "tox"],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
@@ -349,7 +349,7 @@ class TestRun(RunBaseTestCase):
         )
         execute_run.assert_called_once_with(
             ["bash", "--noprofile", "--norc", "-ec", "tox"],
-            cwd=Path("/root/project"),
+            cwd=Path("/root/lpcraft/project"),
             env={},
             stdout=ANY,
             stderr=ANY,
@@ -395,7 +395,7 @@ class TestRun(RunBaseTestCase):
             [
                 call(
                     ["bash", "--noprofile", "--norc", "-ec", "tox"],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
@@ -408,7 +408,7 @@ class TestRun(RunBaseTestCase):
                         "-ec",
                         "pyproject-build",
                     ],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
@@ -450,7 +450,7 @@ class TestRun(RunBaseTestCase):
             [
                 call(
                     ["bash", "--noprofile", "--norc", "-ec", "tox"],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
@@ -524,7 +524,7 @@ class TestRun(RunBaseTestCase):
             [
                 call(
                     ["bash", "--noprofile", "--norc", "-ec", command],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
@@ -583,7 +583,7 @@ class TestRun(RunBaseTestCase):
             [
                 call(
                     ["bash", "--noprofile", "--norc", "-ec", command],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
@@ -636,14 +636,14 @@ class TestRun(RunBaseTestCase):
             [
                 call(
                     ["bash", "--noprofile", "--norc", "-ec", "tox"],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
                 ),
                 call(
                     ["bash", "--noprofile", "--norc", "-ec", "tox"],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
@@ -656,7 +656,7 @@ class TestRun(RunBaseTestCase):
                         "-ec",
                         "pyproject-build",
                     ],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
@@ -698,7 +698,7 @@ class TestRun(RunBaseTestCase):
             [
                 call(
                     ["bash", "--noprofile", "--norc", "-ec", "tox"],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={"TOX_SKIP_ENV": "^(?!lint-)"},
                     stdout=ANY,
                     stderr=ANY,
@@ -776,6 +776,66 @@ class TestRun(RunBaseTestCase):
     @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_output_path_in_immediate_parent(
+        self,
+        mock_get_host_architecture,
+        mock_get_provider,
+        mock_get_project_path,
+    ):
+        def fake_pull_file(source: Path, destination: Path) -> None:
+            destination.touch()
+
+        target_path = Path(self.useFixture(TempDir()).path) / "build"
+        target_path.mkdir()
+        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
+        launcher.return_value.pull_file.side_effect = fake_pull_file
+        config = dedent(
+            """
+            pipeline:
+                - build
+
+            jobs:
+                build:
+                    series: focal
+                    architectures: amd64
+                    run: touch ../test_1.0_all.deb
+                    output:
+                        paths: ["../*.deb"]
+            """
+        )
+        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" / "focal" / "amd64"
+        self.assertEqual(
+            [
+                call(
+                    source=self.tmp_project_path.parent / "test_1.0_all.deb",
+                    destination=job_output / "files" / "test_1.0_all.deb",
+                ),
+            ],
+            launcher.return_value.pull_file.call_args_list,
+        )
+        self.assertEqual(
+            ["files", "properties"],
+            sorted(path.name for path in job_output.iterdir()),
+        )
+        self.assertEqual(
+            ["test_1.0_all.deb"],
+            sorted(path.name for path in (job_output / "files").iterdir()),
+        )
+
+    @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_output_path_escapes_directly(
         self,
         mock_get_host_architecture,
@@ -849,7 +909,7 @@ class TestRun(RunBaseTestCase):
             """
         )
         Path(".launchpad.yaml").write_text(config)
-        Path("symlink.txt").symlink_to("../target.txt")
+        Path("symlink.txt").symlink_to("../../target.txt")
 
         result = self.run_command(
             "run", "--output-directory", str(target_path)
@@ -1264,7 +1324,7 @@ class TestRun(RunBaseTestCase):
                 ),
                 call(
                     ["bash", "--noprofile", "--norc", "-ec", "tox"],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
@@ -1311,14 +1371,14 @@ class TestRun(RunBaseTestCase):
                         "nginx",
                         "apache2",
                     ],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
                 ),
                 call(
                     ["bash", "--noprofile", "--norc", "-ec", "tox"],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
@@ -1359,7 +1419,7 @@ class TestRun(RunBaseTestCase):
             [
                 call(
                     ["apt", "install", "-y", "unknown_package"],
-                    cwd=Path("/root/project"),
+                    cwd=Path("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
@@ -1748,7 +1808,7 @@ class TestRunOne(RunBaseTestCase):
 
         execute_run.assert_called_once_with(
             ["bash", "--noprofile", "--norc", "-ec", "tox"],
-            cwd=Path("/root/project"),
+            cwd=Path("/root/lpcraft/project"),
             env={},
             stdout=ANY,
             stderr=ANY,
@@ -1856,7 +1916,7 @@ class TestRunOne(RunBaseTestCase):
         )
         execute_run.assert_called_once_with(
             ["bash", "--noprofile", "--norc", "-ec", "pyproject-build"],
-            cwd=Path("/root/project"),
+            cwd=Path("/root/lpcraft/project"),
             env={},
             stdout=ANY,
             stderr=ANY,
@@ -1904,7 +1964,7 @@ class TestRunOne(RunBaseTestCase):
         )
         execute_run.assert_called_once_with(
             ["bash", "--noprofile", "--norc", "-ec", "tox"],
-            cwd=Path("/root/project"),
+            cwd=Path("/root/lpcraft/project"),
             env={},
             stdout=ANY,
             stderr=ANY,
diff --git a/lpcraft/env.py b/lpcraft/env.py
index 2e9a0be..0cb51e3 100644
--- a/lpcraft/env.py
+++ b/lpcraft/env.py
@@ -13,4 +13,4 @@ def get_managed_environment_home_path() -> Path:
 
 def get_managed_environment_project_path() -> Path:
     """Path for project when running in managed environment."""
-    return get_managed_environment_home_path() / "project"
+    return get_managed_environment_home_path() / "lpcraft" / "project"
diff --git a/lpcraft/plugin/tests/test_plugins.py b/lpcraft/plugin/tests/test_plugins.py
index f7c2012..dbdd2c2 100644
--- a/lpcraft/plugin/tests/test_plugins.py
+++ b/lpcraft/plugin/tests/test_plugins.py
@@ -59,7 +59,7 @@ class TestPlugins(CommandBaseTestCase):
                         "nginx",
                         "apache2",
                     ],
-                    cwd=PosixPath("/root/project"),
+                    cwd=PosixPath("/root/lpcraft/project"),
                     env={"TOX_TESTENV_PASSENV": "http_proxy https_proxy"},
                     stdout=ANY,
                     stderr=ANY,
@@ -72,7 +72,7 @@ class TestPlugins(CommandBaseTestCase):
                         "-ec",
                         "python3 -m pip install tox==3.24.5; tox",
                     ],
-                    cwd=PosixPath("/root/project"),
+                    cwd=PosixPath("/root/lpcraft/project"),
                     env={"TOX_TESTENV_PASSENV": "http_proxy https_proxy"},
                     stdout=ANY,
                     stderr=ANY,
@@ -151,14 +151,14 @@ class TestPlugins(CommandBaseTestCase):
                         "nginx",
                         "apache2",
                     ],
-                    cwd=PosixPath("/root/project"),
+                    cwd=PosixPath("/root/lpcraft/project"),
                     env={"TOX_TESTENV_PASSENV": "http_proxy https_proxy"},
                     stdout=ANY,
                     stderr=ANY,
                 ),
                 call(
                     ["bash", "--noprofile", "--norc", "-ec", "ls"],
-                    cwd=PosixPath("/root/project"),
+                    cwd=PosixPath("/root/lpcraft/project"),
                     env={"TOX_TESTENV_PASSENV": "http_proxy https_proxy"},
                     stdout=ANY,
                     stderr=ANY,
@@ -203,7 +203,7 @@ class TestPlugins(CommandBaseTestCase):
                         "python3-pip",
                         "python3-venv",
                     ],
-                    cwd=PosixPath("/root/project"),
+                    cwd=PosixPath("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
@@ -216,7 +216,7 @@ class TestPlugins(CommandBaseTestCase):
                         "-ec",
                         "python3 -m pip install build==0.7.0; python3 -m build",  # noqa: E501
                     ],
-                    cwd=PosixPath("/root/project"),
+                    cwd=PosixPath("/root/lpcraft/project"),
                     env={},
                     stdout=ANY,
                     stderr=ANY,
diff --git a/lpcraft/providers/tests/test_lxd.py b/lpcraft/providers/tests/test_lxd.py
index e9b7d87..1a0afd4 100644
--- a/lpcraft/providers/tests/test_lxd.py
+++ b/lpcraft/providers/tests/test_lxd.py
@@ -409,7 +409,7 @@ class TestLXDProvider(TestCase):
                     ),
                     call().lxc.exec(
                         instance_name=expected_instance_name,
-                        command=["rm", "-rf", "/root/project"],
+                        command=["rm", "-rf", "/root/lpcraft/project"],
                         project="test-project",
                         remote="test-remote",
                         runner=subprocess.run,
@@ -421,7 +421,7 @@ class TestLXDProvider(TestCase):
                             "cp",
                             "-a",
                             "/root/tmp-project",
-                            "/root/project",
+                            "/root/lpcraft/project",
                         ],
                         project="test-project",
                         remote="test-remote",
@@ -438,7 +438,7 @@ class TestLXDProvider(TestCase):
             [
                 call().lxc.exec(
                     instance_name=expected_instance_name,
-                    command=["rm", "-rf", "/root/project"],
+                    command=["rm", "-rf", "/root/lpcraft/project"],
                     project="test-project",
                     remote="test-remote",
                     runner=subprocess.run,
@@ -518,7 +518,7 @@ class TestLXDProvider(TestCase):
                 ),
                 call().lxc.exec(
                     instance_name=expected_instance_name,
-                    command=["rm", "-rf", "/root/project"],
+                    command=["rm", "-rf", "/root/lpcraft/project"],
                     project="test-project",
                     remote="test-remote",
                     runner=subprocess.run,
@@ -526,7 +526,12 @@ class TestLXDProvider(TestCase):
                 ),
                 call().lxc.exec(
                     instance_name=expected_instance_name,
-                    command=["cp", "-a", "/root/tmp-project", "/root/project"],
+                    command=[
+                        "cp",
+                        "-a",
+                        "/root/tmp-project",
+                        "/root/lpcraft/project",
+                    ],
                     project="test-project",
                     remote="test-remote",
                     runner=subprocess.run,
@@ -535,7 +540,7 @@ class TestLXDProvider(TestCase):
                 call().unmount(target=Path("/root/tmp-project")),
                 call().lxc.exec(
                     instance_name=expected_instance_name,
-                    command=["rm", "-rf", "/root/project"],
+                    command=["rm", "-rf", "/root/lpcraft/project"],
                     project="test-project",
                     remote="test-remote",
                     runner=subprocess.run,
@@ -565,7 +570,7 @@ class TestLXDProvider(TestCase):
             [
                 call().lxc.exec(
                     instance_name=expected_instance_name,
-                    command=["rm", "-rf", "/root/project"],
+                    command=["rm", "-rf", "/root/lpcraft/project"],
                     project="test-project",
                     remote="test-remote",
                     runner=subprocess.run,
diff --git a/lpcraft/tests/test_env.py b/lpcraft/tests/test_env.py
index 54a8018..6371c03 100644
--- a/lpcraft/tests/test_env.py
+++ b/lpcraft/tests/test_env.py
@@ -16,5 +16,6 @@ class TestEnvironment(TestCase):
 
     def test_get_managed_environment_project_path(self):
         self.assertEqual(
-            Path("/root/project"), env.get_managed_environment_project_path()
+            Path("/root/lpcraft/project"),
+            env.get_managed_environment_project_path(),
         )