From 7ea2bcfbd14bfe6c11424d245c8a0e2fc8de5d6e Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 10 Sep 2020 23:16:04 -0700 Subject: [PATCH] Install dependencies from local path using symlinks Previously, the shared function used to install dependencies from local paths used copy instead of symlinks. The switch to using copy for everything was done for a couple of reasons: - Some functions were already using copy/move to install because they worked from temporary folders that are deleted by the context manager - Use of symlinks on Windows requires the script to be run as administrator, which makes it less friendly to contributors or users running the script locally who are using Windows However, the previous symlinks were essential for the deltas feature to work for libraries or platforms under test because these are installed to a different location than the repository that is checked out to the delta and head refs. When the project under test is a sketch, symlinks are not necessary, since the sketch is run in place, which is why this bug was not detected via minimal informal integration tests. The first reason to copy listed above is easily overcome by changing the code to create temporary folders with the required lifespan. The second remains, but it's easy enough to find the solution to the error message resulting from running the script without the necessary permissions. --- compilesketches/compilesketches.py | 53 +++++++++---------- compilesketches/tests/test_compilesketches.py | 29 ++++------ 2 files changed, 35 insertions(+), 47 deletions(-) diff --git a/compilesketches/compilesketches.py b/compilesketches/compilesketches.py index 3569f937..5182cb36 100644 --- a/compilesketches/compilesketches.py +++ b/compilesketches/compilesketches.py @@ -81,6 +81,7 @@ class RunCommandOutput(enum.Enum): not_applicable_indicator = "N/A" relative_size_report_decimal_places = 2 + temporary_directory = tempfile.TemporaryDirectory(prefix="compilesketches-") arduino_cli_installation_path = pathlib.Path.home().joinpath("bin") arduino_cli_user_directory_path = pathlib.Path.home().joinpath("Arduino") arduino_cli_data_directory_path = pathlib.Path.home().joinpath(".arduino15") @@ -254,7 +255,7 @@ def install_from_download(self, url, source_path, destination_parent_path, desti """ destination_parent_path = pathlib.Path(destination_parent_path) - # Create temporary folder for the download + # Create temporary folder with function duration for the download with tempfile.TemporaryDirectory("-compilesketches-download_folder") as download_folder: download_file_path = pathlib.PurePath(download_folder, url.rsplit(sep="/", maxsplit=1)[1]) @@ -268,23 +269,24 @@ def install_from_download(self, url, source_path, destination_parent_path, desti break out_file.write(block) - # Create temporary folder for the extraction - with tempfile.TemporaryDirectory("-compilesketches-extract_folder") as extract_folder: - # Extract archive - shutil.unpack_archive(filename=str(download_file_path), extract_dir=extract_folder) + # Create temporary folder with script run duration for the extraction + extract_folder = tempfile.mkdtemp(dir=self.temporary_directory.name, prefix="install_from_download-") - archive_root_path = get_archive_root_path(extract_folder) + # Extract archive + shutil.unpack_archive(filename=str(download_file_path), extract_dir=extract_folder) - absolute_source_path = pathlib.Path(archive_root_path, source_path).resolve() + archive_root_path = get_archive_root_path(extract_folder) - if not absolute_source_path.exists(): - print("::error::Archive source path:", source_path, "not found") - sys.exit(1) + absolute_source_path = pathlib.Path(archive_root_path, source_path).resolve() - self.install_from_path(source_path=absolute_source_path, - destination_parent_path=destination_parent_path, - destination_name=destination_name, - force=force) + if not absolute_source_path.exists(): + print("::error::Archive source path:", source_path, "not found") + sys.exit(1) + + self.install_from_path(source_path=absolute_source_path, + destination_parent_path=destination_parent_path, + destination_name=destination_name, + force=force) def install_platforms(self): """Install Arduino boards platforms.""" @@ -537,7 +539,7 @@ def __init__(self): return platform_installation_path def install_from_path(self, source_path, destination_parent_path, destination_name=None, force=False): - """Copy the source path to the destination path. + """Create a symlink to the source path in the destination path. Keyword arguments: source_path -- path to install @@ -563,10 +565,7 @@ def install_from_path(self, source_path, destination_parent_path, destination_na # Create the parent path if it doesn't already exist destination_parent_path.mkdir(parents=True, exist_ok=True) - if source_path.is_dir(): - shutil.copytree(src=source_path, dst=destination_path) - else: - shutil.copy(src=source_path, dst=destination_path) + destination_path.symlink_to(target=source_path, target_is_directory=source_path.is_dir()) def install_platforms_from_repository(self, platform_list): """Install libraries by cloning Git repositories @@ -628,14 +627,14 @@ def install_from_repository(self, # Use the repository name destination_name = url.rstrip("/").rsplit(sep="/", maxsplit=1)[1].rsplit(sep=".", maxsplit=1)[0] - # Clone to a temporary folder to allow installing from subfolders of repos - with tempfile.TemporaryDirectory() as clone_folder: - self.clone_repository(url=url, git_ref=git_ref, destination_path=clone_folder) - # Install to the final location - self.install_from_path(source_path=pathlib.Path(clone_folder, source_path), - destination_parent_path=destination_parent_path, - destination_name=destination_name, - force=force) + # Clone to a temporary folder with script run duration to allow installing from subfolders of repos + clone_folder = tempfile.mkdtemp(dir=self.temporary_directory.name, prefix="install_from_repository-") + self.clone_repository(url=url, git_ref=git_ref, destination_path=clone_folder) + # Install to the final location + self.install_from_path(source_path=pathlib.Path(clone_folder, source_path), + destination_parent_path=destination_parent_path, + destination_name=destination_name, + force=force) def clone_repository(self, url, git_ref, destination_path): """Clone a Git repository to a specified location and check out the specified ref diff --git a/compilesketches/tests/test_compilesketches.py b/compilesketches/tests/test_compilesketches.py index 4072bc1d..ebe95c8f 100644 --- a/compilesketches/tests/test_compilesketches.py +++ b/compilesketches/tests/test_compilesketches.py @@ -1252,14 +1252,15 @@ def test_install_from_path(capsys, exists, force, is_dir): + is_dir = unittest.mock.sentinel.is_dir + compile_sketches = get_compilesketches_object() mocker.patch.object(pathlib.Path, "exists", autospec=True, return_value=exists) mocker.patch("shutil.rmtree", autospec=True) mocker.patch.object(pathlib.Path, "mkdir", autospec=True) mocker.patch.object(pathlib.Path, "is_dir", autospec=True, return_value=is_dir) - mocker.patch("shutil.copytree", autospec=True) - mocker.patch("shutil.copy", autospec=True) + mocker.patch.object(pathlib.Path, "symlink_to", autospec=True) if exists and not force: with pytest.raises(expected_exception=SystemExit, match="1"): @@ -1281,10 +1282,8 @@ def test_install_from_path(capsys, else: shutil.rmtree.assert_not_called() - if is_dir: - shutil.copytree.assert_called_once_with(src=source_path, dst=expected_destination_path) - else: - shutil.copy.assert_called_once_with(src=source_path, dst=expected_destination_path) + pathlib.Path.symlink_to.assert_called_once_with(expected_destination_path, target=source_path, + target_is_directory=is_dir) def test_install_from_path_functional(tmp_path): @@ -2610,19 +2609,7 @@ def test_install_from_repository(mocker, url, source_path, destination_name, exp force = unittest.mock.sentinel.force clone_path = pathlib.PurePath("/foo/ClonePath") - # Stub - class TemporaryDirectory: - def __init__(self, temporary_directory): - self.temporary_directory = temporary_directory - - def __enter__(self): - return self.temporary_directory - - def __exit__(self, *exc): - pass - - mocker.patch("tempfile.TemporaryDirectory", autospec=True, - return_value=TemporaryDirectory(temporary_directory=clone_path)) + mocker.patch("tempfile.mkdtemp", autospec=True, return_value=clone_path) mocker.patch("compilesketches.CompileSketches.clone_repository", autospec=True) mocker.patch("compilesketches.CompileSketches.install_from_path", autospec=True) @@ -2635,7 +2622,9 @@ def __exit__(self, *exc): destination_name=destination_name, force=force) - tempfile.TemporaryDirectory.assert_called_once() + # noinspection PyUnresolvedReferences + tempfile.mkdtemp.assert_called_once_with(dir=compile_sketches.temporary_directory.name, + prefix="install_from_repository-") compile_sketches.clone_repository.assert_called_once_with(compile_sketches, url=url, git_ref=git_ref,