Skip to content

Commit

Permalink
Replace distutils with shutil for 3.12 (#3714)
Browse files Browse the repository at this point in the history
This PR replaces distutils.dir_util.copy_tree(update=1) with a custom
`update_tree` function using `shutil` because Python 3.12 removed
`distutils`. Our tests did not catch this issue because our test
environments include `setuptools`, which vendors `distutils` Added unit
tests to ensure update_tree only copies new or missing files,
maintaining the original behavior in those tasks that used `copy_tree`.

Fixes #3707
  • Loading branch information
jstvz authored Dec 8, 2023
1 parent 68149d8 commit eec6886
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 46 deletions.
12 changes: 5 additions & 7 deletions cumulusci/tasks/metadata/ee_src.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import os

# TODO: Replace this with shutils
from distutils.dir_util import copy_tree, remove_tree
from shutil import copytree, rmtree

from cumulusci.core.exceptions import TaskOptionsError
from cumulusci.core.tasks import BaseTask
from cumulusci.utils import remove_xml_element_directory
from cumulusci.utils import remove_xml_element_directory, update_tree


class CreateUnmanagedEESrc(BaseTask):
Expand Down Expand Up @@ -40,7 +38,7 @@ def _run_task(self):
)

# Copy path to revert_path
copy_tree(self.options["path"], self.options["revert_path"])
copytree(self.options["path"], self.options["revert_path"])

# Edit metadata in path
self.logger.info(
Expand Down Expand Up @@ -88,9 +86,9 @@ def _run_task(self):
self.options["path"], self.options["revert_path"]
)
)
copy_tree(self.options["revert_path"], self.options["path"], update=1)
update_tree(self.options["revert_path"], self.options["path"])
self.logger.info("{} is now reverted".format(self.options["path"]))

# Delete the revert_path
self.logger.info("Deleting {}".format(self.options["revert_path"]))
remove_tree(self.options["revert_path"])
rmtree(self.options["revert_path"])
11 changes: 5 additions & 6 deletions cumulusci/tasks/metadata/managed_src.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
# TODO: Replace this with shutils
from distutils.dir_util import copy_tree, remove_tree
from pathlib import Path
from shutil import copytree, rmtree

from cumulusci.core.exceptions import TaskOptionsError
from cumulusci.core.tasks import BaseTask
from cumulusci.utils import find_replace
from cumulusci.utils import find_replace, update_tree


class CreateManagedSrc(BaseTask):
Expand Down Expand Up @@ -45,7 +44,7 @@ def _run_task(self):
)

# Copy path to revert_path
copy_tree(str(path), str(revert_path))
copytree(str(path), str(revert_path))

# Edit metadata in path
self.logger.info(
Expand Down Expand Up @@ -93,9 +92,9 @@ def _run_task(self):
)

self.logger.info(f"Reverting {path} from {revert_path}")
copy_tree(str(revert_path), str(path), update=1)
update_tree(str(revert_path), str(path))
self.logger.info(f"{path} is now reverted")

# Delete the revert_path
self.logger.info(f"Deleting {str(revert_path)}")
remove_tree(revert_path)
rmtree(revert_path)
67 changes: 51 additions & 16 deletions cumulusci/tasks/metadata/tests/test_ee_src.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import time
from unittest import mock

import pytest
Expand Down Expand Up @@ -52,26 +53,60 @@ def test_run_task__revert_path_already_exists(self):


class TestRevertUnmanagedEESrc:
def test_run_task(self):
with temporary_dir() as revert_path:
with open(os.path.join(revert_path, "file"), "w"):
pass
path = os.path.join(
os.path.dirname(revert_path), os.path.basename(revert_path) + "_orig"
)
project_config = BaseProjectConfig(
UniversalConfig(), config={"noyaml": True}
)
task_config = TaskConfig(
{"options": {"path": path, "revert_path": revert_path}}
)
task = RevertUnmanagedEESrc(project_config, task_config)
task()
assert os.path.exists(os.path.join(path, "file"))
def test_run_task(self, tmp_path):
revert_path = tmp_path / "revert"
revert_path.mkdir()
file_path = revert_path / "file"
file_path.write_text("content")

