Skip to content

Commit 7ea2bcf

Browse files
committed
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.
1 parent 9edb02e commit 7ea2bcf

File tree

2 files changed

+35
-47
lines changed

2 files changed

+35
-47
lines changed

compilesketches/compilesketches.py

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ class RunCommandOutput(enum.Enum):
8181
not_applicable_indicator = "N/A"
8282
relative_size_report_decimal_places = 2
8383

84+
temporary_directory = tempfile.TemporaryDirectory(prefix="compilesketches-")
8485
arduino_cli_installation_path = pathlib.Path.home().joinpath("bin")
8586
arduino_cli_user_directory_path = pathlib.Path.home().joinpath("Arduino")
8687
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
254255
"""
255256
destination_parent_path = pathlib.Path(destination_parent_path)
256257

257-
# Create temporary folder for the download
258+
# Create temporary folder with function duration for the download
258259
with tempfile.TemporaryDirectory("-compilesketches-download_folder") as download_folder:
259260
download_file_path = pathlib.PurePath(download_folder, url.rsplit(sep="/", maxsplit=1)[1])
260261

@@ -268,23 +269,24 @@ def install_from_download(self, url, source_path, destination_parent_path, desti
268269
break
269270
out_file.write(block)
270271

271-
# Create temporary folder for the extraction
272-
with tempfile.TemporaryDirectory("-compilesketches-extract_folder") as extract_folder:
273-
# Extract archive
274-
shutil.unpack_archive(filename=str(download_file_path), extract_dir=extract_folder)
272+
# Create temporary folder with script run duration for the extraction
273+
extract_folder = tempfile.mkdtemp(dir=self.temporary_directory.name, prefix="install_from_download-")
275274

276-
archive_root_path = get_archive_root_path(extract_folder)
275+
# Extract archive
276+
shutil.unpack_archive(filename=str(download_file_path), extract_dir=extract_folder)
277277

278-
absolute_source_path = pathlib.Path(archive_root_path, source_path).resolve()
278+
archive_root_path = get_archive_root_path(extract_folder)
279279

280-
if not absolute_source_path.exists():
281-
print("::error::Archive source path:", source_path, "not found")
282-
sys.exit(1)
280+
absolute_source_path = pathlib.Path(archive_root_path, source_path).resolve()
283281

284-
self.install_from_path(source_path=absolute_source_path,
285-
destination_parent_path=destination_parent_path,
286-
destination_name=destination_name,
287-
force=force)
282+
if not absolute_source_path.exists():
283+
print("::error::Archive source path:", source_path, "not found")
284+
sys.exit(1)
285+
286+
self.install_from_path(source_path=absolute_source_path,
287+
destination_parent_path=destination_parent_path,
288+
destination_name=destination_name,
289+
force=force)
288290

289291
def install_platforms(self):
290292
"""Install Arduino boards platforms."""
@@ -537,7 +539,7 @@ def __init__(self):
537539
return platform_installation_path
538540

539541
def install_from_path(self, source_path, destination_parent_path, destination_name=None, force=False):
540-
"""Copy the source path to the destination path.
542+
"""Create a symlink to the source path in the destination path.
541543
542544
Keyword arguments:
543545
source_path -- path to install
@@ -563,10 +565,7 @@ def install_from_path(self, source_path, destination_parent_path, destination_na
563565
# Create the parent path if it doesn't already exist
564566
destination_parent_path.mkdir(parents=True, exist_ok=True)
565567

566-
if source_path.is_dir():
567-
shutil.copytree(src=source_path, dst=destination_path)
568-
else:
569-
shutil.copy(src=source_path, dst=destination_path)
568+
destination_path.symlink_to(target=source_path, target_is_directory=source_path.is_dir())
570569

571570
def install_platforms_from_repository(self, platform_list):
572571
"""Install libraries by cloning Git repositories
@@ -628,14 +627,14 @@ def install_from_repository(self,
628627
# Use the repository name
629628
destination_name = url.rstrip("/").rsplit(sep="/", maxsplit=1)[1].rsplit(sep=".", maxsplit=1)[0]
630629

631-
# Clone to a temporary folder to allow installing from subfolders of repos
632-
with tempfile.TemporaryDirectory() as clone_folder:
633-
self.clone_repository(url=url, git_ref=git_ref, destination_path=clone_folder)
634-
# Install to the final location
635-
self.install_from_path(source_path=pathlib.Path(clone_folder, source_path),
636-
destination_parent_path=destination_parent_path,
637-
destination_name=destination_name,
638-
force=force)
630+
# Clone to a temporary folder with script run duration to allow installing from subfolders of repos
631+
clone_folder = tempfile.mkdtemp(dir=self.temporary_directory.name, prefix="install_from_repository-")
632+
self.clone_repository(url=url, git_ref=git_ref, destination_path=clone_folder)
633+
# Install to the final location
634+
self.install_from_path(source_path=pathlib.Path(clone_folder, source_path),
635+
destination_parent_path=destination_parent_path,
636+
destination_name=destination_name,
637+
force=force)
639638

640639
def clone_repository(self, url, git_ref, destination_path):
641640
"""Clone a Git repository to a specified location and check out the specified ref

compilesketches/tests/test_compilesketches.py

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,14 +1252,15 @@ def test_install_from_path(capsys,
12521252
exists,
12531253
force,
12541254
is_dir):
1255+
is_dir = unittest.mock.sentinel.is_dir
1256+
12551257
compile_sketches = get_compilesketches_object()
12561258

12571259
mocker.patch.object(pathlib.Path, "exists", autospec=True, return_value=exists)
12581260
mocker.patch("shutil.rmtree", autospec=True)
12591261
mocker.patch.object(pathlib.Path, "mkdir", autospec=True)
12601262
mocker.patch.object(pathlib.Path, "is_dir", autospec=True, return_value=is_dir)
1261-
mocker.patch("shutil.copytree", autospec=True)
1262-
mocker.patch("shutil.copy", autospec=True)
1263+
mocker.patch.object(pathlib.Path, "symlink_to", autospec=True)
12631264

12641265
if exists and not force:
12651266
with pytest.raises(expected_exception=SystemExit, match="1"):
@@ -1281,10 +1282,8 @@ def test_install_from_path(capsys,
12811282
else:
12821283
shutil.rmtree.assert_not_called()
12831284

1284-
if is_dir:
1285-
shutil.copytree.assert_called_once_with(src=source_path, dst=expected_destination_path)
1286-
else:
1287-
shutil.copy.assert_called_once_with(src=source_path, dst=expected_destination_path)
1285+
pathlib.Path.symlink_to.assert_called_once_with(expected_destination_path, target=source_path,
1286+
target_is_directory=is_dir)
12881287

12891288

12901289
def test_install_from_path_functional(tmp_path):
@@ -2610,19 +2609,7 @@ def test_install_from_repository(mocker, url, source_path, destination_name, exp
26102609
force = unittest.mock.sentinel.force
26112610
clone_path = pathlib.PurePath("/foo/ClonePath")
26122611

2613-
# Stub
2614-
class TemporaryDirectory:
2615-
def __init__(self, temporary_directory):
2616-
self.temporary_directory = temporary_directory
2617-
2618-
def __enter__(self):
2619-
return self.temporary_directory
2620-
2621-
def __exit__(self, *exc):
2622-
pass
2623-
2624-
mocker.patch("tempfile.TemporaryDirectory", autospec=True,
2625-
return_value=TemporaryDirectory(temporary_directory=clone_path))
2612+
mocker.patch("tempfile.mkdtemp", autospec=True, return_value=clone_path)
26262613
mocker.patch("compilesketches.CompileSketches.clone_repository", autospec=True)
26272614
mocker.patch("compilesketches.CompileSketches.install_from_path", autospec=True)
26282615

@@ -2635,7 +2622,9 @@ def __exit__(self, *exc):
26352622
destination_name=destination_name,
26362623
force=force)
26372624

2638-
tempfile.TemporaryDirectory.assert_called_once()
2625+
# noinspection PyUnresolvedReferences
2626+
tempfile.mkdtemp.assert_called_once_with(dir=compile_sketches.temporary_directory.name,
2627+
prefix="install_from_repository-")
26392628
compile_sketches.clone_repository.assert_called_once_with(compile_sketches,
26402629
url=url,
26412630
git_ref=git_ref,

0 commit comments

Comments
 (0)