Skip to content
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

fix: use npm update when building in source #579

Merged
merged 4 commits into from
Nov 27, 2023
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
2 changes: 1 addition & 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
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
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",)])
mildaniel marked this conversation as resolved.
Show resolved Hide resolved
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
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,
)
14 changes: 9 additions & 5 deletions tests/unit/workflows/nodejs_npm_esbuild/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@
LinkSinglePathAction,
)
from aws_lambda_builders.architecture import ARM64
from aws_lambda_builders.workflows.nodejs_npm.actions import NodejsNpmInstallAction, NodejsNpmCIAction
from aws_lambda_builders.workflows.nodejs_npm.actions import (
NodejsNpmInstallAction,
NodejsNpmCIAction,
NodejsNpmUpdateAction,
)
from aws_lambda_builders.workflows.nodejs_npm_esbuild import NodejsNpmEsbuildWorkflow
from aws_lambda_builders.workflows.nodejs_npm_esbuild.actions import EsbuildBundleAction
from aws_lambda_builders.workflows.nodejs_npm_esbuild.esbuild import SubprocessEsbuild
Expand Down Expand Up @@ -313,7 +317,7 @@ def test_workflow_uses_production_npm_version(self, get_workflow_mock):
subprocess_npm=ANY,
osutils=ANY,
build_options=None,
install_links=False,
is_building_in_source=False,
)

@patch("aws_lambda_builders.workflows.nodejs_npm_esbuild.workflow.NodejsNpmEsbuildWorkflow._get_esbuild_subprocess")
Expand Down Expand Up @@ -360,7 +364,7 @@ def test_build_in_source(self, install_links_mock):

self.assertEqual(len(workflow.actions), 2)

self.assertIsInstance(workflow.actions[0], NodejsNpmInstallAction)
self.assertIsInstance(workflow.actions[0], NodejsNpmUpdateAction)
self.assertEqual(workflow.actions[0].install_dir, source_dir)
self.assertIsInstance(workflow.actions[1], EsbuildBundleAction)
self.assertEqual(workflow.actions[1]._working_directory, source_dir)
Expand Down Expand Up @@ -405,7 +409,7 @@ def test_workflow_sets_up_npm_actions_with_download_dependencies_without_depende

self.assertEqual(len(workflow.actions), 3)

self.assertIsInstance(workflow.actions[0], NodejsNpmInstallAction)
self.assertIsInstance(workflow.actions[0], NodejsNpmUpdateAction)
self.assertEqual(workflow.actions[0].install_dir, "not_source")
self.assertIsInstance(workflow.actions[1], LinkSinglePathAction)
self.assertEqual(workflow.actions[1]._source, os.path.join("not_source", "node_modules"))
Expand Down Expand Up @@ -439,7 +443,7 @@ 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,
)

@parameterized.expand(
Expand Down
Loading