path = tmp_path / "path"
path.mkdir()
project_config = BaseProjectConfig(UniversalConfig(), config={"noyaml": True})
task_config = TaskConfig(
{"options": {"path": str(path), "revert_path": str(revert_path)}}
)
task = RevertUnmanagedEESrc(project_config, task_config)
task()
assert (path / "file").exists()

def test_run_task__revert_path_not_found(self):
project_config = BaseProjectConfig(UniversalConfig(), config={"noyaml": True})
task_config = TaskConfig({"options": {"path": "bogus", "revert_path": "bogus"}})
task = RevertUnmanagedEESrc(project_config, task_config)
with pytest.raises(TaskOptionsError):
task()

def test_revert_with_update(self, tmp_path):
"""
Test the 'update' behavior of RevertUnmanagedEESrc task with temporary directories.
This test creates a source and a destination directory each with one
file. The file in the source directory has an older timestamp. After
running RevertUnmanagedEESrc, it checks that the destination file is not
overwritten by the older source file, confirming the update logic.
"""
source_dir = tmp_path / "source"
source_dir.mkdir()
source_file = source_dir / "testfile.txt"
source_file.write_text("original content")

dest_dir = tmp_path / "dest"
dest_dir.mkdir()
dest_file = dest_dir / "testfile.txt"
dest_file.write_text("modified content")

# Ensure the source file has an older timestamp
past_time = time.time() - 100
# Use os.utime to modify the timestamp
source_file.touch()
os.utime(str(source_file), (past_time, past_time))

project_config = BaseProjectConfig(UniversalConfig(), config={"noyaml": True})
task_config = TaskConfig(
{"options": {"path": str(dest_dir), "revert_path": str(source_dir)}}
)
task = RevertUnmanagedEESrc(project_config, task_config)
task()

# Verify that the destination file was not updated (due to older source file)
assert dest_file.read_text() == "modified content"
67 changes: 51 additions & 16 deletions cumulusci/tasks/metadata/tests/test_managed_src.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import time

import pytest

Expand Down Expand Up @@ -51,26 +52,60 @@ def test_run_task__revert_path_already_exists(self):


class TestRevertManagedSrc:
def test_run_task(self):
with temporary_dir() as revert_path:
with open(os.path.join(revert_path, "file"), "w"):
pass
path = os.path.join(
os.path.dirname(revert_path), os.path.basename(revert_path) + "_orig"
)
project_config = BaseProjectConfig(
UniversalConfig(), config={"noyaml": True}
)
task_config = TaskConfig(
{"options": {"path": path, "revert_path": revert_path}}
)
task = RevertManagedSrc(project_config, task_config)
task()
assert os.path.exists(os.path.join(path, "file"))
def test_run_task(self, tmp_path):
revert_path = tmp_path / "revert"
revert_path.mkdir()
file_path = revert_path / "file"
file_path.write_text("content")

path = tmp_path / "path"
path.mkdir()
project_config = BaseProjectConfig(UniversalConfig(), config={"noyaml": True})
task_config = TaskConfig(
{"options": {"path": str(path), "revert_path": str(revert_path)}}
)
task = RevertManagedSrc(project_config, task_config)
task()
assert (path / "file").exists()

def test_run_task__revert_path_not_found(self):
project_config = BaseProjectConfig(UniversalConfig(), config={"noyaml": True})
task_config = TaskConfig({"options": {"path": "bogus", "revert_path": "bogus"}})
task = RevertManagedSrc(project_config, task_config)
with pytest.raises(TaskOptionsError):
task()

