Skip to content

feat: implement build in source for custom make #417

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions aws_lambda_builders/workflows/custom_make/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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):
Expand Down
29 changes: 24 additions & 5 deletions aws_lambda_builders/workflows/custom_make/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 = []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could we still do a list comprehension here? for readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure how, given that one item is only added conditionally 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.actions = [CopySrc(...), make_action] if .... else [make_action]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving without blocking.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging without this change as I'm not sure if it improves readability and I feel like it wouldn't be able to nicely handle adding more conditional actions in the future.


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)]
Expand Down
27 changes: 27 additions & 0 deletions tests/integration/workflows/custom_make/test_custom_make.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
36 changes: 6 additions & 30 deletions tests/unit/workflows/custom_make/test_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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
)
32 changes: 32 additions & 0 deletions tests/unit/workflows/custom_make/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)