-
Notifications
You must be signed in to change notification settings - Fork 152
feat: use symlink instead of copy for some build operations #374
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
Changes from all commits
e47a5b1
8f3983a
2f9172b
128b24d
6316124
bb5b153
e19837e
7ab1f2a
5f32fd1
d3aa9c2
68d1db7
855c864
4699737
c34974b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,9 +10,9 @@ | |
| from aws_lambda_builders.actions import ( | ||
| CopySourceAction, | ||
| CleanUpAction, | ||
| CopyDependenciesAction, | ||
| MoveDependenciesAction, | ||
| BaseAction, | ||
| LinkSourceAction, | ||
| ) | ||
| from aws_lambda_builders.utils import which | ||
| from .actions import ( | ||
|
|
@@ -168,13 +168,18 @@ def _accelerate_workflow_actions( | |
| actions += [install_action, CleanUpAction(self.dependencies_dir)] | ||
| if self.combine_dependencies: | ||
| # Auto dependency layer disabled, first build | ||
| actions += [esbuild_with_deps, CopyDependenciesAction(source_dir, scratch_dir, self.dependencies_dir)] | ||
| actions += [ | ||
| esbuild_with_deps, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this require an experimental check? Python requires it and node es build does not?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was a conscious decision. Since is esbuild is still in beta, we figured it would be redundant to have two beta flags for it. |
||
| MoveDependenciesAction(source_dir, scratch_dir, self.dependencies_dir), | ||
| LinkSourceAction(self.dependencies_dir, scratch_dir), | ||
| ] | ||
| else: | ||
| # Auto dependency layer enabled, first build | ||
| actions += esbuild_no_deps + [MoveDependenciesAction(source_dir, scratch_dir, self.dependencies_dir)] | ||
| else: | ||
| if self.dependencies_dir: | ||
| actions.append(CopySourceAction(self.dependencies_dir, scratch_dir)) | ||
| actions.append(LinkSourceAction(self.dependencies_dir, scratch_dir)) | ||
|
|
||
| if self.combine_dependencies: | ||
| # Auto dependency layer disabled, subsequent builds | ||
| actions += [esbuild_with_deps] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| from unittest import TestCase | ||
| from unittest.mock import patch | ||
|
|
||
| from aws_lambda_builders import utils | ||
|
|
||
|
|
||
| class Test_create_symlink_or_copy(TestCase): | ||
| @patch("aws_lambda_builders.utils.Path") | ||
| @patch("aws_lambda_builders.utils.os") | ||
| @patch("aws_lambda_builders.utils.copytree") | ||
| def test_must_create_symlink_with_absolute_path(self, patched_copy_tree, pathced_os, patched_path): | ||
| source_path = "source/path" | ||
| destination_path = "destination/path" | ||
| utils.create_symlink_or_copy(source_path, destination_path) | ||
|
|
||
| pathced_os.symlink.assert_called_with( | ||
| patched_path(source_path).absolute(), patched_path(destination_path).absolute() | ||
| ) | ||
| patched_copy_tree.assert_not_called() | ||
|
|
||
| @patch("aws_lambda_builders.utils.Path") | ||
| @patch("aws_lambda_builders.utils.os") | ||
| @patch("aws_lambda_builders.utils.copytree") | ||
| def test_must_copy_if_symlink_fails(self, patched_copy_tree, pathced_os, patched_path): | ||
| pathced_os.symlink.side_effect = OSError("Unable to create symlink") | ||
|
|
||
| source_path = "source/path" | ||
| destination_path = "destination/path" | ||
| utils.create_symlink_or_copy(source_path, destination_path) | ||
|
|
||
| pathced_os.symlink.assert_called_once() | ||
| patched_copy_tree.assert_called_with(source_path, destination_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I right that this is backwards compatible, in the sense that if we cannot symlink we fall back to copying which was the existing behavior anyways?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's correct.