-
Notifications
You must be signed in to change notification settings - Fork 152
feat: add symlink support in actions #461
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
4c0542e
f0a2910
a832d83
1c87209
dda3bd6
5f760de
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 |
|---|---|---|
|
|
@@ -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) | ||
torresxb1 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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.", | ||
|
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. Should we be prescriptive on the necessary settings? ex: set x
Contributor
Author
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'm not sure that we can pinpoint exactly why it failed or how to resolve it. It might vary by OS and version. |
||
| exc_info=ex if LOG.isEnabledFor(logging.DEBUG) else None, | ||
| ) | ||
| copytree(source, destination) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| import os | ||
|
Contributor
Author
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 added lines 34-91, and 121-127. The rest were just moved from it's previous location ( |
||
| 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)) | ||
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.
We are symlinking and copying?
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.
same question here ... may be I am missing something, but will the copy ovwerwrite the created symlink ?
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.
copystat copies the metadata: https://docs.python.org/3/library/shutil.html#shutil.copystat
Since we are "copying" the symlink by creating a new one that points to the same location as the other one, I thought we might want to copy over the metadata. Let me know if you think this is not necessary.