with clobber true, copy and copySync behave differently if destination file is read only

[bartland]

copy succeeds because it will first do an fs.unlink() on the destination file (in ncp.js).
copySync fails.

To align the behaviour, in copy-file-sync.js change:

if (fs.existsSync(destFile) && !clobber) {
  throw Error('EEXIST');
}

to

if (fs.existsSync(destFile)) {
  if (clobber) {
    fs.unlinkSync(destFile);
  } else {
    throw Error('EEXIST');
  }
}

[bartland]

Hi @jprichardson
BTW: thanks for the great library.

I'll submit a pull request to fix this (without the semi-colons ;)

I've also added a test for it in copy-sync-file.test.js.

        describe('> when clobber is true and dest is readonly', function () {
          it('should copy the file and not throw an error', function () {
            try {
              fs.chmodSync(dest, parseInt('444', 8))
              fs.copySync(src, dest, {clobber: true})
              destData = fs.readFileSync(dest, 'utf8')
              assert.strictEqual(srcData, destData)
            } finally {
              // destination file is readonly so just remove it so we don't affect other tests
              fs.unlinkSync(dest)
            }
          })
        })

Tested on Ubuntu (see below).

On Windows 7, there are 7 tests currently failing, so no change as long as this one passes.
If this test fails, it causes other 3 additional tests to fail. Not sure why. I'll look into it if I get time.

I will test OS X and FreeBSD next week.

Result after the fix:

  429 passing (3s)
  2 pending

Result before the fix (Ubuntu):

  428 passing (3s)
  2 pending
  1 failing

  1) + copySync() > when the source is a file > when clobber option is passed when destination file does exist > when clobber is true and dest is readonly should copy the file and not throw an error:
     Error: EACCES: permission denied, open '/tmp/fs-extra/copy-sync/des-file'
      at Error (native)
      at Object.fs.openSync (fs.js:549:18)
      at copyFileSync (lib/copy-sync/copy-file-sync.js:24:16)
      at Object.copySync (lib/copy-sync/copy-sync.js:31:7)
      at Context.<anonymous> (lib/copy-sync/__tests__/copy-sync-file.test.js:191:18)

[jprichardson]

Thank you, your help is much appreciated! If you submit a PR and it passes TravisCI/Appveyor, that'd be awesome :)

[bartland]

I've worked out why the 7 tests are failing on windows. New issue opened #188.

[bartland]

I'm about to submit a PR for this. It looks clean. Since I did the PR for #188 I just wanted your feedback on what I did to deal with this in my local repository. If ok, I'll submit a PR.

git checkout master
git fetch upstream
git merge upstream/master [did the expected fast-forward merge]
git push origin master

git checkout CopySyncClobberROFile
git rebase master
git push origin CopySyncClobberROFile --force

[jprichardson]

I'm not a Git expert by any means, but AFAICT, it looks good.

[bartland]

hmm! Interesting. Node 0.12+ works but Node 0.10 fails. Nodejs must have changed its behavior. Any suggestions before I investigate?

  1) + copySync() > when the source is a file > when clobber option is passed when destination file does exist > when clobber is true and dest is readonly should copy the file and not throw an error:
     Error: EPERM, operation not permitted 'C:\Users\appveyor\AppData\Local\Temp\1\fs-extra\copy-sync\des-file'
      at Object.fs.unlinkSync (fs.js:772:18)
      at Context.<anonymous> (C:\projects\node-fs-extra\lib\copy-sync\__tests__\copy-sync-file.test.js:196:18)

[jprichardson]

Hmm. I know that I've had a number of issues in the past with trying to synchronously delete in Windows in my tests.

Possibly related: isaacs/rimraf#72 (I posted in this thread awhile back). However this was a ENOTEMPTY error.

If possible, see if you can't dig around a bit. If it's an easy fix, let's do it. If it's difficult, I'm not opposed to dropping support for Node v0.10. But before, I'd at least like to know why.

[bartland]

After some pain setting up a switchable between versions nodejs on Windows, the good news is a simple fix which works in 0.10.40 and 4.2.1. I will resubmit the PR soonish. I just added a chmodSync call.

Fixed code:

if (fs.existsSync(destFile)) {
  if (clobber) {
    fs.chmodSync(destFile, parseInt('777', 8))
    fs.unlinkSync(destFile);
  } else {
    throw Error('EEXIST');
  }
}