ryanmcgrath / wrench-js

Recursive file operations in Node.js
MIT License
435 stars 71 forks source link

Provide option to resolve symlinks (especially for Windows) #69

Closed jsoref closed 10 years ago

jsoref commented 10 years ago

I have a symlink (created by my administrator -- Only administrators can create symlinks, more info about File-Based Symbolic Links).

[cmd-running-as-administrator]> mklink /d contained c:\original
symbolic link created for contained <<===>> c:\original
[cmd]> dir contained*
12/18/2013  06:18 PM    <SYMLINKD>     contained [C:\original]

Node is running as me (I don't have the administrator token), so this:

wrench.copyDirSyncRecursive(parent_of_contained, destination);

... fails:

fs.js:730
  return binding.symlink(preprocessSymlinkDestination(destination, type),
                 ^
Error: EPERM, operation not permitted 'C:\destination\contained'
    at Object.fs.symlinkSync (fs.js:730:18)
    at Object.exports.copyDirSyncRecursive (node_modules\wrench\lib\wrench.js:193:16)
    at Object.exports.copyDirSyncRecursive (node_modules\wrench\lib\wrench.js:190:21)

rsync and xcopy and friends are capable of making copies which don't include symlinks. While it might be nice to preserve symlinks, on Windows your average user can't create any symlinks. So there should at least be a flag to let callers of wrench.copyDirSyncRecursive ask it to serialize when copying things which happen include directory symbolic links.

(I'd argue that on Windows, this should be the default behavior.)

ryanmcgrath commented 10 years ago

Aaah, interesting, I don't use Windows (haven't in somewhere close to 8 years) as my daily driver, wouldn't have caught this one. At the moment I'm unable to attend to this, but what I'm open to is pull requests with a test - happy to pipeline it up if one's provided. Otherwise I'll get to this in time. Thanks!

jsoref commented 10 years ago

Would you like it to automatically convert to real, or would you like a flag? If you suggest the direction and flag name, I can probably provide a PR.

ryanmcgrath commented 10 years ago

Forgive my ignorance - convert to real = make a true directory copy?

It might make more sense to just enforce inflateSymlinks to be true on Windows. Thoughts?

jsoref commented 10 years ago

Yes. Ah, that'd be what I was looking for.

Creating Symlinks on Windows is such a rare thing, that you should probably be explicitly creating them.

ryanmcgrath commented 10 years ago

Hmmm - if you wanted to get a pull request out of all of this, a note in the docs/readme wouldn't be bad. :)

Cheers!