add recursive copy function #3

Open
fjakobs opened this Issue May 5, 2012 · 4 comments

2 participants

@fjakobs
Cloud9 member

No description provided.

@creationix creationix was assigned May 5, 2012
@creationix

What should be the behavior on the various error conditions? Since this is a fairly complex operation many assumptions or options have to be coded.

Some problems include:

  • A file or directory in the source tree is not readable
  • A destination file or directory is not writable.
  • All or part of the destination files already exist.

Should the whole operation fail early on error, should it check for possible errors before starting real work, should it ignore errors as best and continue working where possible?

Also, not errors, but what if the destination is a sub-folder of the source tree? Care needs to be taken to ensure the newly created files aren't copied again recursively.

@fjakobs
Cloud9 member

jsDAV does the following: "copy: This method must work recursively and delete the destination if it exists"

this would get rid of most of the edge cases. An alternative could also be to fail if the destination folder exists. Then the client needs to resolve this conflict.

If the destination is a sub-folder of the source tree we should also return an error.

@creationix

Sounds good, we can error out if the destination folder exists or is a subfolder. That still leaves the edge case of when a source file or folder is not readable. Should we stop of first error, continue past any errors, or check the entire tree in a pre-flight step? (and even with a pre-flight check, things can change after the fact, so errors are still possible, but much less likely)

@fjakobs
Cloud9 member

Sounds good. My suggestion:

  1. check if destination exists or is a subfolder of the source. If so return an error.
  2. copy the source to a temp dir.
  3. if there is an error remove the temp dir and report it
  4. once we copied successfully move the temp dir to the destination 4.1 if this also fails because the target has been added while copying we perform 2.

We could add the prefilght check as an option to step 1. For cloud9 it is not likely to cause any problems without the preflight.

@fjakobs fjakobs closed this in 125da17 May 10, 2012
@fjakobs fjakobs reopened this May 10, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment