Skip to content
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

Fix utils.copy_tree() to create dst if it doesn't exist #1231

Merged
merged 2 commits into from
Sep 4, 2024

Conversation

simu
Copy link
Contributor

@simu simu commented Sep 3, 2024

Proposed Changes

Kapitan's utils.copy_tree() calls shutil.copytree() which will create the destination directory if it doesn't exist yet. However, currently, utils.copy_tree() doesn't support this case and will raise a SafeCopyError when the destination directory doesn't exist.

This PR adjusts the implementation to only raise a SafeCopyError when the destination exists and isn't a directory and will let shutil.copytree() create the destination if the destination doesn't exist at all.

Docs and Tests

  • Tests added
  • Updated documentation

Kapitan's `utils.copy_tree()` calls `shutil.copytree()` which will
create the destination directory if it doesn't exist yet. However,
currently, `utils.copy_tree()` doesn't support this case and will raise
a `SafeCopyError` when the destination directory doesn't exist.

This commit adjusts the implementation to only raise a `SafeCopyError`
when the destination exists and isn't a directory and will let
`shutil.copytree()` create the destination if the destination doesn't
exist at all.
@ademariag ademariag self-requested a review September 3, 2024 15:03
@ademariag ademariag enabled auto-merge (squash) September 3, 2024 15:03
auto-merge was automatically disabled September 3, 2024 15:06

Head branch was pushed to by a user without write access

@ademariag ademariag merged commit 401afb3 into kapicorp:master Sep 4, 2024
8 checks passed
ademariag pushed a commit that referenced this pull request Sep 4, 2024
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants