-
Notifications
You must be signed in to change notification settings - Fork 141
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
Add Include resource functionality to npm and npm/esbuild #431
Changes from 11 commits
e891892
fb98729
9e954a4
c6f627d
b575481
d8aede9
52e7436
63c938f
8074e1f
66f5284
4d857dc
831032c
3a6eea4
f054638
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 |
---|---|---|
|
@@ -6,10 +6,13 @@ | |
import sys | ||
import os | ||
import logging | ||
from glob import glob | ||
from pathlib import Path | ||
from typing import Union | ||
import time | ||
from typing import Union, Dict | ||
|
||
from aws_lambda_builders.architecture import ARM64 | ||
from aws_lambda_builders.exceptions import FileOperationError | ||
|
||
LOG = logging.getLogger(__name__) | ||
|
||
|
@@ -222,3 +225,53 @@ def extract_tarfile(tarfile_path: Union[str, os.PathLike], unpack_dir: Union[str | |
raise tarfile.ExtractError("Attempted Path Traversal in Tar File") | ||
|
||
tar.extractall(unpack_dir) | ||
|
||
|
||
def glob_copy(source: Union[str, list], destination: str) -> None: | ||
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. Let's add type hints and docstrings to all functions. 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. Sure thing |
||
items = source if isinstance(source, list) else [source] | ||
dest_path = Path(destination) | ||
known_paths = [] | ||
for item in items: | ||
if os.path.isabs(item.replace('\\', '/')): | ||
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. Do we need the 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. I had to have the replace in there to get the "absolute" detection to work properly under Windows. "\foo" is considered a relative path, but "/foo" is absolute (go figure). The purposes of this call is to make sure that the requested asset 's path is relative to the project, so either "\foo" or "/foo" should be treated as non-relative. |
||
raise FileOperationError(operation_name='glob_copy', reason='"{item}" is not a relative path'.format(item=item)) | ||
files = glob(item, recursive=True) | ||
if len(files) == 0: | ||
raise FileOperationError(operation_name='glob_copy', reason='"{item}" not found'.format(item=item)) | ||
for file in files: | ||
save_to = str(dest_path.joinpath(file)) | ||
LOG.debug("Copying resource file from source (%s) to destination (%s)", file, save_to) | ||
save_to_dir = os.path.dirname(save_to) | ||
if save_to_dir not in known_paths: | ||
os.makedirs(save_to_dir, exist_ok=True) | ||
known_paths.append(save_to_dir) | ||
shutil.copyfile(file, save_to) | ||
|
||
|
||
def robust_rmtree(path, timeout=1): | ||
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. If this is only used in tests, we should move it there. 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. Moved to test/functional/test_utils.py |
||
"""Robustly tries to delete paths, because shutil.rmtree is broken, | ||
See https://bugs.python.org/issue40143 | ||
|
||
Retries several times if an OSError occurs. | ||
If the final attempt fails, the Exception is propagated | ||
to the caller. | ||
""" | ||
next_retry = 0.001 | ||
while next_retry < timeout: | ||
try: | ||
shutil.rmtree(path) | ||
return # Only hits this on success | ||
except OSError: | ||
# Increase the next_retry and try again | ||
time.sleep(next_retry) | ||
next_retry *= 2 | ||
|
||
# Final attempt, pass any Exceptions up to caller. | ||
shutil.rmtree(path) | ||
|
||
|
||
def get_option_from_args(args: Union[None, Dict[str, any]], option_name: str) -> any: | ||
if args is not None: | ||
options = args.get("options", None) | ||
if options is not None: | ||
return options.get(option_name, None) | ||
return None |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ | |
CleanUpAction, | ||
CopyDependenciesAction, | ||
MoveDependenciesAction, | ||
CopyResourceAction, | ||
) | ||
|
||
from .actions import ( | ||
|
@@ -23,6 +24,7 @@ | |
) | ||
from .utils import OSUtils | ||
from .npm import SubprocessNpm | ||
from ...utils import get_option_from_args | ||
|
||
LOG = logging.getLogger(__name__) | ||
|
||
|
@@ -65,6 +67,12 @@ def __init__(self, source_dir, artifacts_dir, scratch_dir, manifest_path, runtim | |
source_dir, artifacts_dir, scratch_dir, manifest_path, osutils, subprocess_npm | ||
) | ||
|
||
include = get_option_from_args(kwargs, "include") | ||
if isinstance(include, (list, str)): | ||
self.actions.append(CopyResourceAction(source_dir, include, artifacts_dir)) | ||
elif include is not None: | ||
raise ValueError("Resource include items must be strings or lists of strings") | ||
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. Let's use a custom exception here too. 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. Yes sir |
||
|
||
def actions_without_bundler(self, source_dir, artifacts_dir, scratch_dir, manifest_path, osutils, subprocess_npm): | ||
""" | ||
Generate a list of Nodejs build actions without a bundler | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
//excluded | ||
const x = 1; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
//included | ||
const x = 1; |
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.
Make sure to run
make black
to reformat according to our style guide.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.
Sorry, missed that