Skip to content

Commit

Permalink
Fix fetch_git_dependency() to clobber files in the destination if `…
Browse files Browse the repository at this point in the history
…force_fetch=True` (#1232)

## Proposed Changes

When force fetching Git dependencies, we need to be able to overwrite
read-only files in the destination, since Git repos may contain
read-only pack and index files in `.git/`.

To do so, this PR introduces flag `clobber_files` for
`utils.copy_tree()` which will completely clobber existing files in the
destination by setting a custom `copy_function` for `shutil.copytree()`
which deletes existing destination files before copying them with
`shutil.copy2()`.

With this change, `utils.copy_tree()` with `clobber_files=True` behaves
pretty much identically to distutils' `copy_tree()` which was used in
previous versions of Kapitan.

## TODO

- [x] Rebase once #1231 is merged

## Docs and Tests

* [x] Tests added
* [x] Updated documentation
  • Loading branch information
simu authored Sep 4, 2024
1 parent 401afb3 commit 335ab34
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 71 deletions.
4 changes: 3 additions & 1 deletion kapitan/dependency_manager/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,9 @@ def fetch_git_dependency(dep_mapping, save_dir, force, item_type="Dependency"):
"{} {}: subdir {} not found in repo".format(item_type, source, sub_dir)
)
if force:
copied = copy_tree(copy_src_path, output_path)
# We need `clobber_files=True` here, since we otherwise can't overwrite read-only Git
# index and pack files if the destination already contains a copy of the Git repo.
copied = copy_tree(copy_src_path, output_path, clobber_files=True)
else:
copied = safe_copy_tree(copy_src_path, output_path)
if copied:
Expand Down
26 changes: 24 additions & 2 deletions kapitan/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -628,11 +628,28 @@ def safe_copy_tree(src, dst):
return outputs


def copy_tree(src: str, dst: str) -> list:
def force_copy_file(src: str, dst: str, *args, **kwargs):
"""Copy file from `src` to `dst`, forcibly replacing `dst` if it's a file, but preserving the
source file's metadata.
This is suitable to use as `copy_function` in `shutil.copytree()` if the behavior of distutils'
`copy_tree` should be mimicked as closely as possible.
"""
if os.path.isfile(dst):
os.unlink(dst)
shutil.copy2(src, dst, *args, **kwargs)


def copy_tree(src: str, dst: str, clobber_files=False) -> list:
"""Recursively copy a given directory from `src` to `dst`.
If `dst` or a parent of `dst` doesn't exist, the missing directories are created.
If `clobber_files` is set to true, existing files in the destination directory are completely
clobbered. This is necessary to allow use of this function when copying a Git repo into a
destination directory which may already contain an old copy of the repo. Files that are
overwritten this way won't be listed in the return value.
Returns a list of the copied files.
"""
if not os.path.isdir(src):
Expand All @@ -643,6 +660,11 @@ def copy_tree(src: str, dst: str) -> list:

# this will generate an empty set if `dst` doesn't exist
before = set(glob.iglob(f"{dst}/*", recursive=True))
shutil.copytree(src, dst, dirs_exist_ok=True)
if clobber_files:
# use `force_copy_file` to more closely mimic distutils' `copy_tree` behavior
copy_function = force_copy_file
else:
copy_function = shutil.copy2
shutil.copytree(src, dst, dirs_exist_ok=True, copy_function=copy_function)
after = set(glob.iglob(f"{dst}/*", recursive=True))
return list(after - before)
Loading

0 comments on commit 335ab34

Please sign in to comment.