diff --git a/aws_lambda_builders/actions.py b/aws_lambda_builders/actions.py index 4bd3fbc75..3d4a520f7 100644 --- a/aws_lambda_builders/actions.py +++ b/aws_lambda_builders/actions.py @@ -6,10 +6,10 @@ import os import shutil from pathlib import Path -from typing import Iterator, Set, Tuple +from typing import Iterator, Set, Tuple, Union from aws_lambda_builders import utils -from aws_lambda_builders.utils import copytree +from aws_lambda_builders.utils import copytree, create_symlink_or_copy LOG = logging.getLogger(__name__) @@ -105,13 +105,19 @@ class CopySourceAction(BaseAction): PURPOSE = Purpose.COPY_SOURCE - def __init__(self, source_dir, dest_dir, excludes=None): + def __init__(self, source_dir, dest_dir, excludes=None, maintain_symlinks=False): self.source_dir = source_dir self.dest_dir = dest_dir self.excludes = excludes or [] + self.maintain_symlinks = maintain_symlinks def execute(self): - copytree(self.source_dir, self.dest_dir, ignore=shutil.ignore_patterns(*self.excludes)) + copytree( + self.source_dir, + self.dest_dir, + ignore=shutil.ignore_patterns(*self.excludes), + maintain_symlinks=self.maintain_symlinks, + ) class LinkSourceAction(BaseAction): @@ -138,6 +144,24 @@ def execute(self): utils.create_symlink_or_copy(str(source_path), str(destination_path)) +class LinkSinglePathAction(BaseAction): + NAME = "LinkSource" + + DESCRIPTION = "Creates symbolic link at destination, pointing to source" + + PURPOSE = Purpose.LINK_SOURCE + + def __init__(self, source: Union[str, os.PathLike], dest: Union[str, os.PathLike]): + self._source = source + self._dest = dest + + def execute(self): + destination_path = Path(self._dest) + if not destination_path.exists(): + os.makedirs(destination_path.parent, exist_ok=True) + utils.create_symlink_or_copy(str(self._source), str(destination_path)) + + class CopyDependenciesAction(BaseAction): NAME = "CopyDependencies" @@ -145,17 +169,23 @@ class CopyDependenciesAction(BaseAction): PURPOSE = Purpose.COPY_DEPENDENCIES - def __init__(self, source_dir, artifact_dir, destination_dir): + def __init__(self, source_dir, artifact_dir, destination_dir, maintain_symlinks=False): self.source_dir = source_dir self.artifact_dir = artifact_dir self.dest_dir = destination_dir + self.maintain_symlinks = maintain_symlinks def execute(self): deps_manager = DependencyManager(self.source_dir, self.artifact_dir, self.dest_dir) for dependencies_source, new_destination in deps_manager.yield_source_dest(): - if os.path.isdir(dependencies_source): - copytree(dependencies_source, new_destination) + if os.path.islink(dependencies_source) and self.maintain_symlinks: + os.makedirs(os.path.dirname(new_destination), exist_ok=True) + linkto = os.readlink(dependencies_source) + create_symlink_or_copy(linkto, new_destination) + shutil.copystat(dependencies_source, new_destination, follow_symlinks=False) + elif os.path.isdir(dependencies_source): + copytree(dependencies_source, new_destination, maintain_symlinks=self.maintain_symlinks) else: os.makedirs(os.path.dirname(new_destination), exist_ok=True) shutil.copy2(dependencies_source, new_destination) @@ -209,7 +239,9 @@ def execute(self): target_path = os.path.join(self.target_dir, name) LOG.debug("Clean up action: %s is deleted", str(target_path)) - if os.path.isdir(target_path): + if os.path.islink(target_path): + os.unlink(target_path) + elif os.path.isdir(target_path): shutil.rmtree(target_path) else: os.remove(target_path) diff --git a/aws_lambda_builders/utils.py b/aws_lambda_builders/utils.py index a1e55f7df..d8db688fe 100644 --- a/aws_lambda_builders/utils.py +++ b/aws_lambda_builders/utils.py @@ -7,36 +7,40 @@ import shutil import sys from pathlib import Path -from typing import Union +from typing import Callable, List, Optional, Set, Union from aws_lambda_builders.architecture import ARM64 LOG = logging.getLogger(__name__) -def copytree(source, destination, ignore=None, include=None): +def copytree( + source: str, + destination: str, + ignore: Optional[Callable[[str, List[str]], Set[str]]] = None, + include: Optional[Callable[[str], bool]] = None, + maintain_symlinks: bool = False, +) -> None: """ Similar to shutil.copytree except that it removes the limitation that the destination directory should be present. - :type source: str - :param source: - Path to the source folder to copy - - :type destination: str - :param destination: - Path to destination folder - - :type ignore: function - :param ignore: + Parameters + ---------- + source : str + Path to the source folder to copy. + destination : str + Path to destination folder. + ignore : Optional[Callable[[str, List[str]], Set[str]]] A function that returns a set of file names to ignore, given a list of available file names. Similar to the - ``ignore`` property of ``shutils.copytree`` method - - :type include: Callable[[str], bool] - :param include: + ``ignore`` property of ``shutils.copytree`` method. By default None. + include : Optional[Callable[[str], bool]] A function that will decide whether a file should be copied or skipped it. It accepts file name as parameter and return True or False. Returning True will continue copy operation, returning False will skip copy operation - for that file + for that file. By default None. + maintain_symlinks : bool, optional + If True, symbolic links in the source are represented as symbolic links in the destination. + If False, the contents are copied over. By default False. """ if not os.path.exists(source): @@ -74,8 +78,12 @@ def copytree(source, destination, ignore=None, include=None): LOG.debug("File (%s) doesn't satisfy the include rule, skipping it", name) continue - if os.path.isdir(new_source): - copytree(new_source, new_destination, ignore=ignore, include=include) + if os.path.islink(new_source) and maintain_symlinks: + linkto = os.readlink(new_source) + create_symlink_or_copy(linkto, new_destination) + shutil.copystat(new_source, new_destination, follow_symlinks=False) + elif os.path.isdir(new_source): + copytree(new_source, new_destination, ignore=ignore, include=include, maintain_symlinks=maintain_symlinks) else: LOG.debug("Copying source file (%s) to destination (%s)", new_source, new_destination) shutil.copy2(new_source, new_destination) @@ -193,7 +201,8 @@ def create_symlink_or_copy(source: str, destination: str) -> None: os.symlink(Path(source).absolute(), Path(destination).absolute()) except OSError as ex: LOG.warning( - "Symlink operation is failed, falling back to copying files", + "Symbolic link creation failed, falling back to copying files instead. To optimize speed, " + "consider enabling the necessary settings or privileges on your system to support symbolic links.", exc_info=ex if LOG.isEnabledFor(logging.DEBUG) else None, ) copytree(source, destination) diff --git a/tests/functional/test_actions.py b/tests/functional/test_actions.py new file mode 100644 index 000000000..9909bcffe --- /dev/null +++ b/tests/functional/test_actions.py @@ -0,0 +1,119 @@ +import os +from pathlib import Path +import tempfile +from unittest import TestCase +from parameterized import parameterized + + +from aws_lambda_builders.actions import CopyDependenciesAction, LinkSinglePathAction, MoveDependenciesAction +from aws_lambda_builders.utils import copytree +from tests.testing_utils import read_link_without_junction_prefix + + +class TestCopyDependenciesAction(TestCase): + @parameterized.expand( + [ + ("single_file",), + ("multiple_files",), + ("empty_subfolders",), + ] + ) + def test_copy_dependencies_action(self, source_folder): + curr_dir = Path(__file__).resolve().parent + test_folder = os.path.join(curr_dir, "testdata", source_folder) + with tempfile.TemporaryDirectory() as tmpdir: + empty_source = os.path.join(tmpdir, "empty_source") + target = os.path.join(tmpdir, "target") + + os.mkdir(empty_source) + + copy_dependencies_action = CopyDependenciesAction(empty_source, test_folder, target) + copy_dependencies_action.execute() + + self.assertEqual(os.listdir(test_folder), os.listdir(target)) + + def test_must_maintain_symlinks_if_enabled(self): + with tempfile.TemporaryDirectory() as tmpdir: + source_dir = os.path.join(tmpdir, "source") + artifact_dir = os.path.join(tmpdir, "artifact") + destination_dir = os.path.join(tmpdir, "destination") + + source_node_modules = os.path.join(source_dir, "node_modules") + os.makedirs(source_node_modules) + os.makedirs(artifact_dir) + os.symlink(source_node_modules, os.path.join(artifact_dir, "node_modules")) + + copy_dependencies_action = CopyDependenciesAction( + source_dir=source_dir, + artifact_dir=artifact_dir, + destination_dir=destination_dir, + maintain_symlinks=True, + ) + copy_dependencies_action.execute() + + destination_node_modules = os.path.join(destination_dir, "node_modules") + self.assertTrue(os.path.islink(destination_node_modules)) + destination_node_modules_target = read_link_without_junction_prefix(destination_node_modules) + self.assertEqual(destination_node_modules_target, source_node_modules) + + def test_must_not_maintain_symlinks_by_default(self): + with tempfile.TemporaryDirectory() as tmpdir: + source_dir = os.path.join(tmpdir, "source") + artifact_dir = os.path.join(tmpdir, "artifact") + destination_dir = os.path.join(tmpdir, "destination") + + source_node_modules = os.path.join(source_dir, "node_modules") + os.makedirs(os.path.join(source_node_modules, "some_package")) + os.makedirs(artifact_dir) + os.symlink(source_node_modules, os.path.join(artifact_dir, "node_modules")) + + copy_dependencies_action = CopyDependenciesAction( + source_dir=source_dir, artifact_dir=artifact_dir, destination_dir=destination_dir + ) + copy_dependencies_action.execute() + + destination_node_modules = os.path.join(destination_dir, "node_modules") + self.assertFalse(os.path.islink(destination_node_modules)) + self.assertEqual(os.listdir(destination_node_modules), os.listdir(source_node_modules)) + + +class TestLinkSinglePathAction(TestCase): + def test_link_directory(self): + with tempfile.TemporaryDirectory() as tmpdir: + source_dir = os.path.join(tmpdir, "source") + os.makedirs(source_dir) + dest_dir = os.path.join(tmpdir, "dest") + + link_action = LinkSinglePathAction(source_dir, dest_dir) + link_action.execute() + + self.assertTrue(os.path.islink(dest_dir)) + dest_dir_target = read_link_without_junction_prefix(dest_dir) + self.assertEqual(dest_dir_target, source_dir) + + +class TestMoveDependenciesAction(TestCase): + @parameterized.expand( + [ + ("single_file",), + ("multiple_files",), + ("empty_subfolders",), + ] + ) + def test_move_dependencies_action(self, source_folder): + curr_dir = Path(__file__).resolve().parent + test_folder = os.path.join(curr_dir, "testdata", source_folder) + with tempfile.TemporaryDirectory() as tmpdir: + test_source = os.path.join(tmpdir, "test_source") + empty_source = os.path.join(tmpdir, "empty_source") + target = os.path.join(tmpdir, "target") + + os.mkdir(test_source) + os.mkdir(empty_source) + + copytree(test_folder, test_source) + + move_dependencies_action = MoveDependenciesAction(empty_source, test_source, target) + move_dependencies_action.execute() + + self.assertEqual(os.listdir(test_folder), os.listdir(target)) diff --git a/tests/functional/test_utils.py b/tests/functional/test_utils.py index 81e07c42e..a10b57d41 100644 --- a/tests/functional/test_utils.py +++ b/tests/functional/test_utils.py @@ -7,6 +7,7 @@ from unittest import TestCase from aws_lambda_builders.utils import copytree, get_goarch, extract_tarfile +from tests.testing_utils import read_link_without_junction_prefix class TestCopyTree(TestCase): @@ -64,6 +65,58 @@ def test_must_return_valid_go_architecture(self): self.assertEqual(get_goarch("x86_64"), "amd64") self.assertEqual(get_goarch(""), "amd64") + def test_must_maintain_symlinks_if_enabled(self): + # set up symlinked file and directory + source_target_file_path = file(self.source, "targetfile.txt") + source_symlink_file_path = os.path.join(self.source, "symlinkfile.txt") + os.symlink(source_target_file_path, source_symlink_file_path) + + source_target_dir_path = os.path.join(self.source, "targetdir") + os.makedirs(source_target_dir_path) + source_symlink_dir_path = os.path.join(self.source, "symlinkdir") + os.symlink(source_target_dir_path, source_symlink_dir_path) + + # call copytree + copytree(self.source, self.dest, maintain_symlinks=True) + + # assert + self.assertEqual(set(os.listdir(self.dest)), {"targetfile.txt", "symlinkfile.txt", "targetdir", "symlinkdir"}) + + dest_symlink_file_path = os.path.join(self.dest, "symlinkfile.txt") + self.assertTrue(os.path.islink(dest_symlink_file_path)) + dest_symlink_file_target = read_link_without_junction_prefix(dest_symlink_file_path) + self.assertEqual(dest_symlink_file_target, source_target_file_path) + + dest_symlink_dir_path = os.path.join(self.dest, "symlinkdir") + self.assertTrue(os.path.islink(dest_symlink_dir_path)) + dest_symlink_dir_target = read_link_without_junction_prefix(dest_symlink_file_path) + self.assertEqual(dest_symlink_dir_target, source_target_file_path) + + def test_must_not_maintain_symlinks_by_default(self): + # set up symlinked file and directory + source_target_file_path = file(self.source, "targetfile.txt") + source_symlink_file_path = os.path.join(self.source, "symlinkfile.txt") + os.symlink(source_target_file_path, source_symlink_file_path) + + source_target_dir_path = os.path.join(self.source, "targetdir") + os.makedirs(source_target_dir_path) + file(source_target_dir_path, "file_in_dir.txt") + source_symlink_dir_path = os.path.join(self.source, "symlinkdir") + os.symlink(source_target_dir_path, source_symlink_dir_path) + + # call copytree + copytree(self.source, self.dest) + + # assert + self.assertEqual(set(os.listdir(self.dest)), {"targetfile.txt", "symlinkfile.txt", "targetdir", "symlinkdir"}) + + dest_symlink_file_path = os.path.join(self.dest, "symlinkfile.txt") + self.assertFalse(os.path.islink(dest_symlink_file_path)) + + dest_symlink_dir_path = os.path.join(self.dest, "symlinkdir") + self.assertFalse(os.path.islink(dest_symlink_dir_path)) + self.assertEqual(os.listdir(dest_symlink_dir_path), os.listdir(source_target_dir_path)) + class TestExtractTarFile(TestCase): def test_extract_tarfile_unpacks_a_tar(self): @@ -91,3 +144,5 @@ def file(*args): # empty file open(path, "a").close() + + return path diff --git a/tests/integration/testdata/empty_subfolders/sub_folder/sub_folder/test_file.txt b/tests/functional/testdata/empty_subfolders/sub_folder/sub_folder/test_file.txt similarity index 100% rename from tests/integration/testdata/empty_subfolders/sub_folder/sub_folder/test_file.txt rename to tests/functional/testdata/empty_subfolders/sub_folder/sub_folder/test_file.txt diff --git a/tests/integration/testdata/multiple_files/sub_folder/test_file3.txt b/tests/functional/testdata/multiple_files/sub_folder/test_file3.txt similarity index 100% rename from tests/integration/testdata/multiple_files/sub_folder/test_file3.txt rename to tests/functional/testdata/multiple_files/sub_folder/test_file3.txt diff --git a/tests/integration/testdata/multiple_files/test_file.txt b/tests/functional/testdata/multiple_files/test_file.txt similarity index 100% rename from tests/integration/testdata/multiple_files/test_file.txt rename to tests/functional/testdata/multiple_files/test_file.txt diff --git a/tests/integration/testdata/multiple_files/test_file2.txt b/tests/functional/testdata/multiple_files/test_file2.txt similarity index 100% rename from tests/integration/testdata/multiple_files/test_file2.txt rename to tests/functional/testdata/multiple_files/test_file2.txt diff --git a/tests/integration/testdata/single_file/test_file.txt b/tests/functional/testdata/single_file/test_file.txt similarity index 100% rename from tests/integration/testdata/single_file/test_file.txt rename to tests/functional/testdata/single_file/test_file.txt diff --git a/tests/integration/test_actions.py b/tests/integration/test_actions.py deleted file mode 100644 index 4e4f93a31..000000000 --- a/tests/integration/test_actions.py +++ /dev/null @@ -1,59 +0,0 @@ -import os -from pathlib import Path -import tempfile -from unittest import TestCase -from parameterized import parameterized - - -from aws_lambda_builders.actions import CopyDependenciesAction, MoveDependenciesAction -from aws_lambda_builders.utils import copytree - - -class TestCopyDependenciesAction(TestCase): - @parameterized.expand( - [ - ("single_file",), - ("multiple_files",), - ("empty_subfolders",), - ] - ) - def test_copy_dependencies_action(self, source_folder): - curr_dir = Path(__file__).resolve().parent - test_folder = os.path.join(curr_dir, "testdata", source_folder) - with tempfile.TemporaryDirectory() as tmpdir: - empty_source = os.path.join(tmpdir, "empty_source") - target = os.path.join(tmpdir, "target") - - os.mkdir(empty_source) - - copy_dependencies_action = CopyDependenciesAction(empty_source, test_folder, target) - copy_dependencies_action.execute() - - self.assertEqual(os.listdir(test_folder), os.listdir(target)) - - -class TestMoveDependenciesAction(TestCase): - @parameterized.expand( - [ - ("single_file",), - ("multiple_files",), - ("empty_subfolders",), - ] - ) - def test_move_dependencies_action(self, source_folder): - curr_dir = Path(__file__).resolve().parent - test_folder = os.path.join(curr_dir, "testdata", source_folder) - with tempfile.TemporaryDirectory() as tmpdir: - test_source = os.path.join(tmpdir, "test_source") - empty_source = os.path.join(tmpdir, "empty_source") - target = os.path.join(tmpdir, "target") - - os.mkdir(test_source) - os.mkdir(empty_source) - - copytree(test_folder, test_source) - - move_dependencies_action = MoveDependenciesAction(empty_source, test_source, target) - move_dependencies_action.execute() - - self.assertEqual(os.listdir(test_folder), os.listdir(target)) diff --git a/tests/testing_utils.py b/tests/testing_utils.py new file mode 100644 index 000000000..345f62e50 --- /dev/null +++ b/tests/testing_utils.py @@ -0,0 +1,22 @@ +import os + + +def read_link_without_junction_prefix(path: str) -> str: + """ + When our tests run on CI on Windows, it seems to use junctions, which causes symlink targets + have a prefix. This function reads a symlink and returns the target without the prefix (if any). + + Parameters + ---------- + path : str + Path which may or may not have a junction prefix. + + Returns + ------- + str + Path without junction prefix, if any. + """ + target = os.readlink(path) + if target.startswith("\\\\?\\"): # \\?\, with escaped slashes + target = target[4:] + return target diff --git a/tests/unit/test_actions.py b/tests/unit/test_actions.py index f6ffad08c..8381d2bb8 100644 --- a/tests/unit/test_actions.py +++ b/tests/unit/test_actions.py @@ -60,7 +60,7 @@ def test_must_copy(self, copytree_mock): action = CopySourceAction(source_dir, dest_dir, excludes=excludes) action.execute() - copytree_mock.assert_called_with(source_dir, dest_dir, ignore=ANY) + copytree_mock.assert_called_with(source_dir, dest_dir, ignore=ANY, maintain_symlinks=False) class TestCopyDependenciesAction_execute(TestCase): @@ -87,7 +87,7 @@ def test_must_copy( listdir_mock.assert_any_call(source_dir) listdir_mock.assert_any_call(artifact_dir) - copytree_mock.assert_called_once_with("dir1", "dir2") + copytree_mock.assert_called_once_with("dir1", "dir2", maintain_symlinks=False) copy2_mock.assert_called_once_with("file1", "file2") makedirs_mock.assert_called_once_with("parent_dir_1", exist_ok=True) @@ -118,7 +118,7 @@ class TestCleanUpAction_execute(TestCase): @patch("aws_lambda_builders.actions.os.path.isdir") @patch("aws_lambda_builders.actions.os.listdir") @patch("aws_lambda_builders.actions.os.path.join") - def test_must_copy(self, path_mock, listdir_mock, isdir_mock, rmtree_mock, rm_mock): + def test_removes_directories_and_files(self, path_mock, listdir_mock, isdir_mock, rmtree_mock, rm_mock): target_dir = "target" listdir_mock.side_effect = [[1, 2]] @@ -131,6 +131,23 @@ def test_must_copy(self, path_mock, listdir_mock, isdir_mock, rmtree_mock, rm_mo rmtree_mock.assert_any_call("dir") rm_mock.assert_any_call("file") + @patch("aws_lambda_builders.actions.os.unlink") + @patch("aws_lambda_builders.actions.os.path.islink") + @patch("aws_lambda_builders.actions.os.path.isdir") + @patch("aws_lambda_builders.actions.os.listdir") + @patch("aws_lambda_builders.actions.os.path.join") + def test_can_handle_symlinks(self, join_mock, listdir_mock, isdir_mock, islink_mock, unlink_mock): + target_dir = "target" + + isdir_mock.side_effect = [True] + listdir_mock.side_effect = [["link"]] + join_mock.side_effect = ["target/link"] + islink_mock.side_effect = [True] + action = CleanUpAction(target_dir) + action.execute() + + unlink_mock.assert_called_once_with("target/link") + class TestDependencyManager(TestCase): @parameterized.expand(