Skip to content

Commit

Permalink
Merge branch 'develop'
Browse files Browse the repository at this point in the history
  • Loading branch information
aws-sam-cli-bot committed Dec 4, 2023
2 parents 404ee7b + 8bc62be commit c17b01a
Show file tree
Hide file tree
Showing 13 changed files with 157 additions and 42 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/pr-labeler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
pull-requests: write
runs-on: ubuntu-latest
steps:
- uses: actions/github-script@v6
- uses: actions/github-script@v7
with:
github-token: ${{secrets.GITHUB_TOKEN}}
script: |
Expand Down
2 changes: 1 addition & 1 deletion aws_lambda_builders/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
# Changing version will trigger a new release!
# Please make the version change as the last step of your development.

__version__ = "1.42.0"
__version__ = "1.43.0"
RPC_PROTOCOL_VERSION = "0.3"
7 changes: 6 additions & 1 deletion aws_lambda_builders/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class _ActionMetaClass(type):
def __new__(mcs, name, bases, class_dict):
cls = type.__new__(mcs, name, bases, class_dict)

if cls.__name__ == "BaseAction":
if cls.__name__ in ["BaseAction", "NodejsNpmInstallOrUpdateBaseAction"]:
return cls

# Validate class variables
Expand Down Expand Up @@ -156,7 +156,12 @@ def __init__(self, source: Union[str, os.PathLike], dest: Union[str, os.PathLike
self._dest = dest

def execute(self):
source_path = Path(self._source)
destination_path = Path(self._dest)
if not source_path.exists():
# Source path doesn't exist, nothing to symlink
LOG.debug("Source path %s does not exist, skipping generating symlink", source_path)
return
if not destination_path.exists():
os.makedirs(destination_path.parent, exist_ok=True)
utils.create_symlink_or_copy(str(self._source), str(destination_path))
Expand Down
4 changes: 4 additions & 0 deletions aws_lambda_builders/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,10 @@ def create_symlink_or_copy(source: str, destination: str) -> None:
"""Tries to create symlink, if it fails it will copy source into destination"""
LOG.debug("Creating symlink; source: %s, destination: %s", source, destination)
try:
if Path(destination).exists() and Path(destination).is_symlink():
# The symlink is already in place, don't try re-creating it
LOG.debug("Symlink between %s and %s already exists, skipping generating symlink", source, destination)
return
os.symlink(Path(source).absolute(), Path(destination).absolute())
except OSError as ex:
LOG.warning(
Expand Down
56 changes: 44 additions & 12 deletions aws_lambda_builders/workflows/nodejs_npm/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,32 +72,35 @@ def execute(self):
raise ActionFailedError(str(ex))


class NodejsNpmInstallAction(BaseAction):

class NodejsNpmInstallOrUpdateBaseAction(BaseAction):
"""
A Lambda Builder Action that installs NPM project dependencies
A base Lambda Builder Action that is used for installs or updating NPM project dependencies
"""

NAME = "NpmInstall"
DESCRIPTION = "Installing dependencies from NPM"
PURPOSE = Purpose.RESOLVE_DEPENDENCIES

def __init__(self, install_dir: str, subprocess_npm: SubprocessNpm, install_links: Optional[bool] = False):
def __init__(self, install_dir: str, subprocess_npm: SubprocessNpm):
"""
Parameters
----------
install_dir : str
Dependencies will be installed in this directory.
subprocess_npm : SubprocessNpm
An instance of the NPM process wrapper
install_links : Optional[bool]
Uses the --install-links npm option if True, by default False
"""

super(NodejsNpmInstallAction, self).__init__()
super().__init__()
self.install_dir = install_dir
self.subprocess_npm = subprocess_npm
self.install_links = install_links


class NodejsNpmInstallAction(NodejsNpmInstallOrUpdateBaseAction):
"""
A Lambda Builder Action that installs NPM project dependencies
"""

NAME = "NpmInstall"
DESCRIPTION = "Installing dependencies from NPM"

def execute(self):
"""
Expand All @@ -109,9 +112,38 @@ def execute(self):
LOG.debug("NODEJS installing in: %s", self.install_dir)

command = ["install", "-q", "--no-audit", "--no-save", "--unsafe-perm", "--production"]
if self.install_links:
command.append("--install-links")
self.subprocess_npm.run(command, cwd=self.install_dir)

except NpmExecutionError as ex:
raise ActionFailedError(str(ex))


class NodejsNpmUpdateAction(NodejsNpmInstallOrUpdateBaseAction):
"""
A Lambda Builder Action that installs NPM project dependencies
"""

NAME = "NpmUpdate"
DESCRIPTION = "Updating dependencies from NPM"

def execute(self):
"""
Runs the action.
:raises lambda_builders.actions.ActionFailedError: when NPM execution fails
"""
try:
LOG.debug("NODEJS updating in: %s", self.install_dir)

command = [
"update",
"--no-audit",
"--no-save",
"--unsafe-perm",
"--production",
"--no-package-lock",
"--install-links",
]
self.subprocess_npm.run(command, cwd=self.install_dir)

except NpmExecutionError as ex:
Expand Down
18 changes: 10 additions & 8 deletions aws_lambda_builders/workflows/nodejs_npm/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
NodejsNpmPackAction,
NodejsNpmrcAndLockfileCopyAction,
NodejsNpmrcCleanUpAction,
NodejsNpmUpdateAction,
)
from aws_lambda_builders.workflows.nodejs_npm.npm import SubprocessNpm
from aws_lambda_builders.workflows.nodejs_npm.utils import OSUtils
Expand Down Expand Up @@ -119,7 +120,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
subprocess_npm=subprocess_npm,
osutils=osutils,
build_options=self.options,
install_links=is_building_in_source,
is_building_in_source=is_building_in_source,
)
)

Expand Down Expand Up @@ -211,7 +212,7 @@ def get_install_action(
subprocess_npm: SubprocessNpm,
osutils: OSUtils,
build_options: Optional[dict],
install_links: Optional[bool] = False,
is_building_in_source: Optional[bool] = False,
):
"""
Get the install action used to install dependencies.
Expand All @@ -228,8 +229,8 @@ def get_install_action(
An instance of OS Utilities for file manipulation
build_options : Optional[dict]
Object containing build options configurations
install_links : Optional[bool]
Uses the --install-links npm option if True, by default False
is_building_in_source : Optional[bool]
States whether --build-in-source flag is set or not
Returns
-------
Expand All @@ -245,12 +246,13 @@ def get_install_action(

if (osutils.file_exists(lockfile_path) or osutils.file_exists(shrinkwrap_path)) and npm_ci_option:
return NodejsNpmCIAction(
install_dir=install_dir, subprocess_npm=subprocess_npm, install_links=install_links
install_dir=install_dir, subprocess_npm=subprocess_npm, install_links=is_building_in_source
)

return NodejsNpmInstallAction(
install_dir=install_dir, subprocess_npm=subprocess_npm, install_links=install_links
)
if is_building_in_source:
return NodejsNpmUpdateAction(install_dir=install_dir, subprocess_npm=subprocess_npm)

return NodejsNpmInstallAction(install_dir=install_dir, subprocess_npm=subprocess_npm)

@staticmethod
def can_use_install_links(npm_process: SubprocessNpm) -> bool:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim
subprocess_npm=self.subprocess_npm,
osutils=self.osutils,
build_options=self.options,
install_links=is_building_in_source,
is_building_in_source=is_building_in_source,
)
)

Expand Down
4 changes: 2 additions & 2 deletions requirements/dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ pyelftools~=0.30 # Used to verify the generated Go binary architecture in integr

# formatter
black==22.6.0; python_version < "3.8"
black==23.10.1; python_version >= "3.8"
ruff==0.1.4
black==23.11.0; python_version >= "3.8"
ruff==0.1.5
40 changes: 40 additions & 0 deletions tests/integration/workflows/nodejs_npm/test_nodejs_npm.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,46 @@ def test_build_in_source_with_download_dependencies(self, runtime):
output_files = set(os.listdir(self.artifacts_dir))
self.assertEqual(expected_files, output_files)

@parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",), ("nodejs20.x",)])
def test_build_in_source_with_removed_dependencies(self, runtime):
# run a build with default requirements and confirm dependencies are downloaded
source_dir = os.path.join(self.temp_testdata_dir, "npm-deps")

self.builder.build(
source_dir,
self.artifacts_dir,
self.scratch_dir,
os.path.join(source_dir, "package.json"),
runtime=runtime,
build_in_source=True,
)

# dependencies installed in source folder
source_node_modules = os.path.join(source_dir, "node_modules")
self.assertTrue(os.path.isdir(source_node_modules))
expected_node_modules_contents = {"minimal-request-promise", ".package-lock.json"}
self.assertEqual(set(os.listdir(source_node_modules)), expected_node_modules_contents)

# update package.json with empty one and re-run the build then confirm node_modules are cleared up
shutil.copy2(
os.path.join(self.temp_testdata_dir, "no-deps", "package.json"),
os.path.join(self.temp_testdata_dir, "npm-deps", "package.json"),
)

self.builder.build(
source_dir,
self.artifacts_dir,
self.scratch_dir,
os.path.join(source_dir, "package.json"),
runtime=runtime,
build_in_source=True,
)
# dependencies installed in source folder
source_node_modules = os.path.join(source_dir, "node_modules")
self.assertTrue(os.path.isdir(source_node_modules))
self.assertIn(".package-lock.json", set(os.listdir(source_node_modules)))
self.assertNotIn("minimal-request-promise", set(os.listdir(source_node_modules)))

@parameterized.expand([("nodejs12.x",), ("nodejs14.x",), ("nodejs16.x",), ("nodejs18.x",), ("nodejs20.x",)])
def test_build_in_source_with_download_dependencies_local_dependency(self, runtime):
source_dir = os.path.join(self.temp_testdata_dir, "with-local-dependency")
Expand Down
13 changes: 13 additions & 0 deletions tests/unit/test_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
MoveDependenciesAction,
CleanUpAction,
DependencyManager,
LinkSinglePathAction,
)


Expand Down Expand Up @@ -262,3 +263,15 @@ def test_excludes_dependencies_from_source(
@staticmethod
def _convert_strings_to_paths(source_dest_list):
return map(lambda item: (Path(item[0]), Path(item[1])), source_dest_list)


class TestLinkSinglePathAction(TestCase):
@patch("aws_lambda_builders.actions.os.makedirs")
@patch("aws_lambda_builders.utils.create_symlink_or_copy")
def test_skips_non_existent_source(self, mock_create_symlink_or_copy, mock_makedirs):
src = "src/path"
dest = "dest/path"

LinkSinglePathAction(source=src, dest=dest).execute()
mock_create_symlink_or_copy.assert_not_called()
mock_makedirs.assert_not_called()
28 changes: 21 additions & 7 deletions tests/unit/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,27 @@
import platform
from pathlib import Path

from unittest import TestCase
from unittest.mock import patch
from unittest.mock import patch, Mock, MagicMock

from aws_lambda_builders import utils
from aws_lambda_builders.utils import decode


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):
def test_must_create_symlink_with_absolute_path(self, patched_copy_tree, patched_os):
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()
)
p = MagicMock()
p.return_value = False

with patch("aws_lambda_builders.utils.Path.is_symlink", p):
utils.create_symlink_or_copy(source_path, destination_path)

patched_os.symlink.assert_called_with(Path(source_path).absolute(), Path(destination_path).absolute())
patched_copy_tree.assert_not_called()

@patch("aws_lambda_builders.utils.Path")
Expand All @@ -34,6 +37,17 @@ def test_must_copy_if_symlink_fails(self, patched_copy_tree, pathced_os, patched
pathced_os.symlink.assert_called_once()
patched_copy_tree.assert_called_with(source_path, destination_path)

@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):
source_path = "source/path"
destination_path = "destination/path"
utils.create_symlink_or_copy(source_path, destination_path)

pathced_os.symlink.assert_not_called()
patched_copy_tree.assert_not_called()


class TestDecode(TestCase):
def test_does_not_crash_non_utf8_encoding(self):
Expand Down
9 changes: 5 additions & 4 deletions tests/unit/workflows/nodejs_npm/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
NodejsNpmrcCleanUpAction,
NodejsNpmLockFileCleanUpAction,
NodejsNpmCIAction,
NodejsNpmUpdateAction,
)


