feat: implement build in source for custom make (#417)

Author: torresxb1

aws_lambda_builders/workflows/custom_make/actions.py

@@ -25,20 +25,16 @@ class CustomMakeAction(BaseAction):
     def __init__(
         self,
         artifacts_dir,
-        scratch_dir,
         manifest_path,
         osutils,
         subprocess_make,
         build_logical_id,
-        working_directory=None,
+        working_directory,
     ):
         """
         :type artifacts_dir: str
         :param artifacts_dir: directory where artifacts needs to be stored.
 
-        :type scratch_dir: str
-        :param scratch_dir: an existing (writable) directory for temporary files
-
         :type manifest_path: str
         :param manifest_path: path to Makefile of an Make project with the source in same folder.
 
@@ -52,17 +48,15 @@ def __init__(
         :param build_logical_id: the lambda resource logical id that will be built by the custom action.
 
         :type working_directory: str
-        :param working_directory: path to the working directory where the Makefile will be executed. Use the scratch_dir
-        as the working directory if the input working_directory is None
+        :param working_directory: path to the working directory where the Makefile will be executed.
         """
         super(CustomMakeAction, self).__init__()
         self.artifacts_dir = artifacts_dir
-        self.scratch_dir = scratch_dir
         self.manifest_path = manifest_path
         self.osutils = osutils
         self.subprocess_make = subprocess_make
         self.build_logical_id = build_logical_id
-        self.working_directory = working_directory if working_directory else scratch_dir
+        self.working_directory = working_directory
 
     @property
     def artifact_dir_path(self):

aws_lambda_builders/workflows/custom_make/workflow.py

@@ -31,11 +31,10 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
 
         self.os_utils = OSUtils()
 
-        # Find the logical id of the function to be built.
         options = kwargs.get("options") or {}
-        build_logical_id = options.get("build_logical_id", None)
-        working_directory = options.get("working_directory", scratch_dir)
+        build_in_source = kwargs.get("build_in_source")
 
+        build_logical_id = options.get("build_logical_id", None)
         if not build_logical_id:
             raise WorkflowFailedError(
                 workflow_name=self.NAME,
@@ -45,17 +44,37 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
 
         subprocess_make = SubProcessMake(make_exe=self.binaries["make"].binary_path, osutils=self.os_utils)
 
+        # Don't build in source by default (backwards compatibility)
+        if build_in_source is None:
+            build_in_source = False
+
+        # an explicitly definied working directory should take precedence
+        working_directory = options.get("working_directory") or self._select_working_directory(
+            source_dir, scratch_dir, build_in_source
+        )
+
         make_action = CustomMakeAction(
             artifacts_dir,
-            scratch_dir,
             manifest_path,
             osutils=self.os_utils,
             subprocess_make=subprocess_make,
             build_logical_id=build_logical_id,
             working_directory=working_directory,
         )
 
-        self.actions = [CopySourceAction(source_dir, scratch_dir, excludes=self.EXCLUDED_FILES), make_action]
+        self.actions = []
+
+        if not self.build_in_source:
+            # if we're building on scratch_dir, we have to first copy the source there
+            self.actions.append(CopySourceAction(source_dir, scratch_dir, excludes=self.EXCLUDED_FILES))
+
+        self.actions.append(make_action)
+
+    def _select_working_directory(self, source_dir: str, scratch_dir: str, build_in_source: bool):
+        """
+        Returns the directory where the make action should be executed
+        """
+        return source_dir if build_in_source else scratch_dir
 
     def get_resolvers(self):
         return [PathResolver(runtime="provided", binary="make", executable_search_paths=self.executable_search_paths)]

tests/integration/workflows/custom_make/test_custom_make.py

@@ -109,3 +109,30 @@ def test_must_build_python_project_through_makefile_unknown_target(self):
                 runtime=self.runtime,
                 options={"build_logical_id": "HelloWorldFunction2"},
             )
+
+    def test_must_build_python_project_through_makefile_in_source(self):
+        self.builder.build(
+            self.source_dir,
+            self.artifacts_dir,
+            self.scratch_dir,
+            self.manifest_path_valid,
+            runtime=self.runtime,
+            options={"build_logical_id": "HelloWorldFunction"},
+            build_in_source=True,
+        )
+        dependencies_installed = {
+            "chardet",
+            "urllib3",
+            "idna",
+            "urllib3-1.25.11.dist-info",
+            "chardet-3.0.4.dist-info",
+            "certifi-2020.4.5.2.dist-info",
+            "certifi",
+            "idna-2.10.dist-info",
+            "requests",
+            "requests-2.23.0.dist-info",
+        }
+
+        expected_files = self.test_data_files.union(dependencies_installed)
+        output_files = set(os.listdir(self.artifacts_dir))
+        self.assertEqual(expected_files, output_files)

tests/unit/workflows/custom_make/test_actions.py

@@ -23,14 +23,15 @@ def tearDown(self):
     def test_call_makefile_target(self, OSUtilMock, SubprocessMakeMock):
         osutils = OSUtilMock.return_value
         subprocess_make = SubprocessMakeMock.return_value
+        working_directory = "some_dir"
 
         action = CustomMakeAction(
             "artifacts",
-            "scratch_dir",
             "manifest",
             osutils=osutils,
             subprocess_make=subprocess_make,
             build_logical_id="logical_id",
+            working_directory=working_directory,
         )
 
         osutils.dirname.side_effect = lambda value: "/dir:{}".format(value)
@@ -40,48 +41,23 @@ def test_call_makefile_target(self, OSUtilMock, SubprocessMakeMock):
         action.execute()
 
         subprocess_make.run.assert_called_with(
-            ["--makefile", "manifest", "build-logical_id"], cwd="scratch_dir", env=ANY
-        )
-
-    @patch("aws_lambda_builders.workflows.custom_make.utils.OSUtils")
-    @patch("aws_lambda_builders.workflows.custom_make.make.SubProcessMake")
-    def test_call_makefile_target_with_working_directory(self, OSUtilMock, SubprocessMakeMock):
-        osutils = OSUtilMock.return_value
-        subprocess_make = SubprocessMakeMock.return_value
-
-        action = CustomMakeAction(
-            "artifacts",
-            "scratch_dir",
-            "manifest",
-            osutils=osutils,
-            subprocess_make=subprocess_make,
-            build_logical_id="logical_id",
-            working_directory="working_dir",
-        )
-
-        osutils.dirname.side_effect = lambda value: "/dir:{}".format(value)
-        osutils.abspath.side_effect = lambda value: "/abs:{}".format(value)
-        osutils.joinpath.side_effect = lambda a, b: "{}/{}".format(a, b)
-
-        action.execute()
-
-        subprocess_make.run.assert_called_with(
-            ["--makefile", "manifest", "build-logical_id"], cwd="working_dir", env=ANY
+            ["--makefile", "manifest", "build-logical_id"], cwd=working_directory, env=ANY
         )
 
     @patch("aws_lambda_builders.workflows.custom_make.utils.OSUtils")
     @patch("aws_lambda_builders.workflows.custom_make.make.SubProcessMake")
     def test_makefile_target_fails(self, OSUtilMock, SubprocessMakeMock):
         osutils = OSUtilMock.return_value
         subprocess_make = SubprocessMakeMock.return_value
+        working_directory = "some_dir"
 
         action = CustomMakeAction(
             "artifacts",
-            "scratch_dir",
             "manifest",
             osutils=osutils,
             subprocess_make=subprocess_make,
             build_logical_id="logical_id",
+            working_directory=working_directory,
         )
 
         osutils.dirname.side_effect = lambda value: "/dir:{}".format(value)
@@ -93,5 +69,5 @@ def test_makefile_target_fails(self, OSUtilMock, SubprocessMakeMock):
         with self.assertRaises(ActionFailedError):
             action.execute()
         subprocess_make.run.assert_called_with(
-            ["--makefile", "manifest", "build-logical_id"], cwd="scratch_dir", env=ANY
+            ["--makefile", "manifest", "build-logical_id"], cwd=working_directory, env=ANY
         )

tests/unit/workflows/custom_make/test_workflow.py

@@ -64,3 +64,35 @@ def test_use_working_directory(self):
         )
 
         self.assertEqual(workflow.actions[1].working_directory, "working/dir/path")
+
+    def test_build_in_source(self):
+        source_dir = "source"
+
+        workflow = CustomMakeWorkflow(
+            source_dir,
+            "artifacts",
+            "scratch_dir",
+            "manifest",
+            options={"build_logical_id": "hello"},
+            build_in_source=True,
+        )
+
+        self.assertEqual(len(workflow.actions), 1)
+        self.assertIsInstance(workflow.actions[0], CustomMakeAction)
+        self.assertEqual(workflow.actions[0].working_directory, source_dir)
+
+    def test_build_in_source_with_custom_working_directory(self):
+        working_dir = "working/dir/path"
+
+        workflow = CustomMakeWorkflow(
+            "source",
+            "artifacts",
+            "scratch_dir",
+            "manifest",
+            options={"build_logical_id": "hello", "working_directory": working_dir},
+            build_in_source=True,
+        )
+
+        self.assertEqual(len(workflow.actions), 1)
+        self.assertIsInstance(workflow.actions[0], CustomMakeAction)
+        self.assertEqual(workflow.actions[0].working_directory, working_dir)