From b6b92c01ef8da0fa0fd8981c64f0ddf841f5b7b7 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Fri, 9 Dec 2022 09:23:20 -0800 Subject: [PATCH 1/6] feat: implement build in source for custom make --- .../workflows/custom_make/actions.py | 12 ++----- .../workflows/custom_make/workflow.py | 27 ++++++++++++-- .../workflows/custom_make/test_custom_make.py | 27 ++++++++++++++ .../workflows/custom_make/test_actions.py | 36 ++++--------------- .../workflows/custom_make/test_workflow.py | 32 +++++++++++++++++ 5 files changed, 92 insertions(+), 42 deletions(-) diff --git a/aws_lambda_builders/workflows/custom_make/actions.py b/aws_lambda_builders/workflows/custom_make/actions.py index e689012e5..ce28e6129 100644 --- a/aws_lambda_builders/workflows/custom_make/actions.py +++ b/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): diff --git a/aws_lambda_builders/workflows/custom_make/workflow.py b/aws_lambda_builders/workflows/custom_make/workflow.py index 627125667..aa8e92a95 100644 --- a/aws_lambda_builders/workflows/custom_make/workflow.py +++ b/aws_lambda_builders/workflows/custom_make/workflow.py @@ -34,7 +34,6 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim # 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) if not build_logical_id: raise WorkflowFailedError( @@ -45,9 +44,11 @@ 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) + build_in_source = kwargs.get("build_in_source", False) + working_directory = self._get_working_directory(options, source_dir, scratch_dir, build_in_source) + make_action = CustomMakeAction( artifacts_dir, - scratch_dir, manifest_path, osutils=self.os_utils, subprocess_make=subprocess_make, @@ -55,7 +56,27 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim 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 _get_working_directory(self, options, source_dir, scratch_dir, build_in_source): + """ + Gets the directory where the make action should be executed + """ + options_working_directory = options.get("working_directory") + + # an explicitly definied working directory should take precedence + if options_working_directory: + return options_working_directory + elif build_in_source: + return source_dir + else: + return scratch_dir def get_resolvers(self): return [PathResolver(runtime="provided", binary="make", executable_search_paths=self.executable_search_paths)] diff --git a/tests/integration/workflows/custom_make/test_custom_make.py b/tests/integration/workflows/custom_make/test_custom_make.py index 4bd31bef0..2d8065969 100644 --- a/tests/integration/workflows/custom_make/test_custom_make.py +++ b/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) \ No newline at end of file diff --git a/tests/unit/workflows/custom_make/test_actions.py b/tests/unit/workflows/custom_make/test_actions.py index 6f1bdd432..aede6f1ab 100644 --- a/tests/unit/workflows/custom_make/test_actions.py +++ b/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,33 +41,7 @@ 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") @@ -74,14 +49,15 @@ def test_call_makefile_target_with_working_directory(self, OSUtilMock, Subproces 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 ) diff --git a/tests/unit/workflows/custom_make/test_workflow.py b/tests/unit/workflows/custom_make/test_workflow.py index 39f58abcd..028ef2945 100644 --- a/tests/unit/workflows/custom_make/test_workflow.py +++ b/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) From e106eecb32ab3235ae44c2fbf319b7d39dc7d338 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Wed, 14 Dec 2022 16:21:39 -0800 Subject: [PATCH 2/6] set to False when param's value is None --- aws_lambda_builders/workflows/custom_make/workflow.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/aws_lambda_builders/workflows/custom_make/workflow.py b/aws_lambda_builders/workflows/custom_make/workflow.py index aa8e92a95..5aff9519f 100644 --- a/aws_lambda_builders/workflows/custom_make/workflow.py +++ b/aws_lambda_builders/workflows/custom_make/workflow.py @@ -44,7 +44,11 @@ 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) - build_in_source = kwargs.get("build_in_source", False) + build_in_source = kwargs.get("build_in_source") + # Don't build in source by default (backwards compatibility) + if build_in_source is None: + build_in_source = False + working_directory = self._get_working_directory(options, source_dir, scratch_dir, build_in_source) make_action = CustomMakeAction( From f1816f67eb71f83c7b75c338e31a77ee41cb7157 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Thu, 15 Dec 2022 08:55:48 -0800 Subject: [PATCH 3/6] remove trailing whitespace --- aws_lambda_builders/workflows/custom_make/workflow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws_lambda_builders/workflows/custom_make/workflow.py b/aws_lambda_builders/workflows/custom_make/workflow.py index 5aff9519f..14f8fe956 100644 --- a/aws_lambda_builders/workflows/custom_make/workflow.py +++ b/aws_lambda_builders/workflows/custom_make/workflow.py @@ -73,7 +73,7 @@ def _get_working_directory(self, options, source_dir, scratch_dir, build_in_sour Gets the directory where the make action should be executed """ options_working_directory = options.get("working_directory") - + # an explicitly definied working directory should take precedence if options_working_directory: return options_working_directory From dd85f9522dc43690190861fce0535fea2593c423 Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Thu, 15 Dec 2022 09:11:01 -0800 Subject: [PATCH 4/6] black --- tests/integration/workflows/custom_make/test_custom_make.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/workflows/custom_make/test_custom_make.py b/tests/integration/workflows/custom_make/test_custom_make.py index 2d8065969..8d376dca0 100644 --- a/tests/integration/workflows/custom_make/test_custom_make.py +++ b/tests/integration/workflows/custom_make/test_custom_make.py @@ -135,4 +135,4 @@ def test_must_build_python_project_through_makefile_in_source(self): expected_files = self.test_data_files.union(dependencies_installed) output_files = set(os.listdir(self.artifacts_dir)) - self.assertEqual(expected_files, output_files) \ No newline at end of file + self.assertEqual(expected_files, output_files) From ea10dc60ed0e06cfe99e19d1f99560392439fbae Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Thu, 15 Dec 2022 10:56:29 -0800 Subject: [PATCH 5/6] add type hints --- aws_lambda_builders/workflows/custom_make/workflow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws_lambda_builders/workflows/custom_make/workflow.py b/aws_lambda_builders/workflows/custom_make/workflow.py index 14f8fe956..102f7b150 100644 --- a/aws_lambda_builders/workflows/custom_make/workflow.py +++ b/aws_lambda_builders/workflows/custom_make/workflow.py @@ -68,7 +68,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim self.actions.append(make_action) - def _get_working_directory(self, options, source_dir, scratch_dir, build_in_source): + def _get_working_directory(self, options: dict, source_dir: str, scratch_dir: str, build_in_source: bool): """ Gets the directory where the make action should be executed """ From d8279c645d5e6ad61a3c5f5a234a93213610c42d Mon Sep 17 00:00:00 2001 From: Ruperto Torres Date: Thu, 15 Dec 2022 17:18:12 -0800 Subject: [PATCH 6/6] review feedback --- .../workflows/custom_make/workflow.py | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/aws_lambda_builders/workflows/custom_make/workflow.py b/aws_lambda_builders/workflows/custom_make/workflow.py index 102f7b150..37b2cc469 100644 --- a/aws_lambda_builders/workflows/custom_make/workflow.py +++ b/aws_lambda_builders/workflows/custom_make/workflow.py @@ -31,10 +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) + 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, @@ -44,12 +44,14 @@ 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) - build_in_source = kwargs.get("build_in_source") # Don't build in source by default (backwards compatibility) if build_in_source is None: build_in_source = False - working_directory = self._get_working_directory(options, source_dir, scratch_dir, build_in_source) + # 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, @@ -68,19 +70,11 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim self.actions.append(make_action) - def _get_working_directory(self, options: dict, source_dir: str, scratch_dir: str, build_in_source: bool): + def _select_working_directory(self, source_dir: str, scratch_dir: str, build_in_source: bool): """ - Gets the directory where the make action should be executed + Returns the directory where the make action should be executed """ - options_working_directory = options.get("working_directory") - - # an explicitly definied working directory should take precedence - if options_working_directory: - return options_working_directory - elif build_in_source: - return source_dir - else: - return scratch_dir + 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)]