Expand Down Expand Up @@ -107,7 +108,7 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_without_depende
self.assertIsInstance(workflow.actions[3], CopySourceAction)
self.assertEqual(workflow.actions[3].source_dir, "source")
self.assertEqual(workflow.actions[3].dest_dir, "artifacts")
self.assertIsInstance(workflow.actions[4], NodejsNpmInstallAction)
self.assertIsInstance(workflow.actions[4], NodejsNpmUpdateAction)
self.assertEqual(workflow.actions[4].install_dir, "not_source")
self.assertIsInstance(workflow.actions[5], LinkSinglePathAction)
self.assertEqual(workflow.actions[5]._source, os.path.join("not_source", "node_modules"))
Expand Down Expand Up @@ -331,7 +332,7 @@ def test_build_in_source_with_download_dependencies(self, can_use_links_mock):
self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction)
self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction)
self.assertIsInstance(workflow.actions[2], CopySourceAction)
self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction)
self.assertIsInstance(workflow.actions[3], NodejsNpmUpdateAction)
self.assertEqual(workflow.actions[3].install_dir, source_dir)
self.assertIsInstance(workflow.actions[4], LinkSinglePathAction)
self.assertEqual(workflow.actions[4]._source, os.path.join(source_dir, "node_modules"))
Expand All @@ -358,7 +359,7 @@ def test_build_in_source_with_download_dependencies_and_dependencies_dir(self, c
self.assertIsInstance(workflow.actions[0], NodejsNpmPackAction)
self.assertIsInstance(workflow.actions[1], NodejsNpmrcAndLockfileCopyAction)
self.assertIsInstance(workflow.actions[2], CopySourceAction)
self.assertIsInstance(workflow.actions[3], NodejsNpmInstallAction)
self.assertIsInstance(workflow.actions[3], NodejsNpmUpdateAction)
self.assertEqual(workflow.actions[3].install_dir, source_dir)
self.assertIsInstance(workflow.actions[4], LinkSinglePathAction)
self.assertEqual(workflow.actions[4]._source, os.path.join(source_dir, "node_modules"))
Expand Down Expand Up @@ -442,5 +443,5 @@ def test_workflow_revert_build_in_source(self, install_action_mock, install_link
subprocess_npm=ANY,
osutils=ANY,
build_options=ANY,
install_links=False,
is_building_in_source=False,
)
Loading

0 comments on commit c17b01a

Please sign in to comment.