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

[RFC] copy recursively #32

Closed
wants to merge 1 commit into from

Conversation

schnittstabil
Copy link
Collaborator

@schnittstabil schnittstabil commented Jun 22, 2016

At least copying recursively makes it necessary to use a two step approach:

  1. gather all files and folders to process
  2. do the actual copy

Substantial advantages:

  • determine errors due to mutually recursively linked directories in advance (see test below)
  • remove duplicated tasks (same source and destination)
  • determine copy tasks with same destination to throw errors in advance
  • calculate the total size during the first step, and emitting the progress during the second step

The current approach of the first step:

  1. use globby to gather all files and directories
  2. for each directory path: re-glob with path + '/**/*'

@ALL Comments on all aspects are highly welcome.

@jamestalmage
Copy link

jamestalmage commented Jun 22, 2016

When globbing the contents of a directory with /somedir/**/*, you want to avoid adding additional glob tasks for /somedir/subdir/**/*. See how I solved that here.

Also, since you are going to be calling glob/globby many times, you want store and pass in these caching options.

@YurySolovyov
Copy link
Contributor

calculate the total size during the first step, and emitting the progress during the second step

cp-file now has that.

determine copy tasks with same destination to throw errors in advance

Can be racy.

@sindresorhus
Copy link
Owner

sindresorhus commented Aug 13, 2016

When globbing the contents of a directory with /somedir/**/*, you want to avoid adding additional glob tasks for /somedir/subdir/**/*. See how I solved that here.

@jamestalmage Is that something that could maybe be added to globby? Sounds generally useful.

@sindresorhus
Copy link
Owner

I really like the idea of a two step process. My only concern is that it might cause some race issues. What if something in the filesystem hierarchy changes between step 1 and 2? For example, we glob foo/bar.jpg, but by the time we start copying in step 2 that file no longer exist or has moved. How should that be handled? It might be a non-issue as the globbing itself is already racy (node-glob states that in its docs). Just thought I'd bring it up.

@sindresorhus
Copy link
Owner

for each directory path: re-glob with path + '/*/'

Would be better to do this in globby: sindresorhus/globby#33

@sindresorhus
Copy link
Owner

@schnittstabil What are your thoughts on this now?

@sindresorhus
Copy link
Owner

Closing in favor of #61

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.

4 participants