def test_revert_with_update(self, tmp_path):
"""
Test the 'update' behavior of RevertManagedSrc task with temporary directories.
This test creates a source and a destination directory each with one
file. The file in the source directory has an older timestamp. After
running RevertManagedSrc, it checks that the destination file is not
overwritten by the older source file, confirming the update logic.
"""
source_dir = tmp_path / "source"
source_dir.mkdir()
source_file = source_dir / "testfile.txt"
source_file.write_text("original content")

dest_dir = tmp_path / "dest"
dest_dir.mkdir()
dest_file = dest_dir / "testfile.txt"
dest_file.write_text("modified content")

# Ensure the source file has an older timestamp
past_time = time.time() - 100
# Use os.utime to modify the timestamp
source_file.touch()
os.utime(str(source_file), (past_time, past_time))

project_config = BaseProjectConfig(UniversalConfig(), config={"noyaml": True})
task_config = TaskConfig(
{"options": {"path": str(dest_dir), "revert_path": str(source_dir)}}
)
task = RevertManagedSrc(project_config, task_config)
task()

# Verify that the destination file was not updated (due to older source file)
assert dest_file.read_text() == "modified content"
30 changes: 30 additions & 0 deletions cumulusci/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import textwrap
import zipfile
from datetime import datetime
from pathlib import Path
from typing import Union

import requests
import sarge
Expand Down Expand Up @@ -630,3 +632,31 @@ def get_git_config(config_key):
)

return config_value if config_value and not p.returncode else None


def update_tree(src: Union[str, Path], dest: Union[str, Path]):
"""
Copies files from src to dest, same as distutils.copy_tree(update=1).
Copies the entire directory tree from src to dest. If dest exists, only
copies files that are newer in src than in dest, or files that don't exist
in dest.
Args:
src (Union[str, Path]): The source directory to copy files from.
dest (Union[str, Path]): The destination directory to copy files to.
"""
src_path = Path(src)
dest_path = Path(dest)
if not dest_path.exists():
shutil.copytree(src_path, dest_path)
else:
for src_dir in src_path.rglob("*"):
if src_dir.is_file():
dest_file = dest_path / src_dir.relative_to(src_path)
if (
not dest_file.exists()
or src_dir.stat().st_mtime - dest_file.stat().st_mtime > 1
):
dest_file.parent.mkdir(parents=True, exist_ok=True)
shutil.copy2(src_dir, dest_file)
31 changes: 30 additions & 1 deletion cumulusci/utils/tests/test_fileutils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import doctest
import os
import sys
import time
import urllib.request
from io import BytesIO, UnsupportedOperation
from pathlib import Path
Expand All @@ -12,7 +13,7 @@
from fs import errors, open_fs

import cumulusci
from cumulusci.utils import fileutils, temporary_dir
from cumulusci.utils import fileutils, temporary_dir, update_tree
from cumulusci.utils.fileutils import (
FSResource,
load_from_source,
Expand Down Expand Up @@ -253,3 +254,31 @@ class TestFSResourceError:
def test_fs_resource_init_error(self):
with pytest.raises(NotImplementedError):
FSResource()


def test_update_tree(tmpdir):
source_dir = Path(tmpdir.mkdir("source"))
source_file = source_dir / "testfile.txt"
source_file.write_text("original content")

dest_dir = Path(tmpdir.mkdir("dest"))
dest_file = dest_dir / "testfile.txt"
dest_file.write_text("modified content")

# Ensure the source file has an older timestamp
past_time = time.time() - 100
os.utime(str(source_file), (past_time, past_time))

update_tree(source_dir, dest_dir)

assert dest_file.read_text() == "modified content"

# Add a new file to source and run update_tree again
new_source_file = source_dir / "newfile.txt"
new_source_file.write_text("new file content")
update_tree(source_dir, dest_dir)

# Verify that the new file is copied to destination
new_dest_file = dest_dir / "newfile.txt"
assert new_dest_file.exists()
assert new_dest_file.read_text() == "new file content"

0 comments on commit eec6886

Please sign in to comment.