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.

Sign up for GitHub

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

Avoid recursing when installing a project to a subproject via copytree #9734

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Avoid recursing when installing a project to a subproject via copytree #9734

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fb2e01c
Avoid recursing when installing a project to a subproject via copytree
djbelknap Mar 7, 2024
118beed
Add changelog entry
djbelknap Mar 7, 2024
3536557
Merge branch 'main' into deps-local-ignore
dbeatty10 Jul 18, 2024
e10b698
Incorporate `ignore` parameter
dbeatty10 Jul 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/Fixes-20240307-090156.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Avoid recursing when installing a project to a subproject via copytree
time: 2024-03-07T09:01:56.9007199-06:00
custom:
Author: djbelknap
Issue: "9719"
8 changes: 7 additions & 1 deletion core/dbt/deps/local.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,13 @@ def install(self, project, renderer):
system.make_symlink(src_path, dest_path)
except OSError:
fire_event(DepsSymlinkNotAvailable())
shutil.copytree(src_path, dest_path)
shutil.copytree(
src_path,
dest_path,
ignore=lambda directory, contents: project.packages_install_path
if directory == project.project_root
else [],
)
Comment on lines +67 to +73
Copy link
Contributor

@dbeatty10 dbeatty10 Jul 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What

"Don't install yourself into your own dbt_packages folder."

How

shutil.copytree recursively copies an entire directory tree rooted at src to a directory named dst.

The new ignore parameter introduces a base case under which this recursion should stop. It essentially says:

"No dbt project ever needs to install itself within its own packages_install_path folder 😄. So when the current directory being processed is the same as this dbt project, then do not recursively copy the configured packages_install_path directory because it's already covered. Just ignore it!"

Why

The ignore parameter is being added here to prevent a dbt project from installing itself via dbt deps. As mentioned in #9719, this can happen if:

  1. a dbt project is a subfolder of a local dependency listed within packages.yml, and
  2. the host operating system doesn't support symlinks.

The 1st item is common for CI testing for packages like dbt_utils, and the 2nd is most common in legacy versions of Windows.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows 10 users can possibly avoid this code path by turning on Developer Mode (which may allow creating Symbolic Links without requiring admin privileges) [1].

But this PR is specifically intended to cover the case where the user's system doesn't allow symlinks for whatever reason. So Developer Mode for Windows 10 is not a sufficient workaround for all cases.



class LocalUnpinnedPackage(LocalPackageMixin, UnpinnedPackage[LocalPinnedPackage]):
Expand Down
